-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Closing receipt view using device navigation redirects user to expense report #35497
Changes from 17 commits
6144f13
2483bfb
2fda1ad
52883ff
9a31934
0c8a977
f28a3d4
a96b14d
7de3fc1
5bb6c80
4a0b807
767dbbb
9011c0f
723d427
4c53d2d
644b4ff
e231af8
96f90f0
c2006ab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,6 @@ import type {ReactElement} from 'react'; | |
import type {ImageSourcePropType, ViewStyle} from 'react-native'; | ||
import {View} from 'react-native'; | ||
import type {OnyxEntry} from 'react-native-onyx'; | ||
import AttachmentModal from '@components/AttachmentModal'; | ||
import EReceiptThumbnail from '@components/EReceiptThumbnail'; | ||
import * as Expensicons from '@components/Icon/Expensicons'; | ||
import Image from '@components/Image'; | ||
|
@@ -14,10 +13,12 @@ import {ShowContextMenuContext} from '@components/ShowContextMenuContext'; | |
import ThumbnailImage from '@components/ThumbnailImage'; | ||
import useLocalize from '@hooks/useLocalize'; | ||
import useThemeStyles from '@hooks/useThemeStyles'; | ||
import Navigation from '@libs/Navigation/Navigation'; | ||
import * as TransactionUtils from '@libs/TransactionUtils'; | ||
import tryResolveUrlFromApiRoot from '@libs/tryResolveUrlFromApiRoot'; | ||
import variables from '@styles/variables'; | ||
import CONST from '@src/CONST'; | ||
import ROUTES from '@src/ROUTES'; | ||
import type {Transaction} from '@src/types/onyx'; | ||
|
||
type ReportActionItemImageProps = { | ||
|
@@ -36,14 +37,11 @@ type ReportActionItemImageProps = { | |
/** whether thumbnail is refer the local file or not */ | ||
isLocalFile?: boolean; | ||
|
||
/** whether the receipt can be replaced */ | ||
canEditReceipt?: boolean; | ||
/** Whether there are other images displayed in the same parent container */ | ||
isSingleImage?: boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like this unnecessary diff. keep original order, so put isSingleImage under filename ![]() Visit https://github.com/Expensify/App/pull/35497/files and find & fix all occurrences And any reason for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need to use canEditReceipt in this component anymore, we'll use it in |
||
|
||
/** Filename of attachment */ | ||
filename?: string; | ||
|
||
/** Whether there are other images displayed in the same parent container */ | ||
isSingleImage?: boolean; | ||
}; | ||
|
||
/** | ||
|
@@ -52,16 +50,7 @@ type ReportActionItemImageProps = { | |
* and optional preview modal as well. | ||
*/ | ||
|
||
function ReportActionItemImage({ | ||
thumbnail, | ||
image, | ||
enablePreviewModal = false, | ||
transaction, | ||
canEditReceipt = false, | ||
isLocalFile = false, | ||
filename, | ||
isSingleImage = true, | ||
}: ReportActionItemImageProps) { | ||
function ReportActionItemImage({thumbnail, image, enablePreviewModal = false, transaction, isLocalFile = false, isSingleImage = true, filename}: ReportActionItemImageProps) { | ||
const styles = useThemeStyles(); | ||
const {translate} = useLocalize(); | ||
const attachmentModalSource = tryResolveUrlFromApiRoot(image ?? ''); | ||
|
@@ -118,26 +107,14 @@ function ReportActionItemImage({ | |
return ( | ||
<ShowContextMenuContext.Consumer> | ||
{({report}) => ( | ||
<AttachmentModal | ||
source={attachmentModalSource} | ||
isAuthTokenRequired={!isLocalFile} | ||
report={report} | ||
isReceiptAttachment | ||
canEditReceipt={canEditReceipt} | ||
allowDownload={!isEReceipt} | ||
originalFileName={filename} | ||
<PressableWithoutFocus | ||
style={[styles.w100, styles.h100, styles.noOutline as ViewStyle]} | ||
onPress={() => Navigation.navigate(ROUTES.TRANSACTION_RECEIPT.getRoute(report?.reportID ?? '', transaction?.transactionID ?? ''))} | ||
accessibilityLabel={translate('accessibilityHints.viewAttachment')} | ||
accessibilityRole={CONST.ROLE.BUTTON} | ||
> | ||
{({show}) => ( | ||
<PressableWithoutFocus | ||
style={[styles.w100, styles.h100, styles.noOutline as ViewStyle]} | ||
onPress={show} | ||
accessibilityRole={CONST.ROLE.BUTTON} | ||
accessibilityLabel={translate('accessibilityHints.viewAttachment')} | ||
> | ||
{receiptImageComponent} | ||
</PressableWithoutFocus> | ||
)} | ||
</AttachmentModal> | ||
{receiptImageComponent} | ||
</PressableWithoutFocus> | ||
)} | ||
</ShowContextMenuContext.Consumer> | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
import type {StackScreenProps} from '@react-navigation/stack'; | ||
import React, {useEffect} from 'react'; | ||
import type {OnyxEntry} from 'react-native-onyx'; | ||
import {withOnyx} from 'react-native-onyx'; | ||
import AttachmentModal from '@components/AttachmentModal'; | ||
import Navigation from '@libs/Navigation/Navigation'; | ||
import type {AuthScreensParamList} from '@libs/Navigation/types'; | ||
import * as ReceiptUtils from '@libs/ReceiptUtils'; | ||
import * as ReportActionUtils from '@libs/ReportActionsUtils'; | ||
import * as ReportUtils from '@libs/ReportUtils'; | ||
import * as TransactionUtils from '@libs/TransactionUtils'; | ||
import tryResolveUrlFromApiRoot from '@libs/tryResolveUrlFromApiRoot'; | ||
import * as ReportActions from '@userActions/Report'; | ||
import CONST from '@src/CONST'; | ||
import ONYXKEYS from '@src/ONYXKEYS'; | ||
import ROUTES from '@src/ROUTES'; | ||
import type SCREENS from '@src/SCREENS'; | ||
import type {Report, ReportMetadata, Transaction} from '@src/types/onyx'; | ||
|
||
type TransactionReceiptOnyxProps = { | ||
report: OnyxEntry<Report>; | ||
transaction: OnyxEntry<Transaction>; | ||
reportMetadata: OnyxEntry<ReportMetadata>; | ||
}; | ||
|
||
type TransactionReceiptProps = TransactionReceiptOnyxProps & StackScreenProps<AuthScreensParamList, typeof SCREENS.TRANSACTION_RECEIPT>; | ||
|
||
function TransactionReceipt({transaction, report, reportMetadata = {isLoadingInitialReportActions: true}, route}: TransactionReceiptProps) { | ||
const receiptURIs = ReceiptUtils.getThumbnailAndImageURIs(transaction); | ||
|
||
const imageSource = tryResolveUrlFromApiRoot(receiptURIs.image); | ||
|
||
const isLocalFile = receiptURIs.isLocalFile; | ||
|
||
const parentReportAction = ReportActionUtils.getReportAction(report?.parentReportID ?? '', report?.parentReportActionID ?? ''); | ||
const canEditReceipt = ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, CONST.EDIT_REQUEST_FIELD.RECEIPT); | ||
const isEReceipt = transaction && TransactionUtils.hasEReceipt(transaction); | ||
|
||
useEffect(() => { | ||
if (report && transaction) { | ||
return; | ||
} | ||
ReportActions.openReport(route.params.reportID); | ||
// I'm disabling the warning, as it expects to use exhaustive deps, even though we want this useEffect to run only on the first render. | ||
// eslint-disable-next-line react-hooks/exhaustive-deps | ||
}, []); | ||
|
||
return ( | ||
<AttachmentModal | ||
source={imageSource} | ||
isAuthTokenRequired={!isLocalFile} | ||
report={report} | ||
isReceiptAttachment | ||
canEditReceipt={canEditReceipt} | ||
allowDownload={!isEReceipt} | ||
originalFileName={receiptURIs?.filename} | ||
defaultOpen | ||
onModalClose={() => { | ||
Navigation.goBack(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report?.reportID ?? '')); | ||
}} | ||
isLoading={!transaction && reportMetadata?.isLoadingInitialReportActions} | ||
shouldShowNotFoundPage={(report?.parentReportID ?? '') !== transaction?.reportID} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tienifr Is there a special reason to add this condition here ( This line is causing bugs in self DM track expense. |
||
/> | ||
); | ||
} | ||
|
||
TransactionReceipt.displayName = 'TransactionReceipt'; | ||
|
||
export default withOnyx<TransactionReceiptProps, TransactionReceiptOnyxProps>({ | ||
report: { | ||
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.reportID ?? '0'}`, | ||
}, | ||
transaction: { | ||
key: ({route}) => `${ONYXKEYS.COLLECTION.TRANSACTION}${route.params.transactionID ?? '0'}`, | ||
}, | ||
reportMetadata: { | ||
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_METADATA}${route.params.reportID ?? '0'}`, | ||
}, | ||
})(TransactionReceipt); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I updated