Polaris ActionList a11y issue. #12336
Comments
zaquille-oneil
added
Bug
6 tasks
AndrewMusgrave
added a commit
that referenced
this issue
Jul 11, 2024
### WHY are these changes introduced? Fixes #12336 Fixes #12249 ### WHAT is this pull request doing? This PR exposes a `close` function on popovers imperative handle to handle accessibility when closing the popover from an outside source. ### WHY did you take this approach? Originally I thought about using an effect to watch the active status, and automatically manage focus when the popover is closed. However, we can't guarantee that the user will always want popover to manage focus. Perhaps, they'll self manage focus to another area of the page, or navigate entirely. Exposing a `close` function allows us to: * Release this in a minor version, rather than a major one * There's less work required to migrate as you won't need to audit all instance of popover * Allows for accessible focus, while allowing popover to contain the business logic * It allows for the flexibility of how you'll manage the user experience when the popover is closed ### Giphy https://github.com/Shopify/polaris/assets/24610840/7b87b030-11ee-43b2-9ea8-da61265e53f1 ### How to 🎩 **Playground code** ``` import React, {useRef, useState, useEffect} from 'react'; import type {PopoverPublicAPI} from '../src'; import {Page, Button, Popover, ActionList} from '../src'; console.log('Logging active element every 15 seconds'); export const Playground = { tags: ['skip-tests'], render() { useEffect(() => { const interval = setInterval(() => { console.log(document.activeElement); }, 15000); return () => { clearInterval(interval); }; }, []); const popoverRef = useRef<PopoverPublicAPI>(null); const [popoverActive, setPopoverActive] = useState(true); const togglePopoverActive = () => setPopoverActive((popoverActive) => !popoverActive); const activator = ( <Button onClick={togglePopoverActive} disclosure> More actions </Button> ); return ( <Page title="Playground"> <Popover active={popoverActive} activator={activator} autofocusTarget="first-node" onClose={togglePopoverActive} ref={popoverRef} > <ActionList actionRole="menuitem" items={[ { content: 'Focus activator', onAction() { popoverRef.current?.close(); }, }, { content: 'Focus next node', onAction() { popoverRef.current?.close('next-node'); }, }, ]} /> </Popover> <Button>Next focusable node</Button> </Page> ); }, }; ``` ### 🎩 checklist - [ ] Tested a [snapshot](https://github.com/Shopify/polaris/blob/main/documentation/Releasing.md#-snapshot-releases) - [ ] Tested on [mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing) - [ ] Tested on [multiple browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers) - [ ] Tested for [accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md) - [ ] Updated the component's `README.md` with documentation changes - [ ] [Tophatted documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md) changes in the style guide
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Summary
while working on a PR which implements an
<ActionList />nested underneath a<Popover />, I discovered an a11y issue. It seems like after making a selection and closing the<ActionList />, the focus does not return to the activator of the Popover.Expected behavior
I expect the focus to return to the next focusable element in the DOM.
Actual behavior
Video
https://screenshot.click/03-40-r7ldf-hzrsb.mp4Steps to reproduce
Render an action list within a popover.
Are you using React components?
Yes:
Polaris version number
"@shopify/polaris-internal": "13.12.0"
Browser
Chrome
Device
MacOS Sonoma 14.5 Apple M1 Pro 14"
The text was updated successfully, but these errors were encountered: