-
Notifications
You must be signed in to change notification settings - Fork 4k
Fix navigating back to sidebar page in SplitNavigators #59300
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
Changes from all commits
ebecd0b
6507942
125581b
31071a1
28b2ce1
9843e13
d5040a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import Log from '@libs/Log'; | ||
| import Navigation from '@libs/Navigation/Navigation'; | ||
| import navigationRef from '@libs/Navigation/navigationRef'; | ||
| import NAVIGATORS from '@src/NAVIGATORS'; | ||
| import ROUTES from '@src/ROUTES'; | ||
|
|
||
| /** | ||
| * If there are already other screens open in WorkspaceSplitNavigator, we return to the previous one. | ||
| * If not, from the central screen in WorkspaceSplitNavigator we should return to the WorkspaceInitialPage. | ||
| */ | ||
| function goBackFromWorkspaceCentralScreen(policyID: string | undefined) { | ||
| const rootState = navigationRef.getRootState(); | ||
| const lastRoute = rootState.routes.at(-1); | ||
|
|
||
| if (lastRoute?.name !== NAVIGATORS.WORKSPACE_SPLIT_NAVIGATOR) { | ||
| Log.hmmm('[goBackFromWorkspaceCentralScreen] goBackFromWorkspaceCentralScreen was called from a different navigator than WorkspaceSplitNavigator.'); | ||
| return; | ||
| } | ||
|
|
||
| if (lastRoute.state?.routes && lastRoute.state.routes.length > 1) { | ||
| Navigation.goBack(undefined, {shouldPopToTop: true}); | ||
| return; | ||
| } | ||
|
|
||
| Navigation.goBack(ROUTES.WORKSPACE_INITIAL.getRoute(policyID)); | ||
| } | ||
|
|
||
| export default goBackFromWorkspaceCentralScreen; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,7 +53,7 @@ function PreferencesPage() { | |
| shouldUseHeadlineHeader | ||
| shouldShowBackButton={shouldUseNarrowLayout} | ||
| shouldDisplaySearchRouter | ||
| onBackButtonPress={() => Navigation.goBack()} | ||
| onBackButtonPress={() => Navigation.goBack(undefined, {shouldPopToTop: true})} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @WojtekBoman Why this was added here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was added to pop all central screens when navigating back after resizing the window: Screen.Recording.2025-04-04.at.16.24.35.mov
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @WojtekBoman One last thing, I think you forgot About, Troubleshoot, and Save the world pages here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I pushed a new commit that covers it. Thanks for noticing that! |
||
| /> | ||
| <ScrollView contentContainerStyle={styles.pt3}> | ||
| <View style={[styles.flex1, shouldUseNarrowLayout ? styles.workspaceSectionMobile : styles.workspaceSection]}> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ import useLocalize from '@hooks/useLocalize'; | |
| import useThemeStyles from '@hooks/useThemeStyles'; | ||
| import useWorkspaceAccountID from '@hooks/useWorkspaceAccountID'; | ||
| import {getLastFourDigits} from '@libs/BankAccountUtils'; | ||
| import goBackFromWorkspaceCentralScreen from '@libs/Navigation/helpers/goBackFromWorkspaceCentralScreen'; | ||
| import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; | ||
| import Navigation from '@navigation/Navigation'; | ||
| import type {SettingsNavigatorParamList} from '@navigation/types'; | ||
|
|
@@ -48,7 +49,10 @@ function WorkspaceCardSettingsPage({route}: WorkspaceCardSettingsPageProps) { | |
| includeSafeAreaPaddingBottom={false} | ||
| shouldEnableMaxHeight | ||
| > | ||
| <HeaderWithBackButton title={translate('workspace.common.settings')} /> | ||
| <HeaderWithBackButton | ||
| title={translate('workspace.common.settings')} | ||
| onBackButtonPress={() => goBackFromWorkspaceCentralScreen(policyID)} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We placed this logic on the wrong screen, which caused #60451 |
||
| /> | ||
| <ScrollView contentContainerStyle={styles.flexGrow1}> | ||
| <View> | ||
| <OfflineWithFeedback errorRowStyles={styles.mh5}> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,7 @@ import {turnOffMobileSelectionMode} from '@libs/actions/MobileSelectionMode'; | |
| import {clearPolicyTagErrors, deletePolicyTags, downloadTagsCSV, openPolicyTagsPage, setPolicyTagsRequired, setWorkspaceTagEnabled} from '@libs/actions/Policy/Tag'; | ||
| import {canUseTouchScreen} from '@libs/DeviceCapabilities'; | ||
| import localeCompare from '@libs/LocaleCompare'; | ||
| import goBackFromWorkspaceCentralScreen from '@libs/Navigation/helpers/goBackFromWorkspaceCentralScreen'; | ||
| import Navigation from '@libs/Navigation/Navigation'; | ||
| import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; | ||
| import type {WorkspaceSplitNavigatorParamList} from '@libs/Navigation/types'; | ||
|
|
@@ -452,7 +453,13 @@ function WorkspaceTagsPage({route}: WorkspaceTagsPageProps) { | |
| turnOffMobileSelectionMode(); | ||
| return; | ||
| } | ||
| Navigation.goBack(backTo); | ||
|
|
||
| if (backTo) { | ||
| Navigation.goBack(backTo); | ||
| return; | ||
| } | ||
|
|
||
| goBackFromWorkspaceCentralScreen(policyID); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @WojtekBoman I believe there are some pages left like per diem page where you haven't applied these changes.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I pushed a new commit, now it covers all workspace central screens |
||
| }} | ||
| shouldShowThreeDotsButton={!policy?.hasMultipleTagLists} | ||
| threeDotsMenuItems={threeDotsMenuItems} | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.