refactor: add FocusTrapController for trapping focus within a node #3140
Conversation
| return element.matches(':not([disabled])'); | ||
| } | ||
| // Elements that can be focused even if they have [disabled] attribute. | ||
| return element.matches('a[href], area[href], iframe, [tabindex], [contentEditable]'); |
There was a problem hiding this comment.
Should there be more checks regarding accessibility? Like aria-hidden? See e.g. tabindex inside aria-hidden https://www.w3.org/WAI/standards-guidelines/act/rules/aria-hidden-no-focusable-content-6cfa84/ for more details with positive and negative test examples
Edit: there is a highly advanced a11y lib for all kind of utilities if you wanna get some inspiration https://github.com/medialize/ally.js/tree/master/src/is
There was a problem hiding this comment.
Thank you for the references. 👍
About this particular PR, I would prefer to keep the implementation in its original shape as much as possible because the main idea here is more about decomposing and moving functions from the overlay to the component-base package.
Although, regarding the aria-hidden attribute, does it need special care? According to the first link you shared, the attribute doesn't make the element not focusable but rather prevents its announcement so there is no need to check it here, isn't it?
BTW, I extracted these changes into a separate PR, let's continue the discussion there if you want.
There was a problem hiding this comment.
You are right, don't mind me 😀 the first reference is targeting application developer to explain that it's not good to add focusable elements inside aria-hidden because the user has no idea where his focus is once he tabbed into it. So all good!
The second reference was just something that could be interesting for you if you wanna refactor your implementation later on - nothing to be done immediately. 👍
vursen
force-pushed
the
feat/focus-trap-controller
branch
8 times, most recently
from
176635b
to
272c579
Compare
vursen
force-pushed
the
feat/focus-trap-controller
branch
from
272c579
to
c0ac957
Compare
vursen
changed the title
vursen
force-pushed
the
feat/focus-trap-controller
branch
from
ea8a5ee
to
a51e923
Compare
|
SonarCloud Quality Gate failed.
|
|
This ticket/PR has been released with platform 23.0.0.alpha1 and is also targeting the upcoming stable 23.0.0 version. |
Description
The PR introduces a controller for trapping focus within a DOM node that is basically an extraction of the respective logic from the overlay component.
The controller supports two methods:
.trapFocus(node)– activates a focus trap for a DOM node and sets focus on the first focusable element inside the node if focus is not inside already..releaseFocus()– deactivates the focus trapTodo List:
Depends on #3141
Part of #3134
Type of change
Checklist