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

[HOLD #11768] mWeb-lands on wrong screen when pressing the back button from the attachment screen @gadhiyamanan #11372

Closed
kbecciv opened this issue Sep 28, 2022 · 39 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Monthly KSv2

Comments

@kbecciv
Copy link

kbecciv commented Sep 28, 2022

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Go to staging.new.expensify.com
  2. Log in with any account
  3. Go to any chat
  4. Click on any attachment
  5. Press back button

Expected Result:

Should navigate to chat screen

Actual Result:

Navigate to home screen

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Mobile Web

Version Number: 1.2.8.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers): any

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Screen_Recording_20220913_134105_Chrome.mp4

Expensify/Expensify Issue URL:

Issue reported by: @gadhiyamanan

Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1663057441323579

View all open jobs on GitHub

@kbecciv kbecciv added the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Sep 28, 2022
@melvin-bot
Copy link

melvin-bot bot commented Sep 28, 2022

Triggered auto assignment to @alexpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@melvin-bot melvin-bot bot added Daily KSv2 and removed AutoAssignerTriage Auto assign issues for triage to an available triage team member labels Sep 28, 2022
@thesahindia
Copy link
Member

Dupe of #7010 and it was decided to close it (check the reason here)

@alexpensify
Copy link
Contributor

Assigning to Engineering for the check if this one should be closed or give it another review.

cc: @roryabraham and @stitesExpensify

@melvin-bot
Copy link

melvin-bot bot commented Sep 28, 2022

Triggered auto assignment to @cristipaval (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@alexpensify alexpensify removed their assignment Sep 28, 2022
@stitesExpensify
Copy link
Contributor

Hmm so the problem is that the attachment picker is not really a new page, it's the same page with a modal on it, so the back button is technically functioning correctly. IMO this is not a bug, it's user error. If they hit the x everything will work fine. The only reason it's confusing is because it looks like a different page, but that's not something we want to fix right now IMO

@stitesExpensify
Copy link
Contributor

If we do want to fix this, we could artificially add the report page a second time to react-navigation, but that could also cause problems where the flow is:

  1. Navigate to report
  2. Hit add attachment
  3. Click the x
  4. Hit back button
  5. You stay on the same page
  6. Hit back again and you actually go back.

So we would need to artificially add it when you click "add attachment" and then remove it when you hit the "x". Again I think that's not worth it though, happy to hear other opinions

@melvin-bot
Copy link

melvin-bot bot commented Sep 28, 2022

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

@roryabraham
Copy link
Contributor

cc @puneetlath because this is react-navigation related I'm thinking we should put it on HOLD pending the FT you're working on

@cristipaval cristipaval removed their assignment Sep 29, 2022
@cristipaval
Copy link
Contributor

removed the assignment since I see you guys are already triaging it. Thanks!

@roryabraham roryabraham added Weekly KSv2 and removed Daily KSv2 labels Sep 29, 2022
@roryabraham roryabraham changed the title mWeb - Attachment - navigate to the wrong screen when pressing the back button from the attachment screen @gadhiyamanan [HOLD] mWeb - Attachment - navigate to the wrong screen when pressing the back button from the attachment screen @gadhiyamanan Sep 29, 2022
@roryabraham
Copy link
Contributor

roryabraham commented Sep 29, 2022

Putting this on HOLD and added to tracking issue: https://github.com/Expensify/Expensify/issues/221853

(sorry, this tracking issue is internal)

@melvin-bot
Copy link

melvin-bot bot commented Sep 30, 2022

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@JmillsExpensify JmillsExpensify added the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Oct 18, 2022
@melvin-bot melvin-bot bot added the Overdue label Oct 18, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 18, 2022

Triggered auto assignment to @slafortune (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@melvin-bot melvin-bot bot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Oct 18, 2022
@melvin-bot melvin-bot bot removed the Overdue label Dec 27, 2022
@JmillsExpensify
Copy link

Switching out the weekly for monthly given that the navigation project is large and has no ETA.

@JmillsExpensify
Copy link

Still on hold for the navigation project.

@melvin-bot melvin-bot bot added the Overdue label Feb 13, 2023
@JmillsExpensify
Copy link

Same same.

@melvin-bot melvin-bot bot removed the Overdue label Feb 14, 2023
@melvin-bot melvin-bot bot added the Overdue label Mar 17, 2023
@JmillsExpensify
Copy link

Still on hold.

@melvin-bot melvin-bot bot removed the Overdue label Mar 29, 2023
@melvin-bot melvin-bot bot added the Overdue label May 1, 2023
@JmillsExpensify
Copy link

Still on hold

@melvin-bot melvin-bot bot removed the Overdue label May 3, 2023
@melvin-bot melvin-bot bot added the Overdue label Jun 5, 2023
@JmillsExpensify
Copy link

Coming off hold very soon!

@melvin-bot melvin-bot bot removed the Overdue label Jun 7, 2023
@mountiny
Copy link
Contributor

This is not its own screen so I dont think this will be fixed and I am not sure if we should be fixing it in terms of consistency wit desktop. There the attachment is also a modal and not its own page.

@melvin-bot melvin-bot bot added the Overdue label Jul 10, 2023
@JmillsExpensify
Copy link

Ok cool, still aiming to get to this one this week.

@melvin-bot melvin-bot bot removed the Overdue label Jul 12, 2023
@melvin-bot melvin-bot bot added the Overdue label Aug 14, 2023
@JmillsExpensify
Copy link

Still trying to make time. Low priority.

@melvin-bot melvin-bot bot removed the Overdue label Aug 16, 2023
@melvin-bot melvin-bot bot added the Overdue label Sep 18, 2023
@JmillsExpensify
Copy link

Re-tested and this is now fixed.

@melvin-bot melvin-bot bot removed the Overdue label Oct 10, 2023
@JmillsExpensify
Copy link

@gadhiyamanan I just sent you an offer for reporting. Once you've accepted and been paid, we can close this one out.

@gadhiyamanan
Copy link
Contributor

@JmillsExpensify offer accepted, thanks!

@JmillsExpensify
Copy link

Anytime. All paid out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Monthly KSv2
Projects
None yet
Development

No branches or pull requests