Bottom sheets #1335
Bottom sheets #1335
Conversation
|
| @@ -108,7 +108,6 @@ const ScrollScreen = ({ | |||
| alwaysBounceVertical={true} | |||
| onScroll={handleScroll} | |||
| onScrollEndDrag={scrollEndHandler} | |||
| style={{ overflow: SCREEN_OVERFLOW }} | |||
There was a problem hiding this comment.
This comes from a cherry picked commit from one of the expo 52 PRs. This branch is not properly rebased on top of the other 6 branches. I wanted to fix something that I had already fixed.
apps/mobile-wallet/src/contexts/walletConnect/WalletConnectSessionProposalModal.tsx
Show resolved
Hide resolved
apps/mobile-wallet/src/contexts/walletConnect/WalletConnectSessionRequestModal.tsx
Show resolved
Hide resolved
apps/mobile-wallet/src/features/addressesManagement/AddressSettingsModal.tsx
Show resolved
Hide resolved
| const handleDismiss = useCallback(() => { | ||
| dispatch(closeModal({ id: props.modalId })) | ||
| dispatch(removeModal({ id: props.modalId })) | ||
| }, [dispatch, props.modalId]) |
There was a problem hiding this comment.
When the sheet gets dismissed (either by clicking the backdrop, or pulling down, or programmatically) we can completely remove the modal from our Redux state directly since we do not handle the animations ourselves anymore.
There was a problem hiding this comment.
That's so good! But it means we could probably simplify our Redux actions by only keeping closeModal, no?
There was a problem hiding this comment.
Yes, but we still have 1 modal that is complicated to migrate, so I'd keep it for now and focus on other more pressing matters
| @@ -103,7 +103,7 @@ export type OpenModalParams = { | |||
| }[ModalName] | |||
|
|
|||
| export type ModalInstance = { | |||
| id: number | |||
| id: string | |||
There was a problem hiding this comment.
I changed it to string so that I can use it with the sheets lib
|
Need to re-add gaps between sections in some modals |
|
Horizontal padding is too large |
@mvaivre fixed |
| @@ -28,13 +28,14 @@ const AddressSettingsModal = withModal<AddressSettingsModalProps>(({ id, address | |||
| const persistAddressSettings = usePersistAddressSettings() | |||
| const { t } = useTranslation() | |||
| const canDeleteAddress = useCanDeleteAddress(addressHash) | |||
| const { dismiss } = useBottomSheetModal() | |||
There was a problem hiding this comment.
Nit: I'm tempted to rename those dismissModal for clarity. But because we are in a Modal component, I'm also ok with keeping this name.
apps/mobile-wallet/src/features/addressesManagement/AddressTokensListFooter.tsx
Show resolved
Hide resolved
| const handleDismiss = useCallback(() => { | ||
| dispatch(closeModal({ id: props.modalId })) | ||
| dispatch(removeModal({ id: props.modalId })) | ||
| }, [dispatch, props.modalId]) |
There was a problem hiding this comment.
That's so good! But it means we could probably simplify our Redux actions by only keeping closeModal, no?
✅ Migrated 39/40 modals
Further work:
stackBehavior)RegionsModal.tsx(tricky to migrate)stackingBehaviortoopenModalRedux actiondispatch(closeModaland replace withdismisswhere appropriateactivateAppLoading/deactivateAppLoading