fix(dialog): Handle focus trapping correctly #491
Conversation
Codecov Report
@@ Coverage Diff @@
## master #491 +/- ##
==========================================
- Coverage 99.17% 99.13% -0.04%
==========================================
Files 40 40
Lines 2050 1972 -78
Branches 261 250 -11
==========================================
- Hits 2033 1955 -78
Misses 17 17
Continue to review full report at Codecov.
|
|
|
||
| In order for dialogs to be fully accessible, they must conform to the guidelines outlined in | ||
| https://www.w3.org/TR/wai-aria-practices/#dialog_modal. The main implication of these guidelines is |
There was a problem hiding this comment.
"The main implication of these guidelines is that such that the only focusable elements..."
Should read:
"The main implication of these guidelines is that the only focusable elements..."
| }); | ||
|
|
||
| test('#close makes elements untabbable', () => { | ||
| test('#close removes the scroll lock class from dialog background', () => { |
There was a problem hiding this comment.
This is on me for coming up with the naming convention for the component, but potentially we should rephrase "background" to "content underneath dialog" for the test description. Aria calls things behind the dialog "background" which is why I called it this to begin with, but I feel this could too easily be confused with the concept of a dialog backdrop, which we also have.
There was a problem hiding this comment.
np! Changing background to content underneath the dialog
There was a problem hiding this comment.
Actually just changed it to the body to match the spec description above
- Remove all focus trapping logic within the dialog foundation, replacing it with adapter methods set up to handle focus trapping. - Add [focus-trap](https://npmjs.com/focus-trap) as a dependency and use it for implementing the adapter methods at the component level - Add a util method for easily creating properly configured focus-trap instances that framework components can use when implementing their adapters. - Remove use of aria-hidden and replace with visibility styles so that tab-focusing works correctly when dialog is hidden. - [techdebt] Remove all unnecessary adapter API methods and dead code - [techdebt] Make grammatical corrections in README - [techdebt] Remove unneeded passive event listener logic for dialog handlers - [demos] Fix ripples incorrectly appearing in buttons within the demos. Resolves #424 Resolves #409 Resolves #426 NOTE: The additional issues resolved are by-products from switching to focus-trap. BREAKING CHANGE: There are a few changes that need to be taken into account for this commit: - Dialogs no longer require an `aria-hidden="true"` attribute. - Dialogs _do_ require a `style="visibility:hidden"` attribute for correct first render. - `trapFocusOnSurface` and `untrapFocusOnSurface` methods must be implemented for the adapter - `hasClass`, `setAttr`, `registerFocusTrappingHandler`, `deregisterFocusTrappingHandler`, `numFocusableTargets`, `setDialogFocusFirstTarget`, `setInitialFocus`, `getFocusableElements`, `saveElementTabState`, `restoreElementTabState`, `makeElementUntabbable`, `setBodyAttr`, `rmBodyAttr`, `getFocusedTarget`, and `setFocusedTarget` have all been removed from the adapter. - `applyPassive`, `saveElementTabState`, and `restoreElementTabState` have all been removed from `mdcDialog.util`.
traviskaufman
force-pushed
the
fix/dialog-focus
branch
from
14c218c to
a004f17
Compare
replacing it with adapter methods set up to handle focus trapping.
it for implementing the adapter methods at the component level
instances that framework components can use when implementing their
adapters.
tab-focusing works correctly when dialog is hidden.
Resolves #424
Resolves #409
Resolves #426
NOTE: The additional issues resolved are by-products from switching to
focus-trap.
BREAKING CHANGE: There are a few changes that need to be taken into
account for this commit:
aria-hidden="true"attribute.style="visibility:hidden"attribute forcorrect first render.
trapFocusOnSurfaceanduntrapFocusOnSurfacemethods must beimplemented for the adapter
hasClass,setAttr,registerFocusTrappingHandler,deregisterFocusTrappingHandler,numFocusableTargets,setDialogFocusFirstTarget,setInitialFocus,getFocusableElements,saveElementTabState,restoreElementTabState,makeElementUntabbable,setBodyAttr,rmBodyAttr,getFocusedTarget, andsetFocusedTargethave all beenremoved from the adapter.
applyPassive,saveElementTabState, andrestoreElementTabStatehave all been removed from
mdcDialog.util.