Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.32k stars 2.75k forks source link

[$250] The active background screen changes after opening a contact method #47612

Open m-natarajan opened 3 weeks ago

m-natarajan commented 3 weeks ago

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: Reproducible in staging?: Needs Reproduction (Reproduction attempt not made as its live) Reproducible in production?: Needs Reproduction 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: @ZhenjaHorbach Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1723753120436449

Action Performed:

  1. Open app
  2. Open account settings
  3. In general choose save the world
  4. Choose I am a teacher
  5. Press Contact methods or update email address
  6. Choose any contact method

Expected Result:

User should remain in the backgrounded Save the worldpage

Actual Result:

Active backgrounded page changes to profile page

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~010a99315386621f72
  • Upwork Job ID: 1827399721942621412
  • Last Price Increase: 2024-08-24
Issue OwnerCurrent Issue Owner: @Ollyws
melvin-bot[bot] commented 3 weeks ago

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

MelvinBot commented 3 weeks ago

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

ZhenjaHorbach commented 3 weeks ago

@kevinksullivan I reported this issue Plus I can reproduce this issue Can I be a reviewer here please?

bernhardoj commented 3 weeks ago

Edited by proposal-police: This proposal was edited at 2024-08-19 02:01:08 UTC.

Proposal

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

The active background screen when going to contact method from I'm a teacher page changes to profile screen.

What is the root cause of that problem?

When we go to the contact method, getAdaptedStateFromPath is triggered to find the matching root for the contact method RHP. https://github.com/Expensify/App/blob/62722e8bc636f1be2e2c9e8e980aac93b76804c9/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts#L192-L193

Based on the CENTRAL_PANE_TO_RHP_MAPPING file, the matching root for the contact method page is the profile page. https://github.com/Expensify/App/blob/62722e8bc636f1be2e2c9e8e980aac93b76804c9/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts#L138-L145 https://github.com/Expensify/App/blob/62722e8bc636f1be2e2c9e8e980aac93b76804c9/src/libs/Navigation/linkingConfig/CENTRAL_PANE_TO_RHP_MAPPING.ts#L5-L8

So, the profile page central screen is added because there is a state diff. https://github.com/Expensify/App/blob/62722e8bc636f1be2e2c9e8e980aac93b76804c9/src/libs/Navigation/linkTo/index.ts#L135-L141

But, we actually have a way to find the matching root based on the backTo params. https://github.com/Expensify/App/blob/62722e8bc636f1be2e2c9e8e980aac93b76804c9/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts#L110-L136

When we go from I'm a teacher to a contact method, the backTo should be the I'm a teacher page, so the matching root would be the I'm a teacher page. But in our case, the backTo is /. It's because we define the active route once,

https://github.com/Expensify/App/blob/62722e8bc636f1be2e2c9e8e980aac93b76804c9/src/pages/TeachersUnite/ImTeacherUpdateEmailPage.tsx#L17

then use it as the backTo. https://github.com/Expensify/App/blob/62722e8bc636f1be2e2c9e8e980aac93b76804c9/src/pages/TeachersUnite/ImTeacherUpdateEmailPage.tsx#L32

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

Call the get active route when pressing the button in https://github.com/Expensify/App/blob/62722e8bc636f1be2e2c9e8e980aac93b76804c9/src/pages/TeachersUnite/ImTeacherUpdateEmailPage.tsx#L32 https://github.com/Expensify/App/blob/62722e8bc636f1be2e2c9e8e980aac93b76804c9/src/pages/TeachersUnite/ImTeacherUpdateEmailPage.tsx#L42

Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHODS.getRoute(Navigation.getActiveRouteWithoutParams()))

But this still isn't enough because even though the backTo is an RHP, isRHPinState variable here is false. https://github.com/Expensify/App/blob/62722e8bc636f1be2e2c9e8e980aac93b76804c9/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts#L114-L115 https://github.com/Expensify/App/blob/62722e8bc636f1be2e2c9e8e980aac93b76804c9/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts#L123-L129

It's because stateForBackTo.routes contains [BottomTabNavigator, RHPNavigator], so always accessing the first index won't work. We can just remove isRHPinState because we already have rhpNavigator which already finds RHP inside stateForBackTo.routes.

Last, we need to also pass the backTo from contact method page to contact method details page and handle the back button press, just like what we did when navigating to new contact method page.

https://github.com/Expensify/App/blob/62722e8bc636f1be2e2c9e8e980aac93b76804c9/src/pages/settings/Profile/Contacts/ContactMethodsPage.tsx#L93-L94 https://github.com/Expensify/App/blob/62722e8bc636f1be2e2c9e8e980aac93b76804c9/src/pages/settings/Profile/Contacts/NewContactMethodPage.tsx#L80-L85

kevinksullivan commented 3 weeks ago

@ZhenjaHorbach I don't see step 5. Is there anything missing from the reproduction steps?

image

bernhardoj commented 3 weeks ago

@kevinksullivan looking at the code logic, the step 5 will show if we use a public domain email as the default contact method.

melvin-bot[bot] commented 2 weeks ago

Job added to Upwork: https://www.upwork.com/jobs/~010a99315386621f72

melvin-bot[bot] commented 2 weeks ago

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

Ollyws commented 2 weeks ago

Is @ZhenjaHorbach reviewing this one?

ZhenjaHorbach commented 2 weeks ago

@Ollyws I would like to review But since we decided to use auto assignment Then go ahead ! All yours 😌

Ollyws commented 2 weeks ago

@bernhardoj's proposal LGTM. 🎀👀🎀 C+ reviewed

melvin-bot[bot] commented 2 weeks ago

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

bernhardoj commented 2 weeks ago

PR is ready

cc: @Ollyws

Ollyws commented 3 days ago

Due for payment tomorrow @kevinksullivan, Thanks!