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 - Attachment page keeps on loading infinitely. #39290

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
91daa59
implemented offline indicator for video
FitseTLT Mar 29, 2024
58cea0a
Merge branch 'main' into fix-attachement-offline-indicator
FitseTLT Apr 3, 2024
108740d
Merge branch 'main' into fix-attachement-offline-indicator
FitseTLT Apr 4, 2024
7b007e7
Created attachment offline indicator component
FitseTLT Apr 4, 2024
2a25277
Merge branch 'main' into fix-attachement-offline-indicator
FitseTLT Apr 4, 2024
211c18b
Merge branch 'main' into fix-attachement-offline-indicator
FitseTLT Apr 5, 2024
80a7113
did code cleanup
FitseTLT Apr 5, 2024
e6153f2
fix lint and type
FitseTLT Apr 5, 2024
cbf52fe
revert unnecessary change
FitseTLT Apr 5, 2024
b3f14d6
removed unnecessary view
FitseTLT Apr 5, 2024
17cb608
implemented for image view
FitseTLT Apr 5, 2024
29c89d7
implemented for thumbnail preview
FitseTLT Apr 5, 2024
da53445
minor fix
FitseTLT Apr 5, 2024
a2c8cbc
Merge branch 'main' into fix-attachement-offline-indicator
FitseTLT Apr 8, 2024
2a69717
implemented offline indicator for native
FitseTLT Apr 8, 2024
bf1786e
minor fix
FitseTLT Apr 8, 2024
841c61e
Merge branch 'main' into fix-attachement-offline-indicator
FitseTLT Apr 10, 2024
2a7fac5
revert light box change
FitseTLT Apr 10, 2024
bbfda4d
fix buffering case
FitseTLT Apr 10, 2024
6cb699d
fix touch screen case for image view
FitseTLT Apr 11, 2024
cbea4ac
fix logic for zoom scale
FitseTLT Apr 11, 2024
1142478
fix light box offline indicator
FitseTLT Apr 12, 2024
df8edb1
fix cached image case offline indicator
FitseTLT Apr 12, 2024
2d6fe9a
Merge branch 'main' into fix-attachement-offline-indicator
FitseTLT Apr 15, 2024
f083512
fix android loading indicator case
FitseTLT Apr 15, 2024
36ccc93
minor fix
FitseTLT Apr 15, 2024
75a772e
fix receipt offline indicator case
FitseTLT Apr 15, 2024
bd00d41
minor fix
FitseTLT Apr 15, 2024
851f387
Merge branch 'main' into fix-attachement-offline-indicator
FitseTLT Apr 23, 2024
0c01e8f
Merge branch 'main' into fix-attachement-offline-indicator
FitseTLT Apr 25, 2024
243dd47
Merge branch 'main' into fix-attachement-offline-indicator
FitseTLT May 2, 2024
23ed2e3
added delay for offline indicator
FitseTLT May 2, 2024
4de0a96
revert unnecessary change
FitseTLT May 2, 2024
99218b3
Merge branch 'main' into fix-attachement-offline-indicator
FitseTLT May 2, 2024
3e31bf8
Merge branch 'main' into fix-attachement-offline-indicator
FitseTLT May 7, 2024
1dd5ee4
add offline indicator for lightbox
FitseTLT May 7, 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
4 changes: 2 additions & 2 deletions src/components/AttachmentModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ function AttachmentModal({
}
const context = useMemo(
() => ({
pagerItems: [],
pagerItems: [{source: sourceForAttachmentView, index: 0, isActive: true}],
activePage: 0,
pagerRef: undefined,
isPagerScrolling: nope,
Expand All @@ -477,7 +477,7 @@ function AttachmentModal({
onScaleChanged: () => {},
onSwipeDown: closeModal,
}),
[closeModal, nope],
[closeModal, nope, sourceForAttachmentView],
);

return (
Expand Down
57 changes: 57 additions & 0 deletions src/components/AttachmentOfflineIndicator.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import React, {useEffect, useState} from 'react';
import {View} from 'react-native';
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import variables from '@styles/variables';
import Icon from './Icon';
import * as Expensicons from './Icon/Expensicons';
import Text from './Text';

type AttachmentOfflineIndicatorProps = {
/** Whether the offline indicator is displayed for the attachment preview. */
isPreview?: boolean;
};

function AttachmentOfflineIndicator({isPreview = false}: AttachmentOfflineIndicatorProps) {
const theme = useTheme();
const styles = useThemeStyles();
const {isOffline} = useNetwork();
const {translate} = useLocalize();

// We don't want to show the offline indicator when the attachment is a cached one, so
// we delay the display by 200 ms to ensure it is not a cached one.
const [onCacheDelay, setOnCacheDelay] = useState(true);

useEffect(() => {
const timeout = setTimeout(() => setOnCacheDelay(false), 200);

return () => clearTimeout(timeout);
}, []);

if (!isOffline || onCacheDelay) {
return null;
}

return (
<View style={[styles.flexColumn, styles.alignItemsCenter, styles.justifyContentCenter, styles.pAbsolute, styles.h100, styles.w100, isPreview && styles.hoveredComponentBG]}>
<Icon
fill={theme.border}
src={Expensicons.OfflineCloud}
width={variables.iconSizeSuperLarge}
height={variables.iconSizeSuperLarge}
/>
{!isPreview && (
<View>
<Text style={[styles.notFoundTextHeader]}>{translate('common.youAppearToBeOffline')}</Text>
<Text>{translate('common.attachementWillBeAvailableOnceBackOnline')}</Text>
</View>
)}
</View>
);
}

AttachmentOfflineIndicator.displayName = 'AttachmentOfflineIndicator';

export default AttachmentOfflineIndicator;
9 changes: 7 additions & 2 deletions src/components/ImageView/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ import type {SyntheticEvent} from 'react';
import React, {useCallback, useEffect, useRef, useState} from 'react';
import type {GestureResponderEvent, LayoutChangeEvent} from 'react-native';
import {View} from 'react-native';
import AttachmentOfflineIndicator from '@components/AttachmentOfflineIndicator';
import FullscreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import Image from '@components/Image';
import RESIZE_MODES from '@components/Image/resizeModes';
import type {ImageOnLoadEvent} from '@components/Image/types';
import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback';
import useNetwork from '@hooks/useNetwork';
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
import * as DeviceCapabilities from '@libs/DeviceCapabilities';
Expand All @@ -33,6 +35,7 @@ function ImageView({isAuthTokenRequired = false, url, fileName, onError}: ImageV
const [imgHeight, setImgHeight] = useState(0);
const [zoomScale, setZoomScale] = useState(0);
const [zoomDelta, setZoomDelta] = useState<ZoomDelta>();
const {isOffline} = useNetwork();

const scrollableRef = useRef<HTMLDivElement>(null);
const canUseTouchScreen = DeviceCapabilities.canUseTouchScreen();
Expand Down Expand Up @@ -210,7 +213,8 @@ function ImageView({isAuthTokenRequired = false, url, fileName, onError}: ImageV
onLoad={imageLoad}
onError={onError}
/>
{(isLoading || zoomScale === 0) && <FullscreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}
{((isLoading && !isOffline) || (!isLoading && zoomScale === 0)) && <FullscreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}
{isLoading && <AttachmentOfflineIndicator />}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should have also checked if the file is a localfile in case of offline upload, this caused #42657

</View>
);
}
Expand Down Expand Up @@ -243,7 +247,8 @@ function ImageView({isAuthTokenRequired = false, url, fileName, onError}: ImageV
/>
</PressableWithoutFeedback>

{isLoading && <FullscreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}
{isLoading && !isOffline && <FullscreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}
{isLoading && <AttachmentOfflineIndicator />}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same with ImageWithSizeCalculation.

It's noticeable with refreshing the page will show the offline indicator first.

Screen.Recording.2024-04-14.at.13.56.28.mov

</View>
);
}
Expand Down
4 changes: 3 additions & 1 deletion src/components/ImageWithSizeCalculation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import useNetwork from '@hooks/useNetwork';
import useThemeStyles from '@hooks/useThemeStyles';
import Log from '@libs/Log';
import CONST from '@src/CONST';
import AttachmentOfflineIndicator from './AttachmentOfflineIndicator';
import FullscreenLoadingIndicator from './FullscreenLoadingIndicator';
import Image from './Image';
import RESIZE_MODES from './Image/resizeModes';
Expand Down Expand Up @@ -108,7 +109,8 @@ function ImageWithSizeCalculation({url, style, onMeasure, onLoadFailure, isAuthT
onLoad={imageLoadedSuccessfully}
objectPosition={objectPosition}
/>
{isLoading && !isImageCached && <FullscreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}
{isLoading && !isImageCached && !isOffline && <FullscreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}
{isLoading && !isImageCached && <AttachmentOfflineIndicator isPreview />}
FitseTLT marked this conversation as resolved.
Show resolved Hide resolved
</View>
);
}
Expand Down
16 changes: 11 additions & 5 deletions src/components/Lightbox/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ import React, {useCallback, useContext, useEffect, useMemo, useState} from 'reac
import type {LayoutChangeEvent, StyleProp, ViewStyle} from 'react-native';
import {ActivityIndicator, PixelRatio, StyleSheet, View} from 'react-native';
import {useSharedValue} from 'react-native-reanimated';
import AttachmentOfflineIndicator from '@components/AttachmentOfflineIndicator';
import AttachmentCarouselPagerContext from '@components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext';
import Image from '@components/Image';
import type {ImageOnLoadEvent} from '@components/Image/types';
import MultiGestureCanvas, {DEFAULT_ZOOM_RANGE} from '@components/MultiGestureCanvas';
import type {CanvasSize, ContentSize, OnScaleChangedCallback, ZoomRange} from '@components/MultiGestureCanvas/types';
import {getCanvasFitScale} from '@components/MultiGestureCanvas/utils';
import useNetwork from '@hooks/useNetwork';
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
import NUMBER_OF_CONCURRENT_LIGHTBOXES from './numberOfConcurrentLightboxes';
Expand Down Expand Up @@ -47,6 +49,7 @@ function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChan
* we need to create a shared value that can be used in the render function.
*/
const isPagerScrollingFallback = useSharedValue(false);
const {isOffline} = useNetwork();

const attachmentCarouselPagerContext = useContext(AttachmentCarouselPagerContext);
const {
Expand Down Expand Up @@ -219,8 +222,8 @@ function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChan
style={[contentSize ?? styles.invisibleImage]}
isAuthTokenRequired={isAuthTokenRequired}
onError={onError}
onLoad={updateContentSize}
onLoadEnd={() => {
onLoad={(e) => {
updateContentSize(e);
setLightboxImageLoaded(true);
}}
/>
Expand All @@ -236,19 +239,22 @@ function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChan
resizeMode="contain"
style={[fallbackSize ?? styles.invisibleImage]}
isAuthTokenRequired={isAuthTokenRequired}
onLoad={updateContentSize}
onLoadEnd={() => setFallbackImageLoaded(true)}
onLoad={(e) => {
updateContentSize(e);
setFallbackImageLoaded(true);
}}
/>
</View>
)}

{/* Show activity indicator while the lightbox is still loading the image. */}
{isLoading && (
{isLoading && !isOffline && (
<ActivityIndicator
size="large"
style={StyleSheet.absoluteFill}
/>
)}
{isLoading && <AttachmentOfflineIndicator />}
</>
)}
</View>
Expand Down
7 changes: 4 additions & 3 deletions src/components/VideoPlayer/BaseVideoPlayer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {MutableRefObject} from 'react';
import React, {useCallback, useEffect, useLayoutEffect, useRef, useState} from 'react';
import type {GestureResponderEvent} from 'react-native';
import {View} from 'react-native';
import AttachmentOfflineIndicator from '@components/AttachmentOfflineIndicator';
import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import Hoverable from '@components/Hoverable';
import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback';
Expand Down Expand Up @@ -45,6 +46,7 @@ function BaseVideoPlayer({
// user hovers the mouse over the carousel arrows, but this UI bug feels much less troublesome for now.
// eslint-disable-next-line @typescript-eslint/no-unused-vars
isVideoHovered = false,
isPreview,
}: VideoPlayerProps) {
const styles = useThemeStyles();
const {
Expand Down Expand Up @@ -352,9 +354,8 @@ function BaseVideoPlayer({
</View>
)}
</PressableWithoutFeedback>

{(isLoading || isBuffering) && <FullScreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}

{((isLoading && !isOffline) || isBuffering) && <FullScreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}
{isLoading && !isBuffering && <AttachmentOfflineIndicator isPreview={isPreview} />}
{controlsStatus !== CONST.VIDEO_PLAYER.CONTROLS_STATUS.HIDE && !isLoading && (isPopoverVisible || isHovered || canUseTouchScreen) && (
<VideoPlayerControls
duration={duration}
Expand Down
1 change: 1 addition & 0 deletions src/components/VideoPlayer/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type VideoPlayerProps = {
shouldUseControlsBottomMargin?: boolean;
controlsStatus?: ValueOf<typeof CONST.VIDEO_PLAYER.CONTROLS_STATUS>;
shouldPlay?: boolean;
isPreview?: boolean;
};

export type {VideoPlayerProps, VideoWithOnFullScreenUpdate};
1 change: 1 addition & 0 deletions src/components/VideoPlayerPreview/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ function VideoPlayerPreview({videoUrl, thumbnailUrl, reportID, fileName, videoDi
videoDuration={videoDuration}
shouldUseSmallVideoControls
style={[styles.w100, styles.h100]}
isPreview
videoPlayerStyle={styles.videoPlayerPreview}
/>
<View style={[styles.pAbsolute, styles.w100]}>
Expand Down
1 change: 1 addition & 0 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ export default {
conciergeHelp: 'Please reach out to Concierge for help.',
youAppearToBeOffline: 'You appear to be offline.',
thisFeatureRequiresInternet: 'This feature requires an active internet connection to be used.',
attachementWillBeAvailableOnceBackOnline: 'Attachment will become available once back online.',
areYouSure: 'Are you sure?',
verify: 'Verify',
yesContinue: 'Yes, continue',
Expand Down
1 change: 1 addition & 0 deletions src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ export default {
conciergeHelp: 'Por favor, contacta con Concierge para obtener ayuda.',
youAppearToBeOffline: 'Parece que estás desconectado.',
thisFeatureRequiresInternet: 'Esta función requiere una conexión a Internet activa para ser utilizada.',
attachementWillBeAvailableOnceBackOnline: 'El archivo adjunto estará disponible cuando vuelvas a estar en línea.',
areYouSure: '¿Estás seguro?',
verify: 'Verifique',
yesContinue: 'Sí, continuar',
Expand Down
Loading