Skip to content

[Better Expense Report View] Fix video player in expense report view & chat thread #59430

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
73 changes: 65 additions & 8 deletions src/components/VideoPlayerContexts/PlaybackContext.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,56 @@
import type {NavigationState} from '@react-navigation/native';
import type {NavigationState, Route} from '@react-navigation/native';
import {findFocusedRoute} from '@react-navigation/native';
import type {AVPlaybackStatus, AVPlaybackStatusToSet} from 'expo-av';
import React, {useCallback, useContext, useEffect, useMemo, useRef, useState} from 'react';
import type {View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import type {VideoWithOnFullScreenUpdate} from '@components/VideoPlayer/types';
import usePrevious from '@hooks/usePrevious';
import getAttachmentDetails from '@libs/fileDownload/getAttachmentDetails';
import isReportTopmostSplitNavigator from '@libs/Navigation/helpers/isReportTopmostSplitNavigator';
import Navigation from '@libs/Navigation/Navigation';
import {getAllReportActions, getReportActionHtml} from '@libs/ReportActionsUtils';
import {getReportOrDraftReport, isChatThread} from '@libs/ReportUtils';
import Visibility from '@libs/Visibility';
import type {SearchFullscreenNavigatorParamList} from '@navigation/types';
import ROUTES from '@src/ROUTES';
import SCREENS from '@src/SCREENS';
import type {Report} from '@src/types/onyx';
import type ChildrenProps from '@src/types/utils/ChildrenProps';
import type {PlaybackContext, StatusCallback} from './types';

const Context = React.createContext<PlaybackContext | null>(null);

type SearchRoute = Omit<Route<string>, 'key'> | undefined;
type MoneyRequestReportState = {
params: SearchFullscreenNavigatorParamList[typeof SCREENS.SEARCH.MONEY_REQUEST_REPORT];
} & SearchRoute;

function isMoneyRequestReportRouteWithReportIDInParams(route: SearchRoute): route is MoneyRequestReportState {
return !!route && !!route.params && route.name === SCREENS.SEARCH.MONEY_REQUEST_REPORT && 'reportID' in route.params;
}

function findUrlInReportOrAncestorAttachments(currentReport: OnyxEntry<Report>, url: string | null): string | undefined {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
function findUrlInReportOrAncestorAttachments(currentReport: OnyxEntry<Report>, url: string | null): string | undefined {
function findURLInReportOrAncestorAttachments(currentReport: OnyxEntry<Report>, url: string | null): string | undefined {

Copy link
Contributor

Choose a reason for hiding this comment

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

NAB: I think adding more docs to this method would be helpful.

const {parentReportID, reportID} = currentReport ?? {};

const reportActions = getAllReportActions(reportID);
const hasUrlInAttachments = Object.values(reportActions).some((action) => {
const {sourceURL, previewSourceURL} = getAttachmentDetails(getReportActionHtml(action));
return sourceURL === url || previewSourceURL === url;
});

if (hasUrlInAttachments) {
return reportID;
}

if (parentReportID) {
const parentReport = getReportOrDraftReport(parentReportID);
return findUrlInReportOrAncestorAttachments(parentReport, url);
}

return undefined;
}

function PlaybackContextProvider({children}: ChildrenProps) {
const [currentlyPlayingURL, setCurrentlyPlayingURL] = useState<string | null>(null);
const [currentlyPlayingURLReportID, setCurrentlyPlayingURLReportID] = useState<string | undefined>();
Expand Down Expand Up @@ -58,25 +97,43 @@ function PlaybackContextProvider({children}: ChildrenProps) {
*/
const updateCurrentPlayingReportID = useCallback(
(state: NavigationState) => {
if (!isReportTopmostSplitNavigator()) {
setCurrentReportID(undefined);
return;
const focusedRoute = findFocusedRoute(state);
let {reportID} = getReportOrDraftReport(Navigation.getTopmostReportId()) ?? {};

// We need to handle a case where a report is selected via search and is therefore not a topmost report,
// but we still want to be able to play videos in it
if (isMoneyRequestReportRouteWithReportIDInParams(focusedRoute)) {
reportID = focusedRoute.params.reportID;
}
const reportID = Navigation.getTopmostReportId(state);

reportID = Navigation.getActiveRouteWithoutParams() === `/${ROUTES.ATTACHMENTS.route}` ? prevCurrentReportID : reportID;

setCurrentReportID(reportID);
},
[setCurrentReportID],
[setCurrentReportID, prevCurrentReportID],
);

const updateCurrentlyPlayingURL = useCallback(
(url: string | null) => {
if (currentlyPlayingURL && url !== currentlyPlayingURL) {
pauseVideo();
}
setCurrentlyPlayingURLReportID(currentReportID);

// Used for /attachment route
const topMostReport = getReportOrDraftReport(Navigation.getTopmostReportId());
const reportIDFromUrlParams = new URLSearchParams(Navigation.getActiveRoute()).get('reportID') ?? undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think all of these should have URL uppercase in name based on our style guides

Suggested change
const reportIDFromUrlParams = new URLSearchParams(Navigation.getActiveRoute()).get('reportID') ?? undefined;
const reportIDFromURLParams = new URLSearchParams(Navigation.getActiveRoute()).get('reportID') ?? undefined;

const attachmentReportID = Navigation.getActiveRouteWithoutParams() === `/${ROUTES.ATTACHMENTS.route}` ? prevCurrentReportID ?? reportIDFromUrlParams : undefined;
const reportIDWithUrl = isChatThread(topMostReport) ? findUrlInReportOrAncestorAttachments(topMostReport, url) : undefined;

// - if it is a chat thread, use chat thread ID or any ascentor ID since the video could have originally been sent on report many levels up
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// - if it is a chat thread, use chat thread ID or any ascentor ID since the video could have originally been sent on report many levels up
// - if it is a chat thread, use chat thread ID or any ancestor ID since the video could have originally been sent on report many levels up

// - report ID in which we are currently, if it is not a chat thread
// - if it is an attachment route, then we take report ID from the URL params
const currentPlayReportID = [attachmentReportID, reportIDWithUrl, currentReportID].find((id) => id !== undefined);

setCurrentlyPlayingURLReportID(currentPlayReportID);
setCurrentlyPlayingURL(url);
},
[currentlyPlayingURL, currentReportID, pauseVideo],
[currentlyPlayingURL, currentReportID, prevCurrentReportID, pauseVideo],
);

const shareVideoPlayerElements = useCallback(
Expand Down
17 changes: 14 additions & 3 deletions src/components/VideoPlayerPreview/index.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import {useNavigation} from '@react-navigation/native';
import type {VideoReadyForDisplayEvent} from 'expo-av';
import React, {useEffect, useState} from 'react';
import {View} from 'react-native';
import type {GestureResponderEvent} from 'react-native';
import {View} from 'react-native';
import * as Expensicons from '@components/Icon/Expensicons';
import VideoPlayer from '@components/VideoPlayer';
import IconButton from '@components/VideoPlayer/IconButton';
import {usePlaybackContext} from '@components/VideoPlayerContexts/PlaybackContext';
import useFirstRenderRoute from '@hooks/useFirstRenderRoute';
import useLocalize from '@hooks/useLocalize';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useThemeStyles from '@hooks/useThemeStyles';
import useThumbnailDimensions from '@hooks/useThumbnailDimensions';
import ROUTES from '@src/ROUTES';
import VideoPlayerThumbnail from './VideoPlayerThumbnail';

type VideoDimensions = {
Expand Down Expand Up @@ -51,6 +54,10 @@ function VideoPlayerPreview({videoUrl, thumbnailUrl, reportID, fileName, videoDi
const [isThumbnail, setIsThumbnail] = useState(true);
const [measuredDimensions, setMeasuredDimensions] = useState(videoDimensions);
const {thumbnailDimensionsStyles} = useThumbnailDimensions(measuredDimensions.width, measuredDimensions.height);
const navigation = useNavigation();

// We want to play the video only when the user is on the page where it was rendered
const firstRenderRoute = useFirstRenderRoute([`/${ROUTES.ATTACHMENTS.route}`]);

// `onVideoLoaded` is passed to VideoPlayerPreview's `Video` element which is displayed only on web.
// VideoReadyForDisplayEvent type is lacking srcElement, that's why it's added here
Expand All @@ -66,11 +73,15 @@ function VideoPlayerPreview({videoUrl, thumbnailUrl, reportID, fileName, videoDi
};

useEffect(() => {
if (videoUrl !== currentlyPlayingURL || reportID !== currentlyPlayingURLReportID) {
return navigation.addListener('blur', () => !firstRenderRoute.isFocused && setIsThumbnail(true));
}, [navigation, firstRenderRoute]);

useEffect(() => {
if (videoUrl !== currentlyPlayingURL || reportID !== currentlyPlayingURLReportID || !firstRenderRoute.isFocused) {
return;
}
setIsThumbnail(false);
}, [currentlyPlayingURL, currentlyPlayingURLReportID, updateCurrentlyPlayingURL, videoUrl, reportID]);
}, [currentlyPlayingURL, currentlyPlayingURLReportID, updateCurrentlyPlayingURL, videoUrl, reportID, firstRenderRoute]);

return (
<View style={[styles.webViewStyles.tagStyles.video, thumbnailDimensionsStyles]}>
Expand Down
23 changes: 23 additions & 0 deletions src/hooks/useFirstRenderRoute.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import {useEffect, useRef} from 'react';
import Navigation from '@navigation/Navigation';

function useFirstRenderRoute(focusExceptionRoutes?: string[]) {
const initialRoute = useRef<string | null>(null);

useEffect(() => {
initialRoute.current = Navigation.getActiveRouteWithoutParams();
}, []);

return {
get isFocused() {
const activeRoute = Navigation.getActiveRouteWithoutParams();
const allRoutesToConsider = [initialRoute.current, ...(focusExceptionRoutes ?? [])];
return allRoutesToConsider.includes(activeRoute);
},
get value() {
return initialRoute.current;
},
};
}

export default useFirstRenderRoute;