Skip to content

Web - Connect Bank Account - Nonfunctional Back Button when going to step 3 on the Connect bank account manually #24635

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

Closed
1 of 6 tasks
izarutskaya opened this issue Aug 16, 2023 · 14 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2

Comments

@izarutskaya
Copy link

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


Action Performed:

  1. Go to Settings > Workspaces
  2. Select a workspace and access "Connect Bank Account."
  3. Proceed to Step 2 out of 5.
  4. Enter company details and click on the continue button
  5. Click on the back button on the animation page between steps 2 and 3

Expected Result:

Clicking back takes you to Step 2

Actual Result:

Proceeds to step 3

Workaround:

Unknown

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: v1.3.54-11

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

Notes/Photos/Videos: Any additional supporting documentation

4232142525410395537t100_backstep3-1.MP4
Recording.1217.mp4

Expensify/Expensify Issue URL:

Issue reported by: @daveSeife

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1691199712158209

View all open jobs on GitHub

@izarutskaya izarutskaya added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 16, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 16, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Aug 16, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@izarutskaya
Copy link
Author

It’s potentially similar KI #23369

@akinwale
Copy link
Contributor

akinwale commented Aug 16, 2023

Proposal

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

Back button is non functional on the full screen loading animation when navigating to Step 3 in the Connect BA flow.

What is the root cause of that problem?

The achData.currentStep from the reimbursementAccount in the Onyx store is updated with the next step after the Continue button is pressed.

When a user clicks on the back button during the full page loading animation, there is an attempt to navigate back from the current step (Step 2: Company) to the first step. However, the step retrieved from Onyx (Step 3: Personal information) will be rendered after the loading animation is complete. Since the full page loading animation is still active when the back button is pressed, this results in a confusing UX.

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

Solution 1
For an improved UX, hide the back button while the full page loading animation / transition is being displayed.

<HeaderWithBackButton
title={props.translate('reimbursementAccountLoadingAnimation.oneMoment')}
onBackButtonPress={props.onBackButtonPress}
/>

In order to achieve this, we can update the ReimbursementAccountLoadingIndicator component by setting the shouldShowBackButton prop for HeaderWithBackButton to false.

Solution 2
Add an override flag to the goBack method which is passed as the onBackButtonPress handler for the ReimbursementAccountLoadingIndicator component on ReimbursementAccountPage. Use the BankAccounts.openReimbursementAccountPage method to navigate to the correct page as necessary.

Alternatively, implement a separate handler called goBackFromLoading which implements the necessary logic shown below and pass it as the onBackButtonPress prop for ReimbursementAccountLoadingIndicator.

Since we're going back from the loading animation, the idea is to return to the previous page (by overriding) which is essentially still the current step while the animation is active.

  1. Update the onBackButtonPress prop.
<ReimbursementAccountLoadingIndicator
    isSubmittingVerificationsData={isSubmittingVerificationsData}
    onBackButtonPress={() => this.goBack(true)}
/>
  1. Update the goBack handler. Also make use of the subStep parameter if necessary and add all the cases for the other steps as may be required.
goBack(loadingOverride) {
  const achData = lodashGet(this.props.reimbursementAccount, 'achData', {});
  const currentStep = achData.currentStep || CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT;
  
  if (loadingOverride) {
    switch (currentStep) {
        // ... handle other steps
        case CONST.BANK_ACCOUNT.STEP.COMPANY:
            BankAccounts.openReimbursementAccountPage(CONST.BANK_ACCOUNT.STEP.COMPANY, '', '');
            break;
        case CONST.BANK_ACCOUNT.STEP.REQUESTOR:
            BankAccounts.openReimbursementAccountPage(CONST.BANK_ACCOUNT.STEP.REQUESTOR, '', '');
            break;
        // ... handle other steps
    }
    return;
  }

  // ... the rest of the goBack handler
}

Solution 3
Similar to Solution 2 above, but we make use of a state variable to track what the previous / next steps should be, and then navigate back appropriately based on the value.

  1. Add the state value
this.state = {
    // ...
    overrideCurrentStep: null,
};
  1. Update the onBackButtonPress prop.
<ReimbursementAccountLoadingIndicator
    isSubmittingVerificationsData={isSubmittingVerificationsData}
    onBackButtonPress={() => this.goBack(true)}
/>
  1. Update the goBack handler.
goBack(loadingOverride) {
  const achData = lodashGet(this.props.reimbursementAccount, 'achData', {});
  const currentStep = achData.currentStep || CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT;
  
  if (loadingOverride) {
      switch (currentStep) {
          case CONST.BANK_ACCOUNT.STEP.COMPANY:
              this.setState({overrideCurrentStep: CONST.BANK_ACCOUNT.STEP.COMPANY});
              break;
          case CONST.BANK_ACCOUNT.STEP.REQUESTOR:
              this.setState({overrideCurrentStep: CONST.BANK_ACCOUNT.STEP.REQUESTOR});
              break;
          // ... handle other cases
      }
      return;
  }

  // ... the rest of the goBack handler
}
  1. Update currentStep in the componentDidUpdate method.
const currentStep = this.state.overrideCurrentStep || lodashGet(this.props.reimbursementAccount, 'achData.currentStep') || CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT;
  1. Update currentStep in the render method.
const currentStep = this.state.overrideCurrentStep || achData.currentStep || CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT;

What alternative solutions did you explore? (Optional)

None.

@dylanexpensify
Copy link
Contributor

Ah @izarutskaya good shout - will wait for that deploy to then test this again

@akinwale
Copy link
Contributor

Ah @izarutskaya good shout - will wait for that deploy to then test this again

@dylanexpensify I just had a look at the other issue, and it looks like this is a completely different bug based on the videos.

@melvin-bot melvin-bot bot added the Overdue label Aug 20, 2023
@dylanexpensify
Copy link
Contributor

reviewing today!

@dylanexpensify
Copy link
Contributor

Clarifying solution ^

@dylanexpensify
Copy link
Contributor

bumped above

@dylanexpensify
Copy link
Contributor

This should be done by the linked issue!

@akinwale
Copy link
Contributor

@dylanexpensify I just tested on the main branch and this is still happening.

Looking through the other issue, it appears to be dealing with back button navigation skipping over certain steps, for example, attempting to navigate back from Step 4 of the Connect Bank flow, and the user ends up at the first step of the flow instead of the being taken back to Step 3.

For this issue, the problem happens when you click on the Back button while the loading animation between steps is being displayed. To illustrate, when a user is on Step 2, and they click Save and continue, they're supposed to end up at Step 3. This works fine. However, if they click on the Back button that appears during the loading animation between Steps 2 and 3, they will still end up on Step 3, instead of being taken back to Step 2.

Hope this makes sense.

cc @sakluger

@akinwale
Copy link
Contributor

@dylanexpensify This bug is still happening on the main branch if you want to take another look. Thanks.

@DylanDylann
Copy link
Contributor

Proposal

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

  • Back button doesn't work while moving from step 2 to 3 in connect bank account

What is the root cause of that problem?

  • Currently, when user is in CompanyStep, then clicking on submit "Save & Continue" button, and while the loading indicator screen is displayed, clicking on back button, there are main steps:
  1. Call the API to submit data that is inputted in CompanyStep
  2. Set the isLoading to true and waiting for the API done
  3. Because isLoading is true, it will display the loading indicator screen <ReimbursementAccountLoadingIndicator />
  4. After API done, set isLoading to false, and API `s response contains what is the next step, and in this case, it is CompanyStep. Below is the data returned by this API:
 {
        "onyxMethod": "merge",
        "key": "reimbursementAccount",
        "value": {
            "achData": {
                "currentStep": "RequestorStep",
                "bankAccountID": 2100319
            },
            "errors": null
        }
    }
  1. Set the current step to CompanyStep based on the API `s response and render the RequestorStep screen
  • So we can see that in step 5, It always displays the RequestorStep regardless user has clicked on back button or not

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

  • When user clicks on back button while the loading indicator screen is displaying, we should cancel the current API call, clear state (if necessary) end then navigate to the previous step like below:
goBack() {
            ...
            case CONST.BANK_ACCOUNT.STEP.COMPANY:
                if (!isLoadingIndicatorOpen) {
                    BankAccounts.goToWithdrawalAccountSetupStep(CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT, {subStep: CONST.BANK_ACCOUNT.SUBSTEP.MANUAL});
                } else {
                    Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {
                        isLoading: false,
                        errors: null,
                    });
                    BankAccounts.clearOnfidoToken();
                    HttpUtils.cancelPendingRequests();
                }

                break;
  • In the above, isLoadingIndicatorOpen used to check whether the loading indicator is played or not. We cancel the API by HttpUtils.cancelPendingRequests() and clear a few states like isLoading, onfidoToken, ...
  • This bug appear with others steps as well, so we should apply the above solution for these steps

What alternative solutions did you explore? (Optional)

  • We can just disable the back button when the loading indicator screen is displayed

Result

Screencast.from.23-09-2023.12.42.18.webm

@DylanDylann
Copy link
Contributor

@dylanexpensify I still can reproduce this bug. Please help check when you have a chance. Thanks

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
Projects
None yet
Development

No branches or pull requests

4 participants