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.

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 blank area after loading pdf #38010

Merged
merged 60 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
ffb21f1
add POC for fallingback to default attachment view when pdf fails to …
ishpaul777 Mar 8, 2024
bde48fa
adds chnages for attachement view
ishpaul777 Mar 8, 2024
9323594
Merge branch 'fix/blankArea-for-corruptedPDF' into fix-blankArea-afte…
ishpaul777 Mar 8, 2024
757661b
formatting
ishpaul777 Mar 8, 2024
c4fce87
Merge branch 'fix-blankArea-after-loading-pdf' of https://github.com/…
ishpaul777 Mar 8, 2024
26c1f13
fixes conflicts
ishpaul777 Mar 8, 2024
36b5814
fix conflict after merge
ishpaul777 Mar 8, 2024
f7b6cd5
Merge branch 'Expensify:main' into fix-blankArea-after-loading-pdf
ishpaul777 Mar 13, 2024
75a733c
clean up and lint fix
ishpaul777 Mar 13, 2024
1a84b64
Merge branch 'Expensify:main' into fix-blankArea-after-loading-pdf
ishpaul777 Mar 15, 2024
e2b9966
add POC for fallingback to default attachment view when pdf fails to …
ishpaul777 Mar 8, 2024
02f26eb
fixes missing prop
ishpaul777 Mar 15, 2024
2d37945
Merge branch 'fix-blankArea-after-loading-pdf' of https://github.com/…
ishpaul777 Mar 15, 2024
b4ac9a1
remove unused import
ishpaul777 Mar 15, 2024
0372601
change variable name
ishpaul777 Mar 15, 2024
a20811a
fixes lint
ishpaul777 Mar 15, 2024
3ace4d3
use correct conditional and use js docs for props
ishpaul777 Mar 15, 2024
830a392
prettier diffs
ishpaul777 Mar 15, 2024
3813026
Update src/components/Attachments/AttachmentCarousel/CarouselItem.js
ishpaul777 Mar 15, 2024
a8cfafe
remove unused styles
ishpaul777 Mar 15, 2024
5199de9
Merge branch 'Expensify:main' into fix-blankArea-after-loading-pdf
ishpaul777 Mar 16, 2024
ccbcfe0
Merge branch 'Expensify:main' into fix-blankArea-after-loading-pdf
ishpaul777 Mar 19, 2024
a45f6f0
remove error label styles
ishpaul777 Mar 19, 2024
1f489cf
Merge branch 'fix-blankArea-after-loading-pdf' of https://github.com/…
ishpaul777 Mar 19, 2024
cb7ba78
Merge branch 'main' into fix-blankArea-after-loading-pdf
ishpaul777 Mar 20, 2024
81efdb2
Merge branch 'main' into fix-blankArea-after-loading-pdf
ishpaul777 Mar 28, 2024
32f3503
fix conflicts
ishpaul777 Mar 28, 2024
1c814c5
fix lint
ishpaul777 Mar 28, 2024
302fdba
fallback to defaultview component
ishpaul777 Mar 28, 2024
3b51760
fix prop pass
ishpaul777 Mar 28, 2024
7d5d22a
prettier diffs
ishpaul777 Mar 28, 2024
088813d
Merge branch 'Expensify:main' into fix-blankArea-after-loading-pdf
ishpaul777 Mar 29, 2024
0af56c6
remove renderfallbackview prop
ishpaul777 Mar 29, 2024
b18b06a
fix the comment for the prop
ishpaul777 Mar 29, 2024
6436ddb
fixes loading for pdf on web
ishpaul777 Mar 29, 2024
359c54d
fixes proptype warning
ishpaul777 Mar 29, 2024
691e2a8
prettier diffs
ishpaul777 Mar 29, 2024
2b1bff7
Merge branch 'Expensify:main' into fix-blankArea-after-loading-pdf
ishpaul777 Apr 2, 2024
9bd4c96
adds onError prop in react-fast-pdf
ishpaul777 Apr 2, 2024
7003564
Merge branch 'main' into fix-blankArea-after-loading-pdf
ishpaul777 Apr 6, 2024
e714b2f
resolve conflicts
ishpaul777 Apr 6, 2024
15eea96
code formatting
ishpaul777 Apr 6, 2024
7a9565a
remove unnecessary change
ishpaul777 Apr 6, 2024
6912979
fix type checks
ishpaul777 Apr 6, 2024
917095c
lint code
ishpaul777 Apr 6, 2024
c968147
prettier diffs
ishpaul777 Apr 6, 2024
cadf1f1
resolve lint failure
ishpaul777 Apr 6, 2024
54eb295
prettier diffs
ishpaul777 Apr 6, 2024
3044ee9
Merge branch 'Expensify:main' into fix-blankArea-after-loading-pdf
ishpaul777 Apr 11, 2024
9c1527b
remove unnecessary styles
ishpaul777 Apr 11, 2024
3c871f1
Merge branch 'Expensify:main' into fix-blankArea-after-loading-pdf
ishpaul777 Apr 16, 2024
0338f8d
merge main
ishpaul777 May 3, 2024
838e465
prettier diffs
ishpaul777 May 3, 2024
17ed872
Merge branch 'Expensify:main' into fix-blankArea-after-loading-pdf
ishpaul777 May 13, 2024
3763d32
fixes preview not centered in carousel
ishpaul777 May 13, 2024
da41661
fix loading component
ishpaul777 May 13, 2024
c134239
prettier
ishpaul777 May 13, 2024
9c1b2c4
Merge branch 'Expensify:main' into fix-blankArea-after-loading-pdf
ishpaul777 May 15, 2024
e1d69f6
Add-empty-newlines
ishpaul777 May 15, 2024
569b7c1
adds requested changes
ishpaul777 May 15, 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
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ function BaseAnchorForAttachmentsOnly({style, source = '', displayName = '', dow
file={{name: displayName}}
shouldShowDownloadIcon={!isOffline}
shouldShowLoadingSpinnerIcon={isDownloading}
isUsedAsChatAttachment
/>
</PressableWithoutFeedback>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ function CarouselItem({item, onPress, isFocused, isModalHovered}: CarouselItemPr

return (
<View style={[styles.flex1]}>
<View style={[styles.flex1]}>
<View style={[styles.imageModalImageCenterContainer]}>
<AttachmentView
source={item.source}
file={item.file}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ function BaseAttachmentViewPdf({
onScaleChanged: onScaleChangedProp,
onToggleKeyboard,
onLoadComplete,
errorLabelStyles,
style,
isUsedAsChatAttachment,
onLoadError,
}: AttachmentViewPdfProps) {
const attachmentCarouselPagerContext = useContext(AttachmentCarouselPagerContext);
const isScrollEnabled = attachmentCarouselPagerContext === null ? undefined : attachmentCarouselPagerContext.isScrollEnabled;
Expand Down Expand Up @@ -75,7 +76,8 @@ function BaseAttachmentViewPdf({
onToggleKeyboard={onToggleKeyboard}
onScaleChanged={onScaleChanged}
onLoadComplete={onLoadComplete}
errorLabelStyles={errorLabelStyles}
isUsedAsChatAttachment={isUsedAsChatAttachment}
onLoadError={onLoadError}
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, {memo} from 'react';
import PDFView from '@components/PDFView';
import type AttachmentViewPdfProps from './types';

function AttachmentViewPdf({file, encryptedSourceUrl, isFocused, onPress, onToggleKeyboard, onLoadComplete, errorLabelStyles, style}: AttachmentViewPdfProps) {
function AttachmentViewPdf({file, encryptedSourceUrl, isFocused, onPress, onToggleKeyboard, onLoadComplete, style, isUsedAsChatAttachment, onLoadError}: AttachmentViewPdfProps) {
return (
<PDFView
onPress={onPress}
Expand All @@ -12,7 +12,8 @@ function AttachmentViewPdf({file, encryptedSourceUrl, isFocused, onPress, onTogg
style={style}
onToggleKeyboard={onToggleKeyboard}
onLoadComplete={onLoadComplete}
errorLabelStyles={errorLabelStyles}
isUsedAsChatAttachment={isUsedAsChatAttachment}
onLoadError={onLoadError}
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type {StyleProp, TextStyle, ViewStyle} from 'react-native';
import type {StyleProp, ViewStyle} from 'react-native';
import type {AttachmentViewProps} from '..';

type AttachmentViewPdfProps = Pick<AttachmentViewProps, 'file' | 'onPress' | 'isUsedInCarousel' | 'isFocused' | 'onToggleKeyboard'> & {
Expand All @@ -8,11 +8,14 @@ type AttachmentViewPdfProps = Pick<AttachmentViewProps, 'file' | 'onPress' | 'is
/** Additional style props */
style?: StyleProp<ViewStyle>;

/** Styles for the error label */
errorLabelStyles?: StyleProp<TextStyle>;

/** Triggered when the PDF's onScaleChanged event is triggered */
onScaleChanged?: (scale: number) => void;

/** Triggered when the PDF fails to load */
onLoadError?: () => void;

/** Whether the PDF is used as a chat attachment */
isUsedAsChatAttachment?: boolean;
};

export default AttachmentViewPdfProps;
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import React from 'react';
import type {StyleProp, ViewStyle} from 'react-native';
import {ActivityIndicator, View} from 'react-native';
import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import Text from '@components/Text';
import Tooltip from '@components/Tooltip';
import useLocalize from '@hooks/useLocalize';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';

type DefaultAttachmentViewProps = {
/** The name of the file */
fileName?: string;

/** Should show the download icon */
shouldShowDownloadIcon?: boolean;

/** Should show the loading spinner icon */
shouldShowLoadingSpinnerIcon?: boolean;

/** Additional styles for the container */
containerStyles?: StyleProp<ViewStyle>;
};

function DefaultAttachmentView({fileName = '', shouldShowLoadingSpinnerIcon = false, shouldShowDownloadIcon, containerStyles}: DefaultAttachmentViewProps) {
const theme = useTheme();
const styles = useThemeStyles();
const {translate} = useLocalize();

return (
<View style={[styles.defaultAttachmentView, containerStyles]}>
<View style={styles.mr2}>
<Icon
fill={theme.icon}
src={Expensicons.Paperclip}
/>
</View>

<Text style={[styles.textStrong, styles.flexShrink1, styles.breakAll, styles.flexWrap, styles.mw100]}>{fileName}</Text>
{!shouldShowLoadingSpinnerIcon && shouldShowDownloadIcon && (
<Tooltip text={translate('common.download')}>
<View style={styles.ml2}>
<Icon
fill={theme.icon}
src={Expensicons.Download}
/>
</View>
</Tooltip>
)}
{shouldShowLoadingSpinnerIcon && (
<View style={styles.ml2}>
<Tooltip text={translate('common.downloading')}>
<ActivityIndicator
size="small"
color={theme.textSupporting}
/>
</Tooltip>
</View>
)}
</View>
);
}

DefaultAttachmentView.displayName = 'DefaultAttachmentView';

export default DefaultAttachmentView;
ishpaul777 marked this conversation as resolved.
Show resolved Hide resolved
60 changes: 24 additions & 36 deletions src/components/Attachments/AttachmentView/index.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
import Str from 'expensify-common/lib/str';
import React, {memo, useEffect, useState} from 'react';
import type {GestureResponderEvent, StyleProp, ViewStyle} from 'react-native';
import {ActivityIndicator, View} from 'react-native';
import {View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import type {Attachment, AttachmentSource} from '@components/Attachments/types';
import DistanceEReceipt from '@components/DistanceEReceipt';
import EReceipt from '@components/EReceipt';
import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import ScrollView from '@components/ScrollView';
import Text from '@components/Text';
import Tooltip from '@components/Tooltip';
import {usePlaybackContext} from '@components/VideoPlayerContexts/PlaybackContext';
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
Expand All @@ -28,6 +25,7 @@ import type {Transaction} from '@src/types/onyx';
import AttachmentViewImage from './AttachmentViewImage';
import AttachmentViewPdf from './AttachmentViewPdf';
import AttachmentViewVideo from './AttachmentViewVideo';
import DefaultAttachmentView from './DefaultAttachmentView';

type AttachmentViewOnyxProps = {
transaction: OnyxEntry<Transaction>;
Expand Down Expand Up @@ -64,9 +62,14 @@ type AttachmentViewProps = AttachmentViewOnyxProps &
/** Denotes whether it is an icon (ex: SVG) */
maybeIcon?: boolean;

/** Fallback source to use in case of error */
fallbackSource?: AttachmentSource;

/* Whether it is hovered or not */
isHovered?: boolean;

/** Whether the attachment is used as a chat attachment */
isUsedAsChatAttachment?: boolean;
};

function AttachmentView({
Expand All @@ -88,13 +91,15 @@ function AttachmentView({
reportActionID,
isHovered,
duration,
isUsedAsChatAttachment,
}: AttachmentViewProps) {
const {translate} = useLocalize();
const {updateCurrentlyPlayingURL} = usePlaybackContext();
const theme = useTheme();
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
const [loadComplete, setLoadComplete] = useState(false);
const [hasPDFFailedToLoad, setHasPDFFailedToLoad] = useState(false);
const isVideo = (typeof source === 'string' && Str.isVideo(source)) || (file?.name && Str.isVideo(file.name));

useEffect(() => {
Expand Down Expand Up @@ -145,7 +150,9 @@ function AttachmentView({

// Check both source and file.name since PDFs dragged into the text field
// will appear with a source that is a blob
if ((typeof source === 'string' && Str.isPDF(source)) || (file && Str.isPDF(file.name ?? translate('attachmentView.unknownFilename')))) {
const isSourcePDF = typeof source === 'string' && Str.isPDF(source);
const isFilePDF = file && Str.isPDF(file.name ?? translate('attachmentView.unknownFilename'));
if (!hasPDFFailedToLoad && (isSourcePDF || isFilePDF)) {
const encryptedSourceUrl = isAuthTokenRequired ? addEncryptedAuthTokenToURL(source as string) : (source as string);

const onPDFLoadComplete = (path: string) => {
Expand All @@ -158,6 +165,10 @@ function AttachmentView({
}
};

const onPDFLoadError = () => {
setHasPDFFailedToLoad(true);
};

// We need the following View component on android native
// So that the event will propagate properly and
// the Password protected preview will be shown for pdf attachement we are about to send.
Expand All @@ -170,8 +181,9 @@ function AttachmentView({
onPress={onPress}
onToggleKeyboard={onToggleKeyboard}
onLoadComplete={onPDFLoadComplete}
errorLabelStyles={isUsedInAttachmentModal ? [styles.textLabel, styles.textLarge] : [styles.cursorAuto]}
style={isUsedInAttachmentModal ? styles.imageModalPDF : styles.flex1}
isUsedAsChatAttachment={isUsedAsChatAttachment}
onLoadError={onPDFLoadError}
/>
</View>
);
Expand Down Expand Up @@ -228,36 +240,12 @@ function AttachmentView({
}

return (
<View style={[styles.defaultAttachmentView, containerStyles]}>
<View style={styles.mr2}>
<Icon
fill={theme.icon}
src={Expensicons.Paperclip}
/>
</View>

<Text style={[styles.textStrong, styles.flexShrink1, styles.breakAll, styles.flexWrap, styles.mw100]}>{file?.name}</Text>
{!shouldShowLoadingSpinnerIcon && shouldShowDownloadIcon && (
<Tooltip text={translate('common.download')}>
<View style={styles.ml2}>
<Icon
fill={theme.icon}
src={Expensicons.Download}
/>
</View>
</Tooltip>
)}
{shouldShowLoadingSpinnerIcon && (
<View style={styles.ml2}>
<Tooltip text={translate('common.downloading')}>
<ActivityIndicator
size="small"
color={theme.textSupporting}
/>
</Tooltip>
</View>
)}
</View>
<DefaultAttachmentView
fileName={file?.name}
shouldShowDownloadIcon={shouldShowDownloadIcon}
shouldShowLoadingSpinnerIcon={shouldShowLoadingSpinnerIcon}
containerStyles={containerStyles}
/>
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ type BaseInvertedFlatListProps<T> = FlatListProps<T> & {
shouldEnableAutoScrollToTopThreshold?: boolean;
};

const AUTOSCROLL_TO_TOP_THRESHOLD = 128;
const AUTOSCROLL_TO_TOP_THRESHOLD = 250;

function BaseInvertedFlatList<T>(props: BaseInvertedFlatListProps<T>, ref: ForwardedRef<RNFlatList>) {
const {shouldEnableAutoScrollToTopThreshold, ...rest} = props;
Expand Down
27 changes: 16 additions & 11 deletions src/components/PDFView/index.native.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import PDF from 'react-native-pdf';
import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import KeyboardAvoidingView from '@components/KeyboardAvoidingView';
import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback';
import Text from '@components/Text';
import useKeyboardState from '@hooks/useKeyboardState';
import useLocalize from '@hooks/useLocalize';
import useStyleUtils from '@hooks/useStyleUtils';
Expand All @@ -29,7 +28,11 @@ import type {PDFViewNativeProps} from './types';
* so that PDFPasswordForm doesn't bounce when react-native-pdf/PDF
* is (temporarily) rendered.
*/
function PDFView({onToggleKeyboard, onLoadComplete, fileName, onPress, isFocused, onScaleChanged, sourceURL, errorLabelStyles}: PDFViewNativeProps) {

const LOADING_THUMBNAIL_HEIGHT = 250;
const LOADING_THUMBNAIL_WIDTH = 250;

function PDFView({onToggleKeyboard, onLoadComplete, fileName, onPress, isFocused, onScaleChanged, sourceURL, onLoadError, isUsedAsChatAttachment}: PDFViewNativeProps) {
const [shouldRequestPassword, setShouldRequestPassword] = useState(false);
const [shouldAttemptPDFLoad, setShouldAttemptPDFLoad] = useState(true);
const [shouldShowLoadingIndicator, setShouldShowLoadingIndicator] = useState(true);
Expand Down Expand Up @@ -80,6 +83,7 @@ function PDFView({onToggleKeyboard, onLoadComplete, fileName, onPress, isFocused
setShouldShowLoadingIndicator(false);
setShouldRequestPassword(false);
setShouldAttemptPDFLoad(false);
onLoadError?.();
ishpaul777 marked this conversation as resolved.
Show resolved Hide resolved
// eslint-disable-next-line @typescript-eslint/ban-types
}) as (error: object) => void;

Expand Down Expand Up @@ -112,7 +116,9 @@ function PDFView({onToggleKeyboard, onLoadComplete, fileName, onPress, isFocused
};

function renderPDFView() {
const pdfStyles: StyleProp<ViewStyle> = [themeStyles.imageModalPDF, StyleUtils.getWidthAndHeightStyle(windowWidth, windowHeight)];
const pdfWidth = isUsedAsChatAttachment ? LOADING_THUMBNAIL_WIDTH : windowWidth;
const pdfHeight = isUsedAsChatAttachment ? LOADING_THUMBNAIL_HEIGHT : windowHeight;
const pdfStyles: StyleProp<ViewStyle> = [themeStyles.imageModalPDF, StyleUtils.getWidthAndHeightStyle(pdfWidth, pdfHeight)];

// If we haven't yet successfully validated the password and loaded the PDF,
// then we need to hide the react-native-pdf/PDF component so that PDFPasswordForm
Expand All @@ -121,21 +127,20 @@ function PDFView({onToggleKeyboard, onLoadComplete, fileName, onPress, isFocused
if (shouldRequestPassword) {
pdfStyles.push(themeStyles.invisible);
}

const containerStyles = shouldRequestPassword && isSmallScreenWidth ? [themeStyles.w100, themeStyles.flex1] : [themeStyles.alignItemsCenter, themeStyles.flex1];
const containerStyles =
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
isUsedAsChatAttachment || (shouldRequestPassword && isSmallScreenWidth) ? [themeStyles.w100, themeStyles.flex1] : [themeStyles.alignItemsCenter, themeStyles.flex1];
const loadingIndicatorStyles = isUsedAsChatAttachment
? [themeStyles.chatItemPDFAttachmentLoading, StyleUtils.getWidthAndHeightStyle(LOADING_THUMBNAIL_WIDTH, LOADING_THUMBNAIL_HEIGHT)]
: [];

return (
<View style={containerStyles}>
{failedToLoadPDF && (
<View style={[themeStyles.flex1, themeStyles.justifyContentCenter]}>
<Text style={errorLabelStyles}>{translate('attachmentView.failedToLoadPDF')}</Text>
</View>
)}
{shouldAttemptPDFLoad && (
<PDF
fitPolicy={0}
trustAllCerts={false}
renderActivityIndicator={() => <FullScreenLoadingIndicator />}
renderActivityIndicator={() => <FullScreenLoadingIndicator style={loadingIndicatorStyles} />}
source={{uri: sourceURL, cache: true, expiration: 864000}}
style={pdfStyles}
onError={handleFailureToLoadPDF}
Expand Down
Loading
Loading