-
Notifications
You must be signed in to change notification settings - Fork 4k
[Search v1] Implement policy filter #41769
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
Changes from all commits
5bc7f2a
ad29ae5
9adee7b
aa503df
9ba01ac
94580cc
d0a724e
194a44c
71204e2
36edddc
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 @@ | ||
| type SearchParams = { | ||
| query: string; | ||
| policyIDs?: string; | ||
| hash: number; | ||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,12 @@ | ||
| import {getActionFromState} from '@react-navigation/core'; | ||
| import type {NavigationAction, NavigationContainerRef, NavigationState, PartialState} from '@react-navigation/native'; | ||
| import {getPathFromState} from '@react-navigation/native'; | ||
| import type {ValueOf, Writable} from 'type-fest'; | ||
| import type {Writable} from 'type-fest'; | ||
| import getIsNarrowLayout from '@libs/getIsNarrowLayout'; | ||
| import CONST from '@src/CONST'; | ||
| import NAVIGATORS from '@src/NAVIGATORS'; | ||
| import type {Route} from '@src/ROUTES'; | ||
| import ROUTES from '@src/ROUTES'; | ||
| import SCREENS from '@src/SCREENS'; | ||
| import getStateFromPath from './getStateFromPath'; | ||
| import getTopmostCentralPaneRoute from './getTopmostCentralPaneRoute'; | ||
|
|
@@ -35,14 +36,8 @@ function getActionForBottomTabNavigator(action: StackNavigationAction, state: Na | |
| let payloadParams = params?.params as Record<string, string | undefined>; | ||
| let screen = params.screen; | ||
|
|
||
| // Case when the user is on the AllSettingsScreen and selects the specific workspace. The user is redirected then to the specific workspace settings. | ||
| if (screen === SCREENS.ALL_SETTINGS && policyID) { | ||
| screen = SCREENS.WORKSPACE.INITIAL; | ||
| } | ||
|
|
||
| // Alternative case when the user is on the specific workspace settings screen and selects "All" workspace. | ||
| else if (!policyID && screen === SCREENS.WORKSPACE.INITIAL) { | ||
| screen = SCREENS.ALL_SETTINGS; | ||
| if (screen === SCREENS.SEARCH.CENTRAL_PANE) { | ||
| screen = SCREENS.SEARCH.BOTTOM_TAB; | ||
| } | ||
|
|
||
| if (!payloadParams) { | ||
|
|
@@ -65,7 +60,6 @@ export default function switchPolicyID(navigation: NavigationContainerRef<RootSt | |
| if (!navigation) { | ||
| throw new Error("Couldn't find a navigation object. Is your component inside a screen in a navigator?"); | ||
| } | ||
|
|
||
| let root: NavigationRoot = navigation; | ||
| let current: NavigationRoot | undefined; | ||
|
|
||
|
|
@@ -76,7 +70,14 @@ export default function switchPolicyID(navigation: NavigationContainerRef<RootSt | |
| } | ||
|
|
||
| const rootState = navigation.getRootState() as NavigationState<RootStackParamList>; | ||
| const newPath = route ?? getPathFromState({routes: rootState.routes} as State, linkingConfig.config); | ||
| let newPath = route ?? getPathFromState({routes: rootState.routes} as State, linkingConfig.config); | ||
|
|
||
| // Currently, the search page displayed in the bottom tab has the same URL as the page in the central pane, so we need to redirect to the correct search route. | ||
| // Here's the configuration: src/libs/Navigation/AppNavigator/createCustomStackNavigator/index.tsx | ||
| const isOpeningSearchFromBottomTab = newPath.startsWith(CONST.SEARCH_BOTTOM_TAB_URL); | ||
|
Contributor
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. Can we use the screen name instead of creating a new const for it? |
||
| if (isOpeningSearchFromBottomTab) { | ||
| newPath = ROUTES.SEARCH.getRoute(CONST.TAB_SEARCH.ALL); | ||
| } | ||
| const stateFromPath = getStateFromPath(newPath as Route) as PartialState<NavigationState<RootStackParamList>>; | ||
| const action: StackNavigationAction = getActionFromState(stateFromPath, linkingConfig.config); | ||
|
|
||
|
|
@@ -93,51 +94,31 @@ export default function switchPolicyID(navigation: NavigationContainerRef<RootSt | |
| return; | ||
| } | ||
|
|
||
| // The correct route for SearchPage is located in the CentralPane | ||
| const shouldAddToCentralPane = !getIsNarrowLayout() || isOpeningSearchFromBottomTab; | ||
|
|
||
| // If the layout is wide we need to push matching central pane route to the stack. | ||
| if (!getIsNarrowLayout()) { | ||
| // Case when the user selects "All" workspace from the specific workspace settings | ||
| if (checkIfActionPayloadNameIsEqual(actionForBottomTabNavigator, SCREENS.ALL_SETTINGS) && !policyID) { | ||
| root.dispatch({ | ||
| type: CONST.NAVIGATION.ACTION_TYPE.PUSH, | ||
| payload: { | ||
| name: NAVIGATORS.CENTRAL_PANE_NAVIGATOR, | ||
| params: { | ||
| screen: SCREENS.SETTINGS.WORKSPACES, | ||
| params: undefined, | ||
| }, | ||
| }, | ||
| }); | ||
| } else { | ||
| const topmostCentralPaneRoute = getTopmostCentralPaneRoute(rootState); | ||
| const screen = topmostCentralPaneRoute?.name; | ||
| const params: CentralPaneRouteParams = {...topmostCentralPaneRoute?.params}; | ||
| const isWorkspaceScreen = screen && Object.values(SCREENS.WORKSPACE).includes(screen as ValueOf<typeof SCREENS.WORKSPACE>); | ||
|
|
||
| // Only workspace settings screens have to store the policyID in the params. | ||
| // In other case, the policyID is read from the BottomTab params. | ||
| if (!isWorkspaceScreen) { | ||
| delete params.policyID; | ||
| } else { | ||
| params.policyID = policyID; | ||
| } | ||
|
|
||
| // If the user is on the home page and changes the current workspace, then should be displayed a report from the selected workspace. | ||
| // To achieve that, it's necessary to navigate without the reportID param. | ||
| if (checkIfActionPayloadNameIsEqual(actionForBottomTabNavigator, SCREENS.HOME)) { | ||
| delete params.reportID; | ||
| } | ||
|
|
||
| root.dispatch({ | ||
| type: CONST.NAVIGATION.ACTION_TYPE.PUSH, | ||
| payload: { | ||
| name: NAVIGATORS.CENTRAL_PANE_NAVIGATOR, | ||
| params: { | ||
| screen, | ||
| params, | ||
| }, | ||
| }, | ||
| }); | ||
|
Comment on lines
-97
to
-139
Contributor
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. I'm not sure that I understand this diff. Would you mind explaining why we can replace this code?
Contributor
Author
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 code is related to the pages that have been already refactored. Previously Workspace Switcher was available on Account Settings and Workspace Initial Page pages, we don't need this anymore since these pages don't display WS :) |
||
| if (shouldAddToCentralPane) { | ||
| const topmostCentralPaneRoute = getTopmostCentralPaneRoute(rootState); | ||
| const screen = topmostCentralPaneRoute?.name; | ||
| const params: CentralPaneRouteParams = {...topmostCentralPaneRoute?.params}; | ||
|
|
||
| // If the user is on the home page and changes the current workspace, then should be displayed a report from the selected workspace. | ||
| // To achieve that, it's necessary to navigate without the reportID param. | ||
| if (checkIfActionPayloadNameIsEqual(actionForBottomTabNavigator, SCREENS.HOME)) { | ||
| delete params.reportID; | ||
| } | ||
|
|
||
| root.dispatch({ | ||
| type: CONST.NAVIGATION.ACTION_TYPE.PUSH, | ||
| payload: { | ||
| name: NAVIGATORS.CENTRAL_PANE_NAVIGATOR, | ||
| params: { | ||
| screen, | ||
| params, | ||
| }, | ||
| }, | ||
| }); | ||
| } else { | ||
| // If the layout is small we need to pop everything from the central pane so the bottom tab navigator is visible. | ||
| root.dispatch({ | ||
|
|
||
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.
Why did we remove this?