-
Notifications
You must be signed in to change notification settings - Fork 3k
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.
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
[Search v1] Implement Search navigation logic #40280
Changes from all commits
c96e516
1e5150b
eef26cc
3c74507
a707c28
3afd0b9
2206c1d
e7de7ea
b9d88d9
a1b228a
a72d221
633d881
2d78c11
b57d456
2fcae18
18ed94f
1de5ce3
f84911b
8ea25e3
5ea4a9f
2e38d66
f724761
f28c388
1b33065
39a308a
0c538bb
3cb9dbe
faf9a69
87a4076
3b7a83d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import React from 'react'; | ||
import type {CentralPaneNavigatorParamList, NavigationPartialRoute} from '@libs/Navigation/types'; | ||
|
||
const ActiveRouteContext = React.createContext(''); | ||
const ActiveRouteContext = React.createContext<NavigationPartialRoute<keyof CentralPaneNavigatorParamList> | undefined>(undefined); | ||
|
||
export default ActiveRouteContext; |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ import {getActionFromState} from '@react-navigation/core'; | |
import type {NavigationAction, NavigationContainerRef, NavigationState, PartialState} from '@react-navigation/native'; | ||
import type {Writable} from 'type-fest'; | ||
import getIsNarrowLayout from '@libs/getIsNarrowLayout'; | ||
import shallowCompare from '@libs/ObjectUtils'; | ||
import {extractPolicyIDFromPath, getPathWithoutPolicyID} from '@libs/PolicyUtils'; | ||
import CONST from '@src/CONST'; | ||
import NAVIGATORS from '@src/NAVIGATORS'; | ||
|
@@ -13,7 +14,6 @@ import getPolicyIDFromState from './getPolicyIDFromState'; | |
import getStateFromPath from './getStateFromPath'; | ||
import getTopmostBottomTabRoute from './getTopmostBottomTabRoute'; | ||
import getTopmostCentralPaneRoute from './getTopmostCentralPaneRoute'; | ||
import getTopmostReportId from './getTopmostReportId'; | ||
import linkingConfig from './linkingConfig'; | ||
import getAdaptedStateFromPath from './linkingConfig/getAdaptedStateFromPath'; | ||
import getMatchingBottomTabRouteForState from './linkingConfig/getMatchingBottomTabRouteForState'; | ||
|
@@ -152,16 +152,15 @@ export default function linkTo(navigation: NavigationContainerRef<RootStackParam | |
const topRouteName = rootState?.routes?.at(-1)?.name; | ||
const isTargetNavigatorOnTop = topRouteName === action.payload.name; | ||
|
||
const isTargetScreenDifferentThanCurrent = Boolean(topmostCentralPaneRoute && topmostCentralPaneRoute.name !== action.payload.params?.screen); | ||
const areParamsDifferent = !shallowCompare(topmostCentralPaneRoute?.params, action.payload.params?.params); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This also caused #40743 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix for that has been already merged. Could you check if it still occurs in dev environment? |
||
|
||
Comment on lines
+155
to
+157
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a specific case this is fixing? This is causing #41198 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change has been introduced to handle changing filters on the new SearchPage, on this page we can navigate to the same page with a different value of the query param, so that's why we added it. To me, we should create a custom method to compare params. Report ids should be compared as before but in other cases we can use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense! Thanks for the context! |
||
// In case if type is 'FORCED_UP' we replace current screen with the provided. This means the current screen no longer exists in the stack | ||
if (type === CONST.NAVIGATION.TYPE.FORCED_UP) { | ||
action.type = CONST.NAVIGATION.ACTION_TYPE.REPLACE; | ||
|
||
// If this action is navigating to the report screen and the top most navigator is different from the one we want to navigate - PUSH the new screen to the top of the stack | ||
} else if ( | ||
action.payload.name === NAVIGATORS.CENTRAL_PANE_NAVIGATOR && | ||
topmostCentralPaneRoute && | ||
(topmostCentralPaneRoute.name !== action.payload.params?.screen || getTopmostReportId(rootState) !== getTopmostReportId(stateFromPath)) | ||
) { | ||
} else if (action.payload.name === NAVIGATORS.CENTRAL_PANE_NAVIGATOR && (isTargetScreenDifferentThanCurrent || areParamsDifferent)) { | ||
// We need to push a tab if the tab doesn't match the central pane route that we are going to push. | ||
const topmostBottomTabRoute = getTopmostBottomTabRoute(rootState); | ||
const matchingBottomTabRoute = getMatchingBottomTabRouteForState(stateFromPath, policyID); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's add comment here