From 697fcd7da00ca14a260ecb42fddf45b01afd5643 Mon Sep 17 00:00:00 2001 From: David Wheatley Date: Sun, 21 Nov 2021 19:44:31 +0000 Subject: [PATCH] fix(a11y): add focus traps to modals and nav drawer (#3018) * Add focus trap util * Add focus trap to Modals Fixes #2663 * Split tab press into `onTab` handler * Remove deprecated code * Use requestAnimationFrame instead of setTimeout * Reduce code duplication * Implement focus trap in nav drawer Fixes #2665 * Hide drawer when window is resized to be bigger Fixes issue where focus trap would remain on the drawer when it is just the app header, if the drawer was opened then the window was made larger. * Simplify conditional function calls * Fix modal focus trap * Remove debug code * Simplify resize handler conditional statements * Add info about reasoning of resize handler * Prefer native JS methods over jQuery * Update conditional function call to handle `undefined` * Expose screen sizes as CSS custom properties * Use `window.matchMedia` rather than resize handler * Fix spelling error Co-authored-by: David Sevilla Martin * Remove breaking change Co-authored-by: David Sevilla Martin --- framework/core/js/package.json | 1 + framework/core/js/src/common/compat.js | 2 + .../js/src/common/components/ModalManager.tsx | 34 ++++++++- framework/core/js/src/common/utils/Drawer.js | 74 +++++++++++++++---- .../core/js/src/common/utils/focusTrap.ts | 29 ++++++++ .../core/js/src/forum/components/Search.tsx | 2 +- .../js/src/forum/utils/KeyboardNavigatable.ts | 48 +++++++++--- framework/core/js/yarn.lock | 17 +++++ framework/core/less/common/root.less | 7 ++ 9 files changed, 185 insertions(+), 29 deletions(-) create mode 100644 framework/core/js/src/common/utils/focusTrap.ts diff --git a/framework/core/js/package.json b/framework/core/js/package.json index 0fa66d1f9..c311d4f62 100644 --- a/framework/core/js/package.json +++ b/framework/core/js/package.json @@ -9,6 +9,7 @@ "clsx": "^1.1.1", "color-thief-browser": "^2.0.2", "dayjs": "^1.10.7", + "focus-trap": "^6.7.1", "jquery": "^3.6.0", "jquery.hotkeys": "^0.1.0", "mithril": "^2.0.4", diff --git a/framework/core/js/src/common/compat.js b/framework/core/js/src/common/compat.js index d17bd84cc..9111a642a 100644 --- a/framework/core/js/src/common/compat.js +++ b/framework/core/js/src/common/compat.js @@ -31,6 +31,7 @@ import extractText from './utils/extractText'; import formatNumber from './utils/formatNumber'; import mapRoutes from './utils/mapRoutes'; import withAttr from './utils/withAttr'; +import * as FocusTrap from './utils/focusTrap'; import Notification from './models/Notification'; import User from './models/User'; import Post from './models/Post'; @@ -116,6 +117,7 @@ export default { 'utils/withAttr': withAttr, 'utils/throttleDebounce': ThrottleDebounce, 'utils/isObject': isObject, + 'utils/focusTrap': FocusTrap, 'models/Notification': Notification, 'models/User': User, 'models/Post': Post, diff --git a/framework/core/js/src/common/components/ModalManager.tsx b/framework/core/js/src/common/components/ModalManager.tsx index a9d55a124..d46899eec 100644 --- a/framework/core/js/src/common/components/ModalManager.tsx +++ b/framework/core/js/src/common/components/ModalManager.tsx @@ -1,7 +1,9 @@ import Component from '../Component'; -import type Mithril from 'mithril'; +import { createFocusTrap, FocusTrap } from '../utils/focusTrap'; + import type ModalManagerState from '../states/ModalManagerState'; +import type Mithril from 'mithril'; interface IModalManagerAttrs { state: ModalManagerState; @@ -13,7 +15,14 @@ interface IModalManagerAttrs { * overwrite the previous one. */ export default class ModalManager extends Component { - view() { + protected focusTrap: FocusTrap | undefined; + + /** + * Whether a modal is currently shown by this modal manager. + */ + protected modalShown: boolean = false; + + view(vnode: Mithril.VnodeDOM): Mithril.Children { const modal = this.attrs.state.modal; return ( @@ -29,13 +38,28 @@ export default class ModalManager extends Component { ); } - oncreate(vnode: Mithril.VnodeDOM) { + oncreate(vnode: Mithril.VnodeDOM): void { super.oncreate(vnode); // Ensure the modal state is notified about a closed modal, even when the // DOM-based Bootstrap JavaScript code triggered the closing of the modal, // e.g. via ESC key or a click on the modal backdrop. this.$().on('hidden.bs.modal', this.attrs.state.close.bind(this.attrs.state)); + + this.focusTrap = createFocusTrap(this.element as HTMLElement); + } + + onupdate(vnode: Mithril.VnodeDOM): void { + super.onupdate(vnode); + + requestAnimationFrame(() => { + try { + if (this.modalShown) this.focusTrap!.activate?.(); + else this.focusTrap!.deactivate?.(); + } catch { + // We can expect errors to occur here due to the nature of mithril rendering + } + }); } animateShow(readyCallback: () => void): void { @@ -43,6 +67,8 @@ export default class ModalManager extends Component { const dismissible = !!this.attrs.state.modal.componentClass.isDismissible; + this.modalShown = true; + // If we are opening this modal while another modal is already open, // the shown event will not run, because the modal is already open. // So, we need to manually trigger the readyCallback. @@ -64,5 +90,7 @@ export default class ModalManager extends Component { animateHide(): void { // @ts-expect-error: No typings available for Bootstrap modals. this.$().modal('hide'); + + this.modalShown = false; } } diff --git a/framework/core/js/src/common/utils/Drawer.js b/framework/core/js/src/common/utils/Drawer.js index d0bfa7da6..c0003dd1e 100644 --- a/framework/core/js/src/common/utils/Drawer.js +++ b/framework/core/js/src/common/utils/Drawer.js @@ -1,28 +1,72 @@ +import { createFocusTrap } from './focusTrap'; + /** * The `Drawer` class controls the page's drawer. The drawer is the area the * slides out from the left on mobile devices; it contains the header and the * footer. */ export default class Drawer { + /** + * @type {import('./focusTrap').FocusTrap} + */ + focusTrap; + + /** + * @type {HTMLDivElement} + */ + appElement; + constructor() { // Set up an event handler so that whenever the content area is tapped, // the drawer will close. - $('#content').click((e) => { + document.getElementById('content').addEventListener('click', (e) => { if (this.isOpen()) { e.preventDefault(); this.hide(); } }); + + this.appElement = document.getElementById('app'); + this.focusTrap = createFocusTrap('#drawer', { allowOutsideClick: true }); + this.drawerAvailableMediaQuery = window.matchMedia( + `(max-width: ${getComputedStyle(document.documentElement).getPropertyValue('--screen-phone-max')})` + ); } + /** + * Handler for the `resize` event on `window`. + * + * This is used to close the drawer when the viewport is widened past the `phone` size. + * At this point, the drawer turns into the standard header that we see on desktop, but + * the drawer is still registered as 'open' internally. + * + * This causes issues with the focus trap, resulting in focus becoming trapped within + * the header on desktop viewports. + * + * @internal + */ + resizeHandler = ((e) => { + console.log(this, e); + if (!e.matches && this.isOpen()) { + // Drawer is open but we've made window bigger, so hide it. + this.hide(); + } + }).bind(this); + + /** + * @internal + * @type {MediaQueryList} + */ + drawerAvailableMediaQuery; + /** * Check whether or not the drawer is currently open. * - * @return {Boolean} + * @return {boolean} * @public */ isOpen() { - return $('#app').hasClass('drawerOpen'); + return this.appElement.classList.contains('drawerOpen'); } /** @@ -39,18 +83,19 @@ export default class Drawer { * More info: https://github.com/flarum/core/pull/2666#discussion_r595381014 */ - const $app = $('#app'); + this.focusTrap.deactivate(); + this.drawerAvailableMediaQuery.removeListener(this.resizeHandler); - if (!$app.hasClass('drawerOpen')) return; + if (!this.isOpen()) return; const $drawer = $('#drawer'); // Used to prevent `visibility: hidden` from breaking the exit animation $drawer.css('visibility', 'visible').one('transitionend', () => $drawer.css('visibility', '')); - $app.removeClass('drawerOpen'); + this.appElement.classList.remove('drawerOpen'); - if (this.$backdrop) this.$backdrop.remove(); + this.$backdrop?.remove?.(); } /** @@ -59,13 +104,16 @@ export default class Drawer { * @public */ show() { - $('#app').addClass('drawerOpen'); + this.appElement.classList.add('drawerOpen'); - this.$backdrop = $('
') - .addClass('drawer-backdrop fade') - .appendTo('body') - .click(() => this.hide()); + this.drawerAvailableMediaQuery.addListener(this.resizeHandler); - setTimeout(() => this.$backdrop.addClass('in')); + this.$backdrop = $('
').addClass('drawer-backdrop fade').appendTo('body').on('click', this.hide.bind(this)); + + requestAnimationFrame(() => { + this.$backdrop.addClass('in'); + + this.focusTrap.activate(); + }); } } diff --git a/framework/core/js/src/common/utils/focusTrap.ts b/framework/core/js/src/common/utils/focusTrap.ts new file mode 100644 index 000000000..3724aea9a --- /dev/null +++ b/framework/core/js/src/common/utils/focusTrap.ts @@ -0,0 +1,29 @@ +import { createFocusTrap as _createFocusTrap } from 'focus-trap'; + +/** + * Creates a focus trap for the given element with the given options. + * + * This function applies some default options that are different to the library. + * Your own options still override these custom defaults: + * + * ```json + * { + escapeDeactivates: false, + * } + * ``` + * + * @param element The element to be the focus trap, or a selector that will be used to find the element. + * + * @see https://github.com/focus-trap/focus-trap#readme - Library documentation + */ +function createFocusTrap(...args: Parameters): ReturnType { + args[1] = { + escapeDeactivates: false, + ...args[1], + }; + + return _createFocusTrap(...args); +} + +export * from 'focus-trap'; +export { createFocusTrap }; diff --git a/framework/core/js/src/forum/components/Search.tsx b/framework/core/js/src/forum/components/Search.tsx index bf45be748..169197d13 100644 --- a/framework/core/js/src/forum/components/Search.tsx +++ b/framework/core/js/src/forum/components/Search.tsx @@ -202,7 +202,7 @@ export default class Search extends Compone this.navigator .onUp(() => this.setIndex(this.getCurrentNumericIndex() - 1, true)) .onDown(() => this.setIndex(this.getCurrentNumericIndex() + 1, true)) - .onSelect(this.selectResult.bind(this)) + .onSelect(this.selectResult.bind(this), true) .onCancel(this.clear.bind(this)) .bindTo($input); diff --git a/framework/core/js/src/forum/utils/KeyboardNavigatable.ts b/framework/core/js/src/forum/utils/KeyboardNavigatable.ts index 53b1b2d59..667ac10b5 100644 --- a/framework/core/js/src/forum/utils/KeyboardNavigatable.ts +++ b/framework/core/js/src/forum/utils/KeyboardNavigatable.ts @@ -1,6 +1,18 @@ type KeyboardEventHandler = (event: KeyboardEvent) => void; type ShouldHandle = (event: KeyboardEvent) => boolean; +enum Keys { + Enter = 13, + Escape = 27, + Space = 32, + ArrowUp = 38, + ArrowDown = 40, + ArrowLeft = 37, + ArrowRight = 39, + Tab = 9, + Backspace = 8, +} + /** * The `KeyboardNavigatable` class manages lists that can be navigated with the * keyboard, calling callbacks for each actions. @@ -26,7 +38,7 @@ export default class KeyboardNavigatable { * This will be triggered by the Up key. */ onUp(callback: KeyboardEventHandler): KeyboardNavigatable { - this.callbacks.set(38, (e) => { + this.callbacks.set(Keys.ArrowUp, (e) => { e.preventDefault(); callback(e); }); @@ -40,7 +52,7 @@ export default class KeyboardNavigatable { * This will be triggered by the Down key. */ onDown(callback: KeyboardEventHandler): KeyboardNavigatable { - this.callbacks.set(40, (e) => { + this.callbacks.set(Keys.ArrowDown, (e) => { e.preventDefault(); callback(e); }); @@ -51,16 +63,32 @@ export default class KeyboardNavigatable { /** * Provide a callback to be executed when the current item is selected.. * - * This will be triggered by the Return and Tab keys.. + * This will be triggered by the Return key (and Tab key, if not disabled). */ - onSelect(callback: KeyboardEventHandler): KeyboardNavigatable { + onSelect(callback: KeyboardEventHandler, ignoreTabPress: boolean = false): KeyboardNavigatable { + const handler: KeyboardEventHandler = (e) => { + e.preventDefault(); + callback(e); + }; + + if (!ignoreTabPress) this.callbacks.set(Keys.Tab, handler); + this.callbacks.set(Keys.Enter, handler); + + return this; + } + + /** + * Provide a callback to be executed when the current item is tabbed into. + * + * This will be triggered by the Tab key. + */ + onTab(callback: KeyboardEventHandler): KeyboardNavigatable { const handler: KeyboardEventHandler = (e) => { e.preventDefault(); callback(e); }; this.callbacks.set(9, handler); - this.callbacks.set(13, handler); return this; } @@ -71,7 +99,7 @@ export default class KeyboardNavigatable { * This will be triggered by the Escape key. */ onCancel(callback: KeyboardEventHandler): KeyboardNavigatable { - this.callbacks.set(27, (e) => { + this.callbacks.set(Keys.Escape, (e) => { e.stopPropagation(); e.preventDefault(); callback(e); @@ -84,13 +112,9 @@ export default class KeyboardNavigatable { * Provide a callback to be executed when previous input is removed. * * This will be triggered by the Backspace key. - * - * @public - * @param {KeyboardNavigatable~keyCallback} callback - * @return {KeyboardNavigatable} */ onRemove(callback: KeyboardEventHandler): KeyboardNavigatable { - this.callbacks.set(8, (e) => { + this.callbacks.set(Keys.Backspace, (e) => { if (e instanceof KeyboardEvent && e.target instanceof HTMLInputElement && e.target.selectionStart === 0 && e.target.selectionEnd === 0) { callback(e); e.preventDefault(); @@ -112,7 +136,7 @@ export default class KeyboardNavigatable { /** * Set up the navigation key bindings on the given jQuery element. */ - bindTo($element: JQuery) { + bindTo($element: JQuery) { // Handle navigation key events on the navigatable element. $element[0].addEventListener('keydown', this.navigate.bind(this)); } diff --git a/framework/core/js/yarn.lock b/framework/core/js/yarn.lock index 6ba8222f7..ee08d71c4 100644 --- a/framework/core/js/yarn.lock +++ b/framework/core/js/yarn.lock @@ -1404,6 +1404,7 @@ __metadata: expose-loader: ^3.1.0 flarum-tsconfig: ^1.0.2 flarum-webpack-config: ^2.0.0 + focus-trap: ^6.7.1 jquery: ^3.6.0 jquery.hotkeys: ^0.1.0 mithril: ^2.0.4 @@ -2565,6 +2566,15 @@ __metadata: languageName: node linkType: hard +"focus-trap@npm:^6.7.1": + version: 6.7.1 + resolution: "focus-trap@npm:6.7.1" + dependencies: + tabbable: ^5.2.1 + checksum: b96c54a6a2976f8509ed8447ce3a8b76db3801b9c170f278f60b0c878478f2bb2ebc6dbe3ccd7157006b9a7ad9a86c18283efff0f3e387e29ba3ea89d8687b9c + languageName: node + linkType: hard + "follow-redirects@npm:^1.14.0": version: 1.14.5 resolution: "follow-redirects@npm:1.14.5" @@ -3896,6 +3906,13 @@ __metadata: languageName: node linkType: hard +"tabbable@npm:^5.2.1": + version: 5.2.1 + resolution: "tabbable@npm:5.2.1" + checksum: d26e9eeb880c4c78b59244bac2c931ad46f6c64a01e5c15ba6da348dc86442222912733846a9e63373342a81fa15d4afb31267606b38431510d10b0fb9ec9bba + languageName: node + linkType: hard + "tapable@npm:^2.1.1, tapable@npm:^2.2.0": version: 2.2.1 resolution: "tapable@npm:2.2.1" diff --git a/framework/core/less/common/root.less b/framework/core/less/common/root.less index 2790ef75b..1b0e6988a 100644 --- a/framework/core/less/common/root.less +++ b/framework/core/less/common/root.less @@ -105,6 +105,13 @@ // available to the JS code. --flarum-screen: none; + --screen-phone-max: @screen-phone-max; + --screen-tablet: @screen-tablet; + --screen-tablet-max: @screen-tablet-max; + --screen-desktop: @screen-desktop; + --screen-desktop-max: @screen-desktop-max; + --screen-desktop-hd: @screen-desktop-hd; + @media @phone { --flarum-screen: phone; } @media @tablet { --flarum-screen: tablet; } @media @desktop { --flarum-screen: desktop; }