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-04-30][$250] Bank account - Random animations between steps transitions on bank account setup flow #40081

Closed
1 of 6 tasks
izarutskaya opened this issue Apr 11, 2024 · 20 comments
Assignees
Labels
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

@izarutskaya
Copy link

izarutskaya commented Apr 11, 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: v1.4.62-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: https://expensify.testrail.io/index.php?/tests/view/4488988
Email or phone of affected tester (no customers): applausetester+vd_ios041024.2@applause.expensifail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Pre-requisite: user must be logged in, have created a Workspace and have enabled Workflows.

  1. Go to Workspace > Workflows.
  2. Tap on "Connect bank account".
  3. Select "Connect manually" and enter the testing credentials for routing and account numbers, then confirm.
  4. Verify that when "Personal info" page is displayed, a random and inconsistent animation is displayed on the "Legal first name" field.
  5. Fill the form and continue.
  6. Verify that when the date of birth page is displayed, a random and inconsistent animation is displayed on the calendar.
  7. Select a date and continue.
  8. Verify that when the SSN page is displayed, a random and inconsistent animation is displayed on the SSN field.
  9. Tap on the back arrow until you reach the first and last name page.
  10. Once more, tap on the back arrow, make sure to verify that when going to previous step (routing and account numbers), a random and inconsistent animation is displayed on the step numbers at the top.

Expected Result:

There should be no random animations when transitioning between any steps.

Actual Result:

There are some random and inconsistent animations when transitioning between steps.

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

Bug6445392_1712809738710.Chvg8229_1_.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0144ba5dc06cdd83b2
  • Upwork Job ID: 1778359833678811136
  • Last Price Increase: 2024-04-11
  • Automatic offers:
    • getusha | Reviewer | 0
    • ikevin127 | Contributor | 0
@izarutskaya izarutskaya added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Apr 11, 2024
Copy link

melvin-bot bot commented Apr 11, 2024

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

Copy link

melvin-bot bot commented Apr 11, 2024

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

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Apr 11, 2024
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.

@izarutskaya
Copy link
Author

@zanyrenney 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.

@zanyrenney
Copy link
Contributor

Yep can see the weird flash / animation. I agree this is a bug.

@zanyrenney zanyrenney added the External Added to denote the issue can be worked on by a contributor label Apr 11, 2024
@melvin-bot melvin-bot bot changed the title Bank account - Random animations between steps transitions on bank account setup flow [$250] Bank account - Random animations between steps transitions on bank account setup flow Apr 11, 2024
Copy link

melvin-bot bot commented Apr 11, 2024

Job added to Upwork: https://www.upwork.com/jobs/~0144ba5dc06cdd83b2

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

melvin-bot bot commented Apr 11, 2024

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

@Beamanator
Copy link
Contributor

Beamanator commented Apr 11, 2024

I don't think this needs to be a blocker - the weird animations aren't so annoying for the user, and it looks like all the functionality still works fine

  • commented in slack internally here

@Beamanator Beamanator added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Apr 11, 2024
@Beamanator
Copy link
Contributor

Ok ya marking as NAB, but accepting proposals for the fix!

@ikevin127
Copy link
Contributor

Proposal

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

There are some random and inconsistent animations when transitioning between steps on the bank account setup flow.

What is the root cause of that problem?

Offending PR:

Caution

Native code ahead -> more details / context can be found on this react-native issue:

Specifically #42604 (comment) and #42604 (comment), which I'm not going to pretend I understand fully ->
the gist being that there's an issue within:
node_modules/react-native/React/Fabric/Mounting/RCTMountingManager.mm
which has to do with mounting and CATransaction (from Apple docs):

CATransaction
A mechanism for grouping multiple layer-tree operations into atomic updates to the render tree.

The new architecture caused this iOS specific rendering issue of components with border style where the borders appear stretched while rendering a new screen, then animate to their default state shortly (where UI looks good again).

Check out the frame-by-frame of what's actually happening:

iOS: Native
Screen.Recording.2024-04-12.at.01.30.40.mov

Note

This issue is not specific to the BA setup flow, but any page that mounts while having components with border styles (inputs, interactive step, etc).

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

As suggested in #42604 (comment), in order to fix this issue as of right now we need to apply a react-native patch.

Did that, created the patch react-native+0.73.4+014+iOSCoreAnimationBorderRendering.patch, performed a clean install, rebuilt the iOS:Native app and the issue is gone! 🎉

Here's a test branch: test/40081, the diff and steps to test the fix:

  1. Chekout test branch.
  2. Run npm ci to perform a clean install.
  3. Rebuild the iOS: Native app.
  4. Test the fix using OP steps.

Videos (before / after)

iOS: Native
Before After After (OP Steps)
before.mov
after.mov
Screen.Recording.2024-04-12.at.01.59.59.mov

@shubham1206agra
Copy link
Contributor

This might be fixed in RN v0.74.
Commit: facebook/react-native@5fbdc99

@shubham1206agra
Copy link
Contributor

Thanks for the proposal btw @ikevin127.

@ikevin127
Copy link
Contributor

ikevin127 commented Apr 12, 2024

Posted #37374 (comment) in:

I guess it's up to the team to decide whether to add the proposed patch and fix the issue now or HOLD until we're upgraded to v0.74 and see if the issue will be fixed by:

Active conversation in #expensify-open-source Slack 🧵:

  • looks like the consensus12 is to move on with the patch considering it's not a big patch and, after the new arch changes it may take more time to upgrade to RN v0.743.

Footnotes

  1. Slack 🧵 (comment 7)

  2. Slack 🧵 (comment 9)

  3. Slack 🧵 (comment 8)

@melvin-bot melvin-bot bot added the Overdue label Apr 15, 2024
@getusha
Copy link
Contributor

getusha commented Apr 15, 2024

@Beamanator we decided to create a patch, we can assign @ikevin127 #40081 (comment)

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

melvin-bot bot commented Apr 15, 2024

📣 @getusha 🎉 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 Apr 15, 2024

📣 @ikevin127 🎉 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 📖

@ikevin127

This comment was marked as outdated.

@ikevin127
Copy link
Contributor

PR #40243 ready for review! 🎉

@ikevin127
Copy link
Contributor

ikevin127 commented Apr 25, 2024

⚠️ Automation failed here -> this should be on Hold for payment [2024-04-30] according to yesterday's production deploy from #40243 (comment).

cc @zanyrenney

@mallenexpensify mallenexpensify added Daily KSv2 and removed Reviewing Has a PR in review Weekly KSv2 labels Apr 30, 2024
@mallenexpensify mallenexpensify changed the title [$250] Bank account - Random animations between steps transitions on bank account setup flow [Hold for payment] [2024-04-30][$250] Bank account - Random animations between steps transitions on bank account setup flow Apr 30, 2024
@zanyrenney
Copy link
Contributor

zanyrenney commented May 1, 2024

Hey @ikevin127

Payment summary
paid @ikevin127 $250 via Upwork.
paid @getusha $250 via Upwork.

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

No branches or pull requests

7 participants