Skip to content

Commit a75efa6

Browse files
authored
Merge pull request Expensify#81962 from daledah/fix/80421-1
fix: can't play video on a narrow viewport size on IOU report
2 parents a270cc3 + 9bb7e0c commit a75efa6

3 files changed

Lines changed: 86 additions & 13 deletions

File tree

src/components/VideoPlayer/BaseVideoPlayer.tsx

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,9 @@ function BaseVideoPlayer({
5454
onTap,
5555
}: VideoPlayerProps & {reportID: string}) {
5656
const styles = useThemeStyles();
57-
const {currentlyPlayingURL, sharedElement, originalParent, currentVideoPlayerRef, currentVideoViewRef, mountedVideoPlayersRef, playerStatus} = usePlaybackStateContext();
58-
const {pauseVideo, playVideo, replayVideo, shareVideoPlayerElements, updateCurrentURLAndReportID, setCurrentlyPlayingURL, updatePlayerStatus} = usePlaybackActionsContext();
57+
const {currentlyPlayingURL, sharedElement, originalParent, currentVideoPlayerRef, currentVideoViewRef, mountedVideoPlayersRef, playerStatus, shareVersion} = usePlaybackStateContext();
58+
const {pauseVideo, playVideo, replayVideo, shareVideoPlayerElements, updateCurrentURLAndReportID, setCurrentlyPlayingURL, updatePlayerStatus, requestDonorReRegistration} =
59+
usePlaybackActionsContext();
5960
const {isFullScreenRef} = useFullScreenState();
6061

6162
const isOffline = useNetwork().isOffline;
@@ -134,6 +135,12 @@ function BaseVideoPlayer({
134135
const videoPlayerElementRef = useRef<View | HTMLDivElement | null>(null);
135136
const sharedVideoPlayerParentRef = useRef<View | HTMLDivElement | null>(null);
136137
const isReadyForDisplayRef = useRef(false);
138+
const savedCurrentTimeRef = useRef(0);
139+
const shouldUseSharedVideoElementRef = useRef(shouldUseSharedVideoElement);
140+
// This needs to be updated synchronously during render (not in an effect) so that
141+
// cleanup functions of useLayoutEffect always read the latest value.
142+
// eslint-disable-next-line react-hooks/refs
143+
shouldUseSharedVideoElementRef.current = shouldUseSharedVideoElement;
137144
const canUseTouchScreen = canUseTouchScreenLib();
138145
const isCurrentlyURLSet = currentlyPlayingURL === url;
139146
const isUploading = CONST.ATTACHMENT_LOCAL_URL_PREFIX.some((prefix) => url.startsWith(prefix));
@@ -295,6 +302,13 @@ function BaseVideoPlayer({
295302
onSourceLoaded?.(event);
296303
});
297304

305+
useEffect(() => {
306+
if (currentTime <= 0) {
307+
return;
308+
}
309+
savedCurrentTimeRef.current = currentTime;
310+
}, [currentTime]);
311+
298312
useEffect(() => {
299313
if (!videoPlayerRef.current.duration) {
300314
return;
@@ -315,7 +329,11 @@ function BaseVideoPlayer({
315329
// ref url: https://reactjs.org/blog/2020/08/10/react-v17-rc.html#effect-cleanup-timing
316330
useLayoutEffect(
317331
() => () => {
318-
if (shouldUseSharedVideoElement || videoPlayerRef.current !== currentVideoPlayerRef.current) {
332+
// Use ref to read the latest value of shouldUseSharedVideoElement, preventing
333+
// destructive cleanup when this value changes during viewport resize.
334+
// Without the ref, the cleanup captures the stale (old) closure value and
335+
// incorrectly destroys the player when transitioning between shared/non-shared modes.
336+
if (shouldUseSharedVideoElementRef.current || videoPlayerRef.current !== currentVideoPlayerRef.current) {
319337
return;
320338
}
321339
if (currentVideoPlayerRef.current) {
@@ -324,7 +342,8 @@ function BaseVideoPlayer({
324342
currentVideoPlayerRef.current = null;
325343
}
326344
},
327-
[currentVideoPlayerRef, mountedVideoPlayersRef, shouldUseSharedVideoElement, url],
345+
// eslint-disable-next-line react-hooks/exhaustive-deps
346+
[currentVideoPlayerRef, mountedVideoPlayersRef, url],
328347
);
329348

330349
useEffect(() => {
@@ -346,15 +365,30 @@ function BaseVideoPlayer({
346365

347366
useEffect(
348367
() => () => {
349-
if (shouldUseSharedVideoElement || !isCurrentlyURLSetRef.current) {
368+
// Use ref to read the latest value of shouldUseSharedVideoElement, preventing
369+
// premature URL clearing when this value changes during viewport resize.
370+
if (shouldUseSharedVideoElementRef.current || !isCurrentlyURLSetRef.current) {
350371
return;
351372
}
352373

353374
setCurrentlyPlayingURL(null);
354375
},
355-
[setCurrentlyPlayingURL, shouldUseSharedVideoElement],
376+
// eslint-disable-next-line react-hooks/exhaustive-deps
377+
[setCurrentlyPlayingURL],
356378
);
357379

380+
// When transitioning from non-shared to shared mode (e.g. narrow → wide viewport), the stale
381+
// non-shared player refs remain in the context. Request all donors to re-register so the real
382+
// donor (e.g. the chat player) reclaims the context refs.
383+
const prevShouldUseSharedVideoElementRef = useRef(shouldUseSharedVideoElement);
384+
useEffect(() => {
385+
const wasShared = prevShouldUseSharedVideoElementRef.current;
386+
if (shouldUseSharedVideoElement && !wasShared) {
387+
requestDonorReRegistration();
388+
}
389+
prevShouldUseSharedVideoElementRef.current = shouldUseSharedVideoElement;
390+
}, [shouldUseSharedVideoElement, requestDonorReRegistration, url]);
391+
358392
// update shared video elements
359393
useEffect(() => {
360394
// On mobile safari, we need to auto-play when sharing video element here
@@ -373,6 +407,7 @@ function BaseVideoPlayer({
373407
currentlyPlayingURL,
374408
shouldUseSharedVideoElement,
375409
shareVideoPlayerElements,
410+
shareVersion,
376411
url,
377412
isUploading,
378413
reportID,
@@ -409,6 +444,11 @@ function BaseVideoPlayer({
409444
newParentRef.appendChild(sharedElement as HTMLDivElement);
410445
}
411446
}
447+
// Restore the playback position after moving the video element in the DOM.
448+
// Moving elements can reset currentTime to 0 in some browsers/video implementations.
449+
if (videoPlayerRef.current && savedCurrentTimeRef.current > 0 && videoPlayerRef.current.currentTime === 0) {
450+
videoPlayerRef.current.currentTime = savedCurrentTimeRef.current;
451+
}
412452
return () => {
413453
if (!originalParent || !('appendChild' in originalParent)) {
414454
return;

src/components/VideoPlayerContexts/PlaybackContext/index.tsx

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ function PlaybackContextProvider({children}: ChildrenProps) {
1717
const [sharedElement, setSharedElement] = useState<PlaybackStateContextValues['sharedElement']>(null);
1818
const [originalParent, setOriginalParent] = useState<OriginalParent>(null);
1919
const [currentRouteReportID, setCurrentRouteReportID] = useState<ProtectedCurrentRouteReportID>(NO_REPORT_ID);
20+
const [shareVersion, setShareVersion] = useState(0);
2021
const mountedVideoPlayersRef = useRef<string[]>([]);
2122
const playerStatus = useRef<VideoPlayerStatus>('loading');
2223

@@ -55,11 +56,10 @@ function PlaybackContextProvider({children}: ChildrenProps) {
5556
reportIDtoSet = reportID;
5657
}
5758

58-
const routeReportID = getCurrentRouteReportID(url);
59-
60-
if (reportIDtoSet === routeReportID || routeReportID === NO_REPORT_ID_IN_PARAMS) {
61-
setCurrentRouteReportID(reportIDtoSet);
62-
}
59+
// Always set currentRouteReportID so that shareVideoPlayerElements can match.
60+
// When the video is in a thread/child report but the focused route shows a parent report,
61+
// the IDs won't match. We still need to set it so video controls work properly.
62+
setCurrentRouteReportID(reportIDtoSet);
6363

6464
setCurrentlyPlayingURL(url);
6565
},
@@ -70,6 +70,14 @@ function PlaybackContextProvider({children}: ChildrenProps) {
7070
playerStatus.current = newStatus;
7171
}, []);
7272

73+
const requestDonorReRegistration = useCallback(() => {
74+
// Reset currentRouteReportID so the reportID guard in shareVideoPlayerElements is bypassed,
75+
// allowing the real donor (e.g. chat player) to re-register even if its reportID no longer
76+
// matches the stale currentRouteReportID left behind by the non-shared (narrow) player.
77+
setCurrentRouteReportID(NO_REPORT_ID);
78+
setShareVersion((v) => v + 1);
79+
}, []);
80+
7381
const shareVideoPlayerElements: PlaybackActionsContextValues['shareVideoPlayerElements'] = useCallback(
7482
(
7583
videoPlayerRef: VideoPlayer | null,
@@ -79,7 +87,11 @@ function PlaybackContextProvider({children}: ChildrenProps) {
7987
shouldNotAutoPlay: boolean,
8088
{shouldUseSharedVideoElement, url, reportID},
8189
) => {
82-
if (shouldUseSharedVideoElement || url !== currentlyPlayingURL || reportID !== currentRouteReportID) {
90+
// When currentRouteReportID is NO_REPORT_ID it means a forced re-registration was requested
91+
// (e.g. after narrow→wide resize). In that case we skip the reportID check so any non-shared
92+
// player whose URL matches can reclaim the context refs.
93+
const hasReportIDMismatch = currentRouteReportID !== NO_REPORT_ID && reportID !== currentRouteReportID;
94+
if (shouldUseSharedVideoElement || url !== currentlyPlayingURL || hasReportIDMismatch) {
8395
return;
8496
}
8597

@@ -106,7 +118,12 @@ function PlaybackContextProvider({children}: ChildrenProps) {
106118
const isSameReportID = routeReportID === currentRouteReportID || routeReportID === NO_REPORT_ID;
107119
const isOnRouteWithoutReportID = !!currentlyPlayingURL && getCurrentRouteReportID(currentlyPlayingURL) === NO_REPORT_ID_IN_PARAMS;
108120

109-
if (isSameReportID || isOnRouteWithoutReportID) {
121+
// Don't reset if the video URL is still mounted by an active player.
122+
// This prevents resetting when the route's reportID differs from the stored one
123+
// (e.g., video in a thread/child report while the route shows the parent report).
124+
const isURLStillMounted = !!currentlyPlayingURL && mountedVideoPlayersRef.current.includes(currentlyPlayingURL);
125+
126+
if (isSameReportID || isOnRouteWithoutReportID || isURLStillMounted) {
110127
return;
111128
}
112129

@@ -129,6 +146,7 @@ function PlaybackContextProvider({children}: ChildrenProps) {
129146
currentVideoViewRef: video.viewRef,
130147
mountedVideoPlayersRef,
131148
playerStatus,
149+
shareVersion,
132150
};
133151

134152
// Because of the React Compiler we don't need to memoize it manually
@@ -144,6 +162,7 @@ function PlaybackContextProvider({children}: ChildrenProps) {
144162
checkIfVideoIsPlaying: video.isPlaying,
145163
resetVideoPlayerData: video.resetPlayerData,
146164
updatePlayerStatus,
165+
requestDonorReRegistration,
147166
};
148167

149168
return (

src/components/VideoPlayerContexts/PlaybackContext/types.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,13 @@ type PlaybackStateContextValues = {
6969
* Status of the currently used Video Player
7070
*/
7171
playerStatus: RefObject<VideoPlayerStatus>;
72+
73+
/**
74+
* A counter that increments to signal all non-shared (donor) players to re-register their refs.
75+
* This is used when a player transitions from non-shared to shared mode, so the actual donor
76+
* (e.g. the chat player) can reclaim the context refs from the stale non-shared player.
77+
*/
78+
shareVersion: number;
7279
};
7380

7481
/**
@@ -110,6 +117,13 @@ type PlaybackActionsContextValues = {
110117
* @param newStatus New videoPlayer status
111118
*/
112119
updatePlayerStatus: (newStatus: VideoPlayerStatus) => void;
120+
121+
/**
122+
* Requests all non-shared (donor) video player instances to re-register their refs.
123+
* Should be called when a player transitions from non-shared to shared mode, so the real
124+
* donor can reclaim the context refs that were temporarily taken by the non-shared instance.
125+
*/
126+
requestDonorReRegistration: () => void;
113127
};
114128

115129
/**

0 commit comments

Comments
 (0)