Addressing review comments

Expensify · Feb 28, 2024 · 6f92384 · 6f92384
1 parent 90affed
commit 6f92384
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 15 deletions.
diff --git a/src/components/VideoPlayerContexts/PlaybackContext.tsx b/src/components/VideoPlayerContexts/PlaybackContext.tsx
@@ -1,9 +1,9 @@
-import type {Video} from 'expo-av';
+import type {AVPlaybackStatusToSet, Video} from 'expo-av';
 import React, {useCallback, useContext, useEffect, useMemo, useRef, useState} from 'react';
 import type {View} from 'react-native';
 import useCurrentReportID from '@hooks/useCurrentReportID';
 import type ChildrenProps from '@src/types/utils/ChildrenProps';
-import type {PlaybackContext} from './types';
+import type {PlaybackContext, StatusCallback} from './types';
 
 const Context = React.createContext<PlaybackContext | null>(null);
 
@@ -19,15 +19,16 @@ function PlaybackContextProvider({children}: ChildrenProps) {
     }, [currentVideoPlayerRef]);
 
     const stopVideo = useCallback(() => {
-        currentVideoPlayerRef.current?.stopAsync?.();
+        currentVideoPlayerRef.current?.stopAsync();
     }, [currentVideoPlayerRef]);
 
     const playVideo = useCallback(() => {
-        currentVideoPlayerRef.current?.getStatusAsync?.().then((status) => {
+        currentVideoPlayerRef.current?.getStatusAsync().then((status) => {
+            const newStatus: AVPlaybackStatusToSet = {shouldPlay: true};
             if ('durationMillis' in status && status.durationMillis === status.positionMillis) {
-                currentVideoPlayerRef.current?.setStatusAsync({shouldPlay: true, positionMillis: 0});
+                newStatus.positionMillis = 0;
             }
-            currentVideoPlayerRef.current?.setStatusAsync({shouldPlay: true});
+            currentVideoPlayerRef.current?.setStatusAsync(newStatus);
         });
     }, [currentVideoPlayerRef]);
 
@@ -59,7 +60,7 @@ function PlaybackContextProvider({children}: ChildrenProps) {
     );
 
     const checkVideoPlaying = useCallback(
-        (statusCallback: (isPlaying: boolean) => void) => {
+        (statusCallback: StatusCallback) => {
             currentVideoPlayerRef.current?.getStatusAsync().then((status) => {
                 statusCallback('isPlaying' in status && status.isPlaying);
             });

diff --git a/src/components/VideoPlayerContexts/VideoPopoverMenuContext.tsx b/src/components/VideoPlayerContexts/VideoPopoverMenuContext.tsx
@@ -7,18 +7,18 @@ import fileDownload from '@libs/fileDownload';
 import CONST from '@src/CONST';
 import type ChildrenProps from '@src/types/utils/ChildrenProps';
 import {usePlaybackContext} from './PlaybackContext';
-import type {MenuItem, SingularMenuItem, VideoPopoverMenuContext} from './types';
+import type {MenuItem, PlaybackSpeed, SingularMenuItem, VideoPopoverMenuContext} from './types';
 
 const Context = React.createContext<VideoPopoverMenuContext | null>(null);
 
 function VideoPopoverMenuContextProvider({children}: ChildrenProps) {
     const {currentVideoPlayerRef, currentlyPlayingURL} = usePlaybackContext();
     const {translate} = useLocalize();
-    const [currentPlaybackSpeed, setCurrentPlaybackSpeed] = useState<number>(CONST.VIDEO_PLAYER.PLAYBACK_SPEEDS[2]);
+    const [currentPlaybackSpeed, setCurrentPlaybackSpeed] = useState<PlaybackSpeed>(CONST.VIDEO_PLAYER.PLAYBACK_SPEEDS[2]);
     const {isOffline} = useNetwork();
 
     const updatePlaybackSpeed = useCallback(
-        (speed: number) => {
+        (speed: PlaybackSpeed) => {
             setCurrentPlaybackSpeed(speed);
             currentVideoPlayerRef.current?.setStatusAsync({rate: speed});
         },
@@ -29,9 +29,7 @@ function VideoPopoverMenuContextProvider({children}: ChildrenProps) {
         if (currentlyPlayingURL === null) {
             return;
         }
-        const sourceURI =
-            // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing -- nullish coalescing doesn't achieve the same result in this case
-            currentlyPlayingURL.startsWith('blob:') || currentlyPlayingURL.startsWith('file:') ? currentlyPlayingURL : addEncryptedAuthTokenToURL(currentlyPlayingURL);
+        const sourceURI = currentlyPlayingURL.startsWith('blob:') || currentlyPlayingURL.startsWith('file:') ? currentlyPlayingURL : addEncryptedAuthTokenToURL(currentlyPlayingURL);
         fileDownload(sourceURI);
     }, [currentlyPlayingURL]);
 

diff --git a/src/components/VideoPlayerContexts/types.ts b/src/components/VideoPlayerContexts/types.ts
@@ -2,6 +2,7 @@ import type {Video} from 'expo-av';
 import type {MutableRefObject} from 'react';
 import type {View} from 'react-native';
 import type {SharedValue} from 'react-native-reanimated';
+import type CONST from '@src/CONST';
 import type IconAsset from '@src/types/utils/IconAsset';
 
 type PlaybackContext = {
@@ -36,7 +37,11 @@ type MenuItem = {
 
 type VideoPopoverMenuContext = {
     menuItems: Array<SingularMenuItem | MenuItem>;
-    updatePlaybackSpeed: (speed: number) => void;
+    updatePlaybackSpeed: (speed: PlaybackSpeed) => void;
 };
 
-export type {PlaybackContext, VolumeContext, VideoPopoverMenuContext, MenuItem, SingularMenuItem};
+type StatusCallback = (isPlaying: boolean) => void;
+
+type PlaybackSpeed = (typeof CONST.VIDEO_PLAYER.PLAYBACK_SPEEDS)[number];
+
+export type {PlaybackContext, VolumeContext, VideoPopoverMenuContext, MenuItem, SingularMenuItem, StatusCallback, PlaybackSpeed};