Reset stale tab title to "New Expensify" on logout#96916
Conversation
Co-authored-by: Tyler Karaszewski <tylerkaraszewski@users.noreply.github.com>
|
The only failing check is There's no code defect to push a fix for here. That check fails for two reasons, both of which need a human on the PR rather than a code change:
To get this green: open the checklist section in the PR description and check each box you've verified. Once the boxes are filled in, re-running the |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
…umentTitle('') on SignInPage
Co-authored-by: Sahil <thesahindia@users.noreply.github.com>
|
🤖 Switched this PR to the signed-out-root-screen approach per
Net diff is now 2 files (+7 / −24). The PR description has been updated to match. Local |
|
I looked into the failing checks — there's nothing to fix with a code push. The only failing check is
This check only passes once the author ticks the checklist boxes in the PR description — it's not something I can resolve by changing code. Every actual CI check is green: all 8 Jest To get everything green: complete the PR Author Checklist in the description (check every box). No code change is needed. |
|
@MelvinBot mark off all the items in the checklist |
|
🤖 Done — checked off all 40 items in the PR description (both the One heads-up: the checklist automation also logged |
|
@MelvinBot mark the PR as ready for review |
|
@thesahindia 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] |
|
🤖 Done — marked this PR as ready for review. It's now out of draft state. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-07-24.at.4.21.28.PM.mov |
![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: 1eb900109b
ℹ️ 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".
| // Reset the browser tab title on the signed-out root screen so it falls back to the site title ("New Expensify"). | ||
| // Authenticated pages set a page-specific title via useDocumentTitle, but nothing clears it on logout, so the tab | ||
| // would otherwise stay stuck on the last visited page's title. This is a no-op on native. | ||
| useDocumentTitle(''); |
There was a problem hiding this comment.
When an anonymous user is viewing an anonymous-accessible report and clicks Sign in, signOutAndRedirectToSignIn() opens ROUTES.SIGN_IN_MODAL, and SignInModal renders this same SignInPage component. Since useDocumentTitle('') runs from the focused modal too, it clears the report’s document title back to CONFIG.SITE_TITLE even though the user has not landed on the signed-out root screen. Please gate this reset to the public root sign-in route or keep it out of the reusable modal instance.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not sure if it's an issue but @MelvinBot let’s implement it
There was a problem hiding this comment.
Good catch — implemented in f63d144. This is a real issue: SignInModal renders this same SignInPage component, so useDocumentTitle('') fired from the modal too and would clear the underlying anonymous-accessible report's tab title.
Rather than reintroduce route-name coupling, I gated the reset behind an explicit prop:
- Added a
shouldResetTabTitleprop toSignInPage(defaults totrue), and moved theuseDocumentTitle('')call into a tinyResetTabTitleOnFocuschild that only mounts when the prop istrue. Threaded the prop throughSignInPageWrapperand theWithThemeHOC. SignInModalnow passesshouldResetTabTitle={false}, so the reusable modal instance never touches the title.- The public root sign-in screen (registered in
PublicScreens) renders the default export with no prop, so it keeps resetting to "New Expensify" on logout as intended.
Local checks: oxfmt, ESLint (0 errors), tsc typecheck (clean; the one BaseGenericTooltip TS2590 error is pre-existing and unrelated), React Compiler compliance, and the PublicSignInPageRootUrlTest + SessionTest suites all pass.
Gate useDocumentTitle('') behind a shouldResetTabTitle prop (default true) and
pass false from SignInModal so opening the sign-in modal over an
anonymous-accessible report no longer clears that report's tab title.
Co-authored-by: thesahindia <sahhtml@gmail.com>
Co-authored-by: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"} <{"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"}@users.noreply.github.com>
|
@thesahindia 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] |
3 participants
Explanation of Change
On web, after logging out the browser tab title stayed stuck on the last visited authenticated page (e.g. "Profile") instead of resetting to "New Expensify".
The tab title is driven by a module-level
currentPageTitleinupdateUnread/index.ts, which is only ever written (viasetPageTitle) by theuseDocumentTitlehook on screen focus.updateDocumentTitle()only falls back toCONFIG.SITE_TITLE("New Expensify") whencurrentPageTitleis empty. The design deliberately relies on the next focused page overwriting the title — a blur/unmount cleanup used to exist but was removed in #85473 because it raced the async title pipeline and flashed the default title mid-navigation. That assumption holds while navigating between authenticated pages, but breaks on logout: the destination is a public screen (Sign-In), which never callsuseDocumentTitle, so nothing overwrites the stale title.This implements the signed-out-root-screen approach: add
useDocumentTitle('')toSignInPage(the public/signed-out root screen). On logout the app lands onSignInPage, whose focus effect now sets an empty page title, soupdateDocumentTitle()falls back toCONFIG.SITE_TITLE("New Expensify").This follows the natural operation of the hook — every screen owns its own title on focus, and the Sign-In screen simply declares its (empty) title like any other screen — rather than adding an out-of-band cleanup or Onyx subscription. It fires only on focus of the signed-out screen, so it does not reintroduce the #85473 mid-navigation flicker, and
setPageTitleis a no-op on native, so the change is web-safe with no platform guard.Fixed Issues
$ #95208
PROPOSAL: #95208 (comment)
Tests
Offline tests
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari