Skip to content

The central page page is not rendered when resized to narrow and then to wide view again #53466

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.

Sign up for GitHub

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
mountiny merged 3 commits into from
Dec 11, 2024
Merged

The central page page is not rendered when resized to narrow and then to wide view again #53466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c83a60
Add useNavigationResetRootOnLayoutChange
WojtekBoman Dec 3, 2024
5b46385
Add comments to useNavigationResetOnLayoutChange hooks
WojtekBoman Dec 4, 2024
de1a3d3
Add ResizeScreenTests
WojtekBoman Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/libs/Navigation/AppNavigator/createResponsiveStackNavigator/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type {ParamListBase} from '@react-navigation/native';
import {createNavigatorFactory} from '@react-navigation/native';
import useNavigationResetOnLayoutChange from '@libs/Navigation/AppNavigator/useNavigationResetOnLayoutChange';
import useNavigationResetRootOnLayoutChange from '@libs/Navigation/AppNavigator/useNavigationResetRootOnLayoutChange';
import createPlatformStackNavigatorComponent from '@libs/Navigation/PlatformStackNavigation/createPlatformStackNavigatorComponent';
import defaultPlatformStackScreenOptions from '@libs/Navigation/PlatformStackNavigation/defaultPlatformStackScreenOptions';
import type {PlatformStackNavigationEventMap, PlatformStackNavigationOptions, PlatformStackNavigationState} from '@libs/Navigation/PlatformStackNavigation/types';
Expand All @@ -12,7 +12,7 @@ const ResponsiveStackNavigatorComponent = createPlatformStackNavigatorComponent(
createRouter: CustomRouter,
defaultScreenOptions: defaultPlatformStackScreenOptions,
useCustomState: useStateWithSearch,
useCustomEffects: useNavigationResetOnLayoutChange,
useCustomEffects: useNavigationResetRootOnLayoutChange,
ExtraContent: RenderSearchRoute,
});

Expand Down
12 changes: 10 additions & 2 deletions src/libs/Navigation/AppNavigator/useNavigationResetOnLayoutChange.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,23 @@
import type {ParamListBase} from '@react-navigation/native';
import {useEffect} from 'react';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import navigationRef from '@libs/Navigation/navigationRef';
import type {CustomEffectsHookProps} from '@libs/Navigation/PlatformStackNavigation/types';

function useNavigationResetOnLayoutChange() {
/**
* This hook resets the navigation root state when changing the layout size, resetting the state calls the getRehydredState method in CustomFullScreenRouter.tsx.
* When the screen size is changed, it is necessary to check whether the application displays the content correctly.
* When the app is opened on a small layout and the user resizes it to wide, a second screen has to be present in the navigation state to fill the space.
*/
function useNavigationResetOnLayoutChange({navigation}: CustomEffectsHookProps<ParamListBase>) {
const {shouldUseNarrowLayout} = useResponsiveLayout();

useEffect(() => {
if (!navigationRef.isReady()) {
return;
}
navigationRef.resetRoot(navigationRef.getRootState());
navigation.reset(navigation.getState());
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [shouldUseNarrowLayout]);
}

Expand Down
21 changes: 21 additions & 0 deletions src/libs/Navigation/AppNavigator/useNavigationResetRootOnLayoutChange.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import {useEffect} from 'react';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import navigationRef from '@libs/Navigation/navigationRef';

/**
* This hook resets the navigation root state when changing the layout size, resetting the state calls the getRehydredState method in CustomRouter.ts.
* When the screen size is changed, it is necessary to check whether the application displays the content correctly.
* When the app is opened on a small layout and the user resizes it to wide, a second screen has to be present in the navigation state to fill the space.
*/
function useNavigationResetRootOnLayoutChange() {
Copy link
Contributor

@puneetlath puneetlath Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a comment description of what this is used for.

Copy link
Contributor

@mountiny mountiny Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to start setting this up for more unit tests in the navigation are too @WojtekBoman @adamgrzybowski

const {shouldUseNarrowLayout} = useResponsiveLayout();

useEffect(() => {
if (!navigationRef.isReady()) {
return;
}
navigationRef.resetRoot(navigationRef.getRootState());
}, [shouldUseNarrowLayout]);
}

export default useNavigationResetRootOnLayoutChange;
95 changes: 95 additions & 0 deletions tests/ui/ResizeScreenTests.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import {NavigationContainer} from '@react-navigation/native';
import {render, renderHook} from '@testing-library/react-native';
import React from 'react';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import type ResponsiveLayoutResult from '@hooks/useResponsiveLayout/types';
import getIsNarrowLayout from '@libs/getIsNarrowLayout';
import createResponsiveStackNavigator from '@libs/Navigation/AppNavigator/createResponsiveStackNavigator';
import BottomTabNavigator from '@libs/Navigation/AppNavigator/Navigators/BottomTabNavigator';
import useNavigationResetRootOnLayoutChange from '@libs/Navigation/AppNavigator/useNavigationResetRootOnLayoutChange';
import navigationRef from '@libs/Navigation/navigationRef';
import type {AuthScreensParamList} from '@libs/Navigation/types';
import ProfilePage from '@pages/settings/Profile/ProfilePage';
import NAVIGATORS from '@src/NAVIGATORS';
import SCREENS from '@src/SCREENS';

const RootStack = createResponsiveStackNavigator<AuthScreensParamList>();

jest.mock('@hooks/useResponsiveLayout', () => jest.fn());
jest.mock('@libs/getIsNarrowLayout', () => jest.fn());

jest.mock('@pages/settings/InitialSettingsPage');
jest.mock('@pages/settings/Profile/ProfilePage');
jest.mock('@libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar');

const DEFAULT_USE_RESPONSIVE_LAYOUT_VALUE: ResponsiveLayoutResult = {
shouldUseNarrowLayout: true,
isSmallScreenWidth: true,
isInNarrowPaneModal: false,
isExtraSmallScreenHeight: false,
isMediumScreenWidth: false,
isLargeScreenWidth: false,
isExtraSmallScreenWidth: false,
isSmallScreen: false,
onboardingIsMediumOrLargerScreenWidth: false,
};

const INITIAL_STATE = {
routes: [
{
name: NAVIGATORS.BOTTOM_TAB_NAVIGATOR,
state: {
index: 1,
routes: [{name: SCREENS.HOME}, {name: SCREENS.SETTINGS.ROOT}],
},
},
],
};

const mockedGetIsNarrowLayout = getIsNarrowLayout as jest.MockedFunction<typeof getIsNarrowLayout>;
const mockedUseResponsiveLayout = useResponsiveLayout as jest.MockedFunction<typeof useResponsiveLayout>;

describe('Resize screen', () => {
it('Should display the settings profile after resizing the screen with the settings page opened to the wide layout', () => {
Copy link
Contributor

@ntdiary ntdiary Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A great test case! As described, it ensures that when resizing the screen on the /settings page, useNavigationResetRootOnLayoutChange takes effect, and getAdaptedState pushes profile into the routes and then renders it (i.e., /settings/profile).

App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts

Lines 365 to 367 in f9ec4fa

const matchingCentralPaneRoute = getMatchingCentralPaneRouteForState(state);
if (matchingCentralPaneRoute) {
routes.push(matchingCentralPaneRoute);

As for the /settings/workspaces/xxx and /settings/workspaces/xxx/profile pages, their route switching happens here:

App/src/libs/Navigation/AppNavigator/createCustomFullScreenNavigator/CustomFullScreenRouter.tsx

Lines 51 to 54 in f9ec4fa

// If the screen is wide, there should be at least two screens inside:
// - WORKSPACE.INITIAL to cover left pane.
// - WORKSPACE.PROFILE (first workspace settings screen) to cover central pane.
if (!isNarrowLayout) {

If needed, maybe similar test case could be added for them as well? I’m not a test expert, so not entirely sure. :)

cc @mountiny @WojtekBoman

Copy link
Contributor Author

@WojtekBoman WojtekBoman Dec 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's a good point :) We're going to add this test in our PR including nav refactor as it contains many changes related to this code. I believe we can go ahead with this PR for now :)

Copy link
Contributor

@mountiny mountiny Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, I think we can keep on adding more test cases in future, but its good for this PR

// Given the initialized navigation on the narrow layout with the settings screen
mockedGetIsNarrowLayout.mockReturnValue(true);
mockedUseResponsiveLayout.mockReturnValue({...DEFAULT_USE_RESPONSIVE_LAYOUT_VALUE, shouldUseNarrowLayout: true});

const {rerender} = renderHook(() => useNavigationResetRootOnLayoutChange());

render(
<NavigationContainer
ref={navigationRef}
initialState={INITIAL_STATE}
>
<RootStack.Navigator>
<RootStack.Screen
name={NAVIGATORS.BOTTOM_TAB_NAVIGATOR}
component={BottomTabNavigator}
/>

<RootStack.Screen
name={SCREENS.SETTINGS.PROFILE.ROOT}
component={ProfilePage}
/>
</RootStack.Navigator>
</NavigationContainer>,
);

const rootStateBeforeResize = navigationRef.current?.getRootState();

expect(rootStateBeforeResize?.routes.at(0)?.name).toBe(NAVIGATORS.BOTTOM_TAB_NAVIGATOR);
expect(rootStateBeforeResize?.routes.at(1)).toBeUndefined();

// When resizing the screen to the wide layout
mockedGetIsNarrowLayout.mockReturnValue(false);
mockedUseResponsiveLayout.mockReturnValue({...DEFAULT_USE_RESPONSIVE_LAYOUT_VALUE, shouldUseNarrowLayout: false});
rerender({});

const rootStateAfterResize = navigationRef.current?.getRootState();

// Then the settings profile page should be displayed on the screen
expect(rootStateAfterResize?.routes.at(0)?.name).toBe(NAVIGATORS.BOTTOM_TAB_NAVIGATOR);
expect(rootStateAfterResize?.routes.at(1)?.name).toBe(SCREENS.SETTINGS.PROFILE.ROOT);
});
});
Loading