Replace modal screens with modals from @react-navigation
#53493
Comments
chrispader
added
Bug
|
Triggered auto assignment to @Christinadobrzyn ( |
chrispader
changed the title
@react-navigation@react-navigation
mountiny
changed the title
@react-navigation@react-navigation
|
@chrispader we will have to discuss this and make a proposal first |
|
@chrispader, @mountiny, @Christinadobrzyn Eep! 4 days overdue now. Issues have feelings too... |
|
Hey @mountiny or @chrispader - should this remain a daily issue? Are we holding it for this PR - #53362 |
|
No, we are holding this for a proposal/ discussion around it |
|
Made chris the owner of the issue |
|
Just a heads up that I'm going to be ooo Dec 12 - 13th. Back on Monday 16th. I'm not going to assign this to a BZ teammate but if anything is urgent, please reach out to the team for a volunteer. |
|
I've just posted a P/S thread for this here: https://expensify.slack.com/archives/C01GTK53T8Q/p1733938583809829 |
|
Nice work on the P/S @chrispader - looks like it has lots of votes to move forward. Let us know what you're thinking for next steps! |
|
@chrispader, @mountiny, @Christinadobrzyn Huh... This is 4 days overdue. Who can take care of this? |
mountiny
changed the title
@react-navigation@react-navigation
|
I think w can take this off hold, adding this to quality as low as I think it aims to improve UX. @chrispader can you please start with a plan on how to tackle this before writing any code. Also would be great to get agreement from @adamgrzybowski @WoLewicki on the next steps and maybe even work together with them as this is change close to screens and navigation |
|
I'd also add @BartoszGrajdek who is reworking some of the modals to work with reanimated. |
|
I love the plan and approach with many smaller PRs for migrating. Great job analyzing this issue! |
|
Sounds reasonable to me! |
|
@chrispader, @parasharrajat, @mountiny, @Christinadobrzyn Whoops! This issue is 2 days overdue. Let's get this updated quick! |
|
@chrispader, @parasharrajat, @mountiny, @Christinadobrzyn 6 days overdue. This is scarier than being forced to listen to Vogon poetry! |
|
@chrispader What are your next steps? |
I'm going to start working on a holistic change for the |
|
@chrispader, @parasharrajat, @mountiny, @Christinadobrzyn Eep! 4 days overdue now. Issues have feelings too... |
|
@chrispader Please tag me to the PR for reviews. Thanks. Also, do you have any update on this? |
|
@chrispader, @parasharrajat, @mountiny, @Christinadobrzyn Still overdue 6 days?! Let's take care of this! |
|
No update yet. Going to continue working on this PR this week (especially a fix for the |
|
@chrispader Could you confirm if your work will fix #55537? |
|
@chrispader, @parasharrajat, @mountiny, @Christinadobrzyn Huh... This is 4 days overdue. Who can take care of this? |
|
@chrispader, @parasharrajat, @mountiny, @Christinadobrzyn Still overdue 6 days?! Let's take care of this! |
|
@chrispader Can you please leave an update with the current progress and next steps? thanks! |
|
@mountiny @parasharrajat sorry for the delay on this one. This is what i did so far:
|
|
@chrispader, @parasharrajat, @mountiny, @Christinadobrzyn Whoops! This issue is 2 days overdue. Let's get this updated quick! |
|
Thanks! |
melvin-bot
bot
added
Reviewing
P/S Slack thread: expensify.slack.com/archives/C01GTK53T8Q/p1733938583809829
Problem
As discussed in this issue and this Slack thread we should replace modal screens (navigation screens/routes, that only render a modal) and modals that stay open when the containing screen gets unmounted.
The current modals use
react-native-modalwhich uses RN internal Modal under the hood. Using these modals with@react-navigation/native-stackposes issues with animations and navigation, as these modals cannot animate in/out, due to the screen mounting/unmounting at the same time. Additionally, the animations will not look the same as the rest of the screen animations, which doesn't look very nice.Solution
Replace the following types of modals with modal (screens) from
@react-navigationcc @mountiny
List of components and screens to migrate to
@react-navigationmodal screensTracking my progress on an exhaustive list of components and screens that can/should be migrated to modal screens.
The list of components/screens (and other files) are triaged/prioritized as follows:
(File names are listed without file ending, usually
.tsx)🔴 HIGH: Should definitely be migrated, due to current issues with animations and navigation.
These modals are already causing issues with animations, because
causing no or cut off animations.
🟡 MEDIUM: Not causing any issues at the moment, but still worth to migrate to modal screens since the behave "like a screen", to achieve consistent animations and generally improve navigation. (gestures, native feel)
This includes modals that are either
animation: none,🟢 LOW: These screens are animated in from bottom-to-top like actual modals and are never (un-)mounted while at the same time as the screen. Migrating these modals to modal screens (and extra navigation routes) would make animations more consistent with the other screens, but it is not necessary and might not be worth the effort.
Using a custom modal implementation or something like
gorhom/react-native-bottom-sheetprobably makes more sense.Base components to (potentially) re-write
BaseModal
TransactionStartDateSelectorModalPaymentCardCurrencyModalAmountSelectorModalAttachmentModalAvatarCropModalCategorySelectorModalConfirmModalCountrySelectorModalYearPickerModalDecisionModalFeatureTrainingModalPushRowModalRequireTwoFactorAuthenticationModalSearchRouterModalStateSelectorModalTestToolsModalTextSelectorModalValueSelectorModalBusinessTypeSelectorModalNetSuiteCustomListSelectorModalExpenseLimitTypeSelectorModalTransactionStartDateSelectorModalUnitSelectorModalWorkspaceMemberDetailsRoleSelectionModalInitialListValueSelectorModalTypeSelectorModalEditReportFieldDateSetDatePageDateOfBirthPageSearchSelectedNarrowSearchDateFilterBaseDateOfBirthStepDebugDetailsDateTimePickerPageAdditionalDetailsStepIncorporationDateBusinessIOURequestStepDatePopover (uses Modal)
Modalunder the hoodEmojiPickerPopoverMenuPopoverWithMeasuredContentPopoverReportActionContextMenuBasePopoverReactionListProcessMoneyReportHoldMenuWorkspaceCardsListLabelAccountSwitcherAddPaymentMethodMenuAvatarWithImagePickerAttachmentPickerWithMenuTriaging
Modals only visible on native platforms (Android & iOS) are marked with 📱.
🔴 HIGH: Modals causing issues at the moment
Modals animating in from the bottom are marked with ⬆️, modals animating in from the right with ⬅️
AttachmentModal ⬅️ (screen recordings attached)
Comment: Add one or many extra routes?
Components:
AvatarWithImagePickerReportAvatarTransactionReceiptPageReportActionComposeReportAttachmentsProfileAvatarWorkspaceAvatariOS: Animating in not working, because the modal is animating at the same time, as a new screen with
animation: noneis mounting.Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-01-05.at.14.21.11.mp4
Android: Animation out is not working, probably due to the screen unmounting before the modal can animate out
Screen.Recording.2025-01-05.at.14.20.52.mov
ConfirmModal ⬆️ (screen recordings attached)
Comment: Not all
ConfirmModals are causing issuesScreens, where we navigate back while also animating the modal out:
TagSettingsPage(Remove tag)WorkspaceEditTaxPage(Remove tax)CategorySettingsPage(Remove category)PolicyDistanceRateDetailsPage(Remove distance rate)iOS: Modal not able to animate out, because the screen that wraps the modal is already unmounting:
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-01-05.at.14.24.29.mp4
Android: Same issue, though less obvious:
Screen.Recording.2025-01-05.at.14.24.47.mov
🟡 MEDIUM: Screen-like modals - Animating in from right-to-left
SearchRouterModal
AuthScreensAmountSelectorModal -> AmountPicker
FormWorkspaceCreateTaxPageAvatarCropModal -> AvatarWithImagePicker
NewChatConfirmPageReportDetailsPageProfilePageWorkspaceProfilePageCategorySelectorModal
WorkspaceCategoriesSettingsPageCategorySelectorPolicyDistanceRatesSettingsPageWorkspacePerDiemSettingsPage(unused?)YearPickerModal -> CalendarPicker -> DatePicker
SearchDateFilterBaseDateOfBirthStepDebugDetailsDateTimePickerPageEditReportFieldDateAdditionalDetailsStepIncorporationDateBusinessIOURequestStepDateSetDatePageDateOfBirthPageTransactionStartDateSelectorModalTextSelectorModal -> TextPicker
CreateReportFieldsPageWorkspaceCreateTaxPageFormValidateCodeActionModal
MissingPersonalDetailsContent(multiple occurrences)BankAccountStepContactMethodDetailsPage(multiple occurrences)NewContactMethodPageDelegateMagicCodeModalCodesStepBaseGetPhysicalCardExpensifyCardPageReportCardLostPageReportVirtualCardFraudPageVerifyAccountPageConfirmationStepValueSelectorModal -> ValuePicker
WorkspaceNewRoomPageForm(multiple occurrences)FeatureTrainingModal
TrackTrainingPageExplanationModalMigratedUserWelcomeModalOnboardingWelcomeVideoPaymentCardCurrencyModal
ConnectToQuickbooksOnlineFlow 📱
ConnectToXeroFlow 📱
CardAuthenticationModal
PaymentCardWorkspaceOwnerChangeWrapperPageNetSuiteCustomListSelectorModal -> NetSuiteCustomListPicker
ChooseCustomListStepExpenseLimitTypeSelectorModal -> ExpenseLimitTypeSelector
CategoryFlagAmountsOverPageTransactionStartDateSelectorModal
TransactionStartDateStepUnitSelectorModal -> UnitSelector
PolicyDistanceRatesSettingsPageWorkspaceMemberDetailsRoleSelectionModal
WorkspaceMemberDetailsPage(multiple occurrences)InitialListValueSelectorModal -> InitialListValueSelector
CreateReportFieldsPageTypeSelectorModal -> TypeSelector
CreateReportFieldsPage🟢 LOW: Contained modals - Only visible as part of the current screen, animating in from bottom-to-top
ConfirmModal
EditReportFieldPageReportDetailsPageReportParticipantDetailsPageReportParticipantsPageRoomMemberDetailsPageRoomMembersPageEmptySearchViewConsolePageInitialSettingsPageContactMethodDetailsPageVisibilityPageCloseAccountPageSecuritySettingsPageTroubleshootPageWalletPageWorkspaceInitialPageWorkspaceMembersPageWorkspaceMoreFeaturesPageWorkspaceProfilePageWorkspacesListPagePolicyAccountingPageSageIntacctEditUserDimensionsPageNetSuiteImportCustomFieldViewImportedCategoriesPageWorkspaceCategoriesPageWorkspaceCompanyCardDetailsPageWorkspaceCompanyCardsSettingsPagePolicyDistanceRatesPageWorkspaceEditCardLimitPageWorkspaceEditCardLimitTypePageWorkspaceExpensifyCardDetailsPageWorkspaceExpensifyCardPageEmptyStateWorkspaceInvoiceVBASectionImportedMembersPageWorkspaceMemberDetailsPageImportedPerDiemPageWorkspacePerDiemDetailsPageWorkspacePerDiemPageReportFieldsListValuesPageReportFieldsSettingsPageReportFieldsValueSettingsPageWorkspaceReportFieldsPageImportedTagsPageWorkspaceTagsPageWorkspaceViewTagsPageWorkspaceTaxesPageWorkspaceWorkflowsPageWorkspaceWorkflowsApprovalsEditPageExpensifyAccountSwitcherAccountingConnectionConfirmationModalAttachmentModalConfirmModalDelegateNoAccessModalFocusModeNotificationImportSpreadsheetLocationPermissionModalMoneyReportHeaderExportWithDropdownMenuSearchPageHeaderSupportActionRestrictedModalBaseUpdateAppModalHeaderViewPopoverReportActionContextMenuReportDetailsExportPageFloatingActionButtonAndPopoverIOURequestStepScanIOURequestStepWaypointCardSectionWorkspaceResetBankAccountModaluseDeleteSavedSearchDecisionModal
SearchPageHeaderReportDetailsPage(multiple occurrences)WorkspaceMembersPage(multiple occurrences)WorkspaceCategoriesPage(multiple occurrences)WorkspaceTagsPageBaseImportOnyxStateProcessMoneyReportHoldMenuSelectionListModal -> SelectionListWithModal
SearchReportParticipantsPageRoomMembersPageWorkspaceMembersPageWorkspaceCategoriesPagePolicyDistanceRatesPageWorkspacePerDiemPageReportFieldsListValuesPageWorkspaceReportFieldsPageWorkspaceTagsPageWorkspaceViewTagsPageWorkspaceTaxesPageRequireTwoFactorAuthenticationModal
Expensify(multiple occurrences)TestToolsModal
AuthScreensSearchSelectedNarrow -> SearchPageHeader
SearchPageSearchSelectionModaHeader❓ Unknown: Unable to test because the modal is either missing or not accessible
CountrySelectorModal -> CountryPicker -> Address
MissingPersonalDetailsContent->MissingPersonalDetails(unused?)StateSelectorModal -> StatePicker -> Address
MissingPersonalDetailsContent->MissingPersonalDetails(unused?)PushRowModal -> PushRowWithModal (unused?)
ReimburseAccountPageAddressFormFieldsBusinessTypeIncorporationLocationPaymentVolumeConfirmationBusinessTypeSelectorModal -> BusinessTypePicker
ReimburseAccountPageBusinessInfoIssue Owner
Current Issue Owner: @chrispaderThe text was updated successfully, but these errors were encountered: