Skip to content

Commit

Permalink
fix: add isFocusVisible useMenuItem and fix focusRing when typing in …
Browse files Browse the repository at this point in the history
…Autocomplete SearchField (#7625)

* add isFocusVisible support to useMenuItem

* Adding tests and style in storybook

* fix: (WIP) Fix autocomplete searchfield focus ring (#7626)

* for discussion

* fix searchfield focus ring appearing on keypress when wrapped in a autocomplete

* add test

* remove extranous controls
  • Loading branch information
LFDanLu authored Jan 22, 2025
1 parent 2a1c28b commit 9bb6ed9
Show file tree
Hide file tree
Showing 8 changed files with 120 additions and 12 deletions.
10 changes: 7 additions & 3 deletions packages/@react-aria/interactions/src/useFocusVisible.ts
Original file line number Diff line number Diff line change
Expand Up @@ -289,15 +289,18 @@ const nonTextInputTypes = new Set([
* focus visible style can be properly set.
*/
function isKeyboardFocusEvent(isTextInput: boolean, modality: Modality, e: HandlerEvent) {
let document = getOwnerDocument(e?.target as Element);
const IHTMLInputElement = typeof window !== 'undefined' ? getOwnerWindow(e?.target as Element).HTMLInputElement : HTMLInputElement;
const IHTMLTextAreaElement = typeof window !== 'undefined' ? getOwnerWindow(e?.target as Element).HTMLTextAreaElement : HTMLTextAreaElement;
const IHTMLElement = typeof window !== 'undefined' ? getOwnerWindow(e?.target as Element).HTMLElement : HTMLElement;
const IKeyboardEvent = typeof window !== 'undefined' ? getOwnerWindow(e?.target as Element).KeyboardEvent : KeyboardEvent;

// For keyboard events that occur on a non-input element that will move focus into input element (aka ArrowLeft going from Datepicker button to the main input group)
// we need to rely on the user passing isTextInput into here. This way we can skip toggling focus visiblity for said input element
isTextInput = isTextInput ||
(e?.target instanceof IHTMLInputElement && !nonTextInputTypes.has(e?.target?.type)) ||
e?.target instanceof IHTMLTextAreaElement ||
(e?.target instanceof IHTMLElement && e?.target.isContentEditable);
(document.activeElement instanceof IHTMLInputElement && !nonTextInputTypes.has(document.activeElement.type)) ||
document.activeElement instanceof IHTMLTextAreaElement ||
(document.activeElement instanceof IHTMLElement && document.activeElement.isContentEditable);
return !(isTextInput && modality === 'keyboard' && e instanceof IKeyboardEvent && !FOCUS_VISIBLE_INPUT_KEYS[e.key]);
}

Expand All @@ -322,6 +325,7 @@ export function useFocusVisibleListener(fn: FocusVisibleHandler, deps: ReadonlyA

useEffect(() => {
let handler = (modality: Modality, e: HandlerEvent) => {
// We want to early return for any keyboard events that occur inside text inputs EXCEPT for Tab and Escape
if (!isKeyboardFocusEvent(!!(opts?.isTextInput), modality, e)) {
return;
}
Expand Down
3 changes: 3 additions & 0 deletions packages/@react-aria/menu/src/useMenuItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ export interface MenuItemAria {

/** Whether the item is currently focused. */
isFocused: boolean,
/** Whether the item is keyboard focused. */
isFocusVisible: boolean,
/** Whether the item is currently selected. */
isSelected: boolean,
/** Whether the item is currently in a pressed state. */
Expand Down Expand Up @@ -316,6 +318,7 @@ export function useMenuItem<T>(props: AriaMenuItemProps, state: TreeState<T>, re
id: keyboardId
},
isFocused,
isFocusVisible: isFocused && isFocusVisible(),
isSelected,
isPressed,
isDisabled
Expand Down
31 changes: 30 additions & 1 deletion packages/@react-spectrum/s2/stories/Popover.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import type {Meta} from '@storybook/react';
import Org from '../s2wf-icons/S2_Icon_Buildings_20_N.svg';
import Settings from '../s2wf-icons/S2_Icon_Settings_20_N.svg';
import {style} from '../style/spectrum-theme' with {type: 'macro'};
import {UNSTABLE_Autocomplete, useFilter} from 'react-aria-components';
import User from '../s2wf-icons/S2_Icon_User_20_N.svg';
import Users from '../s2wf-icons/S2_Icon_UserGroup_20_N.svg';

Expand Down Expand Up @@ -47,7 +48,7 @@ export const HelpCenter = (args: any) => (
<Tab id="feedback">Feedback</Tab>
</TabList>
<TabPanel id="help">
<SearchField label="Search" styles={style({marginTop: 12, marginX: 12})} />
<SearchField label="Search" styles={style({marginTop: 12, marginX: 12})} />
<Menu aria-label="Help" styles={style({marginTop: 12})}>
<MenuSection>
<MenuItem href="#">
Expand Down Expand Up @@ -163,3 +164,31 @@ AccountMenu.argTypes = {
hideArrow: {table: {disable: true}},
placement: {table: {disable: true}}
};


function Autocomplete(props) {
let {contains} = useFilter({sensitivity: 'base'});
return (
<UNSTABLE_Autocomplete filter={contains} {...props} />
);
}

export const AutocompletePopover = (args: any) => (
<>
<DialogTrigger {...args}>
<ActionButton aria-label="Help" styles={style({marginX: 'auto'})}>
<Help />
</ActionButton>
<Popover {...args}>
<Autocomplete>
<SearchField autoFocus label="Search" styles={style({marginTop: 12, marginX: 12})} />
<Menu aria-label="test menu" styles={style({marginTop: 12})}>
<MenuItem>Foo</MenuItem>
<MenuItem>Bar</MenuItem>
<MenuItem>Baz</MenuItem>
</Menu>
</Autocomplete>
</Popover>
</DialogTrigger>
</>
);
44 changes: 44 additions & 0 deletions packages/@react-spectrum/s2/test/SearchField.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright 2025 Adobe. All rights reserved.
* This file is licensed to you under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. You may obtain a copy
* of the License at http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/

import {Menu, MenuItem, SearchField} from '../src';
import {pointerMap, render} from '@react-spectrum/test-utils-internal';
import React from 'react';
import {UNSTABLE_Autocomplete} from 'react-aria-components';
import userEvent from '@testing-library/user-event';

describe('SearchField', () => {
let user;
beforeAll(() => {
user = userEvent.setup({delay: null, pointerMap});
});

it('should not apply the focus visible styles on the group when typing in the Autocomplete wrapped SearchField', async () => {
let {getByRole} = render(
<UNSTABLE_Autocomplete>
<SearchField autoFocus label="Search" />
<Menu aria-label="test menu">
<MenuItem>Foo</MenuItem>
<MenuItem>Bar</MenuItem>
<MenuItem>Baz</MenuItem>
</Menu>
</UNSTABLE_Autocomplete>
);

let input = getByRole('searchbox');
await user.click(input);
let group = getByRole('group');
expect(group).not.toHaveAttribute('data-focus-visible');
await user.keyboard('Foo');
expect(group).not.toHaveAttribute('data-focus-visible');
});
});
5 changes: 5 additions & 0 deletions packages/react-aria-components/example/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ html {
color: white;
}

.item.focusVisible{
outline: 3px solid black;
outline-offset: -2px;
}

.item.open:not(.focused) {
background: lightslategray;
color: white;
Expand Down
9 changes: 4 additions & 5 deletions packages/react-aria-components/src/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* governing permissions and limitations under the License.
*/

import {AriaMenuProps, FocusScope, mergeProps, useFocusRing, useMenu, useMenuItem, useMenuSection, useMenuTrigger} from 'react-aria';
import {AriaMenuProps, FocusScope, mergeProps, useMenu, useMenuItem, useMenuSection, useMenuTrigger} from 'react-aria';
import {BaseCollection, Collection, CollectionBuilder, createBranchComponent, createLeafComponent} from '@react-aria/collections';
import {MenuTriggerProps as BaseMenuTriggerProps, Collection as ICollection, Node, TreeState, useMenuTriggerState, useTreeState} from 'react-stately';
import {CollectionProps, CollectionRendererContext, ItemRenderProps, SectionContext, SectionProps, usePersistedKeys} from './Collection';
Expand Down Expand Up @@ -343,7 +343,6 @@ export const MenuItem = /*#__PURE__*/ createLeafComponent('item', function MenuI
selectionManager
}, state, ref);

let {isFocusVisible, focusProps} = useFocusRing();
let {hoverProps, isHovered} = useHover({
isDisabled: states.isDisabled
});
Expand All @@ -355,7 +354,7 @@ export const MenuItem = /*#__PURE__*/ createLeafComponent('item', function MenuI
values: {
...states,
isHovered,
isFocusVisible,
isFocusVisible: states.isFocusVisible,
selectionMode: selectionManager.selectionMode,
selectionBehavior: selectionManager.selectionBehavior,
hasSubmenu: !!props['aria-haspopup'],
Expand All @@ -367,13 +366,13 @@ export const MenuItem = /*#__PURE__*/ createLeafComponent('item', function MenuI

return (
<ElementType
{...mergeProps(menuItemProps, focusProps, hoverProps)}
{...mergeProps(menuItemProps, hoverProps)}
{...renderProps}
ref={ref}
data-disabled={states.isDisabled || undefined}
data-hovered={isHovered || undefined}
data-focused={states.isFocused || undefined}
data-focus-visible={isFocusVisible || undefined}
data-focus-visible={states.isFocusVisible || undefined}
data-pressed={states.isPressed || undefined}
data-selected={states.isSelected || undefined}
data-selection-mode={selectionManager.selectionMode === 'none' ? undefined : selectionManager.selectionMode}
Expand Down
5 changes: 3 additions & 2 deletions packages/react-aria-components/stories/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ export const MyMenuItem = (props: MenuItemProps) => {
return (
<MenuItem
{...props}
className={({isFocused, isSelected, isOpen}) => classNames(styles, 'item', {
className={({isFocused, isSelected, isOpen, isFocusVisible}) => classNames(styles, 'item', {
focused: isFocused,
selected: isSelected,
open: isOpen
open: isOpen,
focusVisible: isFocusVisible
})} />
);
};
25 changes: 24 additions & 1 deletion packages/react-aria-components/test/Autocomplete.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

import {AriaAutocompleteTests} from './AriaAutocomplete.test-util';
import {Button, Header, Input, Label, ListBox, ListBoxItem, ListBoxSection, Menu, MenuItem, MenuSection, SearchField, Separator, Text, UNSTABLE_Autocomplete} from '..';
import {pointerMap, render} from '@react-spectrum/test-utils-internal';
import {pointerMap, render, within} from '@react-spectrum/test-utils-internal';
import React, {ReactNode} from 'react';
import {useAsyncList} from 'react-stately';
import {useFilter} from '@react-aria/i18n';
Expand Down Expand Up @@ -221,6 +221,29 @@ describe('Autocomplete', () => {

expect(input).toHaveValue('');
});

it('should apply focusVisible/focused to virtually focused menu items when keyboard navigating', async () => {
let {getByRole} = render(
<AutocompleteWrapper>
<StaticMenu />
</AutocompleteWrapper>
);

let input = getByRole('searchbox');
await user.tab();
expect(document.activeElement).toBe(input);
await user.keyboard('{ArrowDown}');
let menu = getByRole('menu');
let options = within(menu).getAllByRole('menuitem');
expect(input).toHaveAttribute('aria-activedescendant', options[0].id);
expect(options[0]).toHaveAttribute('data-focus-visible');

await user.click(input);
await user.hover(options[1]);
options = within(menu).getAllByRole('menuitem');
expect(options[1]).toHaveAttribute('data-focused');
expect(options[1]).not.toHaveAttribute('data-focus-visible');
});
});

AriaAutocompleteTests({
Expand Down

1 comment on commit 9bb6ed9

@rspbot
Copy link

@rspbot rspbot commented on 9bb6ed9 Jan 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.