-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Extend useDocumentTitle hook to all remaining pages #83388
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
Merged
Merged
Changes from all commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
336f3e2
Extend useDocumentTitle hook to all remaining pages
MelvinBot 279dfa7
Fix: Sort imports to match Prettier configuration
MelvinBot 6b374c2
Merge remote-tracking branch 'origin/main' into claude-extendUseDocum…
MelvinBot 17c76b1
Fix: Update WalletStatementPage for new addEncryptedAuthTokenToURL si…
MelvinBot 30870d4
Extract useWorkspaceDocumentTitle and useDomainDocumentTitle hooks
MelvinBot 044e3fc
Merge remote-tracking branch 'origin/main' into claude-extendUseDocum…
MelvinBot 82a3f70
Gate useDocumentTitle on screen focus to prevent title overwrites fro…
MelvinBot 13ee018
Merge branch 'claude-extendUseDocumentTitle' of https://github.com/Ex…
MelvinBot 3ea905c
Fix: Sort imports to match Prettier configuration
MelvinBot 897a128
Merge main into claude-extendUseDocumentTitle to resolve conflicts
MelvinBot 3937719
Fix: Reset SearchPage, ReportScreen, and WorkspaceOverviewPage to mai…
MelvinBot 2df5cfa
Remove unrelated changes, keep only document-title feature
MelvinBot eb7bd34
Fix: Sort imports to match Prettier configuration
MelvinBot b565d4f
Merge remote-tracking branch 'origin/main' into claude-extendUseDocum…
MelvinBot c360664
Merge remote-tracking branch 'app/main' into claude-extendUseDocument…
MelvinBot 63eae71
Merge remote-tracking branch 'origin/main' into claude-extendUseDocum…
MelvinBot 8f36331
Re-add useDocumentTitle changes from reverted PR #81150
MelvinBot 8573721
Merge remote-tracking branch 'origin/main' into claude-extendUseDocum…
MelvinBot 6f04f02
Re-apply useDocumentTitle changes after merging main
MelvinBot 379638e
Fix: Add missing useCallback import to BaseSelectionListWithSections
MelvinBot bc3fbec
Fix: Add setPageTitle no-op to native updateUnread files and remove i…
MelvinBot eaaae51
Merge remote-tracking branch 'origin/main' into claude-extendUseDocum…
MelvinBot 81b75f0
Fix: Remove unused useCallback import after merge resolution
MelvinBot 8727829
Retrigger CI: test job 5 timed out (no Jest output produced)
MelvinBot c14a421
Retrigger CI: SessionTest.tsx timeout was flaky (passes locally in 36s)
MelvinBot 5f2ab52
Fix: Mock document title hooks in tests to prevent timer accumulation
MelvinBot 80c4046
Fix: Add eslint-disable for __esModule naming convention in jest mocks
MelvinBot b2ce36e
Merge main and resolve conflicts in SearchPage, HomePage, PolicyTrave…
MelvinBot a8153bd
Merge remote-tracking branch 'origin/main' into claude-extendUseDocum…
MelvinBot f745212
Remove unrelated changes: assets, docs, TextPill, useDeepCompareRef, …
MelvinBot 5d92465
Add useDocumentTitle to settings sub-pages
MelvinBot f75b568
Fix: Sort imports alphabetically to pass Prettier check
MelvinBot 1d3bbe5
Fix: Remove useFocusEffect cleanup to fix title race condition in spl…
MelvinBot 84954c6
Fix: Use computed report name for document title instead of raw repor…
MelvinBot a3ec4e6
Fix: Reorder import to satisfy Prettier formatting
MelvinBot bd3dc35
Address review feedback: scope reportAttributes selector and add titl…
MelvinBot 8f22b5f
Merge main and resolve conflict in DomainMembersPage
MelvinBot 9aadf40
Fix: Memoize useOnyx selector with useCallback in ReportScreen
MelvinBot 688e47c
Fix: Prettier formatting in ReportScreen.tsx
MelvinBot 3aa6a49
Merge branch 'main' of github.com:Expensify/App into claude-extendUse…
yuwenmemon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import {useFocusEffect} from '@react-navigation/native'; | ||
| import {useCallback} from 'react'; | ||
| import {setPageTitle} from '@libs/UnreadIndicatorUpdater/updateUnread'; | ||
|
|
||
| function useDocumentTitle(title: string) { | ||
| useFocusEffect( | ||
| useCallback(() => { | ||
| setPageTitle(title); | ||
| return () => setPageTitle(''); | ||
| }, [title]), | ||
| ); | ||
| } | ||
|
|
||
| export default useDocumentTitle; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import type {TranslationPaths} from '@src/languages/types'; | ||
| import useDocumentTitle from './useDocumentTitle'; | ||
| import useLocalize from './useLocalize'; | ||
|
|
||
| /** | ||
| * Sets the browser document title for domain pages using the format "domainName - PageTitle". | ||
| * Falls back to empty string (default title) if domain name is not yet available. | ||
| */ | ||
| function useDomainDocumentTitle(domainName: string | undefined, titleKey: TranslationPaths) { | ||
| const {translate} = useLocalize(); | ||
| useDocumentTitle(domainName ? `${domainName} - ${translate(titleKey)}` : ''); | ||
| } | ||
|
|
||
| export default useDomainDocumentTitle; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import type {TranslationPaths} from '@src/languages/types'; | ||
| import useDocumentTitle from './useDocumentTitle'; | ||
| import useLocalize from './useLocalize'; | ||
|
|
||
| /** | ||
| * Sets the browser document title for workspace pages using the format "PolicyName - PageTitle". | ||
| * Falls back to empty string (default title) if policy name is not yet available. | ||
| */ | ||
| function useWorkspaceDocumentTitle(policyName: string | undefined, titleKey: TranslationPaths) { | ||
| const {translate} = useLocalize(); | ||
| useDocumentTitle(policyName ? `${policyName} - ${translate(titleKey)}` : ''); | ||
| } | ||
|
|
||
| export default useWorkspaceDocumentTitle; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Consider returning a cleanup function that resets the title when the screen loses focus:
Without this, if the user navigates to a screen that doesn't call
useDocumentTitle(e.g., an RHP modal or a page this PR didn't cover), the previous screen's title persists instead of falling back to the default "New Expensify". What do we think?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.
Good call — added a cleanup function that resets the title to empty when the screen loses focus, so uncovered screens fall back to the default "New Expensify" title:
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.
@yuwenmemon @MelvinBot I think we had the clean up method earlier and we removed it as it was causing issues with the title not being updated.
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.
Good catch — you're right. PR #81150 originally introduced
useDocumentTitlewith auseEffect-based cleanup (return () => setPageTitle('')on unmount). That was reverted in #83589 because of #83582: persistent screens (dontDetachScreen: true) never unmount, so the cleanup never fired and the title got stuck as "Settings - Profile".The current PR uses
useFocusEffectinstead ofuseEffect, which addresses the persistent-screen problem since the cleanup now fires on blur rather than unmount. However, #83586 (which was closed in favor of the revert) attempted a similaruseFocusEffectapproach but with additional guards — specifically, it checked whether another screen had already set a new title and whether the blur was caused by an RHP/modal overlay (where the underlying screen is still visible). Without those guards, the simplereturn () => setPageTitle('')here could clear the title when an RHP modal opens on top of a page, briefly showing "New Expensify" even though the user is still contextually on that page.yuwenmemon— given this history, should we add guards similar to what #83586 proposed, or is the simpler approach acceptable here?