From 14c3380f3023a502d1f16eb8d4d4c2addef0718f Mon Sep 17 00:00:00 2001 From: Mark Date: Mon, 28 Nov 2022 10:57:08 +0000 Subject: [PATCH] Improve the menubar accessibility (#465) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Bring keyboard handling closer in line with WCAG2.1 * Code cleanup * Fix focusing the menubar on event * Make the tests check that the activeElement is a descendant * More work on the tests * Fixed the focus / tab tests * Added missing test for the space activation * Cleanup the example * Switch to the ARIA menubar example interaction pattern * Set the activeIndex in onActivateRequest * Remove old code * Code cleanup * Remove test for scenario that cannot occur * Remember the last focused menu item * Test fix * Fix license header job * Update examples/example-menubar/package.json Co-authored-by: Frédéric Collonval * Small rewordings * Small doc changes * Widgets API update Co-authored-by: Frédéric Collonval --- examples/example-menubar/index.html | 16 +++ examples/example-menubar/package.json | 21 ++++ examples/example-menubar/rollup.config.js | 8 ++ examples/example-menubar/src/index.ts | 121 +++++++++++++++++++++ examples/example-menubar/style/content.css | 30 +++++ examples/example-menubar/style/index.css | 19 ++++ examples/example-menubar/tsconfig.json | 17 +++ packages/widgets/src/menubar.ts | 38 +++++-- packages/widgets/tests/src/menubar.spec.ts | 64 ++++++++++- review/api/widgets.api.md | 1 + 10 files changed, 323 insertions(+), 12 deletions(-) create mode 100644 examples/example-menubar/index.html create mode 100644 examples/example-menubar/package.json create mode 100644 examples/example-menubar/rollup.config.js create mode 100644 examples/example-menubar/src/index.ts create mode 100644 examples/example-menubar/style/content.css create mode 100644 examples/example-menubar/style/index.css create mode 100644 examples/example-menubar/tsconfig.json diff --git a/examples/example-menubar/index.html b/examples/example-menubar/index.html new file mode 100644 index 000000000..37608f290 --- /dev/null +++ b/examples/example-menubar/index.html @@ -0,0 +1,16 @@ + + + + + + + + + MenuBar Example + + + + diff --git a/examples/example-menubar/package.json b/examples/example-menubar/package.json new file mode 100644 index 000000000..03ab6fed3 --- /dev/null +++ b/examples/example-menubar/package.json @@ -0,0 +1,21 @@ +{ + "name": "@lumino/example-menubar", + "version": "0.1.0-alpha.6", + "private": true, + "scripts": { + "build": "tsc && rollup -c", + "clean": "rimraf build" + }, + "dependencies": { + "@lumino/default-theme": "^1.0.0-alpha.6", + "@lumino/messaging": "^2.0.0-alpha.6", + "@lumino/widgets": "^2.0.0-alpha.6" + }, + "devDependencies": { + "@rollup/plugin-node-resolve": "^13.3.0", + "rimraf": "^3.0.2", + "rollup": "^2.77.3", + "rollup-plugin-styles": "^4.0.0", + "typescript": "~4.7.3" + } +} diff --git a/examples/example-menubar/rollup.config.js b/examples/example-menubar/rollup.config.js new file mode 100644 index 000000000..11c401c1f --- /dev/null +++ b/examples/example-menubar/rollup.config.js @@ -0,0 +1,8 @@ +/* + * Copyright (c) Jupyter Development Team. + * Distributed under the terms of the Modified BSD License. + */ + +import { createRollupConfig } from '../../rollup.examples.config'; +const rollupConfig = createRollupConfig(); +export default rollupConfig; diff --git a/examples/example-menubar/src/index.ts b/examples/example-menubar/src/index.ts new file mode 100644 index 000000000..6df657ea8 --- /dev/null +++ b/examples/example-menubar/src/index.ts @@ -0,0 +1,121 @@ +// Copyright (c) Jupyter Development Team. +// Distributed under the terms of the Modified BSD License. + +import { CommandRegistry } from '@lumino/commands'; +import { Menu, MenuBar, PanelLayout, Widget } from '@lumino/widgets'; + +import '../style/index.css'; + +/** + * Wrapper widget containing the example application. + */ +class Application extends Widget { + constructor() { + super({ tag: 'main' }); + } +} + +/** + * Skip link to jump to the main content. + */ +class SkipLink extends Widget { + /** + * Create a HTMLElement that statically links to "#content". + */ + static createNode(): HTMLElement { + const node = document.createElement('a'); + node.setAttribute('href', '#content'); + node.innerHTML = 'Skip to the main content'; + node.classList.add('lm-example-skip-link'); + return node; + } + + constructor() { + super({ node: SkipLink.createNode() }); + } +} + +/** + * A Widget containing some content to provide context example. + */ +class Article extends Widget { + /** + * Create the content structure. + */ + static createNode(): HTMLElement { + const node = document.createElement('div'); + node.setAttribute('id', 'content'); + node.setAttribute('tabindex', '-1'); + const h1 = document.createElement('h1'); + h1.innerHTML = 'MenuBar Example'; + node.appendChild(h1); + const button = document.createElement('button'); + button.innerHTML = 'A button you can tab to out of the menubar'; + node.appendChild(button); + return node; + } + + constructor() { + super({ node: Article.createNode() }); + } +} + +/** + * Helper Function to add menu items. + */ +function addMenuItem( + commands: CommandRegistry, + menu: Menu, + command: string, + label: string, + log: string +): void { + commands.addCommand(command, { + label: label, + execute: () => { + console.log(log); + } + }); + menu.addItem({ + type: 'command', + command: command + }); +} + +/** + * Create the MenuBar example application. + */ +function main(): void { + const app = new Application(); + const appLayout = new PanelLayout(); + app.layout = appLayout; + + const skipLink = new SkipLink(); + + const menubar = new MenuBar(); + const commands = new CommandRegistry(); + + const fileMenu = new Menu({ commands: commands }); + fileMenu.title.label = 'File'; + addMenuItem(commands, fileMenu, 'new', 'New', 'File > New'); + addMenuItem(commands, fileMenu, 'open', 'Open', 'File > Open'); + addMenuItem(commands, fileMenu, 'save', 'Save', 'File > Save'); + menubar.addMenu(fileMenu); + + const editMenu = new Menu({ commands: commands }); + editMenu.title.label = 'Edit'; + addMenuItem(commands, editMenu, 'cut', 'Cut', 'Edit > Cut'); + addMenuItem(commands, editMenu, 'copy', 'Copy', 'Edit > Copy'); + addMenuItem(commands, editMenu, 'paste', 'Paste', 'Edit > Paste'); + menubar.addMenu(editMenu); + + const article = new Article(); + + appLayout.addWidget(skipLink); + appLayout.addWidget(menubar); + appLayout.addWidget(article); + + Widget.attach(app, document.body); +} + +window.onload = main; diff --git a/examples/example-menubar/style/content.css b/examples/example-menubar/style/content.css new file mode 100644 index 000000000..6f103a529 --- /dev/null +++ b/examples/example-menubar/style/content.css @@ -0,0 +1,30 @@ +/* + Copyright (c) Jupyter Development Team. + Distributed under the terms of the Modified BSD License. +*/ + +.lm-example-skip-link { + position: absolute; + left: -1000px; + top: -1000px; +} + +.lm-example-skip-link:focus { + position: fixed; + left: 50%; + top: 0; + z-index: 10; + padding: 0.5rem 1rem; + box-shadow: 0 0 5px #000; + border-bottom-left-radius: 0.2rem; + border-bottom-right-radius: 0.2rem; + background: #fff; +} + +textarea { + display: block; +} + +#content { + padding: 0 1rem; +} diff --git a/examples/example-menubar/style/index.css b/examples/example-menubar/style/index.css new file mode 100644 index 000000000..b95a371ce --- /dev/null +++ b/examples/example-menubar/style/index.css @@ -0,0 +1,19 @@ +/* + Copyright (c) Jupyter Development Team. + Distributed under the terms of the Modified BSD License. +*/ +@import '@lumino/default-theme/style/index.css'; +@import './content.css'; + +body { + display: flex; + flex-direction: column; + position: absolute; + top: 0; + left: 0; + right: 0; + bottom: 0; + margin: 0; + padding: 0; + overflow: hidden; +} diff --git a/examples/example-menubar/tsconfig.json b/examples/example-menubar/tsconfig.json new file mode 100644 index 000000000..b14ef2b28 --- /dev/null +++ b/examples/example-menubar/tsconfig.json @@ -0,0 +1,17 @@ +{ + "compilerOptions": { + "declaration": false, + "noImplicitAny": true, + "noEmitOnError": true, + "noUnusedLocals": true, + "strictNullChecks": true, + "sourceMap": true, + "module": "ES6", + "moduleResolution": "node", + "target": "ES2018", + "outDir": "./build", + "lib": ["DOM", "ES2018"], + "types": [] + }, + "include": ["src/*"] +} diff --git a/packages/widgets/src/menubar.ts b/packages/widgets/src/menubar.ts index fbf9d066b..3aebc4e6d 100644 --- a/packages/widgets/src/menubar.ts +++ b/packages/widgets/src/menubar.ts @@ -134,6 +134,11 @@ export class MenuBar extends Widget { // Update the active index. this._activeIndex = value; + // Update the focus index. + if (value !== -1) { + this._tabFocusIndex = value; + } + // Update focus to new active index if ( this._activeIndex >= 0 && @@ -378,7 +383,7 @@ export class MenuBar extends Widget { */ protected onActivateRequest(msg: Message): void { if (this.isAttached) { - this.node.focus(); + this.activeIndex = 0; } } @@ -389,6 +394,10 @@ export class MenuBar extends Widget { let menus = this._menus; let renderer = this.renderer; let activeIndex = this._activeIndex; + let tabFocusIndex = + this._tabFocusIndex >= 0 && this._tabFocusIndex < menus.length + ? this._tabFocusIndex + : 0; let content = new Array(menus.length); for (let i = 0, n = menus.length; i < n; ++i) { let title = menus[i].title; @@ -399,6 +408,7 @@ export class MenuBar extends Widget { content[i] = renderer.renderItem({ title, active, + tabbable: i === tabFocusIndex, onfocus: () => { this.activeIndex = i; } @@ -417,8 +427,9 @@ export class MenuBar extends Widget { // Fetch the key code for the event. let kc = event.keyCode; - // Do not trap the tab key. + // Reset the active index on tab, but do not trap the tab key. if (kc === 9) { + this.activeIndex = -1; return; } @@ -426,8 +437,8 @@ export class MenuBar extends Widget { event.preventDefault(); event.stopPropagation(); - // Enter, Up Arrow, Down Arrow - if (kc === 13 || kc === 38 || kc === 40) { + // Enter, Space, Up Arrow, Down Arrow + if (kc === 13 || kc === 32 || kc === 38 || kc === 40) { this.openActiveMenu(); return; } @@ -652,7 +663,6 @@ export class MenuBar extends Widget { if (!this._childMenu) { return; } - // Remove the active class from the menu bar. this.removeClass('lm-mod-active'); @@ -726,7 +736,10 @@ export class MenuBar extends Widget { this.update(); } + // Track the index of the item that is currently focused. -1 means nothing focused. private _activeIndex = -1; + // Track which item can be focused using the TAB key. Unlike _activeIndex will always point to a menuitem. + private _tabFocusIndex = 0; private _forceItemsPosition: Menu.IOpenOptions; private _menus: Menu[] = []; private _childMenu: Menu | null = null; @@ -772,6 +785,11 @@ export namespace MenuBar { */ readonly active: boolean; + /** + * Whether the user can tab to the item. + */ + readonly tabbable: boolean; + readonly onfocus?: (event: FocusEvent) => void; } @@ -808,7 +826,13 @@ export namespace MenuBar { let dataset = this.createItemDataset(data); let aria = this.createItemARIA(data); return h.li( - { className, dataset, tabindex: '0', onfocus: data.onfocus, ...aria }, + { + className, + dataset, + tabindex: data.tabbable ? '0' : '-1', + onfocus: data.onfocus, + ...aria + }, this.renderIcon(data), this.renderLabel(data) ); @@ -941,8 +965,6 @@ namespace Private { content.className = 'lm-MenuBar-content'; node.appendChild(content); content.setAttribute('role', 'menubar'); - node.tabIndex = 0; - content.tabIndex = 0; return node; } diff --git a/packages/widgets/tests/src/menubar.spec.ts b/packages/widgets/tests/src/menubar.spec.ts index d582bb5c0..b8694b35e 100644 --- a/packages/widgets/tests/src/menubar.spec.ts +++ b/packages/widgets/tests/src/menubar.spec.ts @@ -74,6 +74,15 @@ describe('@lumino/widgets', () => { return bar; } + /** + * Create a MenuBar that has no active menu item. + */ + function createUnfocusedMenuBar(): MenuBar { + const bar = createMenuBar(); + bar.activeIndex = -1; + return bar; + } + before(() => { commands = new CommandRegistry(); const iconRenderer = { @@ -486,6 +495,17 @@ describe('@lumino/widgets', () => { expect(menu.isAttached).to.equal(true); }); + it('should open the active menu on Space', () => { + let menu = bar.activeMenu!; + bar.node.dispatchEvent( + new KeyboardEvent('keydown', { + bubbles, + keyCode: 32 + }) + ); + expect(menu.isAttached).to.equal(true); + }); + it('should open the active menu on Up Arrow', () => { let menu = bar.activeMenu!; bar.node.dispatchEvent( @@ -760,6 +780,36 @@ describe('@lumino/widgets', () => { expect(cancelled).to.equal(true); }); }); + + context('focus', () => { + it('should lose focus on tab key', () => { + let bar = createUnfocusedMenuBar(); + bar.activeIndex = 0; + expect(bar.contentNode.contains(document.activeElement)).to.equal( + true + ); + let event = new KeyboardEvent('keydown', { keyCode: 9, bubbles }); + bar.contentNode.dispatchEvent(event); + expect(bar.activeIndex).to.equal(-1); + bar.dispose(); + }); + + it('should lose focus on shift-tab key', () => { + let bar = createUnfocusedMenuBar(); + bar.activeIndex = 0; + expect(bar.contentNode.contains(document.activeElement)).to.equal( + true + ); + let event = new KeyboardEvent('keydown', { + keyCode: 9, + shiftKey: true, + bubbles + }); + bar.contentNode.dispatchEvent(event); + expect(bar.activeIndex).to.equal(-1); + bar.dispose(); + }); + }); }); describe('#onBeforeAttach()', () => { @@ -808,14 +858,19 @@ describe('@lumino/widgets', () => { let bar = createMenuBar(); Widget.detach(bar); MessageLoop.sendMessage(bar, Widget.Msg.ActivateRequest); - expect(bar.node.contains(document.activeElement)).to.equal(false); + expect(bar.contentNode.contains(document.activeElement)).to.equal( + false + ); bar.dispose(); }); it('should focus the node if attached', () => { - let bar = createMenuBar(); + let bar = createUnfocusedMenuBar(); MessageLoop.sendMessage(bar, Widget.Msg.ActivateRequest); - expect(bar.node.contains(document.activeElement)).to.equal(true); + expect( + bar.contentNode.contains(document.activeElement) && + bar.contentNode !== document.activeElement + ).to.equal(true); bar.dispose(); }); }); @@ -885,7 +940,8 @@ describe('@lumino/widgets', () => { widget.title.closable = true; data = { title: widget.title, - active: true + active: true, + tabbable: true }; }); diff --git a/review/api/widgets.api.md b/review/api/widgets.api.md index 54f23ffac..0453c8a5d 100644 --- a/review/api/widgets.api.md +++ b/review/api/widgets.api.md @@ -777,6 +777,7 @@ export namespace MenuBar { readonly active: boolean; // (undocumented) readonly onfocus?: (event: FocusEvent) => void; + readonly tabbable: boolean; readonly title: Title; } export interface IRenderer {