From 021441f275d800099663b89f683ee07f9cccb437 Mon Sep 17 00:00:00 2001 From: Victoria Zhizhonkova Date: Mon, 20 Jan 2025 08:16:33 +0000 Subject: [PATCH 1/2] fix(ActionSheet): add stopPropagation ignore --- .../ActionSheet/ActionSheet.test.tsx | 36 +++++++++++++++++++ .../components/ActionSheet/ActionSheet.tsx | 5 ++- .../ActionSheet/ActionSheetDropdownMenu.tsx | 16 +++++++-- .../ActionSheet/ActionSheetDropdownSheet.tsx | 16 +++++++-- .../vkui/src/components/ActionSheet/types.ts | 4 +++ .../vkui/src/components/Alert/Alert.test.tsx | 28 +++++++++++++++ packages/vkui/src/components/Alert/Alert.tsx | 17 ++++++++- 7 files changed, 114 insertions(+), 8 deletions(-) diff --git a/packages/vkui/src/components/ActionSheet/ActionSheet.test.tsx b/packages/vkui/src/components/ActionSheet/ActionSheet.test.tsx index 13c6ffe7cc..0921dac104 100644 --- a/packages/vkui/src/components/ActionSheet/ActionSheet.test.tsx +++ b/packages/vkui/src/components/ActionSheet/ActionSheet.test.tsx @@ -261,4 +261,40 @@ describe(ActionSheet, () => { // desktop Android expect(screen.queryByText('Отмена')).toBeFalsy(); }); + + describe('handle allowClickPropagation correctly', () => { + it.each([ + ['menu', ActionSheetMenu], + ['sheet', ActionSheetSheet], + ])('%s', async (_name, ActionSheet) => { + const onClose = jest.fn(); + const onClick = jest.fn(); + const { rerender } = render( +
+ +
+ +
, + ); + await waitForFloatingPosition(); + act(jest.runAllTimers); + + await userEvent.click(screen.getByTestId('content')); + expect(onClick).not.toHaveBeenCalled(); + + rerender( +
+ +
+ +
, + ); + + await waitForFloatingPosition(); + act(jest.runAllTimers); + + await userEvent.click(screen.getByTestId('content')); + expect(onClick).toHaveBeenCalled(); + }); + }); }); diff --git a/packages/vkui/src/components/ActionSheet/ActionSheet.tsx b/packages/vkui/src/components/ActionSheet/ActionSheet.tsx index c1ee5e891c..1c9795adae 100644 --- a/packages/vkui/src/components/ActionSheet/ActionSheet.tsx +++ b/packages/vkui/src/components/ActionSheet/ActionSheet.tsx @@ -23,7 +23,10 @@ export interface ActionSheetOnCloseOptions { } export interface ActionSheetProps - extends Pick, + extends Pick< + SharedDropdownProps, + 'toggleRef' | 'popupOffsetDistance' | 'placement' | 'allowClickPropagation' + >, Omit, Omit, 'autoFocus' | 'title'> { title?: React.ReactNode; diff --git a/packages/vkui/src/components/ActionSheet/ActionSheetDropdownMenu.tsx b/packages/vkui/src/components/ActionSheet/ActionSheetDropdownMenu.tsx index 487265df11..4e6c64a925 100644 --- a/packages/vkui/src/components/ActionSheet/ActionSheetDropdownMenu.tsx +++ b/packages/vkui/src/components/ActionSheet/ActionSheetDropdownMenu.tsx @@ -8,6 +8,8 @@ import { useEventListener } from '../../hooks/useEventListener'; import { usePlatform } from '../../hooks/usePlatform'; import { useDOM } from '../../lib/dom'; import { isRefObject } from '../../lib/isRefObject'; +import { mergeCalls } from '../../lib/mergeCalls'; +import { stopPropagation } from '../../lib/utils'; import { warnOnce } from '../../lib/warnOnce'; import { FocusTrap } from '../FocusTrap/FocusTrap'; import { Popper } from '../Popper/Popper'; @@ -30,6 +32,8 @@ export const ActionSheetDropdownMenu = ({ placement, onAnimationStart, onAnimationEnd, + allowClickPropagation = false, + onClick, ...restProps }: SharedDropdownProps): React.ReactNode => { const { document } = useDOM(); @@ -57,8 +61,6 @@ export const ActionSheetDropdownMenu = ({ }); }, [bodyClickListener, document]); - const onClick = React.useCallback((e: React.MouseEvent) => e.stopPropagation(), []); - const targetRef = React.useMemo(() => { if (isRefObject(toggleRef)) { return toggleRef; @@ -67,6 +69,14 @@ export const ActionSheetDropdownMenu = ({ return { current: toggleRef as HTMLElement }; }, [toggleRef]); + const handleClick = (event: React.MouseEvent) => { + if (!allowClickPropagation) { + stopPropagation(event); + } + }; + + const clickHandlers = mergeCalls({ onClick: handleClick }, { onClick }); + return ( - + {children} diff --git a/packages/vkui/src/components/ActionSheet/ActionSheetDropdownSheet.tsx b/packages/vkui/src/components/ActionSheet/ActionSheetDropdownSheet.tsx index 25a1f8b807..519700fc52 100644 --- a/packages/vkui/src/components/ActionSheet/ActionSheetDropdownSheet.tsx +++ b/packages/vkui/src/components/ActionSheet/ActionSheetDropdownSheet.tsx @@ -4,12 +4,12 @@ import * as React from 'react'; import { classNames } from '@vkontakte/vkjs'; import { useAdaptivityWithJSMediaQueries } from '../../hooks/useAdaptivityWithJSMediaQueries'; import { usePlatform } from '../../hooks/usePlatform'; +import { mergeCalls } from '../../lib/mergeCalls'; +import { stopPropagation } from '../../lib/utils'; import { FocusTrap } from '../FocusTrap/FocusTrap'; import type { SharedDropdownProps } from './types'; import styles from './ActionSheet.module.css'; -const stopPropagation: React.MouseEventHandler = (e) => e.stopPropagation(); - export type ActionSheetDropdownProps = Omit< SharedDropdownProps, 'popupDirection' | 'popupOffsetDistance' | 'placement' @@ -21,15 +21,25 @@ export const ActionSheetDropdownSheet = ({ // these 2 props are only omitted - ActionSheetDesktop compat toggleRef, className, + onClick, + allowClickPropagation = false, ...restProps }: SharedDropdownProps): React.ReactNode => { const { sizeY } = useAdaptivityWithJSMediaQueries(); const platform = usePlatform(); + const handleClick = (event: React.MouseEvent) => { + if (!allowClickPropagation) { + stopPropagation(event); + } + }; + + const clickHandlers = mergeCalls({ onClick: handleClick }, { onClick }); + return ( { descriptionClassNames.forEach((className) => expect(textElement).toHaveClass(className)); }, ); + + it('handle allowClickPropagation correctly', async () => { + const onClose = jest.fn(); + const onClick = jest.fn(); + const action = { + 'title': 'Item', + 'data-testid': '__action__', + 'autoCloseDisabled': true, + 'mode': 'default' as const, + }; + const result = render( +
+ +
, + ); + + await userEvent.click(result.getByTestId('__action__')); + expect(onClick).not.toHaveBeenCalled(); + + result.rerender( +
+ +
, + ); + + await userEvent.click(result.getByTestId('__action__')); + expect(onClick).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/vkui/src/components/Alert/Alert.tsx b/packages/vkui/src/components/Alert/Alert.tsx index c4a0a3f807..bbee0724bb 100644 --- a/packages/vkui/src/components/Alert/Alert.tsx +++ b/packages/vkui/src/components/Alert/Alert.tsx @@ -7,6 +7,7 @@ import { useAdaptivityWithJSMediaQueries } from '../../hooks/useAdaptivityWithJS import { type UseFocusTrapProps } from '../../hooks/useFocusTrap'; import { usePlatform } from '../../hooks/usePlatform'; import { useCSSKeyframesAnimationController } from '../../lib/animation'; +import { mergeCalls } from '../../lib/mergeCalls'; import { stopPropagation } from '../../lib/utils'; import type { AlignType, @@ -73,6 +74,10 @@ export interface AlertProps */ dismissButtonTestId?: string; usePortal?: AppRootPortalProps['usePortal']; + /** + * По умолчанию событие onClick не всплывает + */ + allowClickPropagation?: boolean; } /** @@ -94,6 +99,8 @@ export const Alert = ({ dismissButtonTestId, getRootRef, usePortal, + onClick, + allowClickPropagation = false, ...restProps }: AlertProps): React.ReactNode => { const generatedId = React.useId(); @@ -139,6 +146,14 @@ export const Alert = ({ [close], ); + const handleClick = (event: React.MouseEvent) => { + if (!allowClickPropagation) { + stopPropagation(event); + } + }; + + const clickHandlers = mergeCalls({ onClick: handleClick }, { onClick }); + useScrollLock(); return ( @@ -153,8 +168,8 @@ export const Alert = ({ Date: Fri, 24 Jan 2025 14:08:42 +0000 Subject: [PATCH 2/2] fix review notes --- .../ActionSheet/ActionSheetDropdownMenu.tsx | 16 +++++++--------- .../ActionSheet/ActionSheetDropdownSheet.tsx | 16 +++++++--------- packages/vkui/src/components/Alert/Alert.tsx | 18 ++++++++---------- 3 files changed, 22 insertions(+), 28 deletions(-) diff --git a/packages/vkui/src/components/ActionSheet/ActionSheetDropdownMenu.tsx b/packages/vkui/src/components/ActionSheet/ActionSheetDropdownMenu.tsx index 4e6c64a925..4c09baf0a3 100644 --- a/packages/vkui/src/components/ActionSheet/ActionSheetDropdownMenu.tsx +++ b/packages/vkui/src/components/ActionSheet/ActionSheetDropdownMenu.tsx @@ -8,7 +8,6 @@ import { useEventListener } from '../../hooks/useEventListener'; import { usePlatform } from '../../hooks/usePlatform'; import { useDOM } from '../../lib/dom'; import { isRefObject } from '../../lib/isRefObject'; -import { mergeCalls } from '../../lib/mergeCalls'; import { stopPropagation } from '../../lib/utils'; import { warnOnce } from '../../lib/warnOnce'; import { FocusTrap } from '../FocusTrap/FocusTrap'; @@ -69,13 +68,12 @@ export const ActionSheetDropdownMenu = ({ return { current: toggleRef as HTMLElement }; }, [toggleRef]); - const handleClick = (event: React.MouseEvent) => { - if (!allowClickPropagation) { - stopPropagation(event); - } - }; - - const clickHandlers = mergeCalls({ onClick: handleClick }, { onClick }); + const handleClick = allowClickPropagation + ? onClick + : (event: React.MouseEvent) => { + stopPropagation(event); + onClick?.(event); + }; return ( - + {children} diff --git a/packages/vkui/src/components/ActionSheet/ActionSheetDropdownSheet.tsx b/packages/vkui/src/components/ActionSheet/ActionSheetDropdownSheet.tsx index 519700fc52..d8516ef982 100644 --- a/packages/vkui/src/components/ActionSheet/ActionSheetDropdownSheet.tsx +++ b/packages/vkui/src/components/ActionSheet/ActionSheetDropdownSheet.tsx @@ -4,7 +4,6 @@ import * as React from 'react'; import { classNames } from '@vkontakte/vkjs'; import { useAdaptivityWithJSMediaQueries } from '../../hooks/useAdaptivityWithJSMediaQueries'; import { usePlatform } from '../../hooks/usePlatform'; -import { mergeCalls } from '../../lib/mergeCalls'; import { stopPropagation } from '../../lib/utils'; import { FocusTrap } from '../FocusTrap/FocusTrap'; import type { SharedDropdownProps } from './types'; @@ -28,18 +27,17 @@ export const ActionSheetDropdownSheet = ({ const { sizeY } = useAdaptivityWithJSMediaQueries(); const platform = usePlatform(); - const handleClick = (event: React.MouseEvent) => { - if (!allowClickPropagation) { - stopPropagation(event); - } - }; - - const clickHandlers = mergeCalls({ onClick: handleClick }, { onClick }); + const handleClick = allowClickPropagation + ? onClick + : (event: React.MouseEvent) => { + stopPropagation(event); + onClick?.(event); + }; return ( ) => { - if (!allowClickPropagation) { - stopPropagation(event); - } - }; - - const clickHandlers = mergeCalls({ onClick: handleClick }, { onClick }); - useScrollLock(); + const handleClick = allowClickPropagation + ? onClick + : (event: React.MouseEvent) => { + stopPropagation(event); + onClick?.(event); + }; + return (