Skip to content
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

[navigation-refactor] Use minimal action to navigate inside RHP #21067

Merged
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
102b88b
remove animation for replace inside RHPStack
adamgrzybowski Jun 19, 2023
66652df
add minimalAction utility for linkTo
adamgrzybowski Jun 19, 2023
ebf4c74
add prettier adjustments
adamgrzybowski Jun 19, 2023
0c2f8e5
add prettier adjustments v2
adamgrzybowski Jun 19, 2023
ae9c365
Update src/libs/Navigation/linkTo.js
adamgrzybowski Jun 20, 2023
7765be3
Update src/libs/Navigation/linkTo.js
adamgrzybowski Jun 20, 2023
b9c5e9b
Update src/libs/Navigation/linkTo.js
adamgrzybowski Jun 20, 2023
fc1abd5
Update src/libs/Navigation/linkTo.js
adamgrzybowski Jun 20, 2023
89701ca
Update src/libs/Navigation/linkTo.js
adamgrzybowski Jun 20, 2023
3d91b47
add informations about minimal action in docs
adamgrzybowski Jun 21, 2023
7fe01d4
make adjustments to the linkTo
adamgrzybowski Jun 21, 2023
9e2dbd3
Merge branch 'main' into @swm/navigation-refactor-minimal-action
adamgrzybowski Jun 21, 2023
372be53
Merge branch 'main' into @swm/navigation-refactor-minimal-action
adamgrzybowski Jun 21, 2023
51f5c61
add break condition inside getMinimalAction
adamgrzybowski Jun 22, 2023
93b1938
remove defaultModalScreenOptions
adamgrzybowski Jun 23, 2023
f2efbcf
Merge branch 'main' into @swm/navigation-refactor-minimal-action
adamgrzybowski Jun 23, 2023
cf7c65e
Update contributingGuides/NAVIGATION.md
adamgrzybowski Jun 23, 2023
fc3d7ee
Update contributingGuides/NAVIGATION.md
adamgrzybowski Jun 23, 2023
47e5fbc
Update contributingGuides/NAVIGATION.md
adamgrzybowski Jun 23, 2023
c9585e9
modify docs
adamgrzybowski Jun 26, 2023
9a5fb8e
Merge branch 'main' into @swm/navigation-refactor-minimal-action
adamgrzybowski Jul 4, 2023
8df6227
fix linting error
adamgrzybowski Jul 4, 2023
ab0ee18
Merge branch 'main' into @swm/navigation-refactor-minimal-action
adamgrzybowski Jul 4, 2023
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
71 changes: 71 additions & 0 deletions contributingGuides/NAVIGATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,74 @@ Using [react-freeze](https://github.com/software-mansion/react-freeze) allows us

- To keep the navigation state that was present before changing the layout, we save the state on every change and use it for the `initialState` prop.
Changing the layout means that every component inside `NavigationContainer` is mounted anew.

## Why we need to use minimal action in the `linkTo` function

### The problem
Let's assume that the user wants to navigate like this:

```
1. Settings_root - navigate > Profile
2. Profile - UP > Settings_root
3. Settings_root - navigate > About
4. About - browser back button > Settings_root
```

Without minimal action, expected behavior won't be achieved and the final screen will be `Profile`.

Broken behavior is the outcome of two things:
1. Back button in the browser resets the navigation state with the state saved in step two.

2. `Navigation.navigate` creates action with `getActionFromState` dispatched high in the hierarchy.
adamgrzybowski marked this conversation as resolved.
Show resolved Hide resolved

The reason why `getActionFromState` provided by `react-navigation` is dispatched high in hierarchy is that it doesn't know about current navigation state, only about desired one.
adamgrzybowski marked this conversation as resolved.
Show resolved Hide resolved

In this example it doesn't know if the `RightModalNavigator` and `Settings` are already mounted.


The action for the first step looks like that:
```json
{
"type": "NAVIGATE",
"payload": {
"name": "RightModalNavigator",
"params": {
"initial": true,
"screen": "Settings",
"params": {
"initial": true,
"screen": "Profile",
"path": "/settings/profile"
}
}
}
}
```


That means, the params for the `RightModalNavigator` and `Settings` (also a navigator) will be filled with the information that we want to navigate to the `Profile` screen.
adamgrzybowski marked this conversation as resolved.
Show resolved Hide resolved

This information will stay here even if we pop the `Profile` screen and navigate to `About` screen.

Later on, when the user presses the browser back button expecting that the `Settings_root` screen will appear, the navigation state will be reset with information about the `Profile` screen in the params and this will be used as a source of truth for the navigation.
adamgrzybowski marked this conversation as resolved.
Show resolved Hide resolved

### The solution

If we can create simple action that will only push one screen to the existing navigator, we won't fill any params of the navigators.

The `getMinimalAction` compares action generated by the `getActionFromState` with the current navigation state and tries to find a simpler action.
adamgrzybowski marked this conversation as resolved.
Show resolved Hide resolved

The action for the first step created with `getMinimalAction` looks like this:

```json
{
"type": "NAVIGATE",
"payload": {
"name": "Settings_Profile"
},
"target": "Settings-stack-key-xyz"
}
```

### Deeplinking
There is no minimal action for deeplinking directly to the `Profile` screen. But because the `Settings_root` is not on the stack, pressing UP will reset the params for navigators to the correct ones.
Original file line number Diff line number Diff line change
Expand Up @@ -2,106 +2,87 @@ import React from 'react';
import {createStackNavigator} from '@react-navigation/stack';

import * as ModalStackNavigators from '../ModalStackNavigators';
import defaultModalScreenOptions from '../defaultModalScreenOptions';
adamgrzybowski marked this conversation as resolved.
Show resolved Hide resolved
import RHPScreenOptions from '../RHPScreenOptions';

const Stack = createStackNavigator();

function RigthModalNavigator() {
return (
<Stack.Navigator>
<Stack.Navigator screenOptions={RHPScreenOptions}>
<Stack.Screen
name="Settings"
options={defaultModalScreenOptions}
component={ModalStackNavigators.SettingsModalStackNavigator}
/>
<Stack.Screen
name="NewChat"
options={defaultModalScreenOptions}
component={ModalStackNavigators.NewChatModalStackNavigator}
/>
<Stack.Screen
name="NewGroup"
options={defaultModalScreenOptions}
component={ModalStackNavigators.NewGroupModalStackNavigator}
/>
<Stack.Screen
name="Search"
options={defaultModalScreenOptions}
component={ModalStackNavigators.SearchModalStackNavigator}
/>
<Stack.Screen
name="Details"
options={defaultModalScreenOptions}
component={ModalStackNavigators.DetailsModalStackNavigator}
/>
<Stack.Screen
name="Profile"
options={defaultModalScreenOptions}
component={ModalStackNavigators.ProfileModalStackNavigator}
/>
<Stack.Screen
name="Report_Details"
options={defaultModalScreenOptions}
component={ModalStackNavigators.ReportDetailsModalStackNavigator}
/>
<Stack.Screen
name="Report_Settings"
options={defaultModalScreenOptions}
component={ModalStackNavigators.ReportSettingsModalStackNavigator}
/>
<Stack.Screen
name="Report_WelcomeMessage"
options={defaultModalScreenOptions}
component={ModalStackNavigators.ReportWelcomeMessageModalStackNavigator}
/>
<Stack.Screen
name="Participants"
options={defaultModalScreenOptions}
component={ModalStackNavigators.ReportParticipantsModalStackNavigator}
/>
<Stack.Screen
name="MoneyRequest"
options={defaultModalScreenOptions}
component={ModalStackNavigators.MoneyRequestModalStackNavigator}
/>
<Stack.Screen
name="NewTask"
options={defaultModalScreenOptions}
component={ModalStackNavigators.NewTaskModalStackNavigator}
/>
<Stack.Screen
name="Task_Details"
options={defaultModalScreenOptions}
component={ModalStackNavigators.TaskModalStackNavigator}
/>
<Stack.Screen
name="EnablePayments"
options={defaultModalScreenOptions}
component={ModalStackNavigators.EnablePaymentsStackNavigator}
/>
<Stack.Screen
name="SplitDetails"
options={defaultModalScreenOptions}
component={ModalStackNavigators.SplitDetailsModalStackNavigator}
/>
<Stack.Screen
name="AddPersonalBankAccount"
options={defaultModalScreenOptions}
component={ModalStackNavigators.AddPersonalBankAccountModalStackNavigator}
/>
<Stack.Screen
name="Wallet_Statement"
options={defaultModalScreenOptions}
component={ModalStackNavigators.WalletStatementStackNavigator}
/>
<Stack.Screen
name="Select_Year"
options={defaultModalScreenOptions}
component={ModalStackNavigators.YearPickerStackNavigator}
/>
<Stack.Screen
name="Flag_Comment"
options={defaultModalScreenOptions}
component={ModalStackNavigators.FlagCommentStackNavigator}
/>
</Stack.Navigator>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import {CardStyleInterpolators} from '@react-navigation/stack';
import styles from '../../../styles/styles';

const defaultModalScreenOptions = {
const RHPScreenOptions = {
headerShown: false,
animationEnabled: true,
animationEnabled: false,
gestureDirection: 'horizontal',
cardStyle: styles.navigationScreenCardStyle,
cardStyleInterpolator: CardStyleInterpolators.forHorizontalIOS,
};

export default defaultModalScreenOptions;
export default RHPScreenOptions;
53 changes: 51 additions & 2 deletions src/libs/Navigation/linkTo.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,43 @@ import linkingConfig from './linkingConfig';
import getTopmostReportId from './getTopmostReportId';
import getStateFromPath from './getStateFromPath';

/**
* Motivation for this function is described in NAVIGATION.md
*
* @param {Object} action action generated by getActionFromState
adamgrzybowski marked this conversation as resolved.
Show resolved Hide resolved
* @param {Object} state The root state
* @returns {{minimalAction: Object, targetName: String}} minimal action is the action that we should dispatch and targetName is the name of the target navigator.
* targetName name is necessary to determine if we are going to use REPLACE for navigating between RHP flows.
*/
function getMinimalAction(action, state) {
let currentAction = action;
let currentState = state;
let currentTargetKey = null;
let targetName = null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This PR caused a regression, for more details refer this PR


while (currentState.routes[currentState.index].name === currentAction.payload.name) {
if (!currentState.routes[currentState.index].state) {
break;
}

targetName = currentState.routes[currentState.index].name;
currentState = currentState.routes[currentState.index].state;

currentTargetKey = currentState.key;

// Creating new smaller action
currentAction = {
type: currentAction.type,
payload: {
name: currentAction.payload.params.screen,
params: currentAction.payload.params.params,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
params: currentAction.payload.params.params,
params: currentAction.payload.params.params,
path: currentAction.payload.params.path,

path was missing here and it caused regression - #22308

Copy link
Contributor

Choose a reason for hiding this comment

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

@situchan Can you raise a PR fixing this?

},
target: currentTargetKey,
};
}
return {minimalAction: currentAction, targetName};
}

export default function linkTo(navigation, path, type) {
if (navigation === undefined) {
throw new Error("Couldn't find a navigation object. Is your component inside a screen in a navigator?");
Expand All @@ -29,8 +66,8 @@ export default function linkTo(navigation, path, type) {
if (action.payload.name === NAVIGATORS.CENTRAL_PANE_NAVIGATOR && getTopmostReportId(root.getState()) !== getTopmostReportId(state)) {
action.type = 'PUSH';

// If this action is navigating to the RightModalNavigator and the last route on the root navigator is also RightModalNavigator
// then we want to replace the current RHP state with new one
// If the type is UP, we deeplinked into one of the RHP flows and we want to replace the current screen with the previous one in the flow
// and at the same time we want the back button to go to the page we were before the deeplink
} else if (type === 'UP') {
action.type = 'REPLACE';

Expand All @@ -40,6 +77,18 @@ export default function linkTo(navigation, path, type) {
}
}

if (action.payload.name === NAVIGATORS.RIGHT_MODAL_NAVIGATOR) {
const {minimalAction, targetName} = getMinimalAction(action, navigation.getRootState());
if (minimalAction) {
// If the target name is RHP that means this action is responsible for changing flow within the RHP e.g. from settings to search. In that case we want to use REPLACE.
if (targetName === NAVIGATORS.RIGHT_MODAL_NAVIGATOR) {
minimalAction.type = 'REPLACE';
}
root.dispatch(minimalAction);
return;
}
}

if (action !== undefined) {
root.dispatch(action);
} else {
Expand Down