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

Fix/28098: Clicking back button bring back the workspace #28573

4 changes: 3 additions & 1 deletion src/libs/Navigation/AppNavigator/AuthScreens.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,9 @@ function AuthScreens({isUsingMemoryOnlyKeys, lastUpdateIDAppliedToClient, sessio
} else {
App.reconnectApp(lastUpdateIDAppliedToClient);
}
App.setUpPoliciesAndNavigate(session, !isSmallScreenWidth);

App.setUpPoliciesAndNavigate(session);

App.redirectThirdPartyDesktopSignIn();

// Check if we should be running any demos immediately after signing in.
Expand Down
39 changes: 5 additions & 34 deletions src/libs/actions/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -322,44 +322,17 @@ function endSignOnTransition() {
return resolveSignOnTransitionToFinishPromise();
}

/**
* Create a new workspace and navigate to it
*
* @param {String} [policyOwnerEmail] Optional, the email of the account to make the owner of the policy
* @param {Boolean} [makeMeAdmin] Optional, leave the calling account as an admin on the policy
* @param {String} [policyName] Optional, custom policy name we will use for created workspace
* @param {Boolean} [transitionFromOldDot] Optional, if the user is transitioning from old dot
* @param {Boolean} [shouldNavigateToAdminChat] Optional, navigate to the #admin room after creation
*/
function createWorkspaceAndNavigateToIt(policyOwnerEmail = '', makeMeAdmin = false, policyName = '', transitionFromOldDot = false, shouldNavigateToAdminChat = true) {
const policyID = Policy.generatePolicyID();
const adminsChatReportID = Policy.createWorkspace(policyOwnerEmail, makeMeAdmin, policyName, policyID);
Navigation.isNavigationReady()
.then(() => {
if (transitionFromOldDot) {
// We must call goBack() to remove the /transition route from history
Navigation.goBack(ROUTES.HOME);
}

if (shouldNavigateToAdminChat) {
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(adminsChatReportID));
Copy link
Contributor

Choose a reason for hiding this comment

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

@DylanDylann Why did we remove this line? on staging when we create a new workspace , it should be redirected to admins room.

Copy link
Contributor

Choose a reason for hiding this comment

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

The dismissModal should redirect to #admins room but that doesn’t seem to be the case, can you please check?

Copy link
Contributor Author

@DylanDylann DylanDylann Oct 2, 2023

Choose a reason for hiding this comment

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

I replace it by Navigation.dismissModal(adminsChatReportID);, it still redirects to the admins room, and removes the current RHN modal as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I mentioned in the proposal #28098 (comment):

we can make sure that the current RIGHT_MODAL_NAVIGATOR screen is REPLACE by report screen, rather than PUSH a new report screen to the stack by using Navigation.dismissModal function:

Copy link
Contributor

Choose a reason for hiding this comment

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

it still redirects to the admins room, and removes the current RHN modal as well

It doesn’t work for me.

Can we just dismiss the modal , navigate to admins then open the modal again?

Copy link
Contributor

Choose a reason for hiding this comment

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

the browsers history is just [/r/{reportID}, /settings, /settings/workspaces]`

That's the expected behavior, no? I mean what's the issue here?

Copy link
Contributor Author

@DylanDylann DylanDylann Oct 5, 2023

Choose a reason for hiding this comment

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

or sorry. My bad. I have updated this comment #28573 (comment)

the browser`s history is just [/r/{reportID}, /settings/workspaces]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fedirjh do you have any suggestions for this one? The issue is that window.history does not sync with the react-navigation stacks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fedirjh based on the issue here #28573 (comment), I just updated the PR to fix the error mentioned here #28573 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Result

#28573 (comment)

Screencast.from.06-10-2023.22.40.21.webm
Screencast.from.06-10-2023.22.39.00.webm

}

Navigation.navigate(ROUTES.WORKSPACE_INITIAL.getRoute(policyID));
})
.then(endSignOnTransition);
}

/**
* Create a new draft workspace and navigate to it
*
* @param {String} [policyOwnerEmail] Optional, the email of the account to make the owner of the policy
* @param {String} [policyName] Optional, custom policy name we will use for created workspace
* @param {Boolean} [transitionFromOldDot] Optional, if the user is transitioning from old dot
* @param {Boolean} [makeMeAdmin] Optional, leave the calling account as an admin on the policy
*/
function createWorkspaceWithPolicyDraftAndNavigateToIt(policyOwnerEmail = '', policyName = '', transitionFromOldDot = false) {
function createWorkspaceWithPolicyDraftAndNavigateToIt(policyOwnerEmail = '', policyName = '', transitionFromOldDot = false, makeMeAdmin = false) {
const policyID = Policy.generatePolicyID();
Policy.createDraftInitialWorkspace(policyOwnerEmail, policyName, policyID);
Policy.createDraftInitialWorkspace(policyOwnerEmail, policyName, policyID, makeMeAdmin);

Navigation.isNavigationReady()
.then(() => {
Expand Down Expand Up @@ -403,9 +376,8 @@ function savePolicyDraftByNewWorkspace(policyID, policyName, policyOwnerEmail =
* pass it in as a parameter. withOnyx guarantees that the value has been read
* from Onyx because it will not render the AuthScreens until that point.
* @param {Object} session
* @param {Boolean} shouldNavigateToAdminChat Should we navigate to admin chat after creating workspace
*/
function setUpPoliciesAndNavigate(session, shouldNavigateToAdminChat) {
function setUpPoliciesAndNavigate(session) {
const currentUrl = getCurrentUrl();
if (!session || !currentUrl || !currentUrl.includes('exitTo')) {
return;
Expand All @@ -426,7 +398,7 @@ function setUpPoliciesAndNavigate(session, shouldNavigateToAdminChat) {

const shouldCreateFreePolicy = !isLoggingInAsNewUser && isTransitioning && exitTo === ROUTES.WORKSPACE_NEW;
if (shouldCreateFreePolicy) {
createWorkspaceAndNavigateToIt(policyOwnerEmail, makeMeAdmin, policyName, true, shouldNavigateToAdminChat);
createWorkspaceWithPolicyDraftAndNavigateToIt(policyOwnerEmail, policyName, true, makeMeAdmin);
return;
}
if (!isLoggingInAsNewUser && exitTo) {
Expand Down Expand Up @@ -555,7 +527,6 @@ export {
handleRestrictedEvent,
beginDeepLinkRedirect,
beginDeepLinkRedirectAfterTransition,
createWorkspaceAndNavigateToIt,
getMissingOnyxUpdates,
finalReconnectAppAfterActivatingReliableUpdates,
savePolicyDraftByNewWorkspace,
Expand Down
4 changes: 3 additions & 1 deletion src/libs/actions/Policy.js
Original file line number Diff line number Diff line change
Expand Up @@ -954,8 +954,9 @@ function buildOptimisticCustomUnits() {
* @param {String} [policyOwnerEmail] Optional, the email of the account to make the owner of the policy
* @param {String} [policyName] Optional, custom policy name we will use for created workspace
* @param {String} [policyID] Optional, custom policy id we will use for created workspace
* @param {Boolean} [makeMeAdmin] Optional, leave the calling account as an admin on the policy
*/
function createDraftInitialWorkspace(policyOwnerEmail = '', policyName = '', policyID = generatePolicyID()) {
function createDraftInitialWorkspace(policyOwnerEmail = '', policyName = '', policyID = generatePolicyID(), makeMeAdmin = false) {
const workspaceName = policyName || generateDefaultWorkspaceName(policyOwnerEmail);
const {customUnits} = buildOptimisticCustomUnits();

Expand All @@ -973,6 +974,7 @@ function createDraftInitialWorkspace(policyOwnerEmail = '', policyName = '', pol
outputCurrency: lodashGet(allPersonalDetails, [sessionAccountID, 'localCurrencyCode'], CONST.CURRENCY.USD),
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
customUnits,
makeMeAdmin,
},
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ function FloatingActionButtonAndPopover(props) {
iconHeight: 40,
text: props.translate('workspace.new.newWorkspace'),
description: props.translate('workspace.new.getTheExpensifyCardAndMore'),
onSelected: () => interceptAnonymousUser(() => App.createWorkspaceAndNavigateToIt('', false, '', false, !props.isSmallScreenWidth)),
onSelected: () => interceptAnonymousUser(() => App.createWorkspaceWithPolicyDraftAndNavigateToIt()),
},
]
: []),
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/WorkspaceInitialPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ function WorkspaceInitialPage(props) {
return;
}

App.savePolicyDraftByNewWorkspace(props.policyDraft.id, props.policyDraft.name, '', false);
App.savePolicyDraftByNewWorkspace(props.policyDraft.id, props.policyDraft.name, '', props.policyDraft.makeMeAdmin);
// We only care when the component renders the first time
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/WorkspaceNewRoomPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ function WorkspaceNewRoomPage(props) {
shouldShow={!Permissions.canUsePolicyRooms(props.betas) || !workspaceOptions.length}
shouldShowBackButton={false}
linkKey="workspace.emptyWorkspace.title"
onLinkPress={() => App.createWorkspaceAndNavigateToIt('', false, '', false, false)}
onLinkPress={() => App.createWorkspaceWithPolicyDraftAndNavigateToIt()}
>
<ScreenWrapper
shouldEnableKeyboardAvoidingView={false}
Expand Down
Loading