Skip to content

Commit 0ee935d

Browse files
authored
Merge pull request Expensify#63211 from dmkt9/fix/61983
Fix - Video file loads infinitely and stop playing when open using arrows
2 parents 458da0d + 257d9b4 commit 0ee935d

3 files changed

Lines changed: 69 additions & 26 deletions

File tree

src/components/Attachments/AttachmentCarousel/index.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ function AttachmentCarousel({report, attachmentID, source, onNavigate, setDownlo
108108
report={report}
109109
attachmentID={attachmentID}
110110
source={source}
111+
onNavigate={onNavigate}
111112
/>
112113
);
113114
}

src/components/VideoPlayerPreview/index.tsx

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import VideoPlayer from '@components/VideoPlayer';
99
import IconButton from '@components/VideoPlayer/IconButton';
1010
import {usePlaybackContext} from '@components/VideoPlayerContexts/PlaybackContext';
1111
import useCheckIfRouteHasRemainedUnchanged from '@hooks/useCheckIfRouteHasRemainedUnchanged';
12-
import useFirstRenderRoute from '@hooks/useFirstRenderRoute';
1312
import useLocalize from '@hooks/useLocalize';
1413
import useResponsiveLayout from '@hooks/useResponsiveLayout';
1514
import useThemeStyles from '@hooks/useThemeStyles';
@@ -66,9 +65,8 @@ function VideoPlayerPreview({videoUrl, thumbnailUrl, reportID, fileName, videoDi
6665
const {isOnSearch} = useSearchContext();
6766
const navigation = useNavigation();
6867

69-
const didUserNavigateOutOfReportScreen = useCheckIfRouteHasRemainedUnchanged();
70-
// We want to play the video only when the user is on the page where it was rendered
71-
const firstRenderRoute = useFirstRenderRoute(didUserNavigateOutOfReportScreen);
68+
// We want to play the video only when the user is on the page where it was initially rendered
69+
const doesUserRemainOnFirstRenderRoute = useCheckIfRouteHasRemainedUnchanged(videoUrl);
7270

7371
// `onVideoLoaded` is passed to VideoPlayerPreview's `Video` element which is displayed only on web.
7472
// VideoReadyForDisplayEvent type is lacking srcElement, that's why it's added here
@@ -85,14 +83,15 @@ function VideoPlayerPreview({videoUrl, thumbnailUrl, reportID, fileName, videoDi
8583

8684
useEffect(() => {
8785
return navigation.addListener('blur', () => !isOnAttachmentRoute() && setIsThumbnail(true));
88-
}, [navigation, firstRenderRoute]);
86+
}, [navigation]);
8987

9088
useEffect(() => {
91-
if (videoUrl !== currentlyPlayingURL || reportID !== currentRouteReportID || !firstRenderRoute.isFocused) {
89+
const isFocused = doesUserRemainOnFirstRenderRoute();
90+
if (videoUrl !== currentlyPlayingURL || reportID !== currentRouteReportID || !isFocused) {
9291
return;
9392
}
9493
setIsThumbnail(false);
95-
}, [currentlyPlayingURL, currentRouteReportID, updateCurrentURLAndReportID, videoUrl, reportID, firstRenderRoute, isOnSearch]);
94+
}, [currentlyPlayingURL, currentRouteReportID, updateCurrentURLAndReportID, videoUrl, reportID, doesUserRemainOnFirstRenderRoute, isOnSearch]);
9695

9796
return (
9897
<View style={[styles.webViewStyles.tagStyles.video, thumbnailDimensionsStyles]}>

src/hooks/useCheckIfRouteHasRemainedUnchanged.ts

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,88 @@
11
import {useNavigation} from '@react-navigation/native';
22
import {useCallback, useEffect, useRef} from 'react';
3-
import Navigation from '@navigation/Navigation';
4-
import ROUTES from '@src/ROUTES';
3+
import Navigation, {navigationRef} from '@navigation/Navigation';
4+
import SCREENS from '@src/SCREENS';
5+
import useResponsiveLayout from './useResponsiveLayout';
56

67
/**
7-
* Hook that returns a function to check if the currently active route remains the same as the last known route.
8-
* The last known route reference is updated every time the component experiences a 'blur' event,
9-
* except when opening an attachments modal, which is treated as an exception and does not trigger a reference update.
8+
* Hook that returns a function to check if the currently active route remains the same as the route on which the component was initially rendered.
9+
* The `isOnInitialRenderedRouteRef` is set to `false` every time the component experiences a 'blur' event,
10+
* except when opening an attachments modal, which is treated as an exception and does not trigger a reference update because the attachments modal display overlap and we want to use shared VideoPlayer.
1011
*
11-
* @return Function that checks if the last known route matches the currently active route.
12+
* @return Function that checks if the route where the component was initially rendered matches the current active route.
1213
*/
13-
function useCheckIfRouteHasRemainedUnchanged(): () => boolean {
14-
const lastKnownRouteRef = useRef<string | undefined>(undefined);
14+
function useCheckIfRouteHasRemainedUnchanged(videoUrl: string) {
15+
// Determines whether the component is still rendered on the initially rendered route.
16+
const isOnInitialRenderedRouteRef = useRef<boolean | undefined>(undefined);
1517
const navigation = useNavigation();
18+
const {shouldUseNarrowLayout, isInNarrowPaneModal} = useResponsiveLayout();
1619

17-
// Function to compare the last known route with the current active route
20+
/**
21+
* Return true only when on the initially rendered route or the video is currently playing in attachment modal.
22+
*/
1823
const hasRouteRemainedUnchanged = useCallback(() => {
19-
return lastKnownRouteRef.current === Navigation.getActiveRouteWithoutParams();
20-
}, []);
24+
if (navigation.isFocused()) {
25+
return true;
26+
}
27+
28+
// If navigating away from the initially rendered route or attachment route
29+
if (!isOnInitialRenderedRouteRef.current) {
30+
return false;
31+
}
32+
33+
// If on AttachmentModal, only play when the source parameters match videoUrl ensures correct play VideoPlayer share for this one
34+
const currentRoute = navigationRef.getCurrentRoute();
35+
if (
36+
currentRoute?.name === SCREENS.ATTACHMENTS &&
37+
currentRoute?.params &&
38+
'source' in currentRoute.params &&
39+
currentRoute.params.source === videoUrl &&
40+
// Because the video player is shared only on large screens
41+
// Allow in RHP in cases where we're in RHP on the Search page
42+
(!shouldUseNarrowLayout || isInNarrowPaneModal)
43+
) {
44+
return true;
45+
}
2146

22-
// Initialize the initial route when navigation is ready
47+
return false;
48+
}, [shouldUseNarrowLayout, isInNarrowPaneModal, videoUrl, navigation]);
49+
50+
// Initialize and check if starting with the attachment modal
2351
useEffect(() => {
2452
Navigation.isNavigationReady().then(() => {
25-
if (lastKnownRouteRef.current !== undefined) {
53+
if (isOnInitialRenderedRouteRef.current !== undefined) {
2654
return;
2755
}
2856

29-
lastKnownRouteRef.current = Navigation.getActiveRouteWithoutParams();
57+
const route = navigationRef.getCurrentRoute();
58+
// If the app is launched with the attachment route, it will always remain on the report screen.
59+
// Thus, it can be considered as still being on the rendered route.
60+
isOnInitialRenderedRouteRef.current = navigation.isFocused() || route?.name === SCREENS.ATTACHMENTS;
3061
});
62+
// eslint-disable-next-line react-compiler/react-compiler
63+
// eslint-disable-next-line react-hooks/exhaustive-deps
3164
}, []);
3265

33-
// Update the route reference on 'blur' events, except when opening attachments modal
3466
useEffect(() => {
35-
return navigation.addListener('blur', () => {
36-
const currentRoute = Navigation.getActiveRouteWithoutParams();
37-
if (currentRoute === `/${ROUTES.ATTACHMENTS.route}`) {
67+
const unsubscribeFocus = navigation.addListener('focus', () => {
68+
isOnInitialRenderedRouteRef.current = true;
69+
});
70+
71+
const unsubscribeBlur = navigation.addListener('blur', () => {
72+
const route = navigationRef.getCurrentRoute();
73+
74+
if (route?.name === SCREENS.ATTACHMENTS) {
3875
// Skip route update when attachment modal is opened
3976
return;
4077
}
41-
lastKnownRouteRef.current = currentRoute;
78+
79+
isOnInitialRenderedRouteRef.current = false;
4280
});
81+
82+
return () => {
83+
unsubscribeFocus();
84+
unsubscribeBlur();
85+
};
4386
}, [navigation]);
4487

4588
return hasRouteRemainedUnchanged;

0 commit comments

Comments
 (0)