Migrate Quickbooks and Xero flow modals to use Reanimated modal#65172
Conversation
…quickbooks-modals
…quickbooks-modals
…quickbooks-modals
src/components/Modal/BaseModal.tsx
Outdated
| if (type && PARTIAL_REANIMATED_MODAL_TYPES.includes(type) && shouldUseReanimatedModal) { | ||
| return ( | ||
| <ReanimatedModal | ||
| // eslint-disable-next-line react/jsx-props-no-spreading | ||
| {...(props as ReanimatedModalProps)} | ||
| type={type} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
NAB: What if we remove PARTIAL_REANIMATED_MODAL_TYPES and just rely on shouldUseReanimatedModal?
This way we would end up with just one if:
if ((type && REANIMATED_MODAL_TYPES.includes(type)) || shouldUseReanimatedModal)| case 'fadeIn': | ||
| default: | ||
| return { | ||
| from: {opacity: 0}, | ||
| to: { | ||
| opacity: 0.72, | ||
| easing, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
NAB: Split 'fadeIn' and default to make the logic clearer and better catch unintended animation types
| case 'fadeIn': | |
| default: | |
| return { | |
| from: {opacity: 0}, | |
| to: { | |
| opacity: 0.72, | |
| easing, | |
| }, | |
| }; | |
| case 'fadeIn': | |
| return { | |
| from: {opacity: 0}, | |
| to: { | |
| opacity: 0.72, | |
| easing, | |
| }, | |
| }; | |
| default: | |
| throw ... | |
| }, | ||
| }; | ||
| case 'fadeOut': | ||
| default: |
There was a problem hiding this comment.
Oh this is so much cleaner, good job!
|
@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] |
037b3c5 to
a952677
Compare
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.movAndroid: mWeb Chromeandroid-web.moviOS: HybridAppios-1.movios-2.moviOS: mWeb Safariios-web.movMacOS: Chrome / Safari2025-07-02.20.58.50.movMacOS: Desktop2025-07-02.20.58.50.mov |
|
@ZhenjaHorbach changed |
Thanks ! Weird @mountiny |
…quickbooks-modals
|
@ZhenjaHorbach merged latest main. I think that should fix the "stuck on splash screens" issue |
|
Interesting issue after finishing onboarding 2025-07-03.16.23.57.mov |
|
Weird, I cannot reproduce it :( What IOS version is that ? |
17.5 😅 |
|
Hmm... I see, are you able to get some repro steps or it was a one time glitch ? |
|
@ZhenjaHorbach I managed to get that behaviour, but on the |
Oh But in that case it's not our issue ! |
|
I think I found repro steps. It happens for me on the first account I set up after opening a new emulator, the consecutive accounts are working fine. Gonna check on real device. Screen.Recording.2025-07-04.at.10.27.26.mov |
|
I can reproduce it on Edit: https://swmansion.slack.com/archives/C049HHMV9SM/p1751623695654159 |
|
@ZhenjaHorbach would be awesome if you could check the PR today, we have 3 more lined up depending on this one 😄 |
|
I will complete soon ! |
|
@jmusial 2025-07-04.13.04.39.mov |
|
But actually when I press android system back button |
|
Soo.... I think it's also on main Can you check if you can get in on Screen.Recording.2025-07-04.at.15.56.57.mov |
|
Yes |
|
Prod works fine. |
Yeah And good point |
|
In this case |
|
@ZhenjaHorbach I can post it later, unless you want it :) |
I already did it 😅 And good job ! |
mountiny
left a comment
There was a problem hiding this comment.
Nice great job on the PR and testing!
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.1.77-1 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.1.77-2 🚀
|

Explanation of Change
This PR uses
react-native-reanimatedbased modal forCENTERED_UNSWIPABLEtype modals.Animations used
slideInRight/slideOutRightfor small screensfadeIn/fadeOutfor large screensFixed Issues
$ #64776
$ #64775
Depends on
#64805
PROPOSAL:
https://expensify.slack.com/archives/C05LX9D6E07/p1750147058498549
Tests
Feature Training Modal
Only on large screen devices. On small screens Feature Training Modal uses Bottom Docked, which has already beed migrated.
Xero and QuickBooks Online Modals
Only on native devices. On desktop and web integrations open in a web browser card.
AccountingenabledWorkspaces->Your workspace->AccountingConnecton QuickBooks Online row -> Modal showsConnecton Xero row -> Modal showsOffline tests
Same as tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Xero & Quickbooks Online
0024.xero.quicbooks.android.native.mov
Feature Training
N/A (is another modal type)
Android: mWeb Chrome
Xero & Quickbooks Online
N/A (opens link in another browser tab)
Feature Training
N/A (is another modal type)
iOS: Native
Xero & Quickbooks Online
0024.xero.quickbooks.native.ios.mp4
Feature Training
N/A (is another modal type)
iOS: mWeb Safari
Xero & Quickbooks Online
N/A (opens link in another browser tab)
Feature Training
N/A (is another modal type)
MacOS: Chrome / Safari
Xero & Quickbooks Online
N/A (opens link in another browser tab)
Feature Training
0024.web.chrome.feature.training.mov
MacOS: Desktop
Xero & Quickbooks Online
N/A (opens link in another browser tab)
Feature Training
0024.desktop.native.feature.mov