Open IuliiaHerets opened 3 months ago
We think that this bug might be related to #vip-vsb
Triggered auto assignment to @JmillsExpensify (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
@JmillsExpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors
Edited by proposal-police: This proposal was edited at 2024-08-21 13:09:07 UTC.
Different back button behavior when creating workspace via + and Get started button
activeRoute
which is the workspace selector modal route(/workspace-switcher
) to backTo
param here
https://github.com/Expensify/App/blob/67ad466251078a3b54558a2ed84876ec2fe18608/src/pages/WorkspaceSwitcherPage/WorkspacesSectionHeader.tsx#L37Following the same pattern used in WorkspaceCardCreateAWorkspace page, we shouldn't pass the active route (/workspace-switcher
) to backTo
param, and call createWorkspaceWithPolicyDraftAndNavigateToIt
without any argument here. When there is no backTo param the else
condition will execute -> then the modal will be closed -> and user will be navigated to workspace list page
App.createWorkspaceWithPolicyDraftAndNavigateToIt();
Hmm interesting. I agree that's inconsistent. I think this comes down to whether we should move UP on pressing back or now. @marcaaron thoughts on this consideration? I can kind of see the argument that UP is a more valid action, so you should always be taken back to the workspace list.
Not too passionate about this one as it's a pretty specific set of actions the user will take to uncover this inconsistency and can't imagine anyone feeling too impeded by either result. Both bring you to a list of workspaces. Both lead to the option to create a new workspace if you choose to do that after you just created one (which is probably an uncommon action in any case).
Ignoring the rules of navigation for a second - I see the workspace selector as less of a "page" and almost more of a pop up selector menu from a UX POV so it doesn't really need to be something we can navigate back to.
Ignoring the rules of navigation for a second - I see the workspace selector as less of a "page" and almost more of a pop up selector menu from a UX POV so it doesn't really need to be something we can navigate back to.
Cool, I agree with this. So then let's make sure that "back" goes to the workspace list for both of these cases.
Job added to Upwork: https://www.upwork.com/jobs/~013645bc6cec0aa80d
Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav (External
)
@JmillsExpensify, @mananjadhav Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
@JmillsExpensify @mananjadhav this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
@JmillsExpensify, @mananjadhav Still overdue 6 days?! Let's take care of this!
Still waiting for proposals
I have a proposal here https://github.com/Expensify/App/issues/47603#issuecomment-2294986203
Cool, I agree with this. So then let's make sure that "back" goes to the workspace list for both of these cases.
@etCoderDysto Couldn't understand from your proposals if it covers the expected behavior ^
Cool, I agree with this. So then let's make sure that "back" goes to the workspace list for both of these cases.
@etCoderDysto Couldn't understand from your proposals if it covers the expected behavior ^
@mananjadhav I have updated my proposal to cover the expected behaviour.
WS switcher: Different back button behavior when creating workspace using the + and Get started button.
Inconsistent call to createWorkspaceWithPolicyDraftAndNavigateToIt
in WorkspaceCardCreateAWorkspace.tsx
and src/pages/WorkspaceSwitcherPage/WorkspacesSectionHeader.tsx
I believe the back button should navigate to the previous route. Contrary to the previous suggestion, I suggest adding activeRoute
to the createWorkspaceWithPolicyDraftAndNavigateToIt
function call.
src/pages/workspace/card/WorkspaceCardCreateAWorkspace.tsx
onPress={() => {
const activeRoute = Navigation.getActiveRouteWithoutParams();
interceptAnonymousUser(() => App.createWorkspaceWithPolicyDraftAndNavigateToIt('', '', false, false, activeRoute));
}}
Adding interceptAnonymousUser
is optional, but I recommend including it to prevent other inconsistencies.
N/A
Reviewing the proposals.
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
I believe the back button should navigate to the previous route.
@JmillsExpensify To an extent even I agree with this. wdyt? Will review accordingly.
A kind reminder for C+: My first proposal covers a solution to navigate user to the previous route as the issue report mentions. I only edited my proposal when the expected behaviour was changed here, and when I was requested to address the new expected behaviour here. Please checkout my proposal edit that was made 3 weeks ago.
A kind reminder for C+: My first proposal covers a solution to navigate user to the previous route as the issue report mentions. I only edited my proposal when the expected behaviour was changed. Please checkout https://github.com/Expensify/App/issues/47603#issuecomment-2294986203 that was made 3 weeks ago.
@mananjadhav @JmillsExpensify If that solution was chosen, I want to point out that the proposal was made 3 weeks ago before the External
and Help Wanted
labels were added (2 weeks ago), and the author later changed the solution (not adding it as alternative). I think the official evaluation should consider the solution after the external labels were added.
@JmillsExpensify @mananjadhav this issue is now 4 weeks old, please consider:
Thanks!
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
@JmillsExpensify, @mananjadhav Huh... This is 4 days overdue. Who can take care of this?
Still a lower priority though we'll get back to this.
@JmillsExpensify Can you respond to this question?
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
@JmillsExpensify, @mananjadhav Huh... This is 4 days overdue. Who can take care of this?
Yes, the back button should navigate to the previous route. Sorry, missed this last week.
@JmillsExpensify, @mananjadhav 6 days overdue. This is scarier than being forced to listen to Vogon poetry!
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
@JmillsExpensify, @mananjadhav 10 days overdue. I'm getting more depressed than Marvin.
Not the highest priority, though I think we're still aligned that the back button is the previous route.
@JmillsExpensify, @mananjadhav 12 days overdue. Walking. Toward. The. Light...
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
This issue has not been updated in over 14 days. @JmillsExpensify, @mananjadhav eroding to Weekly issue.
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
I'll review the proposals again on this one.
A gentle reminder to look at this comment when reviewing proposals.
Thanks!
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
Waiting on proposal review
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
Missed this one. I'll check this by tomorrow.
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
Still waiting for proposal review
Sorry for the delay. I checked and I think the @etCoderDysto proposal is good to go.
π π π C+ reviewed.
If you havenβt already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: v9.0.21-4 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com Issue reported by: Applause Internal Team
Action Performed:
Precondition:
Expected Result:
App will open workspace switcher when clicking app back button after creating workspace from Create a workspace" modal (should display the same behavior as when the + button is used).
Actual Result:
App returns to Account settings instead.
Workaround:
Unknown
Platforms:
Screenshots/Videos
https://github.com/user-attachments/assets/b6c6d620-376a-4c2e-8a41-e004ccafbd30
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @mananjadhav