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

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

Closed
1 of 6 tasks
m-natarajan opened this issue Aug 18, 2024 · 19 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Needs Reproduction Reproducible steps needed Reviewing Has a PR in review Weekly KSv2

Comments

@m-natarajan
Copy link

m-natarajan commented Aug 18, 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:
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 world page

Actual Result:

Active backgrounded page changes to profile page

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

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
@m-natarajan m-natarajan added Daily KSv2 Needs Reproduction Reproducible steps needed Bug Something is broken. Auto assigns a BugZero manager. labels Aug 18, 2024
Copy link

melvin-bot bot commented Aug 18, 2024

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
Copy link

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

@ZhenjaHorbach
Copy link
Contributor

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

@bernhardoj
Copy link
Contributor

bernhardoj commented Aug 19, 2024

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.

if (focusedRHPRoute) {
let matchingRootRoute = getMatchingRootRouteForRHPRoute(focusedRHPRoute);

Based on the CENTRAL_PANE_TO_RHP_MAPPING file, the matching root for the contact method page is the profile page.

// Check for CentralPaneNavigator
for (const [centralPaneName, RHPNames] of Object.entries(CENTRAL_PANE_TO_RHP_MAPPING)) {
if (RHPNames.includes(route.name)) {
const paramsFromRoute = getParamsFromRoute(centralPaneName);
return {name: centralPaneName as CentralPaneName, params: pick(route.params, paramsFromRoute)};
}
}

[SCREENS.SETTINGS.PROFILE.ROOT]: [
SCREENS.SETTINGS.PROFILE.DISPLAY_NAME,
SCREENS.SETTINGS.PROFILE.CONTACT_METHODS,
SCREENS.SETTINGS.PROFILE.CONTACT_METHOD_DETAILS,

So, the profile page central screen is added because there is a state diff.

const {adaptedState, metainfo} = getAdaptedStateFromPath(path, linkingConfig.config);
if (adaptedState && (metainfo.isCentralPaneAndBottomTabMandatory || metainfo.isFullScreenNavigatorMandatory)) {
const diff = getPartialStateDiff(rootState, adaptedState as State<RootStackParamList>, metainfo);
const diffActions = getActionsFromPartialDiff(diff);
for (const diffAction of diffActions) {
root.dispatch(diffAction);
}

But, we actually have a way to find the matching root based on the backTo params.

// Check for backTo param. One screen with different backTo value may need diferent screens visible under the overlay.
if (route.params && 'backTo' in route.params && typeof route.params.backTo === 'string') {
const stateForBackTo = getStateFromPath(route.params.backTo, config);
if (stateForBackTo) {
// eslint-disable-next-line @typescript-eslint/no-shadow
const rhpNavigator = stateForBackTo.routes.find((route) => route.name === NAVIGATORS.RIGHT_MODAL_NAVIGATOR);
const centralPaneOrFullScreenNavigator = stateForBackTo.routes.find(
// eslint-disable-next-line @typescript-eslint/no-shadow
(route) => isCentralPaneName(route.name) || route.name === NAVIGATORS.FULL_SCREEN_NAVIGATOR,
);
// If there is rhpNavigator in the state generated for backTo url, we want to get root route matching to this rhp screen.
if (rhpNavigator && rhpNavigator.state) {
const isRHPinState = stateForBackTo.routes[0].name === NAVIGATORS.RIGHT_MODAL_NAVIGATOR;
if (isRHPinState) {
return getMatchingRootRouteForRHPRoute(findFocusedRoute(stateForBackTo) as NavigationPartialRoute);
}
}
// If we know that backTo targets the root route (central pane or full screen) we want to use it.
if (centralPaneOrFullScreenNavigator && centralPaneOrFullScreenNavigator.state) {
return centralPaneOrFullScreenNavigator as NavigationPartialRoute<CentralPaneName | typeof NAVIGATORS.FULL_SCREEN_NAVIGATOR>;
}
}
}

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,

const activeRoute = Navigation.getActiveRouteWithoutParams();

then use it as the backTo.

onLinkPress={() => Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHODS.getRoute(activeRoute))}

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

Call the get active route when pressing the button in

onLinkPress={() => Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHODS.getRoute(activeRoute))}

onPress={() => Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHODS.getRoute(activeRoute))}

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.

// eslint-disable-next-line @typescript-eslint/no-shadow
const rhpNavigator = stateForBackTo.routes.find((route) => route.name === NAVIGATORS.RIGHT_MODAL_NAVIGATOR);

if (rhpNavigator && rhpNavigator.state) {
const isRHPinState = stateForBackTo.routes[0].name === NAVIGATORS.RIGHT_MODAL_NAVIGATOR;
if (isRHPinState) {
return getMatchingRootRouteForRHPRoute(findFocusedRoute(stateForBackTo) as NavigationPartialRoute);
}
}

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.

const onNewContactMethodButtonPress = useCallback(() => {
Navigation.navigate(ROUTES.SETTINGS_NEW_CONTACT_METHOD.getRoute(navigateBackTo));

const onBackButtonPress = useCallback(() => {
if (navigateBackTo === ROUTES.SETTINGS_PROFILE) {
Navigation.goBack(ROUTES.SETTINGS_CONTACT_METHODS.route);
return;
}
Navigation.goBack(ROUTES.SETTINGS_CONTACT_METHODS.getRoute(navigateBackTo));

@melvin-bot melvin-bot bot added the Overdue label Aug 20, 2024
@kevinksullivan
Copy link
Contributor

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

image

@melvin-bot melvin-bot bot removed the Overdue label Aug 20, 2024
@bernhardoj
Copy link
Contributor

@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 melvin-bot bot added the Overdue label Aug 23, 2024
@kevinksullivan kevinksullivan added the External Added to denote the issue can be worked on by a contributor label Aug 24, 2024
@melvin-bot melvin-bot bot changed the title The active background screen changes after opening a contact method [$250] The active background screen changes after opening a contact method Aug 24, 2024
Copy link

melvin-bot bot commented Aug 24, 2024

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

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

melvin-bot bot commented Aug 24, 2024

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

@Ollyws
Copy link
Contributor

Ollyws commented Aug 26, 2024

Is @ZhenjaHorbach reviewing this one?

@ZhenjaHorbach
Copy link
Contributor

ZhenjaHorbach commented Aug 26, 2024

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

@Ollyws
Copy link
Contributor

Ollyws commented Aug 26, 2024

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

Copy link

melvin-bot bot commented Aug 26, 2024

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

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 26, 2024
@melvin-bot melvin-bot bot added the Reviewing Has a PR in review label Aug 26, 2024
@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Aug 26, 2024
@bernhardoj
Copy link
Contributor

PR is ready

cc: @Ollyws

@Ollyws
Copy link
Contributor

Ollyws commented Sep 8, 2024

Due for payment tomorrow @kevinksullivan, Thanks!

@kevinksullivan
Copy link
Contributor

Payment summary:

@bernhardoj
Copy link
Contributor

Requested in ND.

@JmillsExpensify
Copy link

$250 approved for @bernhardoj

@Ollyws
Copy link
Contributor

Ollyws commented Sep 13, 2024

Requested in ND.

@garrettmknight
Copy link
Contributor

$250 approved for @Ollyws

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. External Added to denote the issue can be worked on by a contributor Needs Reproduction Reproducible steps needed Reviewing Has a PR in review Weekly KSv2
Projects
No open projects
Status: No status
Development

No branches or pull requests

9 participants