From fe152fe72007f4e0c21d8828a1c0fdea49d9c23c Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Tue, 30 Jul 2024 15:29:44 +0200 Subject: [PATCH 1/6] enable focus zone for listbox and menu --- .../react/src/ActionList/ActionList.test.tsx | 33 +++++++++++++++++++ packages/react/src/ActionList/List.tsx | 15 ++++++--- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/packages/react/src/ActionList/ActionList.test.tsx b/packages/react/src/ActionList/ActionList.test.tsx index 06a35990b2c..a274070754e 100644 --- a/packages/react/src/ActionList/ActionList.test.tsx +++ b/packages/react/src/ActionList/ActionList.test.tsx @@ -590,4 +590,37 @@ describe('ActionList', () => { expect(mockOnSelect).toHaveBeenCalledTimes(1) }) + + it('should be navigatable with arrow keys for certain roles', async () => { + HTMLRender( + + Option 1 + Option 2 + + Option 3 + + Option 4 + + Option 5 + + , + ) + + const user = userEvent.setup() + await user.tab() // tab into the story, this should focus on the first button + expect(document.activeElement).toHaveTextContent('Option 1') + + await user.keyboard('{ArrowDown}') + expect(document.activeElement).toHaveTextContent('Option 2') + + await user.keyboard('{ArrowDown}') + expect(document.activeElement).not.toHaveTextContent('Option 3') // option 3 is disabled + expect(document.activeElement).toHaveTextContent('Option 4') + + await user.keyboard('{ArrowDown}') + expect(document.activeElement).toHaveAccessibleName('Unavailable due to an outage') + + await user.keyboard('{ArrowUp}') + expect(document.activeElement).toHaveTextContent('Option 4') + }) }) diff --git a/packages/react/src/ActionList/List.tsx b/packages/react/src/ActionList/List.tsx index c6dc26d415a..809bf9fcb19 100644 --- a/packages/react/src/ActionList/List.tsx +++ b/packages/react/src/ActionList/List.tsx @@ -33,15 +33,20 @@ export const List = React.forwardRef( /** if list is inside a Menu, it will get a role from the Menu */ const { - listRole, + listRole: listRoleFromContainer, listLabelledBy, selectionVariant: containerSelectionVariant, // TODO: Remove after DropdownMenu2 deprecation - enableFocusZone, + enableFocusZone: enableFocusZoneFromContainer, } = React.useContext(ActionListContainerContext) const ariaLabelledBy = slots.heading ? slots.heading.props.id ?? headingId : listLabelledBy - + const listRole = role || listRoleFromContainer const listRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) + + let enableFocusZone = false + if (enableFocusZoneFromContainer !== undefined) enableFocusZone = enableFocusZoneFromContainer + else if (listRole) enableFocusZone = ['menu', 'menubar', 'listbox'].includes(listRole) + useFocusZone({ disabled: !enableFocusZone, containerRef: listRef, @@ -54,14 +59,14 @@ export const List = React.forwardRef( variant, selectionVariant: selectionVariant || containerSelectionVariant, showDividers, - role: role || listRole, + role: listRole, headingId, }} > {slots.heading} Date: Tue, 30 Jul 2024 15:32:15 +0200 Subject: [PATCH 2/6] Create actionlist-focuszone.md --- .changeset/lemon-candles-deny.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/lemon-candles-deny.md diff --git a/.changeset/lemon-candles-deny.md b/.changeset/lemon-candles-deny.md new file mode 100644 index 00000000000..afb5158ad1c --- /dev/null +++ b/.changeset/lemon-candles-deny.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +ActionList: Enable focusZone for roles listbox and menu From 2c242bfe7a4de4e97e4cfc8d63c4c2b86358abe0 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Tue, 30 Jul 2024 16:44:24 +0200 Subject: [PATCH 3/6] fix other tests that depend on tab instead of ArrowDown --- .../react/src/ActionList/ActionList.test.tsx | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/packages/react/src/ActionList/ActionList.test.tsx b/packages/react/src/ActionList/ActionList.test.tsx index a274070754e..8524b35dd91 100644 --- a/packages/react/src/ActionList/ActionList.test.tsx +++ b/packages/react/src/ActionList/ActionList.test.tsx @@ -205,23 +205,22 @@ describe('ActionList', () => { it('should focus the button around the leading visual when tabbing to an inactive item', async () => { const component = HTMLRender() const inactiveOptionButton = await waitFor(() => component.getByRole('button', {name: projects[3].inactiveText})) - const inactiveIndex = projects.findIndex(project => project.inactiveText === projects[3].inactiveText) - - for (let i = 0; i < inactiveIndex; i++) { - await userEvent.tab() - } + await userEvent.tab() // get focus on first element + await userEvent.keyboard('{ArrowDown}') + await userEvent.keyboard('{ArrowDown}') expect(inactiveOptionButton).toHaveFocus() }) it('should behave as inactive if both inactiveText and loading props are passed', async () => { const component = HTMLRender() const inactiveOptionButton = await waitFor(() => component.getByRole('button', {name: projects[5].inactiveText})) - const inactiveIndex = projects.findIndex(project => project.inactiveText === projects[5].inactiveText) - for (let i = 0; i < inactiveIndex; i++) { - await userEvent.tab() - } + await userEvent.tab() // get focus on first element + await userEvent.keyboard('{ArrowDown}') + await userEvent.keyboard('{ArrowDown}') + await userEvent.keyboard('{ArrowDown}') + await userEvent.keyboard('{ArrowDown}') expect(inactiveOptionButton).toHaveFocus() }) @@ -606,21 +605,20 @@ describe('ActionList', () => { , ) - const user = userEvent.setup() - await user.tab() // tab into the story, this should focus on the first button + await userEvent.tab() // tab into the story, this should focus on the first button expect(document.activeElement).toHaveTextContent('Option 1') - await user.keyboard('{ArrowDown}') + await userEvent.keyboard('{ArrowDown}') expect(document.activeElement).toHaveTextContent('Option 2') - await user.keyboard('{ArrowDown}') + await userEvent.keyboard('{ArrowDown}') expect(document.activeElement).not.toHaveTextContent('Option 3') // option 3 is disabled expect(document.activeElement).toHaveTextContent('Option 4') - await user.keyboard('{ArrowDown}') + await userEvent.keyboard('{ArrowDown}') expect(document.activeElement).toHaveAccessibleName('Unavailable due to an outage') - await user.keyboard('{ArrowUp}') + await userEvent.keyboard('{ArrowUp}') expect(document.activeElement).toHaveTextContent('Option 4') }) }) From 70e39f675492b2903182f71a8341184659825b42 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Wed, 31 Jul 2024 17:30:23 +0200 Subject: [PATCH 4/6] add failing test --- .../react/src/__tests__/ActionMenu.test.tsx | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/react/src/__tests__/ActionMenu.test.tsx b/packages/react/src/__tests__/ActionMenu.test.tsx index 91533a83b3a..c3054754512 100644 --- a/packages/react/src/__tests__/ActionMenu.test.tsx +++ b/packages/react/src/__tests__/ActionMenu.test.tsx @@ -334,6 +334,29 @@ describe('ActionMenu', () => { }) }) + it('should wrap focus when ArrowDown is pressed on the last element', async () => { + const component = HTMLRender() + const button = component.getByRole('button') + + const user = userEvent.setup() + await user.click(button) + + expect(component.queryByRole('menu')).toBeInTheDocument() + const menuItems = component.getAllByRole('menuitem') + + await user.keyboard('{ArrowDown}') + expect(menuItems[0]).toEqual(document.activeElement) + + await user.keyboard('{ArrowDown}') + await user.keyboard('{ArrowDown}') + await user.keyboard('{ArrowDown}') + await user.keyboard('{ArrowDown}') + expect(menuItems[menuItems.length - 1]).toEqual(document.activeElement) // last elememt + + await user.keyboard('{ArrowDown}') + expect(menuItems[0]).toEqual(document.activeElement) // wrap to first + }) + it('should have no axe violations', async () => { const {container} = HTMLRender() const results = await axe.run(container) From 2eda6fe3032101e5145a266f2a74a0b6c44732d4 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Wed, 31 Jul 2024 17:46:57 +0200 Subject: [PATCH 5/6] disable focuszone inside ActionMenu --- packages/react/src/ActionMenu/ActionMenu.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react/src/ActionMenu/ActionMenu.tsx b/packages/react/src/ActionMenu/ActionMenu.tsx index 619272c40b1..a410257614b 100644 --- a/packages/react/src/ActionMenu/ActionMenu.tsx +++ b/packages/react/src/ActionMenu/ActionMenu.tsx @@ -248,6 +248,7 @@ const Overlay: React.FC> = ({ listLabelledBy: ariaLabelledby || anchorAriaLabelledby || anchorId, selectionAttribute: 'aria-checked', // Should this be here? afterSelect: () => onClose?.('item-select'), + enableFocusZone: false, // AnchoredOverlay takes care of focus zone }} > {children} From fa13d5aae636968727cb45f88f12f4ba9b4c57e7 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Wed, 14 Aug 2024 16:31:09 +0200 Subject: [PATCH 6/6] wrap focus zone for menus --- packages/react/src/ActionList/List.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react/src/ActionList/List.tsx b/packages/react/src/ActionList/List.tsx index 809bf9fcb19..7786f392682 100644 --- a/packages/react/src/ActionList/List.tsx +++ b/packages/react/src/ActionList/List.tsx @@ -51,6 +51,7 @@ export const List = React.forwardRef( disabled: !enableFocusZone, containerRef: listRef, bindKeys: FocusKeys.ArrowVertical | FocusKeys.HomeAndEnd | FocusKeys.PageUpDown, + focusOutBehavior: listRole === 'menu' ? 'wrap' : undefined, }) return (