-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[$500] [Wave Collect] [Ideal Nav] Display the home route when the app restarts on mobile platforms #35927
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
Comments
Triggered auto assignment to @dylanexpensify ( |
Job added to Upwork: https://www.upwork.com/jobs/~01e8861874ab17080a |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Display the home route when the app restarts on mobile platforms. What is the root cause of that problem?NA What changes do you think we should make in order to solve the problem?We should use Line 507 in b5bb986
Lines 138 to 144 in b5bb986
Only run when isNative is true: Since there is no simple way to trigger a function when app is being closed we can set the Demo code, can be simplified and tested during the PR const [lastVisitedPathBeforeInactive, setLastVisitedPathBeforeInactive] = useState(props.lastVisitedPath);
const isAppActive = useRef(false);
const platform = getPlatform();
const isNative = platform === CONST.PLATFORM.IOS || platform === CONST.PLATFORM.ANDROID;
useEffect(() => {
if (!isNative) {
return;
}
const appStateSubscription = AppState.addEventListener('change', (nextAppState) => {
if (nextAppState.match(/inactive|background/)) {
isAppActive.current = false;
setLastVisitedPathBeforeInactive(props.lastVisitedPath);
updateLastVisitedPath('');
} else {
isAppActive.current = true;
updateLastVisitedPath(lastVisitedPathBeforeInactive);
}
});
return () => {
appStateSubscription.remove();
};
}, [props.disableKeyboard, props.lastVisitedPath, lastVisitedPathBeforeInactive, isNative]);
useEffect(() => {
if (!isNative) {
return;
}
if (isAppActive.current) {
updateLastVisitedPath(props.lastVisitedPath);
}
}, [props.lastVisitedPath, isNative]); Resultnavigate_to_homepage_after_restart.mp4app_restart.mp4 |
ProposalPlease re-state the problem that we are trying to solve in this issue.We want to show the home instead of the last visited route when restarting the native app. What is the root cause of that problem?We have a logic here to show the last visited route if exists on all platforms. App/src/libs/Navigation/NavigationRoot.tsx Lines 71 to 89 in 7f4cdce
And now we want to change it. Based on the OP, we want to clear the last visited route/path onyx when the app is closed, but there is no way to know whether the app is closed. What changes do you think we should make in order to solve the problem?Instead of trying to clear it when the app is closed, we can create a new platform-specific file called
What alternative solutions did you explore? (Optional)Or we can create a platform-specific file for Lines 507 to 509 in 7f4cdce
|
@rushatgabhane can we get reviews? |
ProposalPlease re-state the problem that we are trying to solve in this issue.Display the home route when the app is restarted. What is root cause of that problem?Based on the current implementation we always show the last visited path when opening the app but for native platforms we want to chage that and navigate the user home route instead when the app is opened. What Changes do you think we should make in order to solve the problem?When we open the app for the first time or after killing it, we always perform some setup actions while initializing the onyx such as clearing the error which prevents an error from appearing when the app is opened after killing it which means we can also follow a similar approach to address this issue which is to initialize ONYXKEYS.LAST_VISITED_PATH to Lines 33 to 34 in a1b1595
We can do that by adding the following line of code in initialKeyStates
where
given that this is how we perform actions such as clearing an error, we might as well use a similar approach to redirect the user to home route. This works as expected. What alternative solutions did you explore? (Optional)None |
i like @bernhardoj's proposal 🎀👀🎀 |
Current assignee @hayata-suenaga is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new. |
@rushatgabhane What do you think about my proposal? |
I think @HezekielT suggestion is nicer. |
@rushatgabhane can you check @HezekielT's proposal again? @bernhardoj thinks @HezekielT's proposal is better |
@rushatgabhane, @dylanexpensify, @hayata-suenaga Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
bump @rushatgabhane |
there is an additional discussion on this issue It's an internal link. I'll post updates once the discussion concludes. |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
thank you! |
I confirmed that we want to display the Home route when the app restarts on mobile platforms. And by "Home" route, we mean the chat list screen without any workspace filter applied. @rushatgabhane, please look at this comment and confirm who we should assign the issue to. 🙇 |
@hayata-suenaga We can assign @HezekielT, i like their proposal better because |
📣 @HezekielT 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
@HezekielT this is a HIGH priority issue. If you could prioritize this one, we'd appreciate it 🙇 |
Thanks @bernhardoj for the suggestion. I really appreciate that. Thanks @rushatgabhane as well for reconsidering my proposal. |
@hayata-suenaga PR is ready |
PR merged! Pending payout if no regressions! |
coming up! |
created money req - https://staging.new.expensify.com/r/2577190139543546 |
Mind adding a payment summary @dylanexpensify? |
Payment summary:
Please apply/request |
$500 approved for @rushatgabhane based on summary. |
@dylanexpensify Friendly bump for payment |
Done! |
Uh oh!
There was an error while loading. Please reload this page.
Clear the value for the
LAST_VISITED_PATH
Onyx key on mobile platforms (i.e. iOS and Android) so that when the app restarts on these platforms, the home route is displayed instead of the last visited route.Internal Slack discussion
Related issues:
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: