Exit to the proper page when transitioning #8204
Conversation
There was a problem hiding this comment.
I will accept this solution as a hotfix because it's broken on production.
However, this is not really the ideal solution because:
- We don't need to subscribe to the betas in this component
- We are only subscribing because we need this component to "update"
- It's a code smell and indicates we're holding something wrong or missing logic somewhere
Any ideas on what else we can do?
Also let's CP to staging so it gets out sooner than later :D
|
|
I think we could move everything into componentDidMount() {
const accountID = parseInt(lodashGet(this.props.route.params, 'accountID', ''), 10);
const email = lodashGet(this.props.route.params, 'email', '');
const shortLivedToken = lodashGet(this.props.route.params, 'shortLivedToken', '');
const isUserSignedIn = this.props.session && this.props.session.authToken;
if (!isUserSignedIn) {
Session.signInWithShortLivedToken(accountID, email, shortLivedToken);
return;
}
if (this.signOutIfNeeded(email)) {
return;
}
// exitTo is URI encoded because it could contain a variable number of slashes (i.e. "workspace/new" vs "workspace/<ID>/card")
const exitTo = decodeURIComponent(lodashGet(this.props.route.params, 'exitTo', ''));
if (exitTo === ROUTES.WORKSPACE_NEW) {
// New workspace creation is handled in AuthScreens, not in its own screen
return;
}
// In order to navigate to a modal, we first have to dismiss the current modal. But there is no current
// modal you say? I know, it confuses me too. Without dismissing the current modal, if the user cancels out
// of the workspace modal, then they will be routed back to
// /transition/<accountID>/<email>/<authToken>/workspace/<policyID>/card and we don't want that. We want them to go back to `/`
// and by calling dismissModal(), the /transition/... route is removed from history so the user will get taken to `/`
// if they cancel out of the new workspace modal.
Navigation.dismissModal();
Navigation.navigate(exitTo);
} |
|
Yeah that looks like it would work to me. |
|
To see all the cases where we use the transition we have to look at Web-Expensify and I think Mobile-Expensify as well: Maybe others.... |
Exit to the proper page when transitioning (cherry picked from commit 7cd7cc0)
…8204 🍒 Cherry pick PR #8204 to staging 🍒
|
🚀 Cherry-picked to staging by @marcaaron in version: 1.1.44-2 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
|
@neil-marcellini We could not test in Desktop app, Android and iOS. Web is a pass. In mWeb we are redirected app for step 6. Image.from.iOS.MP4 |
cc @marcaaron
Details
As Marc suggested, subscribing to updates in the betas again ensures that componentDidMount runs.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/201705
Tests
@gmail.comemail address.PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issuessection aboveTestssectionQA stepssectionsrc/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNameproperty(i.e.
StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)PR Reviewer Checklist
### Fixed Issuessection aboveTestssectionQA stepssectionsrc/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNameproperty(i.e.
StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)QA Steps
Same as tests but on staging.
Screenshots
Web
Mobile Web
Desktop
iOS
Android