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-07-22][$125] mWeb - IOU - Clicking back from expense title edit page after a refresh takes to LHN #41017

Closed
1 of 6 tasks
lanitochka17 opened this issue Apr 25, 2024 · 52 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@lanitochka17
Copy link

lanitochka17 commented Apr 25, 2024

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: 1.4.66-0
Reproducible in staging?: Y
Reproducible in production?: N
**If this was caught during regression testing, add the test name, ID and link from TestRail:**N/A
Issue reported by: Applause - Internal Team

Issue found when executing PR #40704

Action Performed:

  1. Navigate to staging.new.expensify.com
  2. Sign in with an expensifail account
  3. Create a workspace
  4. Open workspace chat page
  5. Create 2 IOUs
  6. Open expense report page
  7. Click on "Title"
  8. Refresh the page
  9. Click on the back icon at the top left corner

Expected Result:

App takes user back to the expense report page

Actual Result:

App takes user to the 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

Bug6461799_1714061912067.Screen_Recording_20240425_191539_Chrome.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~011da57b8f26d630ba
  • Upwork Job ID: 1783627161453916160
  • Last Price Increase: 2024-06-13
  • Automatic offers:
    • ishpaul777 | Reviewer | 102842730
    • dominictb | Contributor | 102842731
Issue OwnerCurrent Issue Owner: @dylanexpensify
@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Apr 25, 2024
Copy link

melvin-bot bot commented Apr 25, 2024

Triggered auto assignment to @roryabraham (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@lanitochka17
Copy link
Author

@roryabraham 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

@lanitochka17
Copy link
Author

We think that this bug might be related to #vip-vsp

@roryabraham roryabraham added Daily KSv2 External Added to denote the issue can be worked on by a contributor and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Apr 25, 2024
Copy link

melvin-bot bot commented Apr 25, 2024

Job added to Upwork: https://www.upwork.com/jobs/~011da57b8f26d630ba

@melvin-bot melvin-bot bot changed the title mWeb - IOU - Clicking back from expense title edit page after a refresh takes to LHN [$250] mWeb - IOU - Clicking back from expense title edit page after a refresh takes to LHN Apr 25, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 25, 2024
Copy link

melvin-bot bot commented Apr 25, 2024

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

@roryabraham roryabraham changed the title [$250] mWeb - IOU - Clicking back from expense title edit page after a refresh takes to LHN [$125] mWeb - IOU - Clicking back from expense title edit page after a refresh takes to LHN Apr 25, 2024
Copy link

melvin-bot bot commented Apr 25, 2024

Upwork job price has been updated to $125

@allgandalf
Copy link
Contributor

allgandalf commented Apr 25, 2024

Proposal

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

Refreshing the page on expense title exit page and pressing back leads to LHN

What is the root cause of that problem?

We do not have onBackButtonPress defined for the RHP modal:

<HeaderWithBackButton
title={fieldName}
threeDotsMenuItems={menuItems}
shouldShowThreeDotsButton={!!menuItems?.length}
threeDotsAnchorPosition={styles.threeDotsPopoverOffsetNoCloseButton(windowWidth)}
/>

The default fallback is Navigation.goBack()

onBackButtonPress = () => Navigation.goBack(),

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

We need to define onBackButtonPress, which will dismiss the modal:

<HeaderWithBackButton
        title={fieldName}
       threeDotsMenuItems={menuItems}
      shouldShowThreeDotsButton={!!menuItems?.length}
      threeDotsAnchorPosition={styles.threeDotsPopoverOffsetNoCloseButton(windowWidth)}
     onBackButtonPress={() => Navigation.dismissModalWithReport(report)}
/>

Or we can simply use `dismissModal` instead we need to pass reportID there

What alternative solutions did you explore? (Optional)

@CK-codemax
Copy link

Refreshing the page on expense title exit page and pressing back leads to LHN

The Navigation.dismissModalWithReport solution seems really good as it goes indepth .
The report parameter suggests that it might be expecting an object that contains information about the modal or the report that the modal is associated with. This could include details about the modal's state, the data it's displaying, or any other relevant information that might be needed to properly dismiss the modal.

The Navigation.dismissModal dismisses the modal by identifying the modal to be dismissed using a reportID. This suggests a simpler approach where the modal's identity is determined solely by its ID, without the need for additional context or information.

The choice between Navigation.dismissModalWithReport(report) and Navigation.dismissModal(reportID) depends on the specific requirements of this application. If you need to perform additional actions or checks before dismissing the modal, dismissModalWithReport(report) might be the better choice. If your dismissal logic is straightforward and only requires the modal's ID, dismissModal(reportID) could be more suitable.

I also explored other options like using a Global Modal State Management to manage the visibility of modals globally. This approach allows you to control the modal's visibility from anywhere in your application, making it easier to dismiss the modal in response to various events, not just the back button press.

Copy link

melvin-bot bot commented Apr 26, 2024

📣 @CK-codemax! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@CK-codemax
Copy link

Contributor details
Your Expensify account email: whoroochuko@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01598dca7cc455581b

Copy link

melvin-bot bot commented Apr 26, 2024

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@melvin-bot melvin-bot bot added the Overdue label Apr 29, 2024
Copy link

melvin-bot bot commented Apr 29, 2024

@roryabraham, @ishpaul777 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@ishpaul777
Copy link
Contributor

Hey i was OOO past few days and have a few pending issues will get to this one real soon, sorry for delay

@melvin-bot melvin-bot bot removed the Overdue label Apr 30, 2024
Copy link

melvin-bot bot commented May 2, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@ishpaul777
Copy link
Contributor

on my list! will priortixe ASAP

@melvin-bot melvin-bot bot removed the Overdue label Jun 17, 2024
@ishpaul777
Copy link
Contributor

we can base on the following assumption: We should navigate to the ReportScreen if the current RHP route contains the reportID params. Hence the solution is to check for reportID and navigate back to the report screen accordingly

I feel this would be a safe assumption 👍 Lets roll with @dominictb Proposal

🎀 👀 🎀

Copy link

melvin-bot bot commented Jun 19, 2024

Current assignee @roryabraham is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 23, 2024
Copy link

melvin-bot bot commented Jun 23, 2024

📣 @ishpaul777 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Jun 23, 2024

📣 @dominictb 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@roryabraham
Copy link
Contributor

Great, @dominictb we look forward to reviewing your PR!

@dominictb
Copy link
Contributor

Seems like the PR is on production already @roryabraham? Shall we have the Awaiting payment label here?

@roryabraham roryabraham added the Bug Something is broken. Auto assigns a BugZero manager. label Jul 18, 2024
Copy link

melvin-bot bot commented Jul 18, 2024

Triggered auto assignment to @dylanexpensify (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.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 18, 2024
@roryabraham
Copy link
Contributor

Yep, sorry about that.

@roryabraham roryabraham added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review labels Jul 18, 2024
@roryabraham roryabraham changed the title [$125] mWeb - IOU - Clicking back from expense title edit page after a refresh takes to LHN [HOLD for payment 2024-07-22][$125] mWeb - IOU - Clicking back from expense title edit page after a refresh takes to LHN Jul 18, 2024
@melvin-bot melvin-bot bot added the Overdue label Jul 22, 2024
@ishpaul777
Copy link
Contributor

not overdue

Copy link

melvin-bot bot commented Jul 22, 2024

@roryabraham, @dylanexpensify, @ishpaul777, @dominictb Whoops! This issue is 2 days overdue. Let's get this updated quick!

@dylanexpensify
Copy link
Contributor

Payment summary:

Please apply/request!

@melvin-bot melvin-bot bot removed the Overdue label Jul 23, 2024
@dylanexpensify
Copy link
Contributor

Done!

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 Engineering External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

8 participants