[$250] Workspace - Connect bank account one moment page sliding animation is distorted

[lanitochka17]

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.85-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: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Log in with a new expensifail account
2. Create a workspace
3. Enable Workflows
4. Navigate to workspace settings - Workflows
5. Click on "Connect bank account"

Expected Result:

The animation should be smooth, and without any distortions

Actual Result:

Connect bank account one moment page sliding animation is distorted. The sliding animation is choppy and you can see the text at it's intended position before the animation finishes

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

Bug6517246_1718717583417.bandicam_2024-06-18_15-23-30-036.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01747cba36f37ce546
• Upwork Job ID: 1804218709299744642
• Last Price Increase: 2024-07-05

Issue Owner

Current Issue Owner: @getusha

[melvin-bot]

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

[lanitochka17]

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

[bernhardoj]

Proposal

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

The connect bank account page slide animation isn't smooth.

What is the root cause of that problem?

On the bank account page, we have a navigation code that will be called whenever the step is changed.

App/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx

Line 312 in c11ae2f

Navigation.navigate(ROUTES.BANK_ACCOUNT_WITH_STEP_TO_OPEN.getRoute(getRouteForCurrentStep(currentStep), policyID, backTo));

When we open the bank account page, the step is initially empty and will be updated to /new which is mapped to BankAccountStep that shows whether to connect manually or with Plaid.

The navigation code is there to update the step params of the bank account page. However, the navigation code is called while the slide animation is still ongoing which makes it not smooth.

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

Instead of using navigate to update the params, we can use setParams.

Navigation.setParams({stepToOpen: getRouteForCurrentStep(currentStep)});

[melvin-bot]

Triggered auto assignment to @shawnborton (Design), see these Stack Overflow questions for more details.

[mallenexpensify]

@shawnborton 👀 plz, I think this is the bug

[shawnborton]

Yup, agree that it looks like a bug.

[shawnborton]

Seems like @bernhardoj has a good solution but let's get an engineer/C+ assigned to review. I also wonder if this is happening in other places too.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01747cba36f37ce546

[melvin-bot]

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

[mallenexpensify]

Thanks @shawnborton
@getusha and @bernhardoj can you dig around to see if you can find other instances we can fix at the same time as this?

@getusha can you review @bernhardoj 's proposal above too plz

[getusha]

@bernhardoj, the solution seems to work, but I don't think we have a complete RCA.

Check out this branch: https://github.com/software-mansion-labs/expensify-app-fork/tree/exclude-magic-code-from-last-visited-path. I am not able to reproduce the issue. We need to make sure we fix the root cause if possible.

[bernhardoj]

It doesn't happen in https://github.com/software-mansion-labs/expensify-app-fork/tree/exclude-magic-code-from-last-visited-path because navigating to the same page does nothing which is from this PR.

[melvin-bot]

@shawnborton, @mallenexpensify, @getusha Whoops! This issue is 2 days overdue. Let's get this updated quick!

[getusha]

Looks like a regression from #43651

[bernhardoj]

Before #42335, navigation action to the same screen with the same name and params is executed.

After that PR, the action is ignored. #43651 fixed it if the params are different. It's the same case as:

App/src/libs/Navigation/linkTo/index.ts

Lines 84 to 93 in d005e6e

	const areParamsDifferent =
	targetName === SCREENS.REPORT
	? getTopmostReportId(rootState) !== getTopmostReportId(stateFromPath)
	: !shallowCompare(
	omitBy(topmostCentralPaneRoute?.params as Record<string, unknown> | undefined, (value) => value === undefined),
	omitBy(targetParams as Record<string, unknown> | undefined, (value) => value === undefined),
	);
	// If this action is navigating to the report screen and the top most navigator is different from the one we want to navigate - PUSH the new screen to the top of the stack by default
	if (isCentralPaneName(action.payload.name) && (isTargetScreenDifferentThanCurrent || areParamsDifferent)) {

[mallenexpensify]

What's the next step here? @madmax330 and @eh2077 , do you agree with @getusha 's comment above that this looks like a regression from #43651 ?

[mallenexpensify]

Reminder to self to check on this before issue payment

• [HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$250] Workspace switcher search bar is in the wrong place #40826 (comment)

[melvin-bot]

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

[melvin-bot]

@shawnborton @mallenexpensify @getusha this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

@shawnborton, @mallenexpensify, @getusha Eep! 4 days overdue now. Issues have feelings too...

[getusha]

I am not able to repro this after reverting #43651, will dig into this more tomorrow.

[getusha]

Found another place where this issue is happening.

Screen.Recording.2024-07-03.at.10.22.52.at.night.mov

[mallenexpensify]

It's also happening if you click Settings in the RHP too.

2024-07-03_15-11-48.mp4

Are these the repro steps @getusha? If so, Any reason I shouldn't update the OP with the steps?

1. Click on a chat with a workspace
2. click header
3. click Members or Settings
4. then back arrow (and repeat last two steps).

[bernhardoj]

The other RHP sliding animation issue is currently caused by the focus trap which is being handled in #44421 I believe. It affects the bank account page too.

But when the focus trap issue is fixed, our issue here will still exist.

[melvin-bot]

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

[getusha]

Are these the repro steps @getusha? If so, Any reason I shouldn't update the OP with the steps?

I think one instance is enough, they all seem to have a common root cause.

[melvin-bot]

@shawnborton, @mallenexpensify, @getusha Whoops! This issue is 2 days overdue. Let's get this updated quick!

[mallenexpensify]

@bernhardoj does your proposal need to be updated before @getusha reviews?

[bernhardoj]

The solution for our issue is still the same, but looks like I can't repro this anymore

[getusha]

I think this got fixed here #44421
we can close this @mallenexpensify (you're assigned there too 😅 )

[getusha]

for this we can close #39463 without payment for C+

[mallenexpensify]

you're assigned there too 😅 )

d'oh, sorry I didn't catch that sooner.

for #43920 (comment) we can close #39463 without payment for C+

Can you provide more context here, I'm unsure how they're related.

[getusha]

Can you provide more context here, I'm unsure how they're related.

@mallenexpensify I got paid twice (via newDot and upwork) in that issue and we planned to close this without a payment to compensate that.

[mallenexpensify]

Let's keep that issue separate for now @getusha .

Closing