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 for payment 2024-02-20] [Wave 8] [Ideal Nav] Back navigation issues #35626

Closed
6 tasks done
hayata-suenaga opened this issue Feb 2, 2024 · 40 comments
Closed
6 tasks done
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2

Comments

@hayata-suenaga
Copy link
Contributor

hayata-suenaga commented Feb 2, 2024

Action Performed:

There are several issues reported that have to do with the back navigation and wrong navigation animations. I listed those issues below.

### Issue 1:
1. Make sure you're on a mobile phone or on a browser with a smaller width
2. Make sure that the test account has at least one workspace
3. Click on the Settings icon 🔧 on the bottom tab > Workspaces > Select a workspace

video -> #33280 (comment)

Issue 2:

  1. Make sure you're on a mobile phone or on a browser with a smaller width
  2. Click the Chat button on the bottom tab
  3. Select a chat/report
  4. Go back
  5. Confirm that the Chat List page opens from the right when the page should close to the left (watch the video)
  6. Click the Settings (🔧) button on the bottom tab
  7. Confirm that the Setting page opens from the right when the page should not animate. Only the part above the bottom tab should change without animation.
RPReplay_Final1706701018.mp4

Issue 3:

  1. Click the Settings button on the bottom tab > Workspaces
  2. Click the back button (left chevron icon)
  3. Confirm that the Chat List page opens from the right when the Workspace List page should close to left to go back to the Settings page
Screen.Recording.2024-01-26.at.2.56.00.AM.mov

Issue 4:

  1. Test this on a browser. Shrink the window width.
  2. Visit https://dev.new.expensify.com:8082/settings/preferences
  3. Press the back button on the header (left chevron icon next to Preferences)
  4. The app navigates to the Chats page when it should navigate back to the Settings page.

video -> #33280 (comment)

Other related issues

Workaround:

N/A

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

vides are attached above.

View all open jobs on GitHub

@hayata-suenaga hayata-suenaga added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 2, 2024
@hayata-suenaga hayata-suenaga self-assigned this Feb 2, 2024
Copy link

melvin-bot bot commented Feb 2, 2024

Triggered auto assignment to @CortneyOfstad (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@adamgrzybowski
Copy link
Contributor

Hi! Working on it!

@hayata-suenaga
Copy link
Contributor Author

@adamgrzybowski I added several reported issues that probably related to the back navigation to the original post. Please go through them and let me know if there are issues that should be addressed separately from this issue.

@hayata-suenaga
Copy link
Contributor Author

@adamgrzybowski can you also check if this issue can also be fixed by the solution you're thinking about?

Copy link

melvin-bot bot commented Feb 2, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker 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 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.

@CortneyOfstad
Copy link
Contributor

Heads up @adamgrzybowski @hayata-suenaga ^^^

@hayata-suenaga
Copy link
Contributor Author

Closed the issue linked above 🙇

@hayata-suenaga
Copy link
Contributor Author

@adamgrzybowski also another back navigation related issue reported here

@allgandalf
Copy link
Contributor

allgandalf commented Feb 2, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

Pressing the back button on the "Keyboard Shortcuts" page leads to a blank screen

What is the root cause of that problem?

OnBackButtonPress is missing in HeaderWithBackButton component in all the related pages

What changes do you think we should make in order to solve the problem?

Pass the prop onBackButtonPress={() => Navigation.goBack()} in HeaderWithBackButtonin all the pages pertaining this bug.

What alternative solutions did you explore? (Optional)

N/A

@allgandalf
Copy link
Contributor

hey @CortneyOfstad , this issue would be eventually opened to external contributors right :)

Copy link

melvin-bot bot commented Feb 2, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker 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 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.

@hayata-suenaga
Copy link
Contributor Author

@GandalfGwaihir this issue will be handled by an engineer from an expert agency. However, thank you for your proposal.

@hayata-suenaga
Copy link
Contributor Author

This is another instance of back navigation issue: #35736 (comment)

@allgandalf
Copy link
Contributor

found one more similar issue: #35689

@allgandalf
Copy link
Contributor

not 100% sure but maybe this issue #35756 is also in someway related to this root issue

@johncschuster
Copy link
Contributor

Related: #35669

@hayata-suenaga
Copy link
Contributor Author

If you find any back navigation related issues, please add your issue to the Other related issues section in the original post of this issue. 🙇

Screenshot 2024-02-03 at 9 17 48 PM

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Feb 7, 2024
@situchan
Copy link
Contributor

situchan commented Feb 7, 2024

@hayata-suenaga another ones: #36040, #36035

@muas19
Copy link

muas19 commented Feb 7, 2024

Track this #36054

@hayata-suenaga
Copy link
Contributor Author

@muas19 thank you for letting us know about the issue, but that issue is a separate one from this one

@getusha
Copy link
Contributor

getusha commented Feb 8, 2024

@hayata-suenaga on issue 2

Confirm that the Setting page opens from the right when the page should not animate. Only the part above the bottom tab should change without animation.

this is a bit confusing, Settings page should open from the right with animation or just open without any animation?

Screen.Recording.2024-02-08.at.11.57.29.AM.mov

@getusha
Copy link
Contributor

getusha commented Feb 8, 2024

It is almost impossible to go back from workspace settings without reloading

Is this related @hayata-suenaga @adamgrzybowski

Screen.Recording.2024-02-08.at.12.00.24.PM.mov

Copy link

melvin-bot bot commented Feb 8, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker 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 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.

@adamgrzybowski
Copy link
Contributor

this is a bit confusing, Settings page should open from the right with animation or just open without any animation?

The behavior you recorded is as expected. No animation for switching between bottom tabs.

It is almost impossible to go back from workspace settings without reloading

Not sure why impossible. Do you have any specific case? The back button may not be exactly right in that case though. There is a ticket for that #35610

settings.mov

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Feb 13, 2024
@melvin-bot melvin-bot bot changed the title [Wave 8] [Ideal Nav] Back navigation issues [HOLD for payment 2024-02-20] [Wave 8] [Ideal Nav] Back navigation issues Feb 13, 2024
Copy link

melvin-bot bot commented Feb 13, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Feb 13, 2024
Copy link

melvin-bot bot commented Feb 13, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.40-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-02-20. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Feb 13, 2024

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@hayata-suenaga] The PR that introduced the bug has been identified. Link to the PR:
  • [@hayata-suenaga] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@hayata-suenaga] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@adamgrzybowski] Determine if we should create a regression test for this bug.
  • [@adamgrzybowski] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [@CortneyOfstad] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@hayata-suenaga
Copy link
Contributor Author

⬆️ This regression was already known.

@getusha
Copy link
Contributor

getusha commented Feb 14, 2024

@hayata-suenaga could you assign me here for C+ payment?

@hayata-suenaga
Copy link
Contributor Author

@getusha reviewed this PR

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Feb 20, 2024
Copy link

melvin-bot bot commented Feb 20, 2024

Payment Summary

Upwork Job

  • Contributor: @adamgrzybowski is from an agency-contributor and not due payment
  • ROLE: @getusha paid $500 via Upwork (LINK) — paid $500 via Upwork

BugZero Checklist (@CortneyOfstad)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@CortneyOfstad
Copy link
Contributor

For some reason the Upwork Job link just links back to the top of this GH, so linking it here — https://www.upwork.com/jobs/~0194f5dc425897c3e9.

@getusha I've sent you an offer. Let me know once you accept and I can get that paid ASAP. Thanks!

@getusha
Copy link
Contributor

getusha commented Feb 20, 2024

@CortneyOfstad accepted, thank you!

@CortneyOfstad
Copy link
Contributor

Perfect — I completed payment to @getusha and updated the payment summary above. Regression test is already known, so this is all set! Closing this as completed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2
Projects
No open projects
Development

No branches or pull requests

8 participants