Skip to content

Commit

Permalink
fix(TabbarItem): avoid double focus (#8241)
Browse files Browse the repository at this point in the history
Двойной фокус при переходе через `TabbarItem`'s табом происходит в основном из-за того, что внутри `TabbarItem`, который является кнопкой/ссылкой, лежит интерактивный `Tappable`. `Tappable` внутри нужен для того, чтобы на android был ripple эффект.

------

Видел решение #5914. Тоже изначально думал как бы оставить один Tappable, но решил сделать быстрый фикс конкретно этого поведения. Но в целом, в будущем стоит отрефакторить TabbarItem, чтобы избавиться от ненужных нод и упростить компонент.

Изменения
- убраем возможность фокуса на `Tappable` через tabindex="-1".
- добавляем focus-visible у `Tappable` при фокусировании на `TabbarItem`. Так как иначе при фокусе с клавиатуры вообще фокуса видно не будет, из-за tabindex="-1".
  • Loading branch information
andrey-medvedev-vk authored Feb 7, 2025
1 parent fcf48ad commit 2b144f4
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 1 deletion.
52 changes: 52 additions & 0 deletions packages/vkui/src/components/TabbarItem/TabbarItem.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { render, screen } from '@testing-library/react';
import { Icon28NewsfeedOutline } from '@vkontakte/icons';
import {
AppRootContext,
DEFAULT_APP_ROOT_CONTEXT_VALUE,
} from '../../components/AppRoot/AppRootContext';
import { baselineComponent, userEvent } from '../../testing/utils';
import { TabbarItem } from './TabbarItem';
import styles from '../../styles/focusVisible.module.css';

describe('TabbarItem', () => {
baselineComponent((props) => (
Expand Down Expand Up @@ -39,4 +44,51 @@ describe('TabbarItem', () => {
await userEvent.click(screen.getByTestId('test'));
expect(cb).toHaveBeenCalledTimes(1);
});

function renderTabbarItemForFocus({ withKeyboardInput }: { withKeyboardInput: boolean }) {
const onFocusStub = jest.fn();
const onBlurStub = jest.fn();

return {
onFocusStub,
onBlurStub,
...render(
<AppRootContext.Provider
value={{ ...DEFAULT_APP_ROOT_CONTEXT_VALUE, keyboardInput: withKeyboardInput }}
>
<TabbarItem onFocus={onFocusStub} onBlur={onBlurStub} data-testid="test" />,
</AppRootContext.Provider>,
),
};
}

it('shows focus visible on focus with keyboard', async () => {
jest.useFakeTimers();

const component = renderTabbarItemForFocus({ withKeyboardInput: true });

await userEvent.tab();
expect(screen.getByRole('presentation')).toHaveClass(styles['-focus-visible--focused']);

await userEvent.tab();
expect(screen.getByRole('presentation')).not.toHaveClass(styles['-focus-visible--focused']);

expect(component.onFocusStub).toHaveBeenCalledTimes(1);
expect(component.onBlurStub).toHaveBeenCalledTimes(1);
});

it('does not show focus visible on focus without keyboard', async () => {
jest.useFakeTimers();

const component = renderTabbarItemForFocus({ withKeyboardInput: false });

await userEvent.click(screen.getByTestId('test'));
expect(screen.getByRole('presentation')).not.toHaveClass(styles['-focus-visible--focused']);

await userEvent.tab();
expect(screen.getByRole('presentation')).not.toHaveClass(styles['-focus-visible--focused']);

expect(component.onFocusStub).toHaveBeenCalledTimes(1);
expect(component.onBlurStub).toHaveBeenCalledTimes(1);
});
});
19 changes: 18 additions & 1 deletion packages/vkui/src/components/TabbarItem/TabbarItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

import * as React from 'react';
import { classNames, hasReactNode, noop } from '@vkontakte/vkjs';
import { useFocusVisible } from '../../hooks/useFocusVisible';
import { useFocusVisibleClassName } from '../../hooks/useFocusVisibleClassName';
import { usePlatform } from '../../hooks/usePlatform';
import { callMultiple } from '../../lib/callMultiple';
import { COMMON_WARNINGS, warnOnce } from '../../lib/warnOnce';
import type { HasComponent, HasRootRef } from '../../types';
import { RootComponent } from '../RootComponent/RootComponent';
Expand Down Expand Up @@ -38,6 +41,8 @@ export const TabbarItem = ({
href,
Component = href ? 'a' : 'button',
disabled,
onFocus: onFocusProp,
onBlur: onBlurProp,
...restProps
}: TabbarItemProps): React.ReactNode => {
const platform = usePlatform();
Expand All @@ -50,11 +55,22 @@ export const TabbarItem = ({
}
}

const {
focusVisible,
onFocus: handleFocusVisibleOnFocus,
onBlur: handleFocusVisibleOnBlur,
} = useFocusVisible();
const focusVisibleClassNames = useFocusVisibleClassName({
focusVisible,
});

return (
<RootComponent
Component={Component}
{...restProps}
disabled={disabled}
onFocus={callMultiple(handleFocusVisibleOnFocus, onFocusProp)}
onBlur={callMultiple(handleFocusVisibleOnBlur, onBlurProp)}
href={href}
baseClassName={classNames(
styles.host,
Expand All @@ -69,8 +85,9 @@ export const TabbarItem = ({
activeMode={platform === 'ios' ? styles.tappableActive : 'background'}
activeEffectDelay={platform === 'ios' ? 0 : 300}
hasHover={false}
className={styles.tappable}
className={classNames(styles.tappable, focusVisibleClassNames)}
onClick={noop}
tabIndex={-1}
/>
<div className={styles.in}>
<div className={styles.icon}>
Expand Down

0 comments on commit 2b144f4

Please sign in to comment.