From ec4a01e74ba402b2aa782fb5d3759cde7058e39a Mon Sep 17 00:00:00 2001 From: Adam Grzybowski Date: Mon, 28 Oct 2024 12:00:33 +0100 Subject: [PATCH 1/3] Web - Thread - Highlighted message is not dismissed after clicking on the same chat in LHN --- src/libs/Navigation/linkTo/index.ts | 52 +++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/src/libs/Navigation/linkTo/index.ts b/src/libs/Navigation/linkTo/index.ts index 669a844a1443..3354942aa661 100644 --- a/src/libs/Navigation/linkTo/index.ts +++ b/src/libs/Navigation/linkTo/index.ts @@ -7,7 +7,7 @@ import {shallowCompare} from '@libs/ObjectUtils'; import {extractPolicyIDFromPath, getPathWithoutPolicyID} from '@libs/PolicyUtils'; import getStateFromPath from '@navigation/getStateFromPath'; import linkingConfig from '@navigation/linkingConfig'; -import type {RootStackParamList, StackNavigationAction} from '@navigation/types'; +import type {NavigationPartialRoute, ReportsSplitNavigatorParamList, RootStackParamList, StackNavigationAction} from '@navigation/types'; import CONST from '@src/CONST'; import NAVIGATORS from '@src/NAVIGATORS'; import type {Route} from '@src/ROUTES'; @@ -31,18 +31,14 @@ function createActionWithPolicyID(action: StackActionType, policyID: string): St }; } -function shouldDispatchAction(currentState: NavigationState, stateFromPath: PartialState>) { +function areNamesAndParamsEqual(currentState: NavigationState, stateFromPath: PartialState>) { const currentFocusedRoute = findFocusedRoute(currentState); const targetFocusedRoute = findFocusedRoute(stateFromPath); const areNamesEqual = currentFocusedRoute?.name === targetFocusedRoute?.name; const areParamsEqual = shallowCompare(currentFocusedRoute?.params as Record | undefined, targetFocusedRoute?.params as Record | undefined); - if (areNamesEqual && areParamsEqual) { - return false; - } - - return true; + return areNamesEqual && areParamsEqual; } function shouldCheckFullScreenRouteMatching(action: StackNavigationAction): action is StackNavigationAction & {type: 'PUSH'; payload: {name: typeof NAVIGATORS.RIGHT_MODAL_NAVIGATOR}} { @@ -53,6 +49,17 @@ function isNavigatingToAttachmentScreen(focusedRouteName?: string) { return focusedRouteName === SCREENS.ATTACHMENTS; } +function isNavigatingToReportWithSameReportID(currentRoute: NavigationPartialRoute, newRoute: NavigationPartialRoute) { + if (currentRoute.name !== SCREENS.REPORT || newRoute.name !== SCREENS.REPORT) { + return false; + } + + const currentParams = currentRoute.params as ReportsSplitNavigatorParamList[typeof SCREENS.REPORT]; + const newParams = newRoute?.params as ReportsSplitNavigatorParamList[typeof SCREENS.REPORT]; + + return currentParams.reportID === newParams.reportID; +} + export default function linkTo(navigation: NavigationContainerRef | null, path: Route, type?: string) { if (!navigation) { throw new Error("Couldn't find a navigation object. Is your component inside a screen in a navigator?"); @@ -66,30 +73,47 @@ export default function linkTo(navigation: NavigationContainerRef>; - const focusedRouteFromPath = findFocusedRoute(stateFromPath); const currentState = navigation.getRootState() as NavigationState; - const action: StackNavigationAction = getActionFromState(stateFromPath, linkingConfig.config); - // We don't want to dispatch action to push/replace with exactly the same route that is already focused. - if (!shouldDispatchAction(currentState, stateFromPath)) { + const focusedRouteFromPath = findFocusedRoute(stateFromPath); + const currentFocusedRoute = findFocusedRoute(currentState); + + // For type safety. It shouldn't every happen. + if (!focusedRouteFromPath || !currentFocusedRoute) { return; } + const action: StackNavigationAction = getActionFromState(stateFromPath, linkingConfig.config); + // If there is no action, just reset the whole state. if (!action) { navigation.resetRoot(stateFromPath); return; } + // We don't want to dispatch action to push/replace with exactly the same route that is already focused. + if (areNamesAndParamsEqual(currentState, stateFromPath)) { + return; + } + if (type === CONST.NAVIGATION.ACTION_TYPE.REPLACE) { action.type = CONST.NAVIGATION.ACTION_TYPE.REPLACE; } - // Attachments screen is a special case where we want to navigate to it without adding it to the browser history. - else if (action.type === CONST.NAVIGATION.ACTION_TYPE.NAVIGATE && !isNavigatingToAttachmentScreen(focusedRouteFromPath?.name)) { + + // Attachment screen - This is a special case. We want to navigate to it instead of push. If there is no screen on the stack, it will be pushed. + // If not, it will be replaced. This way, navigating between one attachment screen and another won't be added to the browser history. + // Report screen - Also a special case. If we are navigating to the report with same reportID we want to replace it (navigate will do that). + // This covers the case when we open a specific message in report (reportActionID). + else if ( + action.type === CONST.NAVIGATION.ACTION_TYPE.NAVIGATE && + !isNavigatingToAttachmentScreen(focusedRouteFromPath?.name) && + !isNavigatingToReportWithSameReportID(currentFocusedRoute, focusedRouteFromPath) + ) { // We want to PUSH by default to add entries to the browser history. action.type = CONST.NAVIGATION.ACTION_TYPE.PUSH; } + // Handle deep links including policyID as /w/:policyID. if (extractedPolicyID) { const actionWithPolicyID = createActionWithPolicyID(action as StackActionType, extractedPolicyID); if (!actionWithPolicyID) { @@ -100,7 +124,7 @@ export default function linkTo(navigation: NavigationContainerRef Date: Mon, 28 Oct 2024 12:27:12 +0100 Subject: [PATCH 2/3] fix typo --- src/libs/Navigation/linkTo/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/Navigation/linkTo/index.ts b/src/libs/Navigation/linkTo/index.ts index 3354942aa661..b0fae536186f 100644 --- a/src/libs/Navigation/linkTo/index.ts +++ b/src/libs/Navigation/linkTo/index.ts @@ -78,7 +78,7 @@ export default function linkTo(navigation: NavigationContainerRef Date: Mon, 28 Oct 2024 12:38:09 +0100 Subject: [PATCH 3/3] add test --- contributingGuides/NAVIGATION.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/contributingGuides/NAVIGATION.md b/contributingGuides/NAVIGATION.md index 09039c874a65..8079bb41f5b1 100644 --- a/contributingGuides/NAVIGATION.md +++ b/contributingGuides/NAVIGATION.md @@ -410,3 +410,17 @@ Linked issue: https://github.com/Expensify/App/pull/49539#issuecomment-243236062 4. Press arrow on the side of attachment modal to navigate to the second image. 5. Close the modal with X in the corner. 6. Verify that the modal is now fully closed. + +### Navigate instead of push for reports with same reportID + +Linked issue: https://github.com/Expensify/App/pull/49539#issuecomment-2433351709 + +1. Open app on wide layout web. +2. Go to report A (any report). +3. Go to report B (any report with message). +4. Press reply in thread. +5. Press on header subtitle. +6. Press on the report B in the sidebar. +7. Verify that the message you replied to is no longer highlighted. +8. Press the browsers back button. +9. Verify that you are on the A report. \ No newline at end of file