Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
10 changes: 4 additions & 6 deletions src/ROUTES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,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],
},
} as const satisfies DynamicRoutes;

const ROUTES = {
Expand Down Expand Up @@ -783,12 +787,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
37 changes: 37 additions & 0 deletions src/hooks/useReportFromDynamicRoute.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import {useRoute} from '@react-navigation/native';
import {useMemo} from 'react';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Report} from '@src/types/onyx';
import useOnyx from './useOnyx';

type UseReportFromDynamicRouteResult = {
report: Report | null | undefined;
reportID: string;
isLoading: boolean;
};

/**
* Hook to extract reportID from dynamic route path and fetch the report
* Use this for dynamic routes like /r/123/settings/name where reportID is in the URL path
*/
function useReportFromDynamicRoute(): UseReportFromDynamicRouteResult {
const route = useRoute();

// Extract reportID from the current path
const reportID = useMemo(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

❌ CLEAN-REACT-PATTERNS-0 (docs)

React Compiler is enabled in this codebase and automatically memoizes derived values. The useMemo wrapping the regex-based reportID extraction is redundant because the compiler will automatically cache this computation based on its dependency (route.path). Manual memoization adds noise and interferes with the compiler's optimization model.

Remove the useMemo wrapper and compute reportID directly:

const currentPath = route.path ?? '';
const match = currentPath.match(/\/r\/([^/]+)/);
const reportID = match ? match[1] : '';

Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

const currentPath = route.path ?? '';
const match = currentPath.match(/\/r\/([^/]+)/);

Choose a reason for hiding this comment

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

P1 Badge Parse report IDs from search report dynamic routes

useReportFromDynamicRoute only extracts IDs from paths matching /r/:reportID, but this dynamic route is explicitly allowed from SCREENS.RIGHT_MODAL.SEARCH_REPORT, whose URL shape is search/view/:reportID (ROUTES.SEARCH_REPORT). For URLs like /search/view/123/settings/name, reportID becomes empty and isLoading stays true because of !reportID, leaving the page stuck on the loading indicator.

Useful? React with 👍 / 👎.

return match ? match[1] : '';
}, [route.path]);

const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`);
const [isLoadingReportData] = useOnyx(ONYXKEYS.IS_LOADING_REPORT_DATA);

return {
report,
reportID,
isLoading: !reportID || (!report && !!isLoadingReportData),
};
}

export default useReportFromDynamicRoute;
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
4 changes: 1 addition & 3 deletions src/libs/Navigation/linkingConfig/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1399,9 +1399,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
6 changes: 1 addition & 5 deletions src/libs/Navigation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1685,11 +1685,7 @@ 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]: {
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.DYNAMIC_SETTINGS_NAME]: undefined;
[SCREENS.REPORT_SETTINGS.NOTIFICATION_PREFERENCES]: {
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
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/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
41 changes: 41 additions & 0 deletions src/pages/settings/Report/DynamicNamePage.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import React from 'react';
import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView';
import FullscreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import useDynamicBackPath from '@hooks/useDynamicBackPath';
import useReportFromDynamicRoute from '@hooks/useReportFromDynamicRoute';
import {isGroupChat, isTripRoom} from '@libs/ReportUtils';
import GroupChatNameEditPage from '@pages/GroupChatNameEditPage';
import TripChatNameEditPage from '@pages/TripChatNameEditPage';
import {DYNAMIC_ROUTES} from '@src/ROUTES';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import RoomNamePage from './RoomNamePage';

function DynamicNamePage() {
const backPath = useDynamicBackPath(DYNAMIC_ROUTES.REPORT_SETTINGS_NAME.path);
const {report, isLoading} = useReportFromDynamicRoute();

if (isLoading) {
return <FullscreenLoadingIndicator />;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

❌ UI-1 (docs)

FullscreenLoadingIndicator is used in an early return with no navigation component (no HeaderWithBackButton, no close button) in the same conditional branch. If loading hangs, the user is trapped with no way to escape. The shouldUseGoBackButton prop enables an emergency "Go Back" button that appears after a timeout.

Add shouldUseGoBackButton to the loading indicator:

if (isLoading) {
    return <FullscreenLoadingIndicator shouldUseGoBackButton />;
}

Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.


if (isEmptyObject(report)) {
return <FullPageNotFoundView shouldShow />;

Choose a reason for hiding this comment

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

P2 Badge Keep report access guards on dynamic name page

This page now treats any non-empty report object as valid and immediately renders edit screens, but it no longer applies the withReportOrNotFound checks that previously enforced canAccessReport (including errorFields.notFound handling). If a cached report contains a not-found/access-restricted state, users can still reach the rename UI instead of being blocked by NotFound behavior.

Useful? React with 👍 / 👎.

}

if (isTripRoom(report)) {
return <TripChatNameEditPage report={report} />;
}

if (isGroupChat(report)) {
return <GroupChatNameEditPage report={report} />;
}

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

export default DynamicNamePage;
26 changes: 0 additions & 26 deletions src/pages/settings/Report/NamePage.tsx

This file was deleted.

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
Loading