From 140e4d494192043d8fef079e663a2b3743746ac0 Mon Sep 17 00:00:00 2001 From: adids1221 Date: Wed, 26 Feb 2025 17:16:54 +0200 Subject: [PATCH 1/6] implement dismiss state management for closeExpandable control --- .../picker/__tests__/index.spec.tsx | 2 +- src/incubator/expandableOverlay/index.tsx | 23 +++++++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/components/picker/__tests__/index.spec.tsx b/src/components/picker/__tests__/index.spec.tsx index 90cd39eeae..c8c6766ef5 100644 --- a/src/components/picker/__tests__/index.spec.tsx +++ b/src/components/picker/__tests__/index.spec.tsx @@ -114,7 +114,7 @@ describe('Picker', () => { expect(driver.isOpen()).toBeTruthy(); driver.cancel(); expect(driver.isOpen()).toBeFalsy(); - expect(onDismiss).toHaveBeenCalledTimes(2); // TODO: this should be 1 + expect(onDismiss).toHaveBeenCalledTimes(1); }); // TODO: this test is not passing yet diff --git a/src/incubator/expandableOverlay/index.tsx b/src/incubator/expandableOverlay/index.tsx index 8eaf0fe49b..874f99842d 100644 --- a/src/incubator/expandableOverlay/index.tsx +++ b/src/incubator/expandableOverlay/index.tsx @@ -7,6 +7,12 @@ import DialogOld from '../../components/dialog'; import DialogNew, {DialogMigrationProps} from '../dialog'; import {Colors} from 'style'; +enum DismissState { + IDLE = 'idle', + DISMISSING = 'dismissing', + DISMISSED = 'dismissed' +} + export interface ExpandableOverlayMethods { openExpandable: () => void; closeExpandable: () => void; @@ -69,6 +75,7 @@ const ExpandableOverlay = (props: ExpandableOverlayProps, ref: any) => { } = props; const [visible, setExpandableVisible] = useState(false); const containerRef = useRef(null); + const [dismissState, setDismissState] = useState(DismissState.IDLE); const focusAccessibility = useCallback(() => { const reactTag = findNodeHandle(containerRef.current); @@ -79,14 +86,22 @@ const ExpandableOverlay = (props: ExpandableOverlayProps, ref: any) => { const openExpandable = useCallback(() => { setExpandableVisible(true); + setDismissState(DismissState.IDLE); onPress?.(props); }, [onPress, customValue]); const closeExpandable = useCallback(() => { - setExpandableVisible(false); - focusAccessibility(); - useDialog ? dialogProps?.onDismiss?.() : modalProps?.onDismiss?.(); - }, [useDialog, dialogProps?.onDismiss, modalProps?.onDismiss, focusAccessibility]); + if (dismissState === DismissState.IDLE) { + setDismissState(DismissState.DISMISSING); + setExpandableVisible(false); + focusAccessibility(); + useDialog ? dialogProps?.onDismiss?.() : modalProps?.onDismiss?.(); + setDismissState(DismissState.DISMISSED); + } else if (dismissState === DismissState.DISMISSING) { + setExpandableVisible(false); + focusAccessibility(); + } + }, [dismissState, useDialog, dialogProps?.onDismiss, modalProps?.onDismiss, focusAccessibility]); const toggleExpandable = useCallback(() => (visible ? closeExpandable() : openExpandable()), [visible, openExpandable, closeExpandable]); From 1f2b3a7dd58a44aaacd4d8a8ef7d39f7dae24fd2 Mon Sep 17 00:00:00 2001 From: adids1221 Date: Thu, 20 Mar 2025 17:17:15 +0200 Subject: [PATCH 2/6] add imperativeCloseExpandable for better dismiss handling --- src/incubator/expandableOverlay/index.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/incubator/expandableOverlay/index.tsx b/src/incubator/expandableOverlay/index.tsx index 874f99842d..6b071f9b53 100644 --- a/src/incubator/expandableOverlay/index.tsx +++ b/src/incubator/expandableOverlay/index.tsx @@ -103,6 +103,10 @@ const ExpandableOverlay = (props: ExpandableOverlayProps, ref: any) => { } }, [dismissState, useDialog, dialogProps?.onDismiss, modalProps?.onDismiss, focusAccessibility]); + const imperativeCloseExpandable = useCallback(() => { + setExpandableVisible(false); + }, []); + const toggleExpandable = useCallback(() => (visible ? closeExpandable() : openExpandable()), [visible, openExpandable, closeExpandable]); @@ -119,7 +123,7 @@ const ExpandableOverlay = (props: ExpandableOverlayProps, ref: any) => { overlayBackgroundColor={Colors.$backgroundDefault} {...modalProps} visible={visible} - onDismiss={closeExpandable} + onDismiss={imperativeCloseExpandable} onRequestClose={closeExpandable} onBackgroundPress={closeExpandable} > From c637aa6dfce925aceab8d3b2d902528f878c3bb1 Mon Sep 17 00:00:00 2001 From: adids1221 Date: Sun, 23 Mar 2025 20:17:09 +0200 Subject: [PATCH 3/6] remove dismiss state management old solution --- src/incubator/expandableOverlay/index.tsx | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/src/incubator/expandableOverlay/index.tsx b/src/incubator/expandableOverlay/index.tsx index 6b071f9b53..6bb1a0ca2e 100644 --- a/src/incubator/expandableOverlay/index.tsx +++ b/src/incubator/expandableOverlay/index.tsx @@ -7,12 +7,6 @@ import DialogOld from '../../components/dialog'; import DialogNew, {DialogMigrationProps} from '../dialog'; import {Colors} from 'style'; -enum DismissState { - IDLE = 'idle', - DISMISSING = 'dismissing', - DISMISSED = 'dismissed' -} - export interface ExpandableOverlayMethods { openExpandable: () => void; closeExpandable: () => void; @@ -75,7 +69,6 @@ const ExpandableOverlay = (props: ExpandableOverlayProps, ref: any) => { } = props; const [visible, setExpandableVisible] = useState(false); const containerRef = useRef(null); - const [dismissState, setDismissState] = useState(DismissState.IDLE); const focusAccessibility = useCallback(() => { const reactTag = findNodeHandle(containerRef.current); @@ -86,22 +79,14 @@ const ExpandableOverlay = (props: ExpandableOverlayProps, ref: any) => { const openExpandable = useCallback(() => { setExpandableVisible(true); - setDismissState(DismissState.IDLE); onPress?.(props); }, [onPress, customValue]); const closeExpandable = useCallback(() => { - if (dismissState === DismissState.IDLE) { - setDismissState(DismissState.DISMISSING); - setExpandableVisible(false); - focusAccessibility(); - useDialog ? dialogProps?.onDismiss?.() : modalProps?.onDismiss?.(); - setDismissState(DismissState.DISMISSED); - } else if (dismissState === DismissState.DISMISSING) { - setExpandableVisible(false); - focusAccessibility(); - } - }, [dismissState, useDialog, dialogProps?.onDismiss, modalProps?.onDismiss, focusAccessibility]); + setExpandableVisible(false); + focusAccessibility(); + useDialog ? dialogProps?.onDismiss?.() : modalProps?.onDismiss?.(); + }, [useDialog, dialogProps?.onDismiss, modalProps?.onDismiss, focusAccessibility]); const imperativeCloseExpandable = useCallback(() => { setExpandableVisible(false); From faeeec06d5efe15e5f9bc0ac8281d3318d0e3c07 Mon Sep 17 00:00:00 2001 From: adids1221 Date: Thu, 8 May 2025 12:31:27 +0300 Subject: [PATCH 4/6] Rename imperativeCloseExpandable to dismissModal for clarity --- src/incubator/expandableOverlay/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/incubator/expandableOverlay/index.tsx b/src/incubator/expandableOverlay/index.tsx index d4c5773dbf..2d09a8ecc6 100644 --- a/src/incubator/expandableOverlay/index.tsx +++ b/src/incubator/expandableOverlay/index.tsx @@ -88,7 +88,7 @@ const ExpandableOverlay = (props: ExpandableOverlayProps, ref: any) => { useDialog ? dialogProps?.onDismiss?.() : modalProps?.onDismiss?.(); }, [useDialog, dialogProps?.onDismiss, modalProps?.onDismiss, focusAccessibility]); - const imperativeCloseExpandable = useCallback(() => { + const dismissModal = useCallback(() => { setExpandableVisible(false); }, []); @@ -108,7 +108,7 @@ const ExpandableOverlay = (props: ExpandableOverlayProps, ref: any) => { overlayBackgroundColor={Colors.$backgroundDefault} {...modalProps} visible={visible} - onDismiss={imperativeCloseExpandable} + onDismiss={dismissModal} onRequestClose={closeExpandable} onBackgroundPress={closeExpandable} > From 7b71f309eb312b98887445b4a74e67dad156f558 Mon Sep 17 00:00:00 2001 From: adids1221 Date: Thu, 8 May 2025 14:30:39 +0300 Subject: [PATCH 5/6] Update dismissModal to include focusAccessibility for improved accessibility handling --- src/incubator/expandableOverlay/index.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/incubator/expandableOverlay/index.tsx b/src/incubator/expandableOverlay/index.tsx index 2d09a8ecc6..710e24dd3c 100644 --- a/src/incubator/expandableOverlay/index.tsx +++ b/src/incubator/expandableOverlay/index.tsx @@ -90,7 +90,8 @@ const ExpandableOverlay = (props: ExpandableOverlayProps, ref: any) => { const dismissModal = useCallback(() => { setExpandableVisible(false); - }, []); + focusAccessibility(); + }, [focusAccessibility]); const toggleExpandable = useCallback(() => (visible ? closeExpandable() : openExpandable()), [visible, openExpandable, closeExpandable]); From c82b78aec8d692d51310b9dc1616b7453d8dbc46 Mon Sep 17 00:00:00 2001 From: adids1221 Date: Mon, 19 May 2025 11:27:28 +0300 Subject: [PATCH 6/6] closeExpandable using dismissModal --- src/incubator/expandableOverlay/index.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/incubator/expandableOverlay/index.tsx b/src/incubator/expandableOverlay/index.tsx index 710e24dd3c..32f9d09c7f 100644 --- a/src/incubator/expandableOverlay/index.tsx +++ b/src/incubator/expandableOverlay/index.tsx @@ -83,8 +83,7 @@ const ExpandableOverlay = (props: ExpandableOverlayProps, ref: any) => { }, [onPress, customValue]); const closeExpandable = useCallback(() => { - setExpandableVisible(false); - focusAccessibility(); + dismissModal(); useDialog ? dialogProps?.onDismiss?.() : modalProps?.onDismiss?.(); }, [useDialog, dialogProps?.onDismiss, modalProps?.onDismiss, focusAccessibility]);