diff --git a/src/components/Dialog/index.tsx b/src/components/Dialog/index.tsx index 1249897b2b..55798db7f5 100644 --- a/src/components/Dialog/index.tsx +++ b/src/components/Dialog/index.tsx @@ -83,7 +83,7 @@ export function Outer({ const sheetOptions = nativeOptions?.sheet || {} const hasSnapPoints = !!sheetOptions.snapPoints const insets = useSafeAreaInsets() - const closeCallback = React.useRef<() => void>() + const closeCallbacks = React.useRef<(() => void)[]>([]) const {setDialogIsOpen} = useDialogStateControlContext() /* @@ -96,28 +96,51 @@ export function Outer({ */ const isOpen = openIndex > -1 + const callQueuedCallbacks = React.useCallback(() => { + for (const cb of closeCallbacks.current) { + try { + cb() + } catch (e: any) { + logger.error('Error running close callback', e) + } + } + + closeCallbacks.current = [] + }, []) + const open = React.useCallback( ({index} = {}) => { + // Run any leftover callbacks that might have been queued up before calling `.open()` + callQueuedCallbacks() + setDialogIsOpen(control.id, true) // can be set to any index of `snapPoints`, but `0` is the first i.e. "open" setOpenIndex(index || 0) + sheet.current?.snapToIndex(index || 0) }, - [setOpenIndex, setDialogIsOpen, control.id], + [setDialogIsOpen, control.id, callQueuedCallbacks], ) + // This is the function that we call when we want to dismiss the dialog. const close = React.useCallback(cb => { - if (cb && typeof cb === 'function') { - if (closeCallback.current) { - logger.error( - `Dialog close was passed multiple callbacks, you shouldn't do that`, - ) - } - closeCallback.current = cb + if (typeof cb === 'function') { + closeCallbacks.current.push(cb) } - // initiates a close animation, the actual "close" happens in `onCloseInner` sheet.current?.close() }, []) + // This is the actual thing we are doing once we "confirm" the dialog. We want the dialog's close animation to + // happen before we run this. It is passed to the `BottomSheet` component. + const onCloseAnimationComplete = React.useCallback(() => { + // This removes the dialog from our list of stored dialogs. Not super necessary on iOS, but on Android this + // tells us that we need to toggle the accessibility overlay setting + setDialogIsOpen(control.id, false) + setOpenIndex(-1) + + callQueuedCallbacks() + onClose?.() + }, [callQueuedCallbacks, control.id, onClose, setDialogIsOpen]) + useImperativeHandle( control.ref, () => ({ @@ -127,22 +150,6 @@ export function Outer({ [open, close], ) - const onCloseInner = React.useCallback(() => { - setDialogIsOpen(control.id, false) - setOpenIndex(-1) - try { - logger.debug(`Dialog closeCallback`, {controlId: control.id}) - closeCallback.current?.() - } catch (e: any) { - logger.error(`Dialog closeCallback failed`, { - message: e.message, - }) - } finally { - closeCallback.current = undefined - } - onClose?.() - }, [control.id, onClose, setDialogIsOpen]) - const context = React.useMemo(() => ({close}), [close]) return ( @@ -170,7 +177,7 @@ export function Outer({ backdropComponent={Backdrop} handleIndicatorStyle={{backgroundColor: t.palette.primary_500}} handleStyle={{display: 'none'}} - onClose={onCloseInner}> + onClose={onCloseAnimationComplete}> { - setIsOpen(true) setDialogIsOpen(control.id, true) + setIsOpen(true) }, [setIsOpen, setDialogIsOpen, control.id]) const close = React.useCallback( cb => { setDialogIsOpen(control.id, false) - setIsVisible(false) setIsOpen(false) - setIsVisible(true) try { if (cb && typeof cb === 'function') { - cb() + // This timeout ensures that the callback runs at the same time as it would on native. I.e. + // console.log('Step 1') -> close(() => console.log('Step 3')) -> console.log('Step 2') + // This should always output 'Step 1', 'Step 2', 'Step 3', but without the timeout it would output + // 'Step 1', 'Step 3', 'Step 2'. + setTimeout(cb) } } catch (e: any) { logger.error(`Dialog closeCallback failed`, { @@ -113,17 +114,15 @@ export function Outer({ gtMobile ? a.p_lg : a.p_md, {overflowY: 'auto'}, ]}> - {isVisible && ( - - )} + - {isVisible ? children : null} + {children} diff --git a/src/view/screens/Storybook/Dialogs.tsx b/src/view/screens/Storybook/Dialogs.tsx index 4722784cae..f68f9f4ddf 100644 --- a/src/view/screens/Storybook/Dialogs.tsx +++ b/src/view/screens/Storybook/Dialogs.tsx @@ -6,12 +6,13 @@ import {atoms as a} from '#/alf' import {Button, ButtonText} from '#/components/Button' import * as Dialog from '#/components/Dialog' import * as Prompt from '#/components/Prompt' -import {H3, P} from '#/components/Typography' +import {H3, P, Text} from '#/components/Typography' export function Dialogs() { const scrollable = Dialog.useDialogControl() const basic = Dialog.useDialogControl() const prompt = Prompt.usePromptControl() + const testDialog = Dialog.useDialogControl() const {closeAllDialogs} = useDialogStateControlContext() return ( @@ -60,6 +61,15 @@ export function Dialogs() { Open prompt + + This is a prompt @@ -122,6 +132,131 @@ export function Dialogs() { + + + + + + + + Watch the console logs to test each of these dialog edge cases. + Functionality should be consistent across both native and web. If + not then *sad face* something is wrong. + + + + + + + + + + + + + + + + ) }