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

[$500] Tapping on back arrow in the thread takes user to LHN instead of the parent message #36040

Closed
3 of 6 tasks
m-natarajan opened this issue Feb 7, 2024 · 10 comments
Closed
3 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@m-natarajan
Copy link

m-natarajan commented Feb 7, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 1.4.38-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @quinthar
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1707282748260789

Action Performed:

  1. Open any report
  2. Long press and start thread
  3. send a message
  4. Tap back arrow

Expected Result:

Taking user to parent message

Actual Result:

Takes user to LHN

Workaround:

unknown

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

Add any screenshot/video evidence

az_recorder_20240207_110102.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01a2adb75f9035e2a6
  • Upwork Job ID: 1755260136545419264
  • Last Price Increase: 2024-02-07
@m-natarajan m-natarajan added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 7, 2024
@melvin-bot melvin-bot bot changed the title Tapping on back arrow in the thread takes user to LHN instead of the parent message [$500] Tapping on back arrow in the thread takes user to LHN instead of the parent message Feb 7, 2024
Copy link

melvin-bot bot commented Feb 7, 2024

Job added to Upwork: https://www.upwork.com/jobs/~01a2adb75f9035e2a6

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 7, 2024
Copy link

melvin-bot bot commented Feb 7, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (External)

Copy link

melvin-bot bot commented Feb 7, 2024

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

@ZhenjaHorbach
Copy link
Contributor

ZhenjaHorbach commented Feb 7, 2024

Proposal

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

Tapping on back arrow in the thread takes user to LHN instead of the parent message

What is the root cause of that problem?

The main problem with this issue is that when we press back we always return to Home on Report screen page

const goBack = useCallback(() => {
Navigation.goBack(ROUTES.HOME, false, true);
}, []);

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

To fix this issue we can update this function and add new condition

    const isThread = ReportUtils.isChatThread(report);
    const goBack = useCallback(() => {
        if (isThread) {
            Navigation.goBack();
            return;
        }
        Navigation.goBack(ROUTES.HOME, false, true);
    }, [isThread]);

Also, we can make these changes optional for small screens
But I'm not sure that it makes sense

What alternative solutions did you explore? (Optional)

NA

@allgandalf
Copy link
Contributor

Proposal

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

Tapping on back arrow in the thread takes user to LHN instead of the parent message

What is the root cause of that problem?

We reroute all type of ReportScreen views to HOME

const goBack = useCallback(() => {
Navigation.goBack(ROUTES.HOME, false, true);
}, []);

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

Reroute all goBack calls to their respective routes:

 const goBack = useCallback(() => { 
     Navigation.goBack(); 
 }, []); 

What alternative solutions did you explore? (Optional)

N/A

@FitseTLT
Copy link
Contributor

FitseTLT commented Feb 7, 2024

Proposal

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

Tapping on back arrow in the thread takes user to LHN instead of the parent message

What is the root cause of that problem?

This is a regression caused by recent changes (Ideal Nav) #33280 of setting the ROUTES.HOME from empty string to Home now.
So before this change when we goBack with ROUTES.HOME as fallback Navigation.goBack(ROUTES.HOME, false, true); b/c home is empty string it will falsify this

if (isCentralPaneFocused && fallbackRoute && distanceFromPathInRootNavigator > 0) {
navigationRef.current.dispatch(StackActions.pop(distanceFromPathInRootNavigator));

So whenever possible it will navigationRef.current.goBack so it worked fine

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

Now everywhere we are calling Navigation.goBack(ROUTES.HOME, false, true); it is not working right so we have to change in
Navigation.goBack everywhere we used fallbackRoute as a condition we need to check it is not ROUTES.HOME here and here

fallbackRoute && fallbackRoute !== ROUTES.HOME

Or similarly everywhere in the codebase we are using ROUTES.HOME as a fallbackRoute we need to pass empty string to revert back the previous behaviour

What alternative solutions did you explore? (Optional)

@Santhosh-Sellavel Santhosh-Sellavel removed their assignment Feb 7, 2024
@Santhosh-Sellavel
Copy link
Collaborator

Unassiging this one due to low bandwidth, please reassign @garrettmknight!

@situchan
Copy link
Contributor

situchan commented Feb 7, 2024

This should be part of #35626

@situchan
Copy link
Contributor

situchan commented Feb 7, 2024

@adamgrzybowski please confirm that your PR fixes this as well

@hayata-suenaga
Copy link
Contributor

this issue is tracked in this GH issue. Closing this one 🙇

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. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

8 participants