Fix "Go to room" option briefly changing to "Go to workspace" in room details RHP - #93296
Conversation
|
@Krishna2323 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
🤖 Mobile QA — PR #93296 · ✅ PASS (Android)Fix "Go to room" option briefly changing to "Go to workspace" in room details RHP Context
Repro: open a room's Details RHP from the workspace Rooms list → tap "Go to room". What the diff does: Environment
Android — Before (
|
main (before) |
PR fix/93257… (after) |
|
|---|---|---|
| Tap "Go to room" → during RHP close | ❌ flips to "Go to workspace", moves to bottom | ✅ "Go to room" stays put, clean close |
Verdict — ✅ PASS (Android)
The PR resolves the reported flicker: on main, tapping "Go to room" briefly swaps it to "Go to workspace" and reorders the menu during the close; on the PR branch the option is stable through the close and you land in the room cleanly. This is the exact Expected-vs-Actual delta in issue #93257.
Caveats / setup notes (environment, not the PR):
- Rooms-list beta override. The "Go to room" option is only reachable from the Workspace Rooms list, which is gated behind the
workspaceRoomsPagebeta the test account lacked (WorkspaceInitialPage.tsx→isBetaEnabled(CONST.BETAS.WORKSPACE_ROOMS_PAGE)). I enabled it client-side for the test (Onyxbetasmerge via the Metro debugger), applied identically to both branches. It only surfaces the entry point; the bug/fix lives in the render logic, so the before/after is fair. - Entry via deep link. The Rooms list was opened via
workspaces/<policyID>/roomsdeep link (the menu item appears once the beta is on); a room (#qatestroom93296) was created so the list had a room to test. - Metro OOM mid-session. Metro had run out of heap earlier and was restarted fresh with a 12 GB heap (unrelated to the PR).
- Recording note. Videos start at the room Details RHP (the state where the bug manifests) and run through the tap → close → landing.
🤖 Automated mobile QA via argent-qa-pr · Android emulator via Argent/adb · before/after on main vs the PR branch · screen recordings via adb screenrecord, compressed with ffmpeg.
|
@sumo-slonik should I review this? |
|
@sumo-slonik details page shows "Go to workspace" instead of "Go to room":
Monosnap.screencast.2026-06-16.00-51-28.mp4 |
I'm on it! |
Verify that the 'Go to room' row stays visible when the room becomes the topmost report after it is tapped (the frozen useInitialValue check), and that it is hidden when the details page is opened from the room itself.
|
@Krishna2323 Could you please try to reproduce this bug one more time? Or could you tell me Screen.Recording.2026-06-16.at.13.47.54.mov |
@sumo-slonik you forgot to tap "Go to workspace". Monosnap.screencast.2026-06-16.20-05-18.mp4 |
The room-visibility check read live global navigation state, which flickered during the RHP close (Expensify#93257). Deriving it from the page's own route.path keeps it stable across the close and later revisits; for instances recreated by back navigation that have no route.path, fall back to a reactive live topmost-report check so the menu settles on the correct option. Extend the unit tests to cover the route-path and path-less branches.
|
@Krishna2323 fixed 🚀 Screen.Recording.2026-06-16.at.20.51.02.mov |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36486cd047
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const liveIsRoomCurrentlyOpen = useRootNavigationState(() => isReportTopmostSplitNavigator() && Navigation.getTopmostReportId() === report?.reportID); | ||
| const isRoomCurrentlyOpen = route.path ? isReportDetailsOnTopOfRoom(route.path, report?.reportID) : liveIsRoomCurrentlyOpen; |
There was a problem hiding this comment.
Preserve path-less room state after showing Go to room
When route.path is absent, this falls back to a live navigation-state subscription, so a path-less details page that currently shows Go to room still re-evaluates as soon as that action navigates to the room while the RHP is closing. In that path-less flow, liveIsRoomCurrentlyOpen flips to true before unmount and the menu can still swap to Go to workspace, which is the flicker this change is trying to prevent for room details opened away from the room.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@sumo-slonik this seems like a valid concern but I haven't faced this on while testing.
There was a problem hiding this comment.
I'm looking into it now
There was a problem hiding this comment.
It is correct. I'm currently looking for a fix. There is a flicker when no path is available.
Screen.Recording.2026-06-17.at.10.54.53.mov
There was a problem hiding this comment.
Fixed!
Screen.Recording.2026-06-17.at.11.04.29.mov
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid_hybrid.mp4Android: mWeb Chromeandroid_mWeb.mp4iOS: HybridAppios_hybrid.mp4iOS: mWeb Safariios_mWebR.mp4MacOS: Chrome / Safariweb_chrome.mp4 |
Deriving the row from live navigation state re-showed the flicker when the row was tapped on a back-restored details page (no route.path), and the route.path derivation only covered freshly-opened pages. Snapshot the topmost-report check on focus instead: it stays stable while the page is closing after the row is tapped, yet refreshes on later visits, covering every entry path with one mechanism.
|
@trjExpensify I think this one is ready |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.4.10-0 🚀
Bundle Size Analysis (Sentry): |
|
🤖 No help site changes required. I reviewed the changes in this PR against the help site files under What the PR does: It fixes a visual flicker in the report details RHP where tapping Go to room briefly swapped the option to Go to workspace while the page was closing. The fix snapshots the Why no docs are needed:
Since no changes are required, I did not create a draft help site PR. |
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.4.12-0 🚀
Bundle Size Analysis (Sentry): |
|
🤖 No help site changes required. This PR is a pure UI bug fix: it snapshots the I searched Since there are no documented behaviors affected, I did not create a draft help site PR — there is nothing for the docs to add or correct here. @sumo-slonik, if you believe a help article should document this flow (or you'd like one created), let me know and I'll draft it. |
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.4.14-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.4.14-1 🚀
|






Explanation of Change
In the report details RHP, the
isRoomCurrentlyOpencondition (which decides between showingGo to roomandGo to workspace) was re-evaluated on every render, so tappingGo to roomflipped it while the RHP was still closing, briefly swapping the option toGo to workspace. The condition is now computed once when the page is opened (lazyuseStateinitializer), which is safe becausereport.reportIDcomes from the route params and never changes during the lifetime of a details page instance.Fixed Issues
$ #93257
PROPOSAL:
Tests
Offline tests
Unnecessary — the menu rendering and navigation are local, so the behavior is identical offline.
QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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.2026-06-12.at.12.00.16.mov