Skip to content
This repository was archived by the owner on Jan 13, 2025. It is now read-only.

Commit 12bd03e

Browse files
fix(dialog): Handle focus trapping correctly (#491)
- 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`.
1 parent ff772ad commit 12bd03e

File tree

11 files changed

+205
-521
lines changed

11 files changed

+205
-521
lines changed

demos/dialog.html

+7-2
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
<body class="mdc-typography">
4242
<div class="demo-body">
4343
<aside id="mdc-dialog-default"
44+
style="visibility:hidden;"
4445
class="mdc-dialog"
4546
role="alertdialog"
4647
aria-hidden="true"
@@ -64,6 +65,7 @@ <h2 id="mdc-dialog-default-label" class="mdc-dialog__header__title">
6465
</aside>
6566

6667
<aside id="mdc-dialog-with-list"
68+
style="visibility:hidden;"
6769
class="mdc-dialog"
6870
role="alertdialog"
6971
aria-hidden="true"
@@ -186,8 +188,11 @@ <h2>MDC Web Dialog</h2>
186188
document.body.classList[evt.target.checked ? 'add' : 'remove']('mdc-theme--dark');
187189
});
188190

189-
mdc.ripple.MDCRipple.attachTo(document.querySelector('#default-dialog-activation'));
190-
mdc.ripple.MDCRipple.attachTo(document.querySelector('#dialog-with-list-activation'));
191+
// Hack to work around style-loader async loading styles
192+
setTimeout(function() {
193+
mdc.ripple.MDCRipple.attachTo(document.querySelector('#default-dialog-activation'));
194+
mdc.ripple.MDCRipple.attachTo(document.querySelector('#dialog-with-list-activation'));
195+
}, 200);
191196
})();
192197
</script>
193198
</body>

karma.conf.js

+1
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ module.exports = function(config) {
137137
rules: webpackConfig.module.rules.concat([config.singleRun ? {
138138
test: /\.js$/,
139139
include: path.resolve('./packages'),
140+
exclude: /node_modules/,
140141
loader: 'istanbul-instrumenter-loader',
141142
query: {esModules: true},
142143
} : undefined]).filter(Boolean),

packages/mdc-dialog/README.md

+67-50
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ Dialogs inform users about a specific task and may contain critical information
1818

1919
```html
2020
<aside id="my-mdc-dialog"
21+
style="visibility:hidden"
2122
class="mdc-dialog"
2223
role="alertdialog"
23-
aria-hidden="true"
2424
aria-labelledby="my-mdc-dialog-label"
2525
aria-describedby="my-mdc-dialog-description">
2626
<div class="mdc-dialog__surface">
@@ -41,20 +41,17 @@ Dialogs inform users about a specific task and may contain critical information
4141
</aside>
4242
```
4343

44-
> **NOTE**: The `.mdc-dialog__footer__button--accept` element _must_ be the _final focusable element_ within the dialog
45-
in order for this component to function properly.
46-
47-
In the example above, we've created a dialog box in an `aside` element. Note that you can place content inside
44+
In the example above, we've created a dialog box in an `aside` element. Note that you can place content inside
4845
the dialog. There are two types: dialog & dialogs with scrollable content. These are declared using CSS classes.
4946

50-
Some dialogs will not be tall enough to accomodate everything you would like to display in them. For this there is a
47+
Some dialogs will not be tall enough to accomodate everything you would like to display in them. For this there is a
5148
`mdc-dialog__body--scrollable` modifier to allow scrolling in the dialog.
5249

5350
```html
5451
<aside id="mdc-dialog-with-list"
52+
style="visibility:hidden"
5553
class="mdc-dialog"
5654
role="alertdialog"
57-
aria-hidden="true"
5855
aria-labelledby="mdc-dialog-with-list-label"
5956
aria-describedby="mdc-dialog-with-list-description">
6057
<div class="mdc-dialog__surface">
@@ -76,14 +73,14 @@ Some dialogs will not be tall enough to accomodate everything you would like to
7673
<li class="mdc-list-item">Luna</li>
7774
<li class="mdc-list-item">Marimba</li>
7875
<li class="mdc-list-item">Schwifty</li>
79-
</ul>
76+
</ul>
8077
</section>
8178
<footer class="mdc-dialog__footer">
8279
<button type="button" class="mdc-button mdc-dialog__footer__button mdc-dialog__footer__button--cancel">Decline</button>
8380
<button type="button" class="mdc-button mdc-dialog__footer__button mdc-dialog__footer__button--accept">Accept</button>
8481
</footer>
8582
</div>
86-
<div class="mdc-dialog__backdrop"></div>
83+
<div class="mdc-dialog__backdrop"></div>
8784
</aside>
8885
```
8986

@@ -100,7 +97,7 @@ correct dialog behaviors into idiomatic components.
10097
##### ES2015
10198

10299
```javascript
103-
import {MDCDialog, MDCDialogFoundation} from 'mdc-dialog';
100+
import {MDCDialog, MDCDialogFoundation, util} from 'mdc-dialog';
104101
```
105102

106103
##### CommonJS
@@ -109,6 +106,7 @@ import {MDCDialog, MDCDialogFoundation} from 'mdc-dialog';
109106
const mdcDialog = require('mdc-dialog');
110107
const MDCDialog = mdcDialog.MDCDialog;
111108
const MDCDialogFoundation = mdcDialog.MDCDialogFoundation;
109+
const util = mdcDialog.util;
112110
```
113111

114112
##### AMD
@@ -117,6 +115,7 @@ const MDCDialogFoundation = mdcDialog.MDCDialogFoundation;
117115
require(['path/to/mdc-dialog'], mdcDialog => {
118116
const MDCDialog = mdcDrawer.MDCDialog;
119117
const MDCDialogFoundation = mdcDialog.MDCDialogFoundation;
118+
const util = mdcDialog.util;
120119
});
121120
```
122121

@@ -125,6 +124,7 @@ require(['path/to/mdc-dialog'], mdcDialog => {
125124
```javascript
126125
const MDCDialog = mdc.dialog.MDCDialog;
127126
const MDCDialogFoundation = mdc.dialog.MDCDialogFoundation;
127+
const util = mdc.dialog.util;
128128
```
129129

130130
#### Automatic Instantiation
@@ -139,7 +139,7 @@ mdc.dialog.MDCDialog.attachTo(document.querySelector('#my-mdc-dialog'));
139139

140140
#### Manual Instantiation
141141

142-
Dialogs can easily be initialized using their default constructors as well, similar to `attachTo`.
142+
Dialogs can easily be initialized using their default constructors as well, similar to `attachTo`.
143143

144144
```javascript
145145
import {MDCDialog} from 'mdc-dialog';
@@ -167,27 +167,13 @@ document.querySelector('#default-dialog-activation').addEventListener('click', f
167167

168168
### Dialog component API
169169

170-
#### MDCDialog.open
170+
#### MDCDialog.open
171171

172172
Boolean. True when the dialog is shown, false otherwise.
173173

174-
#### MDCDialog.lastFocusedTarget
175-
176-
EventTarget, usually an HTMLElement. Represents the element that was focused on the page before the dialog is shown. If set,
177-
the dialog will return focus to this element when closed. _This property should be set before calls to show()_.
178-
179-
180-
#### MDCDialog.initialize() => void
181-
182-
Attaches ripples to the dialog footer buttons
183-
184-
#### MDCDialog.destroy() => void
185-
186-
Cleans up ripples when dialog is destroyed
187-
188174
#### MDCDialog.show() => void
189175

190-
Shows the dialog
176+
Shows the dialog
191177

192178
#### MDCDialog.close() => void
193179

@@ -208,17 +194,16 @@ Broadcast when a user actions on the `.mdc-dialog__footer__button--cancel` eleme
208194
MDC Dialog ships with an `MDCDialogFoundation` class that external frameworks and libraries can
209195
use to integrate the component. As with all foundation classes, an adapter object must be provided.
210196

211-
> **NOTE**: Components themselves must manage adding ripples to dialog buttons, should they choose to
197+
> **NOTE**: Components themselves must manage adding ripples to dialog buttons, should they choose to
212198
do so. We provide instructions on how to add ripples to buttons within the [mdc-button README](https://github.com/material-components/material-components-web/tree/master/packages/mdc-button#adding-ripples-to-buttons).
213199

214200
### Adapter API
215201

216202
| Method Signature | Description |
217203
| --- | --- |
218-
| `hasClass(className: string) => boolean` | Returns boolean indicating whether the root has a given class. |
219204
| `addClass(className: string) => void` | Adds a class to the root element. |
220205
| `removeClass(className: string) => void` | Removes a class from the root element. |
221-
| `setAttr(attr: string, val: string) => void` | Sets the given attribute to the given value on the root element. |
206+
| `setStyle(propertyName: string, value: string) => void` | Sets a style property `propertyName` on the root element to the `value` specified |
222207
| `addBodyClass(className: string) => void` | Adds a class to the body. |
223208
| `removeBodyClass(className: string) => void` | Removes a class from the body. |
224209
| `eventTargetHasClass(target: EventTarget, className: string) => boolean` | Returns true if target has className, false otherwise. |
@@ -228,45 +213,77 @@ do so. We provide instructions on how to add ripples to buttons within the [mdc-
228213
| `deregisterSurfaceInteractionHandler(evt: string, handler: EventListener) => void` | Deregisters an event handler from the dialog surface element. |
229214
| `registerDocumentKeydownHandler(handler: EventListener) => void` | Registers an event handler on the `document` object for a `keydown` event. |
230215
| `deregisterDocumentKeydownHandler(handler: EventListener) => void` | Deregisters an event handler on the `document` object for a `keydown` event. |
231-
| `registerFocusTrappingHandler(handler: EventListener) => void` | Registers a focus event listener with a given handler at the capture phase on the document. |
232-
| `deregisterFocusTrappingHandler(handler: EventListener) => void` | Deregisters a focus event listener from a given handler at the capture phase on the document. |
233-
| `numFocusableTargets() => number` | Returns the number of focusable elements in the dialog |
234-
| `setDialogFocusFirstTarget() => void` | Resets focus to the first focusable element in the dialog |
235-
| `setInitialFocus() => void` | Sets focus on the `mdc-dialog__footer__button--accept` element. |
236-
| `getFocusableElements() => Array<Element>` | Returns the list of focusable elements inside the dialog. |
237-
| `saveElementTabState(el: Element) => void` | Saves the current tab index for the element in a way which it can be retrieved later on. |
238-
| `restoreElementTabState(el: Element) => void` | Restores the saved tab index (if any) for an element. |
239-
| `makeElementUntabbable(el: Element) => void` | Makes an element untabbable, e.g. by setting the `tabindex` to `-1`. |
240-
| `setBodyAttr(attr: string, val: string) => void` | Sets the given attribute to the given value on the body element. |
241-
| `rmBodyAttr(attr: string) => void` | Removes the given attribute from the body element. |
242-
| `getFocusedTarget() => Element` | Returns the currently focused element, e.g. `document.activeElement`. |
243-
| `setFocusedTarget(target: EventTarget) => void` | Sets focus on the given target, e.g. by calling `focus()` |
244216
| `notifyAccept() => {}` | Broadcasts an event denoting that the user has accepted the dialog. |
245217
| `notifyCancel() => {}` | Broadcasts an event denoting that the user has cancelled the dialog. |
218+
| `trapFocusOnSurface() => {}` | Sets up the DOM which the dialog is contained in such that focusability is restricted to the elements on the dialog surface (see [Handling Focus Trapping](#handling-focus-trapping) below for more details). |
219+
| `untrapFocusOnSurface() => {}` | Removes any affects of focus trapping on the dialog surface from the DOM (see [Handling Focus Trapping](#handling-focus-trapping) below for more details). |
220+
221+
#### Handling Focus Trapping
246222

223+
In order for dialogs to be fully accessible, they must conform to the guidelines outlined in
224+
https://www.w3.org/TR/wai-aria-practices/#dialog_modal. The main implication of these guidelines is
225+
that the only focusable elements are those contained within a dialog surface.
226+
227+
Trapping focus correctly for a modal dialog requires a complex set of events and interaction
228+
patterns that we feel is best not duplicated within the logic of this component. Furthermore,
229+
frameworks and libraries may have their own ways of trapping focus that framework authors may want
230+
to make use of. For this reason, we have two methods on the adapter that should be used to handle
231+
focus trapping:
232+
233+
- *trapFocusOnSurface()* is called when the dialog is open and should set up focus trapping adhering
234+
to the ARIA practices in the link above.
235+
- *untrapFocusOnSurface()* is called when the dialog is closed and should tear down any focus
236+
trapping set up when the dialog was open.
237+
238+
In our `MDCDialog` component, we use the [focus-trap](https://github.com/davidtheclark/focus-trap)
239+
package to handle this. **You can use [util.createFocusTrapInstance](#mdcdialog-util-api) to easily
240+
create a focus trapping solution for your component code.**
247241

248242
### The full foundation API
249243

250-
#### MDCDialogFoundation.open() => void
244+
#### MDCDialogFoundation.open() => void
251245

252-
Opens the dialog, registers appropriate event listners, sets aria attributes, focuses elements.
246+
Opens the dialog, registers appropriate event listeners, sets aria attributes, focuses elements.
253247

254-
#### MDCDialogFoundation.close() => void
248+
#### MDCDialogFoundation.close() => void
255249

256-
Closes the dialog, deregisters appropriate event listners, resets aria attributes, focuses elements.
250+
Closes the dialog, deregisters appropriate event listeners, resets aria attributes, focuses
251+
elements.
257252

258-
#### MDCDialogFoundation.accept(notifyChange = false) => void
253+
#### MDCDialogFoundation.accept(notifyChange = false) => void
259254

260255
Closes the dialog. If `notifyChange` is true, calls the adapter's `notifyAccept()` method.
261256

262-
#### MDCDialogFoundation.cancel(notifyChange = false) => void
257+
#### MDCDialogFoundation.cancel(notifyChange = false) => void
263258

264259
Closes the dialog. If `notifyChange` is true, calls the adapter's `notifyCancel()` method.
265260

266-
#### MDCDialogFoundation.isOpen() => Boolean
261+
#### MDCDialogFoundation.isOpen() => Boolean
267262

268263
Returns true if the dialog is open, false otherwise.
269264

265+
### MDCDialog Util API
266+
267+
#### util.createFocusTrapInstance(surfaceEl, acceptButtonEl, focusTrapFactory = require('focus-trap')) => {activate: () => {}, deactivate: () => {}};
268+
269+
Given a dialog surface element, an accept button element, and an optional focusTrap factory
270+
function, creates a properly configured [focus-trap](https://github.com/davidtheclark/focus-trap)
271+
instance such that:
272+
273+
- The focus is trapped within the `surfaceEl`
274+
- The `acceptButtonEl` receives focus when the focus trap is activated
275+
- Pressing the `escape` key deactivates focus
276+
- Clicking outside the dialog deactivates focus
277+
- Focus is returned to the previously focused element before the focus trap was activated
278+
279+
This focus trap instance can be used to implement the `trapFocusOnSurface` and
280+
`untrapFocusOnSurface` adapter methods by calling `instance.activate()` and `instance.deactivate()`
281+
respectively within those methods.
282+
283+
The `focusTrapFactory` can be used to override the `focus-trap` function used to create the focus
284+
trap. It's API is the same as focus-trap's [createFocusTrap](https://github.com/davidtheclark/focus-trap#focustrap--createfocustrapelement-createoptions) (which is what it defaults to). You can pass in a custom function for mocking out the
285+
actual function within tests, or to modify the arguments passed to the function before it's called.
286+
270287
## Theming - Dark Theme Considerations
271288

272289
When using `mdc-theme--dark` / `mdc-dialog--theme-dark`, the dialog by default sets its background color to `#303030`. You can override this by either overridding the

packages/mdc-dialog/constants.js

-2
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,4 @@ export const strings = {
2727
OPEN_DIALOG_SELECTOR: '.mdc-dialog--open',
2828
DIALOG_SURFACE_SELECTOR: '.mdc-dialog__surface',
2929
ACCEPT_SELECTOR: '.mdc-dialog__footer__button--accept',
30-
FOCUSABLE_ELEMENTS: 'a[href], area[href], input:not([disabled]), select:not([disabled]), textarea:not([disabled]), ' +
31-
'button:not([disabled]), iframe, object, embed, [tabindex], [contenteditable]',
3230
};

0 commit comments

Comments
 (0)