Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix receipt preview in confirmation page #40763

Merged
merged 8 commits into from
May 6, 2024
Merged

Fix receipt preview in confirmation page #40763

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2efb5f3
Fix receipt preview in confirmation page
dukenv0307 Apr 23, 2024
810601f
Merge branch 'main' into fix/34120
dukenv0307 Apr 25, 2024
f788b80
fix distance request thumnail
dukenv0307 Apr 25, 2024
b723078
fix lint
dukenv0307 Apr 25, 2024
a437df0
Merge branch 'main' into fix/34120
dukenv0307 May 6, 2024
11bc78c
Update src/components/ReceiptImage.tsx
dukenv0307 May 6, 2024
9177dae
cast isDistanceRequest to boolean
dukenv0307 May 6, 2024
05360e9
Merge branch 'fix/34120' of https://github.com/dukenv0307/App into fi…
dukenv0307 May 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/components/DistanceEReceipt.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ function DistanceEReceipt({transaction}: DistanceEReceiptProps) {
<ReceiptImage
source={thumbnailSource}
shouldUseThumbnailImage
shouldUseInitialObjectPosition
/>
)}
</View>
Expand Down
62 changes: 38 additions & 24 deletions src/components/MoneyRequestConfirmationList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -949,30 +949,44 @@ function MoneyRequestConfirmationList({
const resolvedReceiptImage = isLocalFile ? receiptImage : tryResolveUrlFromApiRoot(receiptImage ?? '');

const receiptThumbnailContent = useMemo(
() =>
isLocalFile && Str.isPDF(receiptFilename) ? (
<PDFThumbnail
// eslint-disable-next-line @typescript-eslint/non-nullable-type-assertion-style
previewSourceURL={resolvedReceiptImage as string}
style={styles.moneyRequestImage}
// We don't support scaning password protected PDF receipt
enabled={!isAttachmentInvalid}
onPassword={() => setIsAttachmentInvalid(true)}
/>
) : (
<ReceiptImage
style={styles.moneyRequestImage}
isThumbnail={isThumbnail}
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
source={resolvedThumbnail || resolvedReceiptImage || ''}
// AuthToken is required when retrieving the image from the server
// but we don't need it to load the blob:// or file:// image when starting an expense/split
// So if we have a thumbnail, it means we're retrieving the image from the server
isAuthTokenRequired={!!receiptThumbnail}
fileExtension={fileExtension}
/>
),
[isLocalFile, receiptFilename, resolvedThumbnail, styles.moneyRequestImage, isAttachmentInvalid, isThumbnail, resolvedReceiptImage, receiptThumbnail, fileExtension],
() => (
<View style={styles.moneyRequestImage}>
{isLocalFile && Str.isPDF(receiptFilename) ? (
<PDFThumbnail
// eslint-disable-next-line @typescript-eslint/non-nullable-type-assertion-style
previewSourceURL={resolvedReceiptImage as string}
// We don't support scaning password protected PDF receipt
enabled={!isAttachmentInvalid}
onPassword={() => setIsAttachmentInvalid(true)}
/>
) : (
<ReceiptImage
isThumbnail={isThumbnail}
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
source={resolvedThumbnail || resolvedReceiptImage || ''}
// AuthToken is required when retrieving the image from the server
// but we don't need it to load the blob:// or file:// image when starting an expense/split
// So if we have a thumbnail, it means we're retrieving the image from the server
isAuthTokenRequired={!!receiptThumbnail && !isLocalFile}
fileExtension={fileExtension}
shouldUseThumbnailImage
shouldUseInitialObjectPosition={isDistanceRequest}
/>
)}
</View>
),
[
isLocalFile,
receiptFilename,
resolvedThumbnail,
styles.moneyRequestImage,
isAttachmentInvalid,
isThumbnail,
resolvedReceiptImage,
receiptThumbnail,
fileExtension,
isDistanceRequest,
],
);

return (
Expand Down
6 changes: 5 additions & 1 deletion src/components/ReceiptImage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ type ReceiptImageProps = (
/** Whether we should display the receipt with ThumbnailImage component */
shouldUseThumbnailImage?: boolean;

/** Whether we should display the receipt with inital object position */
dukenv0307 marked this conversation as resolved.
Show resolved Hide resolved
shouldUseInitialObjectPosition?: boolean;

/** Whether the receipt image requires an authToken */
isAuthTokenRequired?: boolean;

Expand Down Expand Up @@ -85,6 +88,7 @@ function ReceiptImage({
iconSize,
fallbackIcon,
fallbackIconSize,
shouldUseInitialObjectPosition = false,
}: ReceiptImageProps) {
const styles = useThemeStyles();

Expand Down Expand Up @@ -120,7 +124,7 @@ function ReceiptImage({
shouldDynamicallyResize={false}
fallbackIcon={fallbackIcon}
fallbackIconSize={fallbackIconSize}
objectPosition={CONST.IMAGE_OBJECT_POSITION.TOP}
objectPosition={shouldUseInitialObjectPosition ? CONST.IMAGE_OBJECT_POSITION.INITIAL : CONST.IMAGE_OBJECT_POSITION.TOP}
/>
);
}
Expand Down
3 changes: 3 additions & 0 deletions src/components/ReportActionItem/ReportActionItemImage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ function ReportActionItemImage({
const attachmentModalSource = tryResolveUrlFromApiRoot(image ?? '');
const thumbnailSource = tryResolveUrlFromApiRoot(thumbnail ?? '');
const isEReceipt = transaction && TransactionUtils.hasEReceipt(transaction);
const isDistanceRequest = transaction && TransactionUtils.isDistanceRequest(transaction);

let propsObj: ReceiptImageProps;

Expand All @@ -82,6 +83,7 @@ function ReportActionItemImage({
fallbackIcon: Expensicons.Receipt,
fallbackIconSize: isSingleImage ? variables.iconSizeSuperLarge : variables.iconSizeExtraLarge,
isAuthTokenRequired: true,
shouldUseInitialObjectPosition: isDistanceRequest ?? false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is ?? false needed here? Won't isDistanceRequest always be true or false?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

const isDistanceRequest = transaction && TransactionUtils.isDistanceRequest(transaction);

@Beamanator
Because isDistanceRequest can be null and undefined. I updated to cast this to boolean.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotta love JS :D I expected the && to do some kind of boolean casting for us, seems it doesn't haha

Screenshot 2024-05-06 at 10 11 28 AM

};
} else if (isLocalFile && filename && Str.isPDF(filename) && typeof attachmentModalSource === 'string') {
propsObj = {isPDFThumbnail: true, source: attachmentModalSource};
Expand All @@ -92,6 +94,7 @@ function ReportActionItemImage({
shouldUseThumbnailImage: true,
isAuthTokenRequired: false,
source: thumbnail ?? image ?? '',
shouldUseInitialObjectPosition: isDistanceRequest ?? false,
};
}

Expand Down
1 change: 1 addition & 0 deletions src/styles/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4201,6 +4201,7 @@ const styles = (theme: ThemeColors) =>
height: 200,
borderRadius: 16,
margin: 20,
overflow: 'hidden',
},

reportPreviewBox: {
Expand Down
Loading