Skip to content

Commit

Permalink
fix(ActionSheetItem): fix navigation by arrows in selectable ActionSh…
Browse files Browse the repository at this point in the history
…eetItem (#7216)

h2. Описание

При навигации с помощью стрелочек по  `ActionSheetItems` с `selectable` флагом закрывается `ActionSheet`

h2. Изменения

- Поисследовал проблему с тригером события `click` при навигации по элементам. Нашел вариант как можно ее обойти facebook/react#7407. С помощью проверки можно отличить реальное событие клика. 
- Добавил обработку нажатия кнопки `Enter`, при котором происходит закрытие `ActionSheet`
- Добавил тесты для нового функционала
  • Loading branch information
EldarMuhamethanov authored Jul 30, 2024
1 parent 0e63369 commit 0d94b24
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 28 deletions.
13 changes: 11 additions & 2 deletions packages/vkui/src/components/ActionSheet/ActionSheet.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';
import { act } from 'react';
import { render, screen } from '@testing-library/react';
import { fireEvent, render, screen } from '@testing-library/react';
import { ViewWidth } from '../../lib/adaptivity';
import {
baselineComponent,
Expand Down Expand Up @@ -127,7 +127,16 @@ describe(ActionSheet, () => {
</ActionSheet>,
);
await waitForFloatingPosition();
await userEvent.click(screen.getByTestId('item'));
// эмулируем настоящее событие клика(отличается оно тем, что clientX и clientY != 0)
// @see packages/vkui/src/components/ActionSheetItem/helpers.ts
fireEvent(
screen.getByTestId('item'),
new MouseEvent('click', {
clientX: 1,
clientY: 1,
bubbles: true,
}),
);
act(jest.runAllTimers);

if (onCloseHandler.mock.calls.length > 0) {
Expand Down
2 changes: 1 addition & 1 deletion packages/vkui/src/components/ActionSheet/ActionSheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export const ActionSheet = ({
},
[],
);
const contextValue = useObjectMemo({ onItemClick, mode });
const contextValue = useObjectMemo({ onItemClick, mode, onClose: onCloseWithOther });

const DropdownComponent = mode === 'menu' ? ActionSheetDropdownMenu : ActionSheetDropdownSheet;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export type ItemClickHandler<T extends Element = Element> = (options: {

export type ActionSheetContextType<T extends Element = Element> = {
onItemClick?: ItemClickHandler<T>;
onClose?: () => void;
mode?: 'sheet' | 'menu';
};

Expand Down
17 changes: 11 additions & 6 deletions packages/vkui/src/components/ActionSheet/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ const onClose = () => {
setOpenedPopoutName(null);
};

const [filter, setFilter] = useState('best');
const onChange = (e) => setFilter(e.target.value);
const baseTargetRef = React.useRef(null);
const iconsTargetRef = React.useRef(null);
const subtitleTargetRef = React.useRef(null);
Expand Down Expand Up @@ -140,9 +138,13 @@ const openSubtitle = () =>
</ActionSheet>,
);

const openSelectable = () =>
openActionSheet(
'selectable',
const SelectableActionSheet = () => {
const [filter, setFilter] = useState('best');
const onChange = (e) => {
setFilter(e.target.value);
};

return (
<ActionSheet onClose={onClose} toggleRef={selectableTargetRef}>
<ActionSheetItem
onChange={onChange}
Expand Down Expand Up @@ -189,8 +191,11 @@ const openSelectable = () =>
>
Друзья по вузу
</ActionSheetItem>
</ActionSheet>,
</ActionSheet>
);
};

const openSelectable = () => openActionSheet('selectable', <SelectableActionSheet />);

const openTitle = () =>
openActionSheet(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { render, screen } from '@testing-library/react';
import * as React from 'react';
import { fireEvent, render, screen } from '@testing-library/react';
import { baselineComponent } from '../../testing/utils';
import { ActionSheetContext } from '../ActionSheet/ActionSheetContext';
import { ActionSheetItem, ActionSheetItemProps } from './ActionSheetItem';

const ActionSheetItemTest = (props: ActionSheetItemProps) => (
Expand Down Expand Up @@ -27,4 +29,66 @@ describe('ActionSheetItem', () => {
render(<ActionSheetItemTest selectable>ActionSheetItem</ActionSheetItemTest>);
expect(item().tagName.toLowerCase()).toMatch('label');
});

it('should call close callback when Enter keydown', async () => {
const onCloseCallback = jest.fn();
render(
<ActionSheetContext.Provider
value={{
onClose: onCloseCallback,
}}
>
<ActionSheetItemTest selectable data-testid="action-item">
ActionSheetItem
</ActionSheetItemTest>
</ActionSheetContext.Provider>,
);

await React.act(async () =>
fireEvent.keyDown(screen.getByTestId('action-item'), { key: 'Enter', code: 'Enter' }),
);

expect(onCloseCallback).toHaveBeenCalledTimes(1);
});

it('check call onItemClick callback when click to ActionSheetItem with selectable=true', async () => {
const onItemClickCallback = jest.fn();

render(
<ActionSheetContext.Provider
value={{
onItemClick: onItemClickCallback,
}}
>
<ActionSheetItemTest selectable data-testid="action-item">
ActionSheetItem
</ActionSheetItemTest>
</ActionSheetContext.Provider>,
);

// эмулируем событие клика при навигации стрелочками
await React.act(async () =>
fireEvent(
screen.getByTestId('action-item'),
new MouseEvent('click', {
clientX: 0,
clientY: 0,
bubbles: true,
}),
),
);

expect(onItemClickCallback).toHaveBeenCalledTimes(0);

// эмулируем настоящее событие клика(отличается оно тем, что clientX и clientY != 0)
// @see packages/vkui/src/components/ActionSheetItem/helpers.ts
const newMouseEvent = new MouseEvent('click', {
clientX: 1,
clientY: 1,
bubbles: true,
});
await React.act(async () => fireEvent(screen.getByTestId('action-item'), newMouseEvent));

expect(onItemClickCallback).toHaveBeenCalledTimes(1);
});
});
61 changes: 43 additions & 18 deletions packages/vkui/src/components/ActionSheetItem/ActionSheetItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ import * as React from 'react';
import { classNames, noop } from '@vkontakte/vkjs';
import { useAdaptivityWithJSMediaQueries } from '../../hooks/useAdaptivityWithJSMediaQueries';
import { usePlatform } from '../../hooks/usePlatform';
import { Keys, pressedKey } from '../../lib/accessibility';
import { ActionSheetContext, type ActionSheetContextType } from '../ActionSheet/ActionSheetContext';
import { Tappable } from '../Tappable/Tappable';
import { Subhead } from '../Typography/Subhead/Subhead';
import { Text } from '../Typography/Text/Text';
import { Title } from '../Typography/Title/Title';
import { isRealClickEvent } from './helpers';
import { Radio } from './subcomponents/Radio/Radio';
import styles from './ActionSheetItem.module.css';

Expand Down Expand Up @@ -74,29 +76,57 @@ export const ActionSheetItem = ({
...restProps
}: ActionSheetItemProps): React.ReactNode => {
const platform = usePlatform();
const { onItemClick = () => noop, mode: actionSheetMode } =
React.useContext<ActionSheetContextType<HTMLElement>>(ActionSheetContext);
const {
onItemClick = () => noop,
mode: actionSheetMode,
onClose: onActionSheetClose,
} = React.useContext<ActionSheetContextType<HTMLElement>>(ActionSheetContext);
const { sizeY } = useAdaptivityWithJSMediaQueries();

const Component: React.ElementType | undefined = selectable ? 'label' : undefined;

const isRich = subtitle || meta || selectable;
const isCentered = !isRich && !before && platform === 'ios';

const onItemClickHandler = React.useCallback(
(e: React.MouseEvent) => {
onItemClick({
action: onClick,
immediateAction: onImmediateClick,
autoClose: !autoCloseDisabled,
isCancelItem: Boolean(isCancelItem),
})?.(e);
},
[autoCloseDisabled, isCancelItem, onClick, onImmediateClick, onItemClick],
);

const onKeyDown: React.KeyboardEventHandler<HTMLElement> = React.useCallback(
(event) => {
if (pressedKey(event) === Keys.ENTER) {
onActionSheetClose?.();
}
},
[onActionSheetClose],
);

const onItemClickImpl: React.MouseEventHandler<HTMLElement> = React.useCallback(
(event) => {
if (selectable) {
if (isRealClickEvent(event)) {
onItemClickHandler(event);
}
} else {
onItemClickHandler(event);
}
},
[onItemClickHandler, selectable],
);

return (
<Tappable
{...(Component && { Component })}
{...restProps}
onClick={
selectable
? onClick
: onItemClick({
action: onClick,
immediateAction: onImmediateClick,
autoClose: !autoCloseDisabled,
isCancelItem: Boolean(isCancelItem),
})
}
onClick={onItemClickImpl}
activeMode={platform === 'ios' ? styles['ActionSheetItem--active'] : undefined}
className={classNames(
styles['ActionSheetItem'],
Expand All @@ -109,6 +139,7 @@ export const ActionSheetItem = ({
selectable && styles['ActionSheetItem--selectable'],
className,
)}
onKeyDown={onKeyDown}
>
{before && <div className={styles['ActionSheetItem__before']}>{before}</div>}
<div
Expand Down Expand Up @@ -146,12 +177,6 @@ export const ActionSheetItem = ({
name={name}
value={value}
onChange={onChange}
onClick={onItemClick({
action: noop,
immediateAction: noop,
autoClose: !autoCloseDisabled,
isCancelItem: Boolean(isCancelItem),
})}
defaultChecked={defaultChecked}
checked={checked}
disabled={restProps.disabled}
Expand Down
11 changes: 11 additions & 0 deletions packages/vkui/src/components/ActionSheetItem/helpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* По дизайну `ActionSheet` должен закрывать при клике на `ActionSheetItem`.
* В режиме `selectable` в реализации используются нативный input type=radio
* И при навигации стрелочками по элементам происходит событие `click` из-за чего `ActionSheet` закрывается.
* Поэтому нужно как-то отличить реальное событие клика
* @see https://github.com/facebook/react/issues/7407
* @see https://github.com/VKCOM/VKUI/issues/6954
*/
export const isRealClickEvent = (event: React.MouseEvent) => {
return event.type === 'click' && event.clientX !== 0 && event.clientY !== 0;
};

0 comments on commit 0d94b24

Please sign in to comment.