Fix Not-here page flashing when deleting workspace from Overview - #97261
Conversation
Latch the deleted state in WorkspacePageWithSections so FullPageNotFoundView does not flash while goBackFromInvalidPolicy()'s exit animation plays out and the navigation state re-renders the still-mounted Overview screen. Fixes Expensify#96977 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbb840df33
ℹ️ 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 [hasWorkspaceBeenDeleted, setHasWorkspaceBeenDeleted] = useState(false); | ||
| if (!hasWorkspaceBeenDeleted && (isPendingDelete || (prevIsPendingDelete && isEmptyObject(policy)))) { |
There was a problem hiding this comment.
When the same WorkspacePageWithSections instance is reused for another route.params.policyID after a deletion has been latched, this state never clears, so shouldShow stays forced to false for the new workspace too. In that case an empty or inaccessible replacement workspace renders the normal children instead of the not-found/unauthorized view; key this latch by policyID or clear it when the route policy changes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. I reworked the latch so it stores the deleted policyID instead of a plain boolean. hasWorkspaceBeenDeleted is now derived as deletedPolicyID === policyID, so the suppression is scoped to the exact workspace being deleted and clears on its own the moment this still-mounted screen is reused for a different policyID.
I also guarded the pending-delete -> removed-from-Onyx transition with prevPolicyID === policyID. That branch reads prevPolicy, which belongs to the previous render's policyID, so without the guard a policyID switch could re-latch the new workspace off a stale prevPolicy. Now an empty or inaccessible replacement workspace renders the normal not-found view as expected.
Added a test that reuses the same screen instance for a different, inaccessible policyID after a delete and asserts the not-found view shows.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@DylanDylann 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] |
|
@codex review |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-08-03.at.12.16.16.movAndroid: mWeb ChromeScreen.Recording.2026-08-03.at.11.58.30.moviOS: HybridAppScreen.Recording.2026-08-03.at.12.15.59.moviOS: mWeb SafariScreen.Recording.2026-08-03.at.11.56.35.movMacOS: Chrome / SafariScreen.Recording.2026-08-03.at.11.55.57.mov |
|
@dilshodmackbook-sketch Could you merge the latest main too? |
| // The workspace we're viewing is being deleted (optimistically pending delete, or pending delete last render | ||
| // and now gone from Onyx). Latch it so FullPageNotFoundView doesn't flash while goBackFromInvalidPolicy()'s | ||
| // exit animation plays out and the navigation state re-renders this (still-mounted) screen. | ||
| // We latch the deleted policyID (rather than a plain boolean) so the suppression is scoped to that specific | ||
| // workspace and clears automatically if this still-mounted screen is later reused for a different policyID. |
There was a problem hiding this comment.
| // The workspace we're viewing is being deleted (optimistically pending delete, or pending delete last render | |
| // and now gone from Onyx). Latch it so FullPageNotFoundView doesn't flash while goBackFromInvalidPolicy()'s | |
| // exit animation plays out and the navigation state re-renders this (still-mounted) screen. | |
| // We latch the deleted policyID (rather than a plain boolean) so the suppression is scoped to that specific | |
| // workspace and clears automatically if this still-mounted screen is later reused for a different policyID. |
There was a problem hiding this comment.
Done, removed the long comments and merged the latest main.
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…orkspace-notfound-flash
|
Done, merged main. |
2 participants
Explanation of Change
When deleting a workspace from Workspace → Overview → More → Delete → Delete, the "Hmm... it's not here" (Not found) page flashed briefly before landing on the Workspaces list — most noticeable on mWeb where the page-exit animation is slower.
The Overview page renders inside
WorkspacePageWithSections, which decides whether to showFullPageNotFoundViewvia its ownshouldShowmemo. Deleting online is a two-step Onyx transition: the policy is first optimistically markedpendingAction: DELETE, then (on backend success) removed from Onyx entirely. Once the policy is gone, the empty-policy branch ofshouldShowreturnstrue, and it has no pending-delete guard — so the Not-found view paints on the still-mounted Overview screen whilegoBackFromInvalidPolicy()'s navigation/exit animation plays out.This mirrors the transient
WorkspaceInitialPagealready protects against by latching its not-found flag.WorkspacePageWithSectionshad no such latch.The fix detects that the workspace being viewed is the one the user is deleting (it's optimistically pending delete, or it was pending delete last render and is now gone from Onyx) and latches that in state. While latched,
shouldShowstaysfalsefor the rest of the screen's life, so the Not-found view doesn't flash during the exit animation. It only latches on a real, user-initiated deletion, so genuine "not accessible / not found" cases are unaffected, and offline deletes (where the policy keepspendingAction: DELETEand is never emptied) keep relying on the existing guard.Fixed Issues
$ #96977
PROPOSAL: #96977 (comment)
Tests
npx jest tests/ui/WorkspacePageWithSectionsTest.tsxpasses, including the two new cases covering the delete transition and the genuine not-found negative case.Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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-07-29.at.18.18.49.mov
Android: mWeb Chrome
Screen.Recording.2026-07-29.at.15.08.34.mov
iOS: Native
Screen.Recording.2026-07-29.at.18.23.16.mov
iOS: mWeb Safari
Screen.Recording.2026-07-29.at.15.09.00.mov
MacOS: Chrome / Safari
Screen.Recording.2026-07-29.at.15.06.19.mov