-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[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
Changes from all commits
8f150cc
94d72fb
67e7722
633938f
52488f0
2b7f1b0
c74aedb
893a0a1
dc26009
3a7ff8d
4b89080
f5bf193
703ea6c
798156a
5c45bf5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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>(); | ||||||
|
@@ -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; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
// - 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( | ||||||
|
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.