Reanimated modal for validate code action modal by jmusial · Pull Request #66046 · Expensify/App

jmusial · 2025-07-14T10:48:45Z

Explanation of Change

This PR uses react-native-reanimated based modal for Validate Code Action Modal.

Fixed Issues

PROPOSAL:

https://expensify.slack.com/archives/C05LX9D6E07/p1750147058498549

Tests

Note: Some of the tests have full flow, in some just showing the modal and going back - there seems to be a limit on number of codes that 1 user can generate in given time.

Tests for 2 out of many places that the modal exists in:

Add new contact method

Open app
Go to Settings -> Profile -> Contact method
Click "Add new contact method"
Enter email
Enter Validation Code no. 1 / or cancel by pressing back arrow
Enter Validation Code no. 2
New Contact method is connected

Issue new Expensify card

Open app
Open a workspace that you can issue Expensify Cards in (needs to have a connected bank account and currency USD)
Click "Issue card"
Select account
Select Virtual card
Select "Smart limit"
Set a limit
Give it a name
Issue card
Enter code / or cancel by pressing back arrow
Card is issued

Offline tests

N / A

QA Steps

Same as tests

PR Author Checklist

Screenshots/Videos

Android: Native

0032.android.native.mov

Android: mWeb Chrome

0032.android.chrome.mov

iOS: Native

0032.ios.native.mp4

iOS: mWeb Safari

0032.ios.safari.mp4

MacOS: Chrome / Safari

0032.mac.chrome.mov

MacOS: Desktop

0032.mac.native.mov

…on-modal

jmusial · 2025-07-22T10:11:32Z

Found this bug while retesting, but it's not connected to the modal inplementation change. Already posted on slack

Go to staging.new.expensify.com on any WEB
Go to Account -> Profile -> Contact method
Type in the magic code

Expected Result: Second modal code request appears (to confirm new contact email.)
Actual Result: Not found page appears

Platforms: any web

Screen.Recording.2025-07-22.at.11.52.19.mov

It's here
#65234

melvin-bot · 2025-07-22T10:11:45Z

@ZhenjaHorbach Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

ZhenjaHorbach · 2025-07-23T11:07:44Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
I verified tests pass on all platforms & I tested again on:
- Android: HybridApp
- Android: mWeb Chrome
- iOS: HybridApp
- iOS: mWeb Safari
- MacOS: Chrome / Safari
- MacOS: Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified there are no new alerts related to the canBeMissing param for useOnyx
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp

2025-07-23.12.57.22.mov

Android: mWeb Chrome

2025-07-23.12.55.15.mov

iOS: HybridApp

ios.mov

iOS: mWeb Safari

ios-web.mov

MacOS: Chrome / Safari

web.mov

MacOS: Desktop

web.mov

ZhenjaHorbach · 2025-07-23T11:10:04Z

@jmusial
Could you update the branch to the latest version of main ?

ZhenjaHorbach · 2025-07-23T11:11:33Z

@@ -72,6 +72,7 @@ function ValidateCodeActionModal({
            useNativeDriver


As I remenber ReanimatedModal doesn't use this prop

ZhenjaHorbach · 2025-07-23T11:29:58Z

small bug
But when we close a modal by clicking on background
Modals close with at different speeds

ZhenjaHorbach · 2025-07-23T11:45:08Z

And Is it just me or is the animation too fast?

2025-07-23.13.43.59.mov

…on-modal

jmusial · 2025-07-25T09:28:39Z

small bug But when we close a modal by clicking on background Modals close with at different speeds

I think this is the same as here. We have Navigator modal and and a reanimated one on top. We'll be working on aligning the transition timings as followups.

jmusial · 2025-07-25T09:34:31Z

And Is it just me or is the animation too fast?

2025-07-23.13.43.59.mov

Did some testing and I think this is just because default Navigator transitions are longer on ios.

The timings for our transition is 300 in 200 . Default for navigator is 500 in out for IOS and 350 in out for Android.

I think that's why it looks a bit out of place on IOS. @Expensify/design should we tweak transition times for RHP mimicing modals to be more in line with navigator ?

Below 500ms transition for that modal

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-07-25.at.11.31.25.mp4

ZhenjaHorbach · 2025-07-25T10:00:45Z

small bug But when we close a modal by clicking on background Modals close with at different speeds

I think this is the same as here. We have Navigator modal and and a reanimated one on top. We'll be working on aligning the transition timings as followups.

Oh yeah
I didn't notice that this screen is not part of the navigation
Yes, I also think that these are similar cases !

shawnborton · 2025-07-25T11:03:04Z

Hmm I don't think I am exactly following what you are saying, mind rephrasing?

Otherwise I think the video you are showing looks okay to me? Curious what others think though.

ZhenjaHorbach · 2025-07-25T11:09:57Z

And Is it just me or is the animation too fast?

2025-07-23.13.43.59.mov

@shawnborton
You can check this video
There is a difference in animation speed for validate code action modal

2025-07-23.13.43.59.mov

shawnborton · 2025-07-25T11:13:01Z

Ah okay yeah, totally agree that feels way too fast.

…-for-validate-code-action-modal

jmusial · 2025-07-28T12:03:39Z

Yes Now animation looks good ! Thanks ! but I noticed this bug

2025-07-25.15.31.10.mov

@ZhenjaHorbach this validate screen comes from Navigation, It's not a regression from modal migration.

Sorry, now I see that I should have chosen less confusing flow for testing this one :(

EDIT: Ok, it's kinda connected there is the same issue when using the old modal but it's more visible on the new one. It's due to how VerifyAccountPage is implemented it's a RHP page that renders a modal, so there are 2 slide right animations playing and those are not in sync -.-'

old.modal.issue.mov

I'm gonna check how many places have similar issue and probably rewrite it.

ZhenjaHorbach · 2025-07-28T12:30:42Z

Yes Now animation looks good ! Thanks ! but I noticed this bug
2025-07-25.15.31.10.mov

@ZhenjaHorbach this validate screen comes from Navigation, It's not a regression from modal migration.

Sorry, now I see that I should have chosen less confusing flow for testing this one :(

EDIT: Ok, it's kinda connected there is the same issue when using the old modal but it's more visible on the new one. It's due to how VerifyAccountPage is implemented it's a RHP page that renders a modal, so there are 2 slide right animations playing and those are not in sync -.-'

old.modal.issue.mov
I'm gonna check how many places have similar issue and probably rewrite it.

Yeah 😅
I'm pretty sure this appeared after last commits

…on-modal

jmusial · 2025-07-30T14:24:37Z

Hey @ZhenjaHorbach, sorry have been sidetracked by other issues. Coming back to this one:

This recording is from current main (on IOS). (So old modal) You can see there 2 different modals, and all the bugs you mentioned :(

Screen.Recording.2025-07-30.at.14.40.43.mov

Web has the same issues (slightly less visible).

It's a bit tricky this one because it's a modal over RHP, but it is used as an auth screen so its the first thing user sees and RHP and modal open at the same time.

I think keeping current implementation (ValidateCodeAction) as a modal it's going to be hard to get rid of those. The best solution that comes to mind would be to do them all as RHP with navigation path (ie not a modal). Similar to how it's done for /settings/profile/contact-methods/verify.

@Expensify/design are issues shown one the video above passable (it usually happens over 200ms so without slo mo barely noticable)?

shawnborton · 2025-07-30T15:10:06Z

Hmm I suppose if it's not perceivable by the user it's okay, but let's get an opinion from our engineers too.

jmusial · 2025-07-31T08:29:58Z

Hey @ZhenjaHorbach, sorry have been sidetracked by other issues. Coming back to this one:

This recording is from current main (on IOS). (So old modal) You can see there 2 different modals, and all the bugs you mentioned :(

Screen.Recording.2025-07-30.at.14.40.43.mov
Web has the same issues (slightly less visible).

It's a bit tricky this one because it's a modal over RHP, but it is used as an auth screen so its the first thing user sees and RHP and modal open at the same time.

I think keeping current implementation (ValidateCodeAction) as a modal it's going to be hard to get rid of those. The best solution that comes to mind would be to do them all as RHP with navigation path (ie not a modal). Similar to how it's done for /settings/profile/contact-methods/verify.

@Expensify/design are issues shown one the video above passable (it usually happens over 200ms so without slo mo barely noticable)?

@mountiny what's your opinion on this ?

ZhenjaHorbach · 2025-08-01T08:13:51Z

I think keeping current implementation (ValidateCodeAction) as a modal it's going to be hard to get rid of those. The best solution that comes to mind would be to do them all as RHP with navigation path (ie not a modal). Similar to how it's done for /settings/profile/contact-methods/verify.

I think it's a good idea !
But since the issue is not very noticeable
I don't mind leaving everything as it is for now
And maybe in the future fix it as a separate issue !

But yes
Let's wait for @mountiny opinion !

mountiny · 2025-08-01T14:09:33Z

Yea I do think that making them its own route and page in the stack makes the most sense. What are downsides to doing that? @jmusial

Otherwise it seems like no new issues are introduced here so we can move this ahead

mountiny

Changes look fine to me, though I would imagine it makes sense to update some of those flows to be its own pages instead of modals, I am not sure why they are modals in a first place. Maybe as we tried to reuse the same verification component in bunch of flows that was easier

mountiny · 2025-08-01T14:11:22Z

@ZhenjaHorbach do you approve of the PR?

ZhenjaHorbach · 2025-08-01T14:27:52Z

@ZhenjaHorbach do you approve of the PR?

Yeah
It's good !

mountiny · 2025-08-01T14:49:56Z

@ZhenjaHorbach, Can you please write down which modals suffered from the bug you listed above? We could then see if we should migrate them to their own screens.

ZhenjaHorbach · 2025-08-01T14:56:13Z

@ZhenjaHorbach, Can you please write down which modals suffered from the bug you listed above? We could then see if we should migrate them to their own screens.

In this PR we worked only with ValidateCodeActionModal
And this modal is used in few places
So I guess we can reuse the same content for few validate code screens but with different requests (Or just make one screen through conditions)

And I can't remember similar issues in other modals 😅
But I'll double check other migrations
Maybe in other modals these issues is not so noticeable

OSBotify · 2025-08-01T15:00:47Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

jmusial · 2025-08-04T08:56:26Z

@ZhenjaHorbach, Can you please write down which modals suffered from the bug you listed above? We could then see if we should migrate them to their own screens.
@mountiny
Yeah this is just for ValidateCodeActionModal It's use cases:

Confirming Expensify Card Issue
Report Virtual Card Fraud
Reporting lost card
2FA
Adding delegate
Updating delegate
New contact method
Verify Contact method (first you verify your current account, then new account)
Verify bank account
Missing personal details (not sure how to get to this one)

If you want I can go manually through the flows and make recommendations.

ZhenjaHorbach · 2025-08-04T09:02:21Z

Missing personal details (not sure how to get to this one)

You can directly open https://staging.new.expensify.com/missing-personal-details
And after all steps you will see ValidateCodeActionModal

jmusial · 2025-08-04T09:18:59Z

@ZhenjaHorbach yeah I mean on web it's ez 😄. But dunno how to get to this flow other than direct link

mountiny · 2025-08-04T15:52:13Z

@getusha @hungvu193 I remember you worked on the validate modal, right? I was also involved, but I cannot recall what was the reason for not creating these as separate pages?

hungvu193 · 2025-08-04T16:39:59Z

I remember I decided to create modal instead of separate page to reuse it every where. Most of the flows at the time only use magic code to verify the user. They shared the same flow (2Fa, contact method, connect bank account..etc). Creating a page also requires several steps and might need extra effort to handle routing, go back cases...

github-actions · 2025-08-04T19:59:00Z

🚀 Deployed to staging by https://github.com/mountiny in version: 9.1.89-1 🚀

platform	result
🖥 desktop 🖥	success ✅
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

github-actions · 2025-08-06T19:29:56Z

🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.1.89-21 🚀

platform	result
🖥 desktop 🖥	success ✅
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	failure ❌

Migrate ValidateCodeActionModal to ReanimatedModal

b9b439d

jmusial mentioned this pull request Jul 14, 2025

[Due for payment 2025-09-02] [Due for payment 2025-08-20] [Due for payment 2025-08-13] Use react-native-reanimated in RIGHT_DOCKED Modals #64778

Closed

fix wide screen issues

7f6c1e0

jmusial changed the title ~~Feat/reanimated modal for validate code action modal~~ Reanimated modal for validate code action modal Jul 14, 2025

jmusial added 5 commits July 14, 2025 16:59

fix ts

edfa2bc

add forgotten stashed changes lol

8aced10

bump to fix tests

c5095d4

Merge branch 'main' into feat/reanimated-modal-for-validate-code-acti…

9eee36a

…on-modal

clean changes after merge

b1015ab

jmusial marked this pull request as ready for review July 22, 2025 10:11

jmusial requested a review from a team as a code owner July 22, 2025 10:11

melvin-bot Bot requested review from ZhenjaHorbach and removed request for a team July 22, 2025 10:11

ZhenjaHorbach reviewed Jul 23, 2025

View reviewed changes

jmusial added 4 commits July 23, 2025 13:46

Merge branch 'main' into feat/reanimated-modal-for-validate-code-acti…

88f6027

…on-modal

remove usused props

bc4f174

Merge branch 'main' into feat/reanimated-modal-for-validate-code-acti…

4d68fdf

…on-modal

Merge branch 'main' into feat/reanimated-modal-for-validate-code-acti…

5373878

…on-modal

Merge remote-tracking branch 'origin/main' into feat/reanimated-modal…

26836a0

…-for-validate-code-action-modal

jmusial added 2 commits July 29, 2025 16:19

Merge branch 'main' into feat/reanimated-modal-for-validate-code-acti…

5e23d6a

…on-modal

Merge branch 'main' into feat/reanimated-modal-for-validate-code-acti…

f43fad6

…on-modal

jmusial mentioned this pull request Jul 31, 2025

[Due for payment 2025-09-02] [HOLD https://github.com/Expensify/App/issues/69171] [$250] Delegate Confirmation Modal Lacks Exit Animation #65576

Closed

16 tasks

mountiny assigned jmusial Aug 1, 2025

mountiny approved these changes Aug 1, 2025

View reviewed changes

ZhenjaHorbach approved these changes Aug 1, 2025

View reviewed changes

melvin-bot Bot requested a review from mountiny August 1, 2025 14:27

mountiny merged commit 38de4ae into Expensify:main Aug 1, 2025
19 checks passed

github-actions Bot mentioned this pull request Aug 4, 2025

Deploy Checklist: New Expensify 2025-08-04 #67808

Closed

jmusial deleted the feat/reanimated-modal-for-validate-code-action-modal branch August 26, 2025 08:11

		@@ -72,6 +72,7 @@ function ValidateCodeActionModal({
		useNativeDriver

Conversation

jmusial commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Add new contact method

Issue new Expensify card

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

jmusial commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

melvin-bot Bot commented Jul 22, 2025

Uh oh!

ZhenjaHorbach commented Jul 23, 2025

Reviewer Checklist

Screenshots/Videos

Uh oh!

ZhenjaHorbach commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZhenjaHorbach Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZhenjaHorbach commented Jul 23, 2025

Uh oh!

ZhenjaHorbach commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmusial commented Jul 25, 2025

Uh oh!

jmusial commented Jul 25, 2025

Uh oh!

ZhenjaHorbach commented Jul 25, 2025

Uh oh!

shawnborton commented Jul 25, 2025

Uh oh!

ZhenjaHorbach commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shawnborton commented Jul 25, 2025

Uh oh!

jmusial commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZhenjaHorbach commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmusial commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shawnborton commented Jul 30, 2025

Uh oh!

jmusial commented Jul 31, 2025

Uh oh!

ZhenjaHorbach commented Aug 1, 2025

Uh oh!

mountiny commented Aug 1, 2025

Uh oh!

mountiny left a comment

Choose a reason for hiding this comment

Uh oh!

mountiny commented Aug 1, 2025

Uh oh!

ZhenjaHorbach commented Aug 1, 2025

Uh oh!

Uh oh!

mountiny commented Aug 1, 2025

Uh oh!

ZhenjaHorbach commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OSBotify commented Aug 1, 2025

Uh oh!

jmusial commented Aug 4, 2025

jmusial commented Jul 14, 2025 •

edited

Loading

jmusial commented Jul 22, 2025 •

edited

Loading

ZhenjaHorbach commented Jul 23, 2025 •

edited

Loading

ZhenjaHorbach Jul 23, 2025 •

edited

Loading

ZhenjaHorbach commented Jul 23, 2025 •

edited

Loading

ZhenjaHorbach commented Jul 25, 2025 •

edited

Loading

jmusial commented Jul 28, 2025 •

edited

Loading

ZhenjaHorbach commented Jul 28, 2025 •

edited

Loading

jmusial commented Jul 30, 2025 •

edited

Loading

ZhenjaHorbach commented Aug 1, 2025 •

edited

Loading

hungvu193 commented Aug 4, 2025 •

edited

Loading