feat(web, a11y): focus management for modals and popups #8298
Conversation
| @@ -43,12 +43,12 @@ export function clickOutside(node: HTMLElement, options: Options = {}): ActionRe | |||
| }; | |||
|
|
|||
| document.addEventListener('click', handleClick, true); | |||
| document.addEventListener('keydown', handleKey, true); | |||
| node.addEventListener('keydown', handleKey, false); | |||
There was a problem hiding this comment.
Setting the listener on the document was causing both modals to close at once in scenarios where 2 overlapping modals are open at once. Setting the listener on the element itself and letting the event bubble up in the DOM solves this problem.
This does mean that modals now need tabindex="-1" on the body - if a mouse user is navigating the modal and presses escape without a form element focus, this guarantees that the modal will still close.
| }); | ||
|
|
||
| const getFocusableElements = () => { | ||
| return Array.from( |
There was a problem hiding this comment.
For reference, another example implementation for finding focusable elements is the tabbable package.
| @@ -182,7 +182,12 @@ | |||
| {#if !asset.isReadOnly || !asset.isExternal} | |||
| <CircleIconButton isOpacity={true} icon={mdiDeleteOutline} on:click={() => dispatch('delete')} title="Delete" /> | |||
| {/if} | |||
| <div use:clickOutside on:outclick={() => (isShowAssetOptions = false)}> | |||
| <div | |||
| use:clickOutside={{ | |||
There was a problem hiding this comment.
I didn't know you could do this! Amazing!
There was a problem hiding this comment.
It's from the cool work done in #7677!
| @@ -44,7 +44,7 @@ | |||
| }; | |||
| </script> | |||
|
|
|||
| <BaseModal on:close={() => dispatch('close')}> | |||
| <BaseModal on:close on:escape> | |||
There was a problem hiding this comment.
I don't think we use on:escape here, correct? Which event's reaction does it do?
There was a problem hiding this comment.
This is needed now, since I added escape key handling for a couple of the modals inside of the asset viewer.
There was a problem hiding this comment.
Ah, so this propagates the event out for this component?
| @@ -0,0 +1,62 @@ | |||
| <script lang="ts"> | |||
There was a problem hiding this comment.
What is the rule of thumb to wrap other component with FocusTrap?
There was a problem hiding this comment.
I don't have a hard and fast rule, but it's something to consider any time you're making an element appear over the screen and have a use:clickOutside to dismiss it. The idea is that if there's an element on the screen that's preventing mouse users from interacting with elements behind it, keyboard users should also have the same experience.
Description
Creating a new
FocusTrapcomponent, that helps maintain accessible focus in modals and popup menus. See the "Keyboard Interaction" section of the WCAG spec for more information about the recommended focus management:https://www.w3.org/WAI/ARIA/apg/patterns/dialog-modal/#keyboardinteraction
New functionality in the focus trap:
Also in this PR:
Caveats:
autofocusattribute break the ability of the focus trap to focus the first element, and return focus back to the triggering element.Screenshots
Not applicable - everything visually looks the same.
How Has This Been Tested?
Navigate the UI entirely by keyboard. Any time a modal is opened, focus should automatically be moved to the modal without need for a click, and return back to where it was when closed.
Focus should never go underneath the asset viewer if a photo is clicked.
Checklist: