From 9bb6ed9d1c6dd6efeaf459a3ffe4dc84602b7aaa Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Wed, 22 Jan 2025 11:07:17 -0800 Subject: [PATCH] fix: add isFocusVisible useMenuItem and fix focusRing when typing in 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 --- .../interactions/src/useFocusVisible.ts | 10 +++-- packages/@react-aria/menu/src/useMenuItem.ts | 3 ++ .../s2/stories/Popover.stories.tsx | 31 ++++++++++++- .../s2/test/SearchField.test.tsx | 44 +++++++++++++++++++ .../react-aria-components/example/index.css | 5 +++ packages/react-aria-components/src/Menu.tsx | 9 ++-- .../react-aria-components/stories/utils.tsx | 5 ++- .../test/Autocomplete.test.tsx | 25 ++++++++++- 8 files changed, 120 insertions(+), 12 deletions(-) create mode 100644 packages/@react-spectrum/s2/test/SearchField.test.tsx diff --git a/packages/@react-aria/interactions/src/useFocusVisible.ts b/packages/@react-aria/interactions/src/useFocusVisible.ts index 5190494d832..0d56e412729 100644 --- a/packages/@react-aria/interactions/src/useFocusVisible.ts +++ b/packages/@react-aria/interactions/src/useFocusVisible.ts @@ -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]); } @@ -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; } diff --git a/packages/@react-aria/menu/src/useMenuItem.ts b/packages/@react-aria/menu/src/useMenuItem.ts index a088687cfbc..af3094df09c 100644 --- a/packages/@react-aria/menu/src/useMenuItem.ts +++ b/packages/@react-aria/menu/src/useMenuItem.ts @@ -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. */ @@ -316,6 +318,7 @@ export function useMenuItem(props: AriaMenuItemProps, state: TreeState, re id: keyboardId }, isFocused, + isFocusVisible: isFocused && isFocusVisible(), isSelected, isPressed, isDisabled diff --git a/packages/@react-spectrum/s2/stories/Popover.stories.tsx b/packages/@react-spectrum/s2/stories/Popover.stories.tsx index 8c4f04c9c6c..12446f0d8c8 100644 --- a/packages/@react-spectrum/s2/stories/Popover.stories.tsx +++ b/packages/@react-spectrum/s2/stories/Popover.stories.tsx @@ -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'; @@ -47,7 +48,7 @@ export const HelpCenter = (args: any) => ( Feedback - + @@ -163,3 +164,31 @@ AccountMenu.argTypes = { hideArrow: {table: {disable: true}}, placement: {table: {disable: true}} }; + + +function Autocomplete(props) { + let {contains} = useFilter({sensitivity: 'base'}); + return ( + + ); +} + +export const AutocompletePopover = (args: any) => ( + <> + + + + + + + + + Foo + Bar + Baz + + + + + +); diff --git a/packages/@react-spectrum/s2/test/SearchField.test.tsx b/packages/@react-spectrum/s2/test/SearchField.test.tsx new file mode 100644 index 00000000000..be8b2ca04c9 --- /dev/null +++ b/packages/@react-spectrum/s2/test/SearchField.test.tsx @@ -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( + + + + Foo + Bar + Baz + + + ); + + 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'); + }); +}); diff --git a/packages/react-aria-components/example/index.css b/packages/react-aria-components/example/index.css index 64e6c5c6557..829a370ffae 100644 --- a/packages/react-aria-components/example/index.css +++ b/packages/react-aria-components/example/index.css @@ -105,6 +105,11 @@ html { color: white; } +.item.focusVisible{ + outline: 3px solid black; + outline-offset: -2px; +} + .item.open:not(.focused) { background: lightslategray; color: white; diff --git a/packages/react-aria-components/src/Menu.tsx b/packages/react-aria-components/src/Menu.tsx index 5956e6d9140..c2d887bf1cd 100644 --- a/packages/react-aria-components/src/Menu.tsx +++ b/packages/react-aria-components/src/Menu.tsx @@ -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'; @@ -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 }); @@ -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'], @@ -367,13 +366,13 @@ export const MenuItem = /*#__PURE__*/ createLeafComponent('item', function MenuI return ( { return ( classNames(styles, 'item', { + className={({isFocused, isSelected, isOpen, isFocusVisible}) => classNames(styles, 'item', { focused: isFocused, selected: isSelected, - open: isOpen + open: isOpen, + focusVisible: isFocusVisible })} /> ); }; diff --git a/packages/react-aria-components/test/Autocomplete.test.tsx b/packages/react-aria-components/test/Autocomplete.test.tsx index 44fc0ef93dc..c3ff2092c26 100644 --- a/packages/react-aria-components/test/Autocomplete.test.tsx +++ b/packages/react-aria-components/test/Autocomplete.test.tsx @@ -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'; @@ -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( + + + + ); + + 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({