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

Remove reportID param from dismissModal function #58191

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion contributingGuides/NAVIGATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ When creating RHP flows, you have to remember a couple of things:

- We use a custom `goBack` function to handle the browser and the `react-navigation` history stack. Under the hood, it resolves to either replacing the current screen with the one we navigate to (deeplinking scenario) or just going back if we reached the current page by navigating in App (pops the screen). It ensures the requested behaviors on web, which is navigating back to the place from where you deeplinked when going into the RHP flow by it.

- If you want to navigate to a certain report after completing a flow related to it, e.g. `RequestMoney` flow with a certain group/user, you should use `Navigation.dismissModal` with this `reportID` as an argument. If, in the future, we would like to navigate to something different than the report after such flows, the API should be rather easy to change. We do it like that in order to replace the RHP flow with the new report instead of pushing it, so pressing the back button does not navigate back to the ending page of the flow. If we were to navigate to the same report, we just pop the RHP modal.
- If you want to navigate to a certain report after completing a flow related to it, e.g. `RequestMoney` flow with a certain group/user, you should use `Navigation.dismissModalWithReport` with this `reportID` as an argument. If, in the future, we would like to navigate to something different than the report after such flows, the API should be rather easy to change. We do it like that in order to replace the RHP flow with the new report instead of pushing it, so pressing the back button does not navigate back to the ending page of the flow. If we were to navigate to the same report, we just pop the RHP modal.

### Example of usage

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type {RootNavigatorParamList, State} from '@libs/Navigation/types';
import * as SearchQueryUtils from '@libs/SearchQueryUtils';
import NAVIGATORS from '@src/NAVIGATORS';
import SCREENS from '@src/SCREENS';
import type {OpenWorkspaceSplitActionType, PushActionType, SwitchPolicyIdActionType} from './types';
import type {OpenWorkspaceSplitActionType, PushActionType, ReplaceActionType, SwitchPolicyIdActionType} from './types';

const MODAL_ROUTES_TO_DISMISS: string[] = [
NAVIGATORS.WORKSPACE_SPLIT_NAVIGATOR,
Expand Down Expand Up @@ -237,6 +237,29 @@ function handlePushSearchPageAction(
return stackRouter.getStateForAction(state, updatedAction, configOptions);
}

function handleReplaceReportsSplitNavigatorAction(
state: StackNavigationState<ParamListBase>,
action: ReplaceActionType,
configOptions: RouterConfigOptions,
stackRouter: Router<StackNavigationState<ParamListBase>, CommonActions.Action | StackActionType>,
) {
const stateWithReportsSplitNavigator = stackRouter.getStateForAction(state, action, configOptions);

if (!stateWithReportsSplitNavigator) {
Log.hmmm('[handleReplaceReportsSplitNavigatorAction] ReportsSplitNavigator has not been found in the navigation state.');
return null;
}

const lastReportsSplitNavigator = stateWithReportsSplitNavigator.routes.at(-1);

// ReportScreen should always be opened with an animation when replacing the navigator
if (lastReportsSplitNavigator?.key) {
reportsSplitsWithEnteringAnimation.add(lastReportsSplitNavigator.key);
}

return stateWithReportsSplitNavigator;
}

/**
* Handles the DISMISS_MODAL action.
* If the last route is a modal route, it has to be popped from the navigation stack.
Expand Down Expand Up @@ -275,6 +298,7 @@ export {
handleDismissModalAction,
handlePushReportSplitAction,
handlePushSearchPageAction,
handleReplaceReportsSplitNavigatorAction,
handleSwitchPolicyIDAction,
handleSwitchPolicyIDFromSearchAction,
handleNavigatingToModalFromModal,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,19 @@ import {
handleOpenWorkspaceSplitAction,
handlePushReportSplitAction,
handlePushSearchPageAction,
handleReplaceReportsSplitNavigatorAction,
handleSwitchPolicyIDAction,
} from './GetStateForActionHandlers';
import syncBrowserHistory from './syncBrowserHistory';
import type {DismissModalActionType, OpenWorkspaceSplitActionType, PushActionType, RootStackNavigatorAction, RootStackNavigatorRouterOptions, SwitchPolicyIdActionType} from './types';
import type {
DismissModalActionType,
OpenWorkspaceSplitActionType,
PushActionType,
ReplaceActionType,
RootStackNavigatorAction,
RootStackNavigatorRouterOptions,
SwitchPolicyIdActionType,
} from './types';

function isOpenWorkspaceSplitAction(action: RootStackNavigatorAction): action is OpenWorkspaceSplitActionType {
return action.type === CONST.NAVIGATION.ACTION_TYPE.OPEN_WORKSPACE_SPLIT;
Expand All @@ -31,6 +40,10 @@ function isPushAction(action: RootStackNavigatorAction): action is PushActionTyp
return action.type === CONST.NAVIGATION.ACTION_TYPE.PUSH;
}

function isReplaceAction(action: RootStackNavigatorAction): action is ReplaceActionType {
return action.type === CONST.NAVIGATION.ACTION_TYPE.REPLACE;
}

function isDismissModalAction(action: RootStackNavigatorAction): action is DismissModalActionType {
return action.type === CONST.NAVIGATION.ACTION_TYPE.DISMISS_MODAL;
}
Expand Down Expand Up @@ -82,6 +95,10 @@ function RootStackRouter(options: RootStackNavigatorRouterOptions) {
return handleDismissModalAction(state, configOptions, stackRouter);
}

if (isReplaceAction(action) && action.payload.name === NAVIGATORS.REPORTS_SPLIT_NAVIGATOR) {
return handleReplaceReportsSplitNavigatorAction(state, action, configOptions, stackRouter);
}

if (isPushAction(action)) {
if (action.payload.name === NAVIGATORS.REPORTS_SPLIT_NAVIGATOR) {
return handlePushReportSplitAction(state, action, configOptions, stackRouter, setActiveWorkspaceID);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ type SwitchPolicyIdActionType = RootStackNavigatorActionType & {

type PushActionType = StackActionType & {type: typeof CONST.NAVIGATION.ACTION_TYPE.PUSH};

type ReplaceActionType = StackActionType & {type: typeof CONST.NAVIGATION.ACTION_TYPE.REPLACE};

type DismissModalActionType = RootStackNavigatorActionType & {
type: typeof CONST.NAVIGATION.ACTION_TYPE.DISMISS_MODAL;
};
Expand All @@ -49,6 +51,7 @@ export type {
OpenWorkspaceSplitActionType,
SwitchPolicyIdActionType,
PushActionType,
ReplaceActionType,
DismissModalActionType,
RootStackNavigatorAction,
RootStackNavigatorActionType,
Expand Down
49 changes: 37 additions & 12 deletions src/libs/Navigation/Navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {CommonActions, getPathFromState, StackActions} from '@react-navigation/n
import omit from 'lodash/omit';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import type {Writable} from 'type-fest';
import type {MergeExclusive, Writable} from 'type-fest';
import getIsNarrowLayout from '@libs/getIsNarrowLayout';
import Log from '@libs/Log';
import {shallowCompare} from '@libs/ObjectUtils';
Expand All @@ -24,6 +24,7 @@ import originalCloseRHPFlow from './helpers/closeRHPFlow';
import getPolicyIDFromState from './helpers/getPolicyIDFromState';
import getStateFromPath from './helpers/getStateFromPath';
import getTopmostReportParams from './helpers/getTopmostReportParams';
import {isFullScreenName} from './helpers/isNavigatorName';
import isReportOpenInRHP from './helpers/isReportOpenInRHP';
import linkTo from './helpers/linkTo';
import getMinimalAction from './helpers/linkTo/getMinimalAction';
Expand Down Expand Up @@ -468,20 +469,25 @@ function waitForProtectedRoutes() {
});
}

type NavigateToReportWithPolicyCheckPayload = {report?: OnyxEntry<Report>; reportID?: string; reportActionID?: string; referrer?: string; policyIDToCheck?: string};
// It should not be possible to pass a report and a reportID at the same time.
type NavigateToReportWithPolicyCheckPayload = MergeExclusive<{report: OnyxEntry<Report>}, {reportID: string}> & {
reportActionID?: string;
referrer?: string;
policyIDToCheck?: string;
};

/**
* Navigates to a report passed as a param (as an id or report object) and checks whether the target object belongs to the currently selected workspace.
* If not, the current workspace is set to global.
*/
function navigateToReportWithPolicyCheck({report, reportID, reportActionID, referrer, policyIDToCheck}: NavigateToReportWithPolicyCheckPayload, ref = navigationRef) {
function navigateToReportWithPolicyCheck({report, reportID, reportActionID, referrer, policyIDToCheck}: NavigateToReportWithPolicyCheckPayload, forceReplace = false, ref = navigationRef) {
const targetReport = reportID ? {reportID, ...allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]} : report;
const policyID = policyIDToCheck ?? getPolicyIDFromState(navigationRef.getRootState() as State<RootNavigatorParamList>);
const policyMemberAccountIDs = getPolicyEmployeeAccountIDs(policyID);
const shouldOpenAllWorkspace = isEmptyObject(targetReport) ? true : !doesReportBelongToWorkspace(targetReport, policyMemberAccountIDs, policyID);

if ((shouldOpenAllWorkspace && !policyID) || !shouldOpenAllWorkspace) {
linkTo(ref.current, ROUTES.REPORT_WITH_ID.getRoute(targetReport?.reportID, reportActionID, referrer));
linkTo(ref.current, ROUTES.REPORT_WITH_ID.getRoute(targetReport?.reportID, reportActionID, referrer), {forceReplace: !!forceReplace});
return;
}

Expand All @@ -497,6 +503,17 @@ function navigateToReportWithPolicyCheck({report, reportID, reportActionID, refe
params.referrer = referrer;
}

if (forceReplace) {
ref.dispatch(
StackActions.replace(NAVIGATORS.REPORTS_SPLIT_NAVIGATOR, {
policyID: undefined,
screen: SCREENS.REPORT,
params,
}),
);
return;
}

ref.dispatch(
StackActions.push(NAVIGATORS.REPORTS_SPLIT_NAVIGATOR, {
policyID: undefined,
Expand Down Expand Up @@ -527,23 +544,31 @@ function getReportRouteByID(reportID?: string, routes: NavigationRoute[] = navig
/**
* Closes the modal navigator (RHP, LHP, onboarding).
*/
const dismissModal = (reportID?: string, ref = navigationRef) => {
const dismissModal = (ref = navigationRef) => {
isNavigationReady().then(() => {
ref.dispatch({type: CONST.NAVIGATION.ACTION_TYPE.DISMISS_MODAL});
if (!reportID) {
return;
}
navigateToReportWithPolicyCheck({reportID});
});
};

/**
* Dismisses the modal and opens the given report.
*/
const dismissModalWithReport = (report: OnyxEntry<Report>, ref = navigationRef) => {
const dismissModalWithReport = (navigateToReportPayload: NavigateToReportWithPolicyCheckPayload, ref = navigationRef) => {
isNavigationReady().then(() => {
ref.dispatch({type: CONST.NAVIGATION.ACTION_TYPE.DISMISS_MODAL});
navigateToReportWithPolicyCheck({report});
if (getIsNarrowLayout()) {
const topmostReportID = getTopmostReportId();
const areReportsIDsDefined = !!topmostReportID && !!navigateToReportPayload.reportID;
const isReportsSplitTopmostFullScreen = ref.getRootState().routes.findLast((route) => isFullScreenName(route.name))?.name === NAVIGATORS.REPORTS_SPLIT_NAVIGATOR;
if (topmostReportID === navigateToReportPayload.reportID && areReportsIDsDefined && isReportsSplitTopmostFullScreen) {
dismissModal();
return;
}
const forceReplace = true;
navigateToReportWithPolicyCheck(navigateToReportPayload, forceReplace);
return;
}
dismissModal();
navigateToReportWithPolicyCheck(navigateToReportPayload);
});
};

Expand Down
60 changes: 49 additions & 11 deletions src/libs/actions/IOU.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4717,7 +4717,11 @@ function requestMoney(requestMoneyInformation: RequestMoneyInformation) {
}

InteractionManager.runAfterInteractions(() => removeDraftTransaction(CONST.IOU.OPTIMISTIC_TRANSACTION_ID));
Navigation.dismissModal(isSearchTopmostFullScreenRoute() ? undefined : activeReportID);
if (isSearchTopmostFullScreenRoute() || !activeReportID) {
Navigation.dismissModal();
} else {
Navigation.dismissModalWithReport({reportID: activeReportID});
}

const trackReport = Navigation.getReportRouteByID(linkedTrackedExpenseReportAction?.childReportID);
if (trackReport?.key) {
Expand Down Expand Up @@ -4801,7 +4805,12 @@ function submitPerDiemExpense(submitPerDiemExpenseInformation: PerDiemExpenseInf
API.write(WRITE_COMMANDS.CREATE_PER_DIEM_REQUEST, parameters, onyxData);

InteractionManager.runAfterInteractions(() => removeDraftTransaction(CONST.IOU.OPTIMISTIC_TRANSACTION_ID));
Navigation.dismissModal(isSearchTopmostFullScreenRoute() ? undefined : activeReportID);
if (isSearchTopmostFullScreenRoute() || !activeReportID) {
Navigation.dismissModal();
} else {
Navigation.dismissModalWithReport({reportID: activeReportID});
}

if (activeReportID) {
notifyNewAction(activeReportID, payeeAccountID);
}
Expand Down Expand Up @@ -4868,7 +4877,7 @@ function sendInvoice(
if (isSearchTopmostFullScreenRoute()) {
Navigation.dismissModal();
} else {
Navigation.dismissModalWithReport(invoiceRoom);
Navigation.dismissModalWithReport({report: invoiceRoom});
}

notifyNewAction(invoiceRoom.reportID, receiver.accountID);
Expand Down Expand Up @@ -5094,7 +5103,11 @@ function trackExpense(params: CreateTrackExpenseParams) {
}
}
InteractionManager.runAfterInteractions(() => removeDraftTransaction(CONST.IOU.OPTIMISTIC_TRANSACTION_ID));
Navigation.dismissModal(isSearchTopmostFullScreenRoute() ? undefined : activeReportID);
if (isSearchTopmostFullScreenRoute() || !activeReportID) {
Navigation.dismissModal();
} else {
Navigation.dismissModalWithReport({reportID: activeReportID});
}

if (action === CONST.IOU.ACTION.SHARE) {
if (isSearchTopmostFullScreenRoute() && activeReportID) {
Expand Down Expand Up @@ -5676,7 +5689,12 @@ function splitBill({
API.write(WRITE_COMMANDS.SPLIT_BILL, parameters, onyxData);
InteractionManager.runAfterInteractions(() => removeDraftTransaction(CONST.IOU.OPTIMISTIC_TRANSACTION_ID));

Navigation.dismissModal(isSearchTopmostFullScreenRoute() ? undefined : existingSplitChatReportID);
if (isSearchTopmostFullScreenRoute() || !existingSplitChatReportID) {
Navigation.dismissModal();
} else {
Navigation.dismissModalWithReport({reportID: existingSplitChatReportID});
}

notifyNewAction(splitData.chatReportID, currentUserAccountID);
}

Expand Down Expand Up @@ -5749,7 +5767,11 @@ function splitBillAndOpenReport({
API.write(WRITE_COMMANDS.SPLIT_BILL_AND_OPEN_REPORT, parameters, onyxData);
InteractionManager.runAfterInteractions(() => removeDraftTransaction(CONST.IOU.OPTIMISTIC_TRANSACTION_ID));

Navigation.dismissModal(isSearchTopmostFullScreenRoute() ? undefined : splitData.chatReportID);
if (isSearchTopmostFullScreenRoute()) {
Navigation.dismissModal();
} else {
Navigation.dismissModalWithReport({reportID: splitData.chatReportID});
}
notifyNewAction(splitData.chatReportID, currentUserAccountID);
}

Expand Down Expand Up @@ -6072,7 +6094,7 @@ function startSplitBill({

API.write(WRITE_COMMANDS.START_SPLIT_BILL, parameters, {optimisticData, successData, failureData});

Navigation.dismissModalWithReport(splitChatReport);
Navigation.dismissModalWithReport({report: splitChatReport});
notifyNewAction(splitChatReport.reportID, currentUserAccountID);
}

Expand Down Expand Up @@ -6331,7 +6353,11 @@ function completeSplitBill(

API.write(WRITE_COMMANDS.COMPLETE_SPLIT_BILL, parameters, {optimisticData, successData, failureData});
InteractionManager.runAfterInteractions(() => removeDraftTransaction(CONST.IOU.OPTIMISTIC_TRANSACTION_ID));
Navigation.dismissModal(isSearchTopmostFullScreenRoute() ? undefined : chatReportID);
if (isSearchTopmostFullScreenRoute()) {
Navigation.dismissModal();
} else {
Navigation.dismissModalWithReport({reportID: chatReportID});
}
notifyNewAction(chatReportID, sessionAccountID);
}

Expand Down Expand Up @@ -6514,7 +6540,11 @@ function createDistanceRequest(distanceRequestInformation: CreateDistanceRequest
API.write(WRITE_COMMANDS.CREATE_DISTANCE_REQUEST, parameters, onyxData);
InteractionManager.runAfterInteractions(() => removeDraftTransaction(CONST.IOU.OPTIMISTIC_TRANSACTION_ID));
const activeReportID = isMoneyRequestReport && report?.reportID ? report.reportID : parameters.chatReportID;
Navigation.dismissModal(isSearchTopmostFullScreenRoute() ? undefined : activeReportID);
if (isSearchTopmostFullScreenRoute() || !activeReportID) {
Navigation.dismissModal();
} else {
Navigation.dismissModalWithReport({reportID: activeReportID});
}
notifyNewAction(activeReportID, userAccountID);
}

Expand Down Expand Up @@ -8202,7 +8232,11 @@ function sendMoneyElsewhere(report: OnyxEntry<OnyxTypes.Report>, amount: number,

API.write(WRITE_COMMANDS.SEND_MONEY_ELSEWHERE, params, {optimisticData, successData, failureData});

Navigation.dismissModal(isSearchTopmostFullScreenRoute() ? undefined : params.chatReportID);
if (isSearchTopmostFullScreenRoute()) {
Navigation.dismissModal();
} else {
Navigation.dismissModalWithReport({reportID: params.chatReportID});
}
notifyNewAction(params.chatReportID, managerID);
}

Expand All @@ -8215,7 +8249,11 @@ function sendMoneyWithWallet(report: OnyxEntry<OnyxTypes.Report>, amount: number

API.write(WRITE_COMMANDS.SEND_MONEY_WITH_WALLET, params, {optimisticData, successData, failureData});

Navigation.dismissModal(isSearchTopmostFullScreenRoute() ? undefined : params.chatReportID);
if (isSearchTopmostFullScreenRoute()) {
Navigation.dismissModal();
} else {
Navigation.dismissModalWithReport({reportID: params.chatReportID});
}
notifyNewAction(params.chatReportID, managerID);
}

Expand Down
10 changes: 8 additions & 2 deletions src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ import getEnvironment from '@libs/Environment/getEnvironment';
import type EnvironmentType from '@libs/Environment/getEnvironment/types';
import * as ErrorUtils from '@libs/ErrorUtils';
import fileDownload from '@libs/fileDownload';
import getIsNarrowLayout from '@libs/getIsNarrowLayout';
import HttpUtils from '@libs/HttpUtils';
import isPublicScreenRoute from '@libs/isPublicScreenRoute';
import * as Localize from '@libs/Localize';
Expand Down Expand Up @@ -1187,6 +1188,11 @@ function navigateToAndOpenReport(
// We want to pass newChat here because if anything is passed in that param (even an existing chat), we will try to create a chat on the server
openReport(report?.reportID, '', userLogins, newChat, undefined, undefined, undefined, avatarFile);
if (shouldDismissModal) {
if (getIsNarrowLayout()) {
Navigation.dismissModalWithReport({report});
return;
}

Navigation.dismissModal();
}

Expand Down Expand Up @@ -2394,7 +2400,7 @@ function navigateToConciergeChat(shouldDismissModal = false, checkIfCurrentPageA
navigateToAndOpenReport([CONST.EMAIL.CONCIERGE], shouldDismissModal);
});
} else if (shouldDismissModal) {
Navigation.dismissModal(conciergeChatReportID);
Navigation.dismissModalWithReport({reportID: conciergeChatReportID});
} else {
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(conciergeChatReportID), linkToOptions);
}
Expand Down Expand Up @@ -2503,7 +2509,7 @@ function addPolicyReport(policyReport: OptimisticChatReport) {
};

API.write(WRITE_COMMANDS.ADD_WORKSPACE_ROOM, parameters, {optimisticData, successData, failureData});
Navigation.dismissModalWithReport(policyReport);
Navigation.dismissModalWithReport({report: policyReport});
}

/** Deletes a report, along with its reportActions, any linked reports, and any linked IOU report. */
Expand Down
Loading
Loading