From 3c84495c0b7b61c4f124230e3a7a6e29cc964392 Mon Sep 17 00:00:00 2001 From: GitHub Date: Fri, 27 Sep 2024 13:29:34 +1000 Subject: [PATCH 1/5] Fix Select keyboard focus trap --- .../@react-aria/select/src/HiddenSelect.tsx | 42 +++++++++- .../react-aria-components/test/Select.test.js | 76 ++++++++++++++++++- 2 files changed, 116 insertions(+), 2 deletions(-) diff --git a/packages/@react-aria/select/src/HiddenSelect.tsx b/packages/@react-aria/select/src/HiddenSelect.tsx index 824977ea1c2..71268289a7d 100644 --- a/packages/@react-aria/select/src/HiddenSelect.tsx +++ b/packages/@react-aria/select/src/HiddenSelect.tsx @@ -11,6 +11,7 @@ */ import {FocusableElement, RefObject} from '@react-types/shared'; +import {focusSafely, getFocusableTreeWalker} from '@react-aria/focus'; import React, {ReactNode, useRef} from 'react'; import {selectData} from './useSelect'; import {SelectState} from '@react-stately/select'; @@ -108,7 +109,30 @@ export function useHiddenSelect(props: AriaHiddenSelectOptions, state: Select type: 'text', tabIndex: modality == null || state.isFocused || state.isOpen ? -1 : 0, style: {fontSize: 16}, - onFocus: () => triggerRef.current.focus(), + onFocus: (e) => { + let position = e.relatedTarget?.compareDocumentPosition(e.currentTarget); + let walker = getFocusableTreeWalker(document.body, {tabbable: true}); + walker.currentNode = e.relatedTarget; + if (position & Node.DOCUMENT_POSITION_PRECEDING ) { + // if focus is coming from "after" this element, we know it's a shift tab + let prevNode = walker.previousNode() as FocusableElement | null; + if (prevNode) { + focusElement(prevNode, true); + } else { + triggerRef.current.focus(); + } + } else if ( position & Node.DOCUMENT_POSITION_FOLLOWING ) { + // if focus is coming from "before" this element, we know it's a tab + let nextNode = walker.nextNode() as FocusableElement | null; + if (nextNode) { + focusElement(nextNode, true); + } else { + triggerRef.current.focus(); + } + } else { + triggerRef.current.focus(); + } + }, disabled: isDisabled }, selectProps: { @@ -172,3 +196,19 @@ export function HiddenSelect(props: HiddenSelectProps) { return null; } + +function focusElement(element: FocusableElement | null, scroll = false) { + if (element != null && !scroll) { + try { + focusSafely(element); + } catch (err) { + // ignore + } + } else if (element != null) { + try { + element.focus(); + } catch (err) { + // ignore + } + } +} diff --git a/packages/react-aria-components/test/Select.test.js b/packages/react-aria-components/test/Select.test.js index 1d620f85f40..b7b2c1e795d 100644 --- a/packages/react-aria-components/test/Select.test.js +++ b/packages/react-aria-components/test/Select.test.js @@ -11,7 +11,7 @@ */ import {act, pointerMap, render, within} from '@react-spectrum/test-utils-internal'; -import {Button, FieldError, Label, ListBox, ListBoxItem, Popover, Select, SelectContext, SelectValue, Text} from '../'; +import {Button, FieldError, Label, ListBox, ListBoxItem, Popover, Select, SelectContext, SelectStateContext, SelectValue, Text} from '../'; import React from 'react'; import {User} from '@react-aria/test-utils'; import userEvent from '@testing-library/user-event'; @@ -278,4 +278,78 @@ describe('Select', () => { expect(button).toHaveTextContent('0'); }); + + function SelectClearButton() { + let state = React.useContext(SelectStateContext); + return ( + + ); + } + + it('should support extra children for use with the state', async () => { + let onChangeSpy = jest.fn(); + let {getByTestId} = render( + <> + + + + + ); + + let beforeInput = getByTestId('before'); + let afterInput = getByTestId('after'); + let wrapper = getByTestId('select'); + let clearButton = getByTestId('clear'); + let selectTester = testUtilUser.createTester('Select', {root: wrapper}); + + await user.tab(); + await user.tab(); + expect(document.activeElement).toBe(selectTester.trigger); + + await user.tab(); + expect(document.activeElement).toBe(clearButton); + + await user.tab(); + expect(document.activeElement).toBe(afterInput); + + await user.tab({shift: true}); + expect(document.activeElement).toBe(clearButton); + + await user.tab({shift: true}); + expect(document.activeElement).toBe(selectTester.trigger); + + await user.tab({shift: true}); + expect(document.activeElement).toBe(beforeInput); + + await user.tab(); + await selectTester.selectOption({optionText: 'Dog'}); + + expect(onChangeSpy).toHaveBeenCalledTimes(1); + expect(onChangeSpy).toHaveBeenLastCalledWith('dog'); + + await user.click(clearButton); + expect(onChangeSpy).toHaveBeenCalledTimes(2); + expect(onChangeSpy).toHaveBeenLastCalledWith(null); + }); }); From ec66e5d11052e91ec35d7cfcb6f72dd53c0d3094 Mon Sep 17 00:00:00 2001 From: GitHub Date: Fri, 27 Sep 2024 13:58:59 +1000 Subject: [PATCH 2/5] fix lint --- packages/@react-aria/select/package.json | 1 + packages/@react-aria/select/src/HiddenSelect.tsx | 7 ++++--- packages/react-aria-components/test/Select.test.js | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/@react-aria/select/package.json b/packages/@react-aria/select/package.json index b05d20a55cb..092648e6254 100644 --- a/packages/@react-aria/select/package.json +++ b/packages/@react-aria/select/package.json @@ -22,6 +22,7 @@ "url": "https://github.com/adobe/react-spectrum" }, "dependencies": { + "@react-aria/focus": "^3.18.2", "@react-aria/form": "^3.0.9", "@react-aria/i18n": "^3.12.3", "@react-aria/interactions": "^3.22.3", diff --git a/packages/@react-aria/select/src/HiddenSelect.tsx b/packages/@react-aria/select/src/HiddenSelect.tsx index 71268289a7d..f0ab442495a 100644 --- a/packages/@react-aria/select/src/HiddenSelect.tsx +++ b/packages/@react-aria/select/src/HiddenSelect.tsx @@ -112,8 +112,8 @@ export function useHiddenSelect(props: AriaHiddenSelectOptions, state: Select onFocus: (e) => { let position = e.relatedTarget?.compareDocumentPosition(e.currentTarget); let walker = getFocusableTreeWalker(document.body, {tabbable: true}); - walker.currentNode = e.relatedTarget; - if (position & Node.DOCUMENT_POSITION_PRECEDING ) { + if (position & Node.DOCUMENT_POSITION_PRECEDING) { + walker.currentNode = e.relatedTarget; // if focus is coming from "after" this element, we know it's a shift tab let prevNode = walker.previousNode() as FocusableElement | null; if (prevNode) { @@ -121,7 +121,8 @@ export function useHiddenSelect(props: AriaHiddenSelectOptions, state: Select } else { triggerRef.current.focus(); } - } else if ( position & Node.DOCUMENT_POSITION_FOLLOWING ) { + } else if (position & Node.DOCUMENT_POSITION_FOLLOWING) { + walker.currentNode = e.relatedTarget; // if focus is coming from "before" this element, we know it's a tab let nextNode = walker.nextNode() as FocusableElement | null; if (nextNode) { diff --git a/packages/react-aria-components/test/Select.test.js b/packages/react-aria-components/test/Select.test.js index b7b2c1e795d..2f7c1dae1b5 100644 --- a/packages/react-aria-components/test/Select.test.js +++ b/packages/react-aria-components/test/Select.test.js @@ -283,7 +283,7 @@ describe('Select', () => { let state = React.useContext(SelectStateContext); return (