Skip to content

Commit cc0ebe5

Browse files
mountinymountiny
authored
Merge pull request #59896 from software-mansion-labs/korytko/video-fix-followup
Rewrite video player & its context logic
2 parents bc79c0f + f53c782 commit cc0ebe5

File tree

15 files changed

+399
-310
lines changed

15 files changed

+399
-310
lines changed

‎src/components/AttachmentModal.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,7 @@ function AttachmentModal({
616616
isUsedInAttachmentModal
617617
transactionID={transaction?.transactionID}
618618
isUploaded={!isEmptyObject(report)}
619+
reportID={!isEmptyObject(report) ? report.reportID : undefined}
619620
/>
620621
</AttachmentCarouselPagerContext.Provider>
621622
)

‎src/components/Attachments/AttachmentView/index.tsx

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import PerDiemEReceipt from '@components/PerDiemEReceipt';
1111
import ScrollView from '@components/ScrollView';
1212
import Text from '@components/Text';
1313
import {usePlaybackContext} from '@components/VideoPlayerContexts/PlaybackContext';
14+
import useFirstRenderRoute from '@hooks/useFirstRenderRoute';
1415
import useLocalize from '@hooks/useLocalize';
1516
import useNetwork from '@hooks/useNetwork';
1617
import useOnyx from '@hooks/useOnyx';
@@ -120,7 +121,7 @@ function AttachmentView({
120121
}: AttachmentViewProps) {
121122
const [transaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`);
122123
const {translate} = useLocalize();
123-
const {updateCurrentlyPlayingURL} = usePlaybackContext();
124+
const {updateCurrentURLAndReportID} = usePlaybackContext();
124125

125126
const theme = useTheme();
126127
const {safeAreaPaddingBottomStyle} = useSafeAreaPaddings();
@@ -130,13 +131,15 @@ function AttachmentView({
130131
const [isHighResolution, setIsHighResolution] = useState<boolean>(false);
131132
const [hasPDFFailedToLoad, setHasPDFFailedToLoad] = useState(false);
132133
const isVideo = (typeof source === 'string' && Str.isVideo(source)) || (file?.name && Str.isVideo(file.name));
134+
const firstRenderRoute = useFirstRenderRoute();
135+
const isInFocusedModal = firstRenderRoute.isFocused && isFocused === undefined;
133136

134137
useEffect(() => {
135-
if (!isFocused && !(file && isUsedInAttachmentModal)) {
138+
if (!isFocused && !isInFocusedModal && !(file && isUsedInAttachmentModal)) {
136139
return;
137140
}
138-
updateCurrentlyPlayingURL(isVideo && typeof source === 'string' ? source : null);
139-
}, [file, isFocused, isUsedInAttachmentModal, isVideo, source, updateCurrentlyPlayingURL]);
141+
updateCurrentURLAndReportID(isVideo && typeof source === 'string' ? source : undefined, reportID);
142+
}, [file, isFocused, isInFocusedModal, isUsedInAttachmentModal, isVideo, reportID, source, updateCurrentURLAndReportID]);
140143

141144
const [imageError, setImageError] = useState(false);
142145

‎src/components/HTMLEngineProvider/HTMLRenderers/VideoRenderer.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ function VideoRenderer({tnode, key}: VideoRendererProps) {
3131
<ShowContextMenuContext.Consumer>
3232
{({report}) => (
3333
<AttachmentContext.Consumer>
34-
{({accountID, type, hashKey}) => (
34+
{({accountID, type, hashKey, reportID}) => (
3535
<VideoPlayerPreview
3636
key={key}
3737
videoUrl={sourceURL}
38-
reportID={report?.reportID}
38+
reportID={reportID ?? report?.reportID}
3939
fileName={fileName}
4040
thumbnailUrl={thumbnailUrl}
4141
videoDimensions={{width, height}}

‎src/components/VideoPlayer/BaseVideoPlayer.tsx

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import AttachmentOfflineIndicator from '@components/AttachmentOfflineIndicator';
1111
import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
1212
import Hoverable from '@components/Hoverable';
1313
import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback';
14-
import {useSearchContext} from '@components/Search/SearchContext';
1514
import {useFullScreenContext} from '@components/VideoPlayerContexts/FullScreenContext';
1615
import {usePlaybackContext} from '@components/VideoPlayerContexts/PlaybackContext';
1716
import type {PlaybackSpeed} from '@components/VideoPlayerContexts/types';
@@ -65,10 +64,9 @@ function BaseVideoPlayer({
6564
originalParent,
6665
shareVideoPlayerElements,
6766
currentVideoPlayerRef,
68-
updateCurrentlyPlayingURL,
67+
updateCurrentURLAndReportID,
6968
videoResumeTryNumberRef,
7069
setCurrentlyPlayingURL,
71-
currentlyPlayingURLReportID,
7270
} = usePlaybackContext();
7371
const {isFullScreenRef} = useFullScreenContext();
7472
const {isOffline} = useNetwork();
@@ -107,19 +105,18 @@ function BaseVideoPlayer({
107105
const {videoPopoverMenuPlayerRef, currentPlaybackSpeed, setCurrentPlaybackSpeed, setSource: setPopoverMenuSource} = useVideoPopoverMenuContext();
108106
const {source} = videoPopoverMenuPlayerRef.current?.props ?? {};
109107
const shouldUseNewRate = typeof source === 'number' || !source || source.uri !== sourceURL;
110-
const {isOnSearch} = useSearchContext();
111108

112109
const togglePlayCurrentVideo = useCallback(() => {
113110
setIsEnded(false);
114111
videoResumeTryNumberRef.current = 0;
115112
if (!isCurrentlyURLSet) {
116-
updateCurrentlyPlayingURL(url);
113+
updateCurrentURLAndReportID(url, reportID);
117114
} else if (isPlaying) {
118115
pauseVideo();
119116
} else {
120117
playVideo();
121118
}
122-
}, [isCurrentlyURLSet, isPlaying, pauseVideo, playVideo, updateCurrentlyPlayingURL, url, videoResumeTryNumberRef]);
119+
}, [isCurrentlyURLSet, isPlaying, pauseVideo, playVideo, reportID, updateCurrentURLAndReportID, url, videoResumeTryNumberRef]);
123120

124121
const hideControl = useCallback(() => {
125122
if (isEnded) {
@@ -355,18 +352,15 @@ function BaseVideoPlayer({
355352

356353
// update shared video elements
357354
useEffect(() => {
358-
// in search page, we don't have active report id, so comparing reportID is not necessary
359-
if (shouldUseSharedVideoElement || url !== currentlyPlayingURL || (reportID !== currentlyPlayingURLReportID && !isOnSearch)) {
360-
return;
361-
}
362355
// On mobile safari, we need to auto-play when sharing video element here
363356
shareVideoPlayerElements(
364357
videoPlayerRef.current,
365358
videoPlayerElementParentRef.current,
366359
videoPlayerElementRef.current,
367360
isUploading || isFullScreenRef.current || (!isReadyForDisplayRef.current && !isMobileSafari()),
361+
{shouldUseSharedVideoElement, url, reportID},
368362
);
369-
}, [currentlyPlayingURL, shouldUseSharedVideoElement, shareVideoPlayerElements, url, isUploading, isFullScreenRef, reportID, currentlyPlayingURLReportID, isOnSearch]);
363+
}, [currentlyPlayingURL, shouldUseSharedVideoElement, shareVideoPlayerElements, url, isUploading, isFullScreenRef, reportID]);
370364

371365
// Call bindFunctions() through the refs to avoid adding it to the dependency array of the DOM mutation effect, as doing so would change the DOM when the functions update.
372366
const bindFunctionsRef = useRef<(() => void) | null>(null);
@@ -413,14 +407,14 @@ function BaseVideoPlayer({
413407
}
414408
newParentRef.childNodes[0]?.remove();
415409
};
416-
}, [currentVideoPlayerRef, currentlyPlayingURL, currentlyPlayingURLReportID, isFullScreenRef, originalParent, reportID, sharedElement, shouldUseSharedVideoElement, url]);
410+
}, [currentVideoPlayerRef, currentlyPlayingURL, isFullScreenRef, originalParent, reportID, sharedElement, shouldUseSharedVideoElement, url]);
417411

418412
useEffect(() => {
419413
if (!shouldPlay) {
420414
return;
421415
}
422-
updateCurrentlyPlayingURL(url);
423-
}, [shouldPlay, updateCurrentlyPlayingURL, url]);
416+
updateCurrentURLAndReportID(url, reportID);
417+
}, [reportID, shouldPlay, updateCurrentURLAndReportID, url]);
424418

425419
useEffect(() => {
426420
videoPlayerRef.current?.setStatusAsync({isMuted: true});
@@ -531,6 +525,7 @@ function BaseVideoPlayer({
531525
togglePlayCurrentVideo={togglePlayCurrentVideo}
532526
controlsStatus={controlStatusState}
533527
showPopoverMenu={showPopoverMenu}
528+
reportID={reportID}
534529
/>
535530
)}
536531
</View>

‎src/components/VideoPlayer/VideoPlayerControls/ProgressBar/index.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@ function getProgress(currentPosition: number, maxPosition: number): number {
2323

2424
function ProgressBar({duration, position, seekPosition}: ProgressBarProps) {
2525
const styles = useThemeStyles();
26-
const {pauseVideo, playVideo, checkVideoPlaying} = usePlaybackContext();
26+
const {pauseVideo, playVideo, checkIfVideoIsPlaying} = usePlaybackContext();
2727
const [sliderWidth, setSliderWidth] = useState(1);
2828
const [isSliderPressed, setIsSliderPressed] = useState(false);
2929
const progressWidth = useSharedValue(0);
3030
const wasVideoPlayingOnCheck = useSharedValue(false);
3131

32-
const onCheckVideoPlaying = (isPlaying: boolean) => {
32+
const onCheckIfVideoIsPlaying = (isPlaying: boolean) => {
3333
wasVideoPlayingOnCheck.set(isPlaying);
3434
};
3535

@@ -47,7 +47,7 @@ function ProgressBar({duration, position, seekPosition}: ProgressBarProps) {
4747
.runOnJS(true)
4848
.onBegin((event) => {
4949
setIsSliderPressed(true);
50-
checkVideoPlaying(onCheckVideoPlaying);
50+
checkIfVideoIsPlaying(onCheckIfVideoIsPlaying);
5151
pauseVideo();
5252
progressBarInteraction(event);
5353
})

‎src/components/VideoPlayer/VideoPlayerControls/index.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ type VideoPlayerControlsProps = {
4646
togglePlayCurrentVideo: (event?: GestureResponderEvent | KeyboardEvent) => void | Promise<void>;
4747

4848
controlsStatus: ValueOf<typeof CONST.VIDEO_PLAYER.CONTROLS_STATUS>;
49+
50+
reportID: string | undefined;
4951
};
5052

5153
function VideoPlayerControls({
@@ -59,10 +61,11 @@ function VideoPlayerControls({
5961
showPopoverMenu,
6062
togglePlayCurrentVideo,
6163
controlsStatus = CONST.VIDEO_PLAYER.CONTROLS_STATUS.SHOW,
64+
reportID,
6265
}: VideoPlayerControlsProps) {
6366
const styles = useThemeStyles();
6467
const {translate} = useLocalize();
65-
const {updateCurrentlyPlayingURL} = usePlaybackContext();
68+
const {updateCurrentURLAndReportID} = usePlaybackContext();
6669
const {isFullScreenRef} = useFullScreenContext();
6770
const [shouldShowTime, setShouldShowTime] = useState(false);
6871
const iconSpacing = small ? styles.mr3 : styles.mr4;
@@ -74,9 +77,9 @@ function VideoPlayerControls({
7477
const enterFullScreenMode = useCallback(() => {
7578
// eslint-disable-next-line react-compiler/react-compiler
7679
isFullScreenRef.current = true;
77-
updateCurrentlyPlayingURL(url);
80+
updateCurrentURLAndReportID(url, reportID);
7881
videoPlayerRef.current?.presentFullscreenPlayer();
79-
}, [isFullScreenRef, updateCurrentlyPlayingURL, url, videoPlayerRef]);
82+
}, [isFullScreenRef, reportID, updateCurrentURLAndReportID, url, videoPlayerRef]);
8083

8184
const seekPosition = useCallback(
8285
(newPosition: number) => {

0 commit comments

Comments
 (0)