Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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 contributingGuides/NAVIGATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -711,10 +711,10 @@ Do not use dynamic routes when:
- `path`: The URL suffix (e.g. `'verify-account'`).
- `entryScreens`: List of screen names that are allowed to have this suffix appended (access control; see [Entry Screens (Access Control)](#entry-screens-access-control)).

`createDynamicRoute(suffix)` — [`createDynamicRoute.ts`](src/libs/Navigation/helpers/createDynamicRoute.ts). Accepts a `DynamicRouteSuffix` (from `DYNAMIC_ROUTES`), appends it to the current active route and returns the full route. Use the following when navigating to a dynamic route:
`createDynamicRoute(suffix)` — [`createDynamicRoute.ts`](src/libs/Navigation/helpers/dynamicRoutesUtils/createDynamicRoute.ts). Accepts a `DynamicRouteSuffix` (from `DYNAMIC_ROUTES`), appends it to the current active route and returns the full route. Use the following when navigating to a dynamic route:

```ts
import createDynamicRoute from '@libs/Navigation/helpers/createDynamicRoute';
import createDynamicRoute from '@libs/Navigation/helpers/dynamicRoutesUtils/createDynamicRoute';
import Navigation from '@libs/Navigation/Navigation';
import ROUTES, {DYNAMIC_ROUTES} from '@src/ROUTES';

Expand Down
10 changes: 4 additions & 6 deletions src/ROUTES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ const DYNAMIC_ROUTES = {
path: 'owner-selector',
entryScreens: [],
},
REPORT_SETTINGS_NAME: {
path: 'settings/name',
entryScreens: [SCREENS.REPORT_DETAILS.ROOT, SCREENS.RIGHT_MODAL.REPORT_SETTINGS, SCREENS.REPORT, SCREENS.RIGHT_MODAL.SEARCH_REPORT, SCREENS.SEARCH.ROOT],
},
ADDRESS_COUNTRY: {
path: 'country',
entryScreens: [
Expand Down Expand Up @@ -792,12 +796,6 @@ const ROUTES = {
// eslint-disable-next-line no-restricted-syntax -- Legacy route generation
getRoute: (reportID: string, backTo?: string) => getUrlWithBackToParam(`r/${reportID}/settings` as const, backTo),
},
REPORT_SETTINGS_NAME: {
route: 'r/:reportID/settings/name',

// eslint-disable-next-line no-restricted-syntax -- Legacy route generation
getRoute: (reportID: string, backTo?: string) => getUrlWithBackToParam(`r/${reportID}/settings/name` as const, backTo),
},
REPORT_SETTINGS_NOTIFICATION_PREFERENCES: {
route: 'r/:reportID/settings/notification-preferences',

Expand Down
2 changes: 1 addition & 1 deletion src/SCREENS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ const SCREENS = {

REPORT_SETTINGS: {
ROOT: 'Report_Settings_Root',
NAME: 'Report_Settings_Name',
DYNAMIC_SETTINGS_NAME: 'Dynamic_Report_Settings_Name',
NOTIFICATION_PREFERENCES: 'Report_Settings_Notification_Preferences',
WRITE_CAPABILITY: 'Report_Settings_Write_Capability',
VISIBILITY: 'Report_Settings_Visibility',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ const ReportChangeApproverModalStackNavigator = createModalStackNavigator<Report

const ReportSettingsModalStackNavigator = createModalStackNavigator<ReportSettingsNavigatorParamList>({
[SCREENS.REPORT_SETTINGS.ROOT]: () => require<ReactComponentModule>('../../../../pages/settings/Report/ReportSettingsPage').default,
[SCREENS.REPORT_SETTINGS.NAME]: () => require<ReactComponentModule>('../../../../pages/settings/Report/NamePage').default,
[SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_NAME]: () => require<ReactComponentModule>('../../../../pages/settings/Report/DynamicNamePage').default,
[SCREENS.REPORT_SETTINGS.NOTIFICATION_PREFERENCES]: () => require<ReactComponentModule>('../../../../pages/settings/Report/NotificationPreferencePage').default,
[SCREENS.REPORT_SETTINGS.WRITE_CAPABILITY]: () => require<ReactComponentModule>('../../../../pages/settings/Report/WriteCapabilityPage').default,
[SCREENS.REPORT_SETTINGS.VISIBILITY]: () => require<ReactComponentModule>('../../../../pages/settings/Report/VisibilityPage').default,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import splitPathAndQuery from './splitPathAndQuery';
type LeafRoute = {
name: string;
path: string;
params?: Record<string, string>;
params?: Record<string, unknown>;
};

type NestedRoute = {
Expand Down Expand Up @@ -54,7 +54,7 @@ function getRouteNamesForDynamicRoute(dynamicRouteName: DynamicRouteSuffix): str
return null;
}

function getStateForDynamicRoute(path: string, dynamicRouteName: keyof typeof DYNAMIC_ROUTES) {
function getStateForDynamicRoute(path: string, dynamicRouteName: keyof typeof DYNAMIC_ROUTES, parentRouteParams?: Record<string, unknown>) {
const routeConfig = getRouteNamesForDynamicRoute(DYNAMIC_ROUTES[dynamicRouteName].path);
const [, query] = splitPathAndQuery(path);
const params = getParamsFromQuery(query);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@collectioneur How about const params = getParamsFromQuery(query);? Do we still need this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we do need to keep this. Dynamic routes can also have query parameters, and this seems to be the most reliable way to extract them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated. thanks

Expand All @@ -67,12 +67,15 @@ function getStateForDynamicRoute(path: string, dynamicRouteName: keyof typeof DY
const buildNestedState = (routes: string[], currentIndex: number): RouteNode => {
const currentRoute = routes.at(currentIndex);

// If this is the last route, create leaf node with path
// If this is the last route, create leaf node with path and inherited params
if (currentIndex === routes.length - 1) {
return {
name: currentRoute ?? '',
path,
params,
params: {
...params,
...(parentRouteParams ?? {}),
},
};
}

Expand Down
2 changes: 1 addition & 1 deletion src/libs/Navigation/helpers/getStateFromPath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function getStateFromPath(path: Route): PartialState<NavigationState> {
if (focusedRoute?.name) {
if (entryScreens.includes(focusedRoute.name as Screen)) {
// Generate navigation state for the dynamic route
const dynamicRouteState = getStateForDynamicRoute(normalizedPath, dynamicRoute as DynamicRouteKey);
const dynamicRouteState = getStateForDynamicRoute(normalizedPath, dynamicRoute as DynamicRouteKey, focusedRoute?.params as Record<string, unknown> | undefined);
return dynamicRouteState;
}

Expand Down
4 changes: 1 addition & 3 deletions src/libs/Navigation/linkingConfig/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1396,9 +1396,7 @@ const config: LinkingOptions<RootNavigatorParamList>['config'] = {
[SCREENS.REPORT_SETTINGS.ROOT]: {
path: ROUTES.REPORT_SETTINGS.route,
},
[SCREENS.REPORT_SETTINGS.NAME]: {
path: ROUTES.REPORT_SETTINGS_NAME.route,
},
[SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_NAME]: DYNAMIC_ROUTES.REPORT_SETTINGS_NAME.path,
[SCREENS.REPORT_SETTINGS.NOTIFICATION_PREFERENCES]: {
path: ROUTES.REPORT_SETTINGS_NOTIFICATION_PREFERENCES.route,
},
Expand Down
4 changes: 1 addition & 3 deletions src/libs/Navigation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1683,10 +1683,8 @@ type ReportSettingsNavigatorParamList = {
// eslint-disable-next-line no-restricted-syntax -- `backTo` usages in this file are legacy. Do not add new `backTo` params to screens. See contributingGuides/NAVIGATION.md
backTo?: Routes;
};
[SCREENS.REPORT_SETTINGS.NAME]: {
[SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_NAME]: {
reportID: string;
// eslint-disable-next-line no-restricted-syntax -- `backTo` usages in this file are legacy. Do not add new `backTo` params to screens. See contributingGuides/NAVIGATION.md
backTo?: Routes;
};
[SCREENS.REPORT_SETTINGS.NOTIFICATION_PREFERENCES]: {
reportID: string;
Expand Down
7 changes: 5 additions & 2 deletions src/pages/ReportDetailsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useThemeStyles from '@hooks/useThemeStyles';
import getBase62ReportID from '@libs/getBase62ReportID';
import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
import createDynamicRoute from '@libs/Navigation/helpers/dynamicRoutesUtils/createDynamicRoute';
import Navigation, {navigationRef} from '@libs/Navigation/Navigation';
import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types';
import type {ReportDetailsNavigatorParamList, RightModalNavigatorParamList} from '@libs/Navigation/types';
Expand Down Expand Up @@ -125,7 +126,7 @@ import CONST from '@src/CONST';
import type {TranslationPaths} from '@src/languages/types';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Route} from '@src/ROUTES';
import ROUTES from '@src/ROUTES';
import ROUTES, {DYNAMIC_ROUTES} from '@src/ROUTES';
import SCREENS from '@src/SCREENS';
import type * as OnyxTypes from '@src/types/onyx';
import type DeepValueOf from '@src/types/utils/DeepValueOf';
Expand Down Expand Up @@ -818,7 +819,9 @@ function ReportDetailsPage({policy, report, route, reportMetadata}: ReportDetail
furtherDetails={chatRoomSubtitle && !isGroupChat ? additionalRoomDetails : ''}
furtherDetailsNumberOfLines={isWorkspaceChat ? 0 : undefined}
furtherDetailsStyle={isWorkspaceChat ? [styles.textAlignCenter, styles.breakWord] : undefined}
onPress={() => Navigation.navigate(ROUTES.REPORT_SETTINGS_NAME.getRoute(report.reportID, backTo))}
onPress={() => {
Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.REPORT_SETTINGS_NAME.path));
}}
numberOfLinesTitle={isThread ? 2 : 0}
shouldBreakWord
/>
Expand Down
1 change: 1 addition & 0 deletions src/pages/inbox/report/withReportOrNotFound.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ type ScreenProps =
| PlatformStackScreenProps<ReportDetailsNavigatorParamList, typeof SCREENS.REPORT_DETAILS.ROOT>
| PlatformStackScreenProps<ReportDetailsNavigatorParamList, typeof SCREENS.REPORT_DETAILS.SHARE_CODE>
| PlatformStackScreenProps<ReportSettingsNavigatorParamList, typeof SCREENS.REPORT_SETTINGS.ROOT>
| PlatformStackScreenProps<ReportSettingsNavigatorParamList, typeof SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_NAME>
| PlatformStackScreenProps<RoomMembersNavigatorParamList, typeof SCREENS.ROOM_MEMBERS.DETAILS>
| PlatformStackScreenProps<ReportChangeWorkspaceNavigatorParamList, typeof SCREENS.REPORT_CHANGE_WORKSPACE.ROOT>
| PlatformStackScreenProps<ReportChangeApproverParamList, typeof SCREENS.REPORT_CHANGE_APPROVER.ROOT>;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
import React from 'react';
import useDynamicBackPath from '@hooks/useDynamicBackPath';
import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types';
import {isGroupChat, isTripRoom} from '@libs/ReportUtils';
import type {ReportSettingsNavigatorParamList} from '@navigation/types';
import GroupChatNameEditPage from '@pages/GroupChatNameEditPage';
import withReportOrNotFound from '@pages/inbox/report/withReportOrNotFound';
import type {WithReportOrNotFoundProps} from '@pages/inbox/report/withReportOrNotFound';
import TripChatNameEditPage from '@pages/TripChatNameEditPage';
import {DYNAMIC_ROUTES} from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import RoomNamePage from './RoomNamePage';

type NamePageProps = WithReportOrNotFoundProps & PlatformStackScreenProps<ReportSettingsNavigatorParamList, typeof SCREENS.REPORT_SETTINGS.NAME>;
type DynamicNamePageProps = WithReportOrNotFoundProps & PlatformStackScreenProps<ReportSettingsNavigatorParamList, typeof SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_NAME>;

function DynamicNamePage({report}: DynamicNamePageProps) {
const backPath = useDynamicBackPath(DYNAMIC_ROUTES.REPORT_SETTINGS_NAME.path);

function NamePage({report}: NamePageProps) {
if (isTripRoom(report)) {
return <TripChatNameEditPage report={report} />;
}
Expand All @@ -20,7 +24,12 @@ function NamePage({report}: NamePageProps) {
return <GroupChatNameEditPage report={report} />;
}

return <RoomNamePage report={report} />;
return (
<RoomNamePage
report={report}
navigateBackTo={backPath}
/>
);
}

export default withReportOrNotFound()(NamePage);
export default withReportOrNotFound()(DynamicNamePage);
16 changes: 6 additions & 10 deletions src/pages/settings/Report/RoomNamePage.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {useIsFocused, useRoute} from '@react-navigation/native';
import {useIsFocused} from '@react-navigation/native';
import React, {useCallback, useRef} from 'react';
import {View} from 'react-native';
import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView';
Expand All @@ -15,35 +15,31 @@ import useReportIsArchived from '@hooks/useReportIsArchived';
import useThemeStyles from '@hooks/useThemeStyles';
import {addErrorMessage} from '@libs/ErrorUtils';
import Navigation from '@libs/Navigation/Navigation';
import type {PlatformStackRouteProp} from '@libs/Navigation/PlatformStackNavigation/types';
import type {ReportSettingsNavigatorParamList} from '@libs/Navigation/types';
import {shouldDisableRename} from '@libs/ReportUtils';
import {isExistingRoomName, isReservedRoomName, isValidRoomNameWithoutLimits} from '@libs/ValidationUtils';
import {updatePolicyRoomName as updatePolicyRoomNameReportAction} from '@userActions/Report';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import type {Route} from '@src/ROUTES';
import INPUT_IDS from '@src/types/form/RoomNameForm';
import type {Report} from '@src/types/onyx';

type RoomNamePageProps = {
report: Report;
navigateBackTo?: Route;
};

function RoomNamePage({report}: RoomNamePageProps) {
const route = useRoute<PlatformStackRouteProp<ReportSettingsNavigatorParamList, typeof SCREENS.REPORT_SETTINGS.NAME>>();
function RoomNamePage({report, navigateBackTo}: RoomNamePageProps) {
const styles = useThemeStyles();
const roomNameInputRef = useRef<AnimatedTextInputRef>(null);
const isFocused = useIsFocused();
const {translate} = useLocalize();
const reportID = report?.reportID;
const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT);
const isReportArchived = useReportIsArchived(report?.reportID);

const goBack = useCallback(() => {
Navigation.setNavigationActionToMicrotaskQueue(() => Navigation.goBack(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(reportID, route.params.backTo)));
}, [reportID, route.params.backTo]);
Navigation.setNavigationActionToMicrotaskQueue(() => Navigation.goBack(navigateBackTo));
}, [navigateBackTo]);

const validate = useCallback(
(values: FormOnyxValues<typeof ONYXKEYS.FORMS.ROOM_NAME_FORM>) => {
Expand Down
20 changes: 20 additions & 0 deletions tests/navigation/getStateForDynamicRouteTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ jest.mock('@src/ROUTES', () => ({
type LeafRoute = {
name: string;
path: string;
params?: Record<string, unknown>;
};

type NestedRoute = {
Expand Down Expand Up @@ -111,4 +112,23 @@ describe('getStateForDynamicRoute', () => {
expect(state?.index).toBe(0);
expect(Array.isArray(state?.routes)).toBe(true);
});

it('should inherit parent route params on the leaf node', () => {
const path = '/r/12345/settings/name';
const parentParams = {reportID: '12345'};
const result = getStateForDynamicRoute(path, KEY_TEST as unknown as keyof typeof DYNAMIC_ROUTES, parentParams);

const rootRoute = result.routes.at(0) as NestedRoute | undefined;
const leafRoute = rootRoute?.state.routes.at(0) as LeafRoute | undefined;
expect(leafRoute?.params).toEqual(parentParams);
});

it('should not include params on the leaf node when parentRouteParams is undefined', () => {
const path = '/some/path/test-path';
const result = getStateForDynamicRoute(path, KEY_TEST as unknown as keyof typeof DYNAMIC_ROUTES);

const rootRoute = result.routes.at(0) as NestedRoute | undefined;
const leafRoute = rootRoute?.state.routes.at(0) as LeafRoute | undefined;
expect(leafRoute?.params).toBeUndefined();
});
});
5 changes: 3 additions & 2 deletions tests/navigation/getStateFromPathTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,18 @@ describe('getStateFromPath', () => {
it('should generate dynamic state when authorized screen is focused', () => {
const fullPath = '/settings/wallet/verify-account';
const baseRouteState = {routes: [{name: 'Wallet'}]};
const focusedRouteParams = {walletID: '456'};

mockRNGetStateFromPath.mockReturnValue(baseRouteState);
mockFindFocusedRoute.mockReturnValue({name: 'Wallet'});
mockFindFocusedRoute.mockReturnValue({name: 'Wallet', params: focusedRouteParams});

const expectedDynamicState = {routes: [{name: 'DynamicRoot'}]};
mockGetStateForDynamicRoute.mockReturnValue(expectedDynamicState);

const result = getStateFromPath(fullPath as unknown as Route);

expect(result).toBe(expectedDynamicState);
expect(mockGetStateForDynamicRoute).toHaveBeenCalledWith(fullPath, 'VERIFY_ACCOUNT');
expect(mockGetStateForDynamicRoute).toHaveBeenCalledWith(fullPath, 'VERIFY_ACCOUNT', focusedRouteParams);
});

it('should fallback to standard RN parsing if focused screen is NOT authorized for dynamic route', () => {
Expand Down
Loading