Create super wide RHP modal to display expense reports when accessed via Reports page or report previews#73731
Conversation
|
🚧 @trjExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
@ZhenjaHorbach Feel free to start on some early testing! |
| setShouldRenderThirdOverlay(true); | ||
| Animated.timing(thirdOverlayProgress, { | ||
| toValue: 1, | ||
| duration: 300, |
There was a problem hiding this comment.
I suppose it's better to use constants for that
| {!isSmallScreenWidth && shouldRenderSecondaryOverlay && route.name === SCREENS.RIGHT_MODAL.SEARCH_MONEY_REQUEST_REPORT && !!isWideRhpFocused && !isFocused ? ( | ||
| <Overlay | ||
| progress={secondOverlayProgress} | ||
| positionLeftValue={modalStackOverlayWideRHPWidth} | ||
| /> | ||
| ) : null} | ||
| {!isSmallScreenWidth && shouldRenderSecondaryOverlay && route.name === SCREENS.RIGHT_MODAL.SEARCH_MONEY_REQUEST_REPORT && !isWideRhpFocused && !isFocused ? ( | ||
| <Overlay | ||
| progress={secondOverlayProgress} | ||
| positionLeftValue={modalStackOverlaySuperWideRHPWidth} | ||
| /> | ||
| ) : null} |
There was a problem hiding this comment.
{!isSmallScreenWidth &&
shouldRenderSecondaryOverlay &&
route.name === SCREENS.RIGHT_MODAL.SEARCH_MONEY_REQUEST_REPORT &&
!isFocused && (
<Overlay
progress={secondOverlayProgress}
positionLeftValue={
isWideRhpFocused
? modalStackOverlayWideRHPWidth
: modalStackOverlaySuperWideRHPWidth
}
/>
)}
| right: 0, | ||
| position: 'absolute', | ||
| overflow: 'hidden', | ||
| width: shouldUseNarrowLayout ? '100%' : (animatedWidth as unknown as number), |
There was a problem hiding this comment.
I think it's bad practice to use unknown
| // Progress of the secondary overlay, the one covering wider RHP screen | ||
| secondOverlayProgress: Animated.Value; | ||
|
|
||
| // Progress of the secondary overlay, the one covering wider RHP screen |
There was a problem hiding this comment.
// Progress of the tertiary overlay, the one covering wider RHP screen
| // If the secondary overlay should be rendered. This value takes into account the delay of closing transition. | ||
| shouldRenderSecondaryOverlay: boolean; | ||
|
|
||
| // If the secondary overlay should be rendered. This value takes into account the delay of closing transition. |
There was a problem hiding this comment.
// If the tertiary overlay should be rendered. This value takes into account the delay of closing transition.
mountiny
left a comment
mountiny
left a comment
There was a problem hiding this comment.
We should make sure we add unit tests for all the hooks and helper methods added here but since we are trying to push this one ahead for release for customers asap, lets get this to staging. Jason, @ZhenjaHorbach and design team have all tested it and generally the flows work good, no major issues found so lets see if QA can find any
| const {shouldUseNarrowLayout: shouldUseNarrowLayoutByDefault, isSmallScreenWidth} = useResponsiveLayout(); | ||
| const {superWideRHPRouteKeys} = useContext(WideRHPContext); | ||
| const shouldUseNarrowLayout = shouldUseNarrowLayoutByDefault && (superWideRHPRouteKeys.length === 0 || isSmallScreenWidth); |
There was a problem hiding this comment.
I feel like this should be abstracted out
| <Animated.View style={styles.wideRHPMoneyRequestReceiptViewContainer}> | ||
| <ScrollView contentContainerStyle={styles.wideRHPMoneyRequestReceiptViewScrollViewContainer}> | ||
| <MoneyRequestReceiptView | ||
| allReports={allReports} |
There was a problem hiding this comment.
Unrelated, but I dont understand why this component should be getting full reports collection
There was a problem hiding this comment.
the components needs parent and chat report so we could have just passed the two, I believe @adamgrzybowski is in blame so ideally SWM can take a look at this
| markReportIDAsMultiTransactionExpense(reportID); | ||
| } else { | ||
| unmarkReportIDAsMultiTransactionExpense(reportID); |
There was a problem hiding this comment.
should these be added to the useCallback deps?
| ); | ||
| }; | ||
|
|
||
| function getCurrentWideRHPKeys(allWideRHPKeys: string[], lastVisibleRHPRouteKey: string | undefined) { |
There was a problem hiding this comment.
Can you add plain English explanation for this one, please?
| if (!lastRHPRoute) { | ||
| return []; | ||
| } | ||
| const getIsSuperWideRHPBelowFocusedScreen = useCallback( |
There was a problem hiding this comment.
can you please add a comment here explaining what the value represents/ what it is for in plain english
| [allSuperWideRHPRouteKeys], | ||
| ); | ||
|
|
||
| const getIsWideRHPBelowFocusedScreen = useCallback( |
There was a problem hiding this comment.
can you please add a comment here explaining what the value represents/ what it is for in plain english
| } | ||
|
|
||
| const newKey = route.key; | ||
| // If the key is in the array, don't add it. |
There was a problem hiding this comment.
| // If the key is in the array, don't add it. | |
| // If the key is in the array, don't add it. |
|
A better safe than sorry revert PR here |
|
I'm going to have to start calling you WARP. ZeroTrust |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.2.62-0 🚀
|
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.2.62-5 🚀
|
11 participants
Explanation of Change
Super Wide RHP
This PR introduces a third width of RHP called Super Wide RHP.
The maximum width of this RHP is
1260px.This type of RHP is available only for the following pages in the app:
SCREENS.EXPENSE_REPORT_RHP (e/:reportID)- it's a new page added to the app. It's responsible for displaying expense reports in super wide / wide RHP in the Inbox tabSCREENS.SEARCH.MONEY_REQUEST_REPORT (search/r/:reportID)Super wide RHP is displayed for these pages only if the open RHP has more than one visible transaction, otherwise these pages are opened in Wide RHP.
Overlays
There are two levels of nesting in RHP:
After adding the third RHP width, it became necessary to add a third overlay displayed when we overlap Super Wide RHP -> Wide RHP -> RHP.
There are two levels of nesting in RHP:
This information is important for correctly adding a new overlay.
When a new overlay is added, it is needed to be adjusted in 2 files:
src/libs/Navigation/AppNavigator/Navigators/RightModalNavigator.tsxHere, an overlay occupying the space outside the screen displayed in RHP must be added, when clicking on this overlay, the screen in RHP is closed.
src/libs/Navigation/AppNavigator/ModalStackNavigators/index.tsxIn case 2 or more RHPs of different widths are displayed, the RHP should also be added here.
In
RightModalNavigator, an overlay is shown from the left edge of the screen to the edge of the screen displayed as the modal. InModalStackNavigator, the overlay should be displayed below the currently displayed modal. This way, when opening another modal, you can see that there's also an overlay underneath it.More information about how overlays are displayed and their positioning can be found directly in the comments in the files mentioned above.
Fixed Issues
$ #71821
PROPOSAL:
Tests
Initial conditions:
Create a report containing at least two expenses.
Test 1
Test 2
Test 3
Test 4
Test 5
Test 6
Offline tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2025-11-20.at.10.16.43.mp4
Android: mWeb Chrome
Screen.Recording.2025-11-18.at.09.01.46.mov
iOS: Native
Screen.Recording.2025-11-17.at.21.47.55.mp4
Screen.Recording.2025-11-17.at.21.45.42.mp4
iOS: mWeb Safari
Screen.Recording.2025-11-17.at.21.51.27.mp4
MacOS: Chrome / Safari
Screen.Recording.2025-11-18.at.16.47.43.mov
MacOS: Desktop