Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the menubar accessibility #465

Merged
merged 20 commits into from
Nov 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/license-header.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:
id: changed-files
uses: tj-actions/changed-files@v34.3.2
with:
base_sha: 'HEAD~1'
since_last_remote_commit: "true"
sha: 'HEAD'

- name: Push fixes
Expand Down
16 changes: 16 additions & 0 deletions examples/example-menubar/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<!--
~ Copyright (c) Jupyter Development Team.
~ Distributed under the terms of the Modified BSD License.
-->

<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<link href="https://maxcdn.bootstrapcdn.com/font-awesome/4.2.0/css/font-awesome.min.css" rel="stylesheet">
<script type="text/javascript" src="build/bundle.example.js"></script>
<title>MenuBar Example</title>
</head>
<body>
</body>
</html>
21 changes: 21 additions & 0 deletions examples/example-menubar/package.json
Original file line number Diff line number Diff line change
@@ -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"
scmmmh marked this conversation as resolved.
Show resolved Hide resolved
},
"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"
}
}
8 changes: 8 additions & 0 deletions examples/example-menubar/rollup.config.js
Original file line number Diff line number Diff line change
@@ -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;
121 changes: 121 additions & 0 deletions examples/example-menubar/src/index.ts
Original file line number Diff line number Diff line change
@@ -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);

fcollonval marked this conversation as resolved.
Show resolved Hide resolved
Widget.attach(app, document.body);
}

window.onload = main;
30 changes: 30 additions & 0 deletions examples/example-menubar/style/content.css
Original file line number Diff line number Diff line change
@@ -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;
}
19 changes: 19 additions & 0 deletions examples/example-menubar/style/index.css
Original file line number Diff line number Diff line change
@@ -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;
}
17 changes: 17 additions & 0 deletions examples/example-menubar/tsconfig.json
Original file line number Diff line number Diff line change
@@ -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/*"]
}
38 changes: 30 additions & 8 deletions packages/widgets/src/menubar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down Expand Up @@ -378,7 +383,7 @@ export class MenuBar extends Widget {
*/
protected onActivateRequest(msg: Message): void {
if (this.isAttached) {
this.node.focus();
this.activeIndex = 0;
}
}

Expand All @@ -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<VirtualElement>(menus.length);
for (let i = 0, n = menus.length; i < n; ++i) {
let title = menus[i].title;
Expand All @@ -399,6 +408,7 @@ export class MenuBar extends Widget {
content[i] = renderer.renderItem({
title,
active,
tabbable: i === tabFocusIndex,
onfocus: () => {
this.activeIndex = i;
}
Expand All @@ -417,17 +427,18 @@ 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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces an inconsistency that makes me uncomfortable. The active index gets reset when you tab out of the menubar but not when you click out. On the example page, if you tab to the File menu, then click on the button, the File menu stays shaded. But if you tab to the File menu, then tab to the button, the File menu loses its shading.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can live with this inconsistency for now, but we will need to address it in the future.

return;
}

// A menu bar handles all other keydown events.
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;
}
Expand Down Expand Up @@ -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');

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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)
);
Expand Down Expand Up @@ -941,8 +965,6 @@ namespace Private {
content.className = 'lm-MenuBar-content';
node.appendChild(content);
content.setAttribute('role', 'menubar');
node.tabIndex = 0;
scmmmh marked this conversation as resolved.
Show resolved Hide resolved
content.tabIndex = 0;
return node;
}

Expand Down
Loading