Skip to content

Commit 7e184c0

Browse files
Restore workspace sub-page on tab re-entry via URL navigation
1 parent 314bc88 commit 7e184c0

4 files changed

Lines changed: 310 additions & 12 deletions

File tree

src/hooks/useRestoreWorkspacesTabOnNavigate.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,9 @@ function useRestoreWorkspacesTabOnNavigate() {
105105
domain: lastViewedDomain,
106106
lastWorkspacesTabNavigatorRoute,
107107
topmostFullScreenRoute,
108+
workspacesTabState,
108109
});
109-
}, [shouldUseNarrowLayout, currentUserLogin, lastViewedPolicy, lastViewedDomain, lastWorkspacesTabNavigatorRoute, topmostFullScreenRoute]);
110+
}, [shouldUseNarrowLayout, currentUserLogin, lastViewedPolicy, lastViewedDomain, lastWorkspacesTabNavigatorRoute, topmostFullScreenRoute, workspacesTabState]);
110111
}
111112

112113
export default useRestoreWorkspacesTabOnNavigate;

src/libs/Navigation/helpers/navigateToWorkspacesPage.ts

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,24 +6,42 @@ import navigationRef from '@libs/Navigation/navigationRef';
66
import {isPendingDeletePolicy, shouldShowPolicy as shouldShowPolicyUtil} from '@libs/PolicyUtils';
77
import NAVIGATORS from '@src/NAVIGATORS';
88
import ROUTES from '@src/ROUTES';
9+
import type {Route} from '@src/ROUTES';
910
import SCREENS from '@src/SCREENS';
1011
import type {Domain, Policy} from '@src/types/onyx';
1112
import getActiveTabName from './getActiveTabName';
13+
import getPathFromState from './getPathFromState';
1214

1315
type RouteType = NavigationState['routes'][number] | PartialState<NavigationState>['routes'][number];
1416

17+
/**
18+
* Wraps a leaf navigation state in successive ancestor navigators (outermost first).
19+
* Used to reconstruct the linking-config hierarchy that `getPathFromState` walks when
20+
* resolving a state subtree to a URL.
21+
*/
22+
function wrapStateInNavigators(state: PartialState<NavigationState>, navigators: readonly string[]): PartialState<NavigationState> {
23+
return navigators.reduceRight<PartialState<NavigationState>>((acc, name) => ({routes: [{name, state: acc}], index: 0}), state);
24+
}
25+
1526
type Params = {
1627
currentUserLogin?: string;
1728
shouldUseNarrowLayout: boolean;
1829
policy?: Policy;
1930
domain?: Domain;
2031
lastWorkspacesTabNavigatorRoute?: RouteType;
2132
topmostFullScreenRoute?: RouteType;
33+
/**
34+
* The full WorkspaceSplitNavigator inner state captured by the hook.
35+
* Wrapped in a synthetic outer node and fed to `getPathFromState` to reconstruct
36+
* the deep URL the user was on (e.g. `/workspaces/POLICY_ID/workflows`). Navigating
37+
* via that URL goes through `getStateFromPath` which produces a fully-formed
38+
* navigation state — bypassing custom router actions that don't seed nested state
39+
* when pushing a fresh TabNavigator on top of an existing fullscreen stack.
40+
*/
41+
workspacesTabState?: NavigationState | PartialState<NavigationState>;
2242
};
2343

24-
// Navigates to the appropriate workspace tab or workspace list page.
25-
// eslint-disable-next-line @typescript-eslint/no-unused-vars -- shouldUseNarrowLayout kept for API compat with callers
26-
const navigateToWorkspacesPage = ({currentUserLogin, shouldUseNarrowLayout, policy, domain, lastWorkspacesTabNavigatorRoute, topmostFullScreenRoute}: Params) => {
44+
const navigateToWorkspacesPage = ({currentUserLogin, shouldUseNarrowLayout, policy, domain, lastWorkspacesTabNavigatorRoute, topmostFullScreenRoute, workspacesTabState}: Params) => {
2745
const rootState = navigationRef.getRootState();
2846
const focusedRoute = rootState ? findFocusedRoute(rootState) : undefined;
2947
const isOnWorkspacesList = focusedRoute?.name === SCREENS.WORKSPACES_LIST;
@@ -61,9 +79,26 @@ const navigateToWorkspacesPage = ({currentUserLogin, shouldUseNarrowLayout, poli
6179
return;
6280
}
6381

64-
// Restore to last-visited workspace — navigate through standard routing which switches the tab
6582
if (policy?.id) {
66-
Navigation.navigate(ROUTES.WORKSPACE_INITIAL.getRoute(policy.id));
83+
// Synthesize a URL from the captured WorkspaceSplitNavigator inner state and navigate
84+
// to it. URL-based navigation goes through `getStateFromPath`, which produces a fully
85+
// formed nested state and reliably handles pushing a fresh TabNavigator on top of an
86+
// existing fullscreen stack. The state has to be wrapped with its full ancestor chain
87+
// (TAB_NAVIGATOR > WORKSPACE_NAVIGATOR > WORKSPACE_SPLIT_NAVIGATOR) so `getPathFromState`
88+
// can match the linking-config hierarchy and produce a real URL like
89+
// `/workspaces/POLICY_ID/workflows`; otherwise the resolver falls back to navigator
90+
// names as path segments and the result hits 404. Narrow layouts skip the deep-restore
91+
// and go to the workspace's initial page (mirrors mobile behavior).
92+
const wrappedState =
93+
!shouldUseNarrowLayout && workspacesTabState
94+
? wrapStateInNavigators(workspacesTabState as PartialState<NavigationState>, [
95+
NAVIGATORS.TAB_NAVIGATOR,
96+
NAVIGATORS.WORKSPACE_NAVIGATOR,
97+
NAVIGATORS.WORKSPACE_SPLIT_NAVIGATOR,
98+
])
99+
: undefined;
100+
const targetPath = (wrappedState ? getPathFromState(wrappedState) : ROUTES.WORKSPACE_INITIAL.getRoute(policy.id)) as Route;
101+
Navigation.navigate(targetPath);
67102
}
68103
return;
69104
}

tests/unit/hooks/useRestoreWorkspacesTabOnNavigate.test.ts

Lines changed: 189 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import {renderHook} from '@testing-library/react-native';
2+
import getPathFromState from '@libs/Navigation/helpers/getPathFromState';
23
import Navigation from '@libs/Navigation/Navigation';
34
import NAVIGATORS from '@src/NAVIGATORS';
45
import ROUTES from '@src/ROUTES';
6+
import SCREENS from '@src/SCREENS';
57
import createRandomPolicy from '../../utils/collections/policies';
68

79
jest.mock('@libs/Navigation/AppNavigator/createSplitNavigator/usePreserveNavigatorState', () => ({
@@ -12,7 +14,8 @@ jest.mock('@libs/Navigation/helpers/lastVisitedTabPathUtils', () => ({
1214
getWorkspacesTabStateFromSessionStorage: jest.fn(() => undefined),
1315
}));
1416

15-
jest.mock('@hooks/useResponsiveLayout', () => () => ({shouldUseNarrowLayout: false}));
17+
const mockResponsiveLayout = jest.fn(() => ({shouldUseNarrowLayout: false}));
18+
jest.mock('@hooks/useResponsiveLayout', () => () => mockResponsiveLayout());
1619

1720
jest.mock('@hooks/useCurrentUserPersonalDetails', () => () => ({login: 'test@example.com'}));
1821

@@ -24,7 +27,11 @@ jest.mock('@hooks/useOnyx', () => (key: unknown, opts?: unknown) => mockUseOnyx(
2427

2528
jest.mock('@libs/interceptAnonymousUser', () => (cb: () => void) => cb());
2629

27-
jest.mock('@libs/Navigation/navigationRef', () => ({getRootState: jest.fn(() => ({routes: []})), isReady: jest.fn(() => true)}));
30+
jest.mock('@libs/Navigation/navigationRef', () => ({
31+
getRootState: jest.fn(() => ({routes: []})),
32+
isReady: jest.fn(() => true),
33+
dispatch: jest.fn(),
34+
}));
2835

2936
jest.mock('@react-navigation/native', () => ({
3037
findFocusedRoute: jest.fn(() => ({name: 'some-screen'})),
@@ -35,6 +42,11 @@ jest.mock('@libs/Navigation/Navigation', () => ({
3542
goBack: jest.fn(),
3643
}));
3744

45+
jest.mock('@libs/Navigation/helpers/getPathFromState', () => ({
46+
__esModule: true,
47+
default: jest.fn(),
48+
}));
49+
3850
// eslint-disable-next-line no-restricted-syntax
3951
jest.mock('@libs/PolicyUtils', () => ({
4052
shouldShowPolicy: jest.fn(() => true),
@@ -43,13 +55,17 @@ jest.mock('@libs/PolicyUtils', () => ({
4355

4456
const fakePolicyID = 'ABCD1234';
4557
const mockPolicy = {...createRandomPolicy(0), id: fakePolicyID};
58+
const mockedGetPathFromState = getPathFromState as jest.MockedFunction<typeof getPathFromState>;
4659

4760
// eslint-disable-next-line @typescript-eslint/no-require-imports
4861
const useRestoreWorkspacesTabOnNavigate = (require('@hooks/useRestoreWorkspacesTabOnNavigate') as {default: () => () => void}).default;
4962

5063
// eslint-disable-next-line @typescript-eslint/no-require-imports, no-restricted-syntax
5164
const PolicyUtils = require('@libs/PolicyUtils') as {shouldShowPolicy: jest.Mock; isPendingDeletePolicy: jest.Mock};
5265

66+
// eslint-disable-next-line @typescript-eslint/no-require-imports
67+
const lastVisitedTabPathUtils = require('@libs/Navigation/helpers/lastVisitedTabPathUtils') as {getWorkspacesTabStateFromSessionStorage: jest.Mock};
68+
5369
function setupOnyxForPolicy() {
5470
mockUseOnyx.mockImplementation((_key: unknown, opts?: {selector?: (data: unknown) => unknown}) => {
5571
if (opts?.selector) {
@@ -83,18 +99,30 @@ describe('useRestoreWorkspacesTabOnNavigate', () => {
8399
beforeEach(() => {
84100
jest.clearAllMocks();
85101
mockUseOnyx.mockReturnValue([undefined]);
102+
mockResponsiveLayout.mockReturnValue({shouldUseNarrowLayout: false});
103+
lastVisitedTabPathUtils.getWorkspacesTabStateFromSessionStorage.mockReturnValue(undefined);
86104
PolicyUtils.shouldShowPolicy.mockReturnValue(true);
87105
PolicyUtils.isPendingDeletePolicy.mockReturnValue(false);
106+
mockedGetPathFromState.mockReset();
88107
});
89108

90109
it('restores to the last visited workspace when re-entering the Workspaces tab', () => {
91110
setupOnyxForPolicy();
92-
mockRootState.mockReturnValue(buildStateWithUserOnDifferentTab([{name: NAVIGATORS.WORKSPACE_SPLIT_NAVIGATOR, state: {routes: [{params: {policyID: fakePolicyID}}]}}]));
111+
const restoredPath = `/workspaces/${fakePolicyID}` as const;
112+
mockedGetPathFromState.mockReturnValue(restoredPath);
113+
mockRootState.mockReturnValue(
114+
buildStateWithUserOnDifferentTab([
115+
{
116+
name: NAVIGATORS.WORKSPACE_SPLIT_NAVIGATOR,
117+
state: {routes: [{name: SCREENS.WORKSPACE.INITIAL, params: {policyID: fakePolicyID}}]},
118+
},
119+
]),
120+
);
93121

94122
const {result} = renderHook(() => useRestoreWorkspacesTabOnNavigate());
95123
result.current();
96124

97-
expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.WORKSPACE_INITIAL.getRoute(fakePolicyID));
125+
expect(Navigation.navigate).toHaveBeenCalledWith(restoredPath);
98126
});
99127

100128
it('falls back to the workspaces list when no workspace was previously visited', () => {
@@ -117,11 +145,167 @@ describe('useRestoreWorkspacesTabOnNavigate', () => {
117145
PolicyUtils.isPendingDeletePolicy.mockReturnValue(true);
118146

119147
setupOnyxForPolicy();
120-
mockRootState.mockReturnValue(buildStateWithUserOnDifferentTab([{name: NAVIGATORS.WORKSPACE_SPLIT_NAVIGATOR, state: {routes: [{params: {policyID: fakePolicyID}}]}}]));
148+
mockRootState.mockReturnValue(
149+
buildStateWithUserOnDifferentTab([
150+
{
151+
name: NAVIGATORS.WORKSPACE_SPLIT_NAVIGATOR,
152+
state: {routes: [{name: SCREENS.WORKSPACE.INITIAL, params: {policyID: fakePolicyID}}]},
153+
},
154+
]),
155+
);
121156

122157
const {result} = renderHook(() => useRestoreWorkspacesTabOnNavigate());
123158
result.current();
124159

125160
expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.WORKSPACES_LIST.route);
126161
});
162+
163+
// Regression: clicking the Workspaces tab from any other tab should land the user on the *exact* sub-page
164+
// they had open inside the workspace (e.g. Workflows), not the workspace's initial page.
165+
it('preserves the focused workspace sub-page (Workflows) when restoring on a wide layout', () => {
166+
setupOnyxForPolicy();
167+
const restoredPath = `/workspaces/${fakePolicyID}/workflows` as const;
168+
mockedGetPathFromState.mockReturnValue(restoredPath);
169+
mockRootState.mockReturnValue(
170+
buildStateWithUserOnDifferentTab([
171+
{
172+
name: NAVIGATORS.WORKSPACE_SPLIT_NAVIGATOR,
173+
state: {
174+
index: 1,
175+
routes: [
176+
{name: SCREENS.WORKSPACE.INITIAL, params: {policyID: fakePolicyID}},
177+
{name: SCREENS.WORKSPACE.WORKFLOWS, params: {policyID: fakePolicyID}},
178+
],
179+
},
180+
},
181+
]),
182+
);
183+
184+
const {result} = renderHook(() => useRestoreWorkspacesTabOnNavigate());
185+
result.current();
186+
187+
expect(Navigation.navigate).toHaveBeenCalledWith(restoredPath);
188+
});
189+
190+
// Regression for the original bug (#89106): when an RHP-driven navigation pushes a fresh TabNavigator above
191+
// the modal, the new TabNavigator's WORKSPACE_NAVIGATOR is empty. The hook must reach into the *older*
192+
// TabNavigator instance still alive in the root stack to recover the user's last workspace sub-page.
193+
it('reads workspace state from an older TabNavigator instance when the topmost one is empty', () => {
194+
setupOnyxForPolicy();
195+
const restoredPath = `/workspaces/${fakePolicyID}/workflows` as const;
196+
mockedGetPathFromState.mockReturnValue(restoredPath);
197+
mockRootState.mockReturnValue({
198+
routes: [
199+
// Older TabNavigator: still holds the workspace state with WORKFLOWS focused.
200+
{
201+
name: NAVIGATORS.TAB_NAVIGATOR,
202+
state: {
203+
index: 1,
204+
routes: [
205+
{name: NAVIGATORS.REPORTS_SPLIT_NAVIGATOR},
206+
{
207+
name: NAVIGATORS.WORKSPACE_NAVIGATOR,
208+
state: {
209+
routes: [
210+
{
211+
name: NAVIGATORS.WORKSPACE_SPLIT_NAVIGATOR,
212+
state: {
213+
index: 1,
214+
routes: [
215+
{name: SCREENS.WORKSPACE.INITIAL, params: {policyID: fakePolicyID}},
216+
{name: SCREENS.WORKSPACE.WORKFLOWS, params: {policyID: fakePolicyID}},
217+
],
218+
},
219+
},
220+
],
221+
},
222+
},
223+
],
224+
},
225+
},
226+
// Newer TabNavigator pushed above the modal: WORKSPACE_NAVIGATOR is empty.
227+
{
228+
name: NAVIGATORS.TAB_NAVIGATOR,
229+
state: {
230+
index: 0,
231+
routes: [{name: NAVIGATORS.REPORTS_SPLIT_NAVIGATOR}, {name: NAVIGATORS.WORKSPACE_NAVIGATOR}],
232+
},
233+
},
234+
],
235+
});
236+
237+
const {result} = renderHook(() => useRestoreWorkspacesTabOnNavigate());
238+
result.current();
239+
240+
expect(Navigation.navigate).toHaveBeenCalledWith(restoredPath);
241+
});
242+
243+
// On narrow layouts (mobile), the URL-based restore is skipped: we always land on the workspace's
244+
// initial page so the user can navigate inward via the side-list — matches mobile UX and the docs.
245+
it('falls back to the workspace initial page on narrow layouts even when a sub-page is focused', () => {
246+
mockResponsiveLayout.mockReturnValue({shouldUseNarrowLayout: true});
247+
setupOnyxForPolicy();
248+
mockRootState.mockReturnValue(
249+
buildStateWithUserOnDifferentTab([
250+
{
251+
name: NAVIGATORS.WORKSPACE_SPLIT_NAVIGATOR,
252+
state: {
253+
index: 1,
254+
routes: [
255+
{name: SCREENS.WORKSPACE.INITIAL, params: {policyID: fakePolicyID}},
256+
{name: SCREENS.WORKSPACE.WORKFLOWS, params: {policyID: fakePolicyID}},
257+
],
258+
},
259+
},
260+
]),
261+
);
262+
263+
const {result} = renderHook(() => useRestoreWorkspacesTabOnNavigate());
264+
result.current();
265+
266+
expect(mockedGetPathFromState).not.toHaveBeenCalled();
267+
expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.WORKSPACE_INITIAL.getRoute(fakePolicyID));
268+
});
269+
270+
// Cold-start path: when no workspace route exists anywhere in the live nav tree, fall back to the
271+
// sessionStorage-persisted state so a fresh page-load still restores the user's last workspace sub-page.
272+
it('hydrates from sessionStorage when the live navigation tree has no workspace route', () => {
273+
setupOnyxForPolicy();
274+
const restoredPath = `/workspaces/${fakePolicyID}/workflows` as const;
275+
mockedGetPathFromState.mockReturnValue(restoredPath);
276+
mockRootState.mockReturnValue({
277+
routes: [
278+
{
279+
name: NAVIGATORS.TAB_NAVIGATOR,
280+
state: {index: 0, routes: [{name: NAVIGATORS.REPORTS_SPLIT_NAVIGATOR}]},
281+
},
282+
],
283+
});
284+
lastVisitedTabPathUtils.getWorkspacesTabStateFromSessionStorage.mockReturnValue({
285+
routes: [
286+
{
287+
name: NAVIGATORS.WORKSPACE_NAVIGATOR,
288+
state: {
289+
routes: [
290+
{
291+
name: NAVIGATORS.WORKSPACE_SPLIT_NAVIGATOR,
292+
state: {
293+
index: 1,
294+
routes: [
295+
{name: SCREENS.WORKSPACE.INITIAL, params: {policyID: fakePolicyID}},
296+
{name: SCREENS.WORKSPACE.WORKFLOWS, params: {policyID: fakePolicyID}},
297+
],
298+
},
299+
},
300+
],
301+
},
302+
},
303+
],
304+
});
305+
306+
const {result} = renderHook(() => useRestoreWorkspacesTabOnNavigate());
307+
result.current();
308+
309+
expect(Navigation.navigate).toHaveBeenCalledWith(restoredPath);
310+
});
127311
});

0 commit comments

Comments
 (0)