From d684f54703ff128809fa102c197925dd88a2c309 Mon Sep 17 00:00:00 2001 From: Dickson Tan Date: Sun, 29 Mar 2020 09:32:58 +0000 Subject: [PATCH] Bug 1545727 - make the JSON tree viewer more accessible. r=Honza Make the JSON tree view more accessible. 1. Automatically set focus to the first node on initial focus. Previously, the only way to interact with the tree view through the keyboard was to click on a node so that subsequent key presses would register. 2. Fix inaccurate aria-level and aria-expanded attributes causing confusing announcements for screen reader users. 3. Previously, pressing left arrow the first time on a leaf child node would not move focus to its parent, but subsequent tries would work. This has been fixed. 4. Implement first-letter navigation for quick movement via keyboard. Differential Revision: https://phabricator.services.mozilla.com/D28274 --HG-- extra : moz-landing-system : lando --- .../test/chrome/test_tree-view_01.html | 8 ++++ .../client/shared/components/tree/TreeRow.js | 9 +++- .../client/shared/components/tree/TreeView.js | 47 ++++++++++++++++--- 3 files changed, 56 insertions(+), 8 deletions(-) diff --git a/devtools/client/shared/components/test/chrome/test_tree-view_01.html b/devtools/client/shared/components/test/chrome/test_tree-view_01.html index 393913df38be..0acae4c1dcd0 100644 --- a/devtools/client/shared/components/test/chrome/test_tree-view_01.html +++ b/devtools/client/shared/components/test/chrome/test_tree-view_01.html @@ -113,6 +113,14 @@ window.onload = function() { name: "Selected row should remain on first on ArrowUp.", event: { type: "keyDown", el: treeViewEl, options: { key: "ArrowUp" }}, state: { selected: "/B" }, + }, { + name: "Selected row should move to the next matching row with first letter navigation.", + event: { type: "keyDown", el: treeViewEl, options: { key: "C" }}, + state: { selected: "/C" }, + }, { + name: "Selected row should not change when there are no more visible nodes matching first letter navigation.", + event: { type: "keyDown", el: treeViewEl, options: { key: "C" }}, + state: { selected: "/C" }, }, { name: "Selected row should be updated to last on End.", event: { type: "keyDown", el: treeViewEl, options: { key: "End" }}, diff --git a/devtools/client/shared/components/tree/TreeRow.js b/devtools/client/shared/components/tree/TreeRow.js index 83104af95766..52af1d949c1e 100644 --- a/devtools/client/shared/components/tree/TreeRow.js +++ b/devtools/client/shared/components/tree/TreeRow.js @@ -219,12 +219,17 @@ define(function(require, exports, module) { if (member.hasChildren) { classNames.push("hasChildren"); - props["aria-expanded"] = false; + + // There are 2 situations where hasChildren is true: + // 1. it is an object with children. Only set aria-expanded in this situation + // 2. It is a long string (> 50 chars) that can be expanded to fully display it + if (member.type !== "string") { + props["aria-expanded"] = member.open; + } } if (member.open) { classNames.push("opened"); - props["aria-expanded"] = true; } if (member.loading) { diff --git a/devtools/client/shared/components/tree/TreeView.js b/devtools/client/shared/components/tree/TreeView.js index 372e35b16314..035487a5e5fe 100644 --- a/devtools/client/shared/components/tree/TreeView.js +++ b/devtools/client/shared/components/tree/TreeView.js @@ -225,6 +225,7 @@ define(function(require, exports, module) { this.toggle = this.toggle.bind(this); this.isExpanded = this.isExpanded.bind(this); + this.onFocus = this.onFocus.bind(this); this.onKeyDown = this.onKeyDown.bind(this); this.onClickRow = this.onClickRow.bind(this); this.getSelectedRow = this.getSelectedRow.bind(this); @@ -253,7 +254,7 @@ define(function(require, exports, module) { componentDidUpdate() { const selected = this.getSelectedRow(); - if (selected) { + if (selected || this.state.active) { return; } @@ -304,9 +305,23 @@ define(function(require, exports, module) { // Event Handlers + onFocus(_event) { + // Set focus to the first element, if none is selected or activated + // This is needed because keyboard navigation won't work without an element being selected + this.componentDidUpdate(); + } + // eslint-disable-next-line complexity onKeyDown(event) { - if (!SUPPORTED_KEYS.includes(event.key)) { + const keyEligibleForFirstLetterNavigation = + event.key.length === 1 && + !event.ctrlKey && + !event.metaKey && + !event.altKey; + if ( + !SUPPORTED_KEYS.includes(event.key) && + !keyEligibleForFirstLetterNavigation + ) { return; } @@ -317,15 +332,25 @@ define(function(require, exports, module) { const rows = this.visibleRows; const index = rows.indexOf(row); + const { hasChildren, open } = row.props.member; + switch (event.key) { case "ArrowRight": - const { hasChildren, open } = row.props.member; - if (hasChildren && !open) { - this.toggle(this.state.selected); + if (hasChildren) { + if (open) { + const firstChildRow = this.rows + .slice(index + 1) + .find(r => r.props.member.level > row.props.member.level); + if (firstChildRow) { + this.selectRow(firstChildRow, { alignTo: "bottom" }); + } + } else { + this.toggle(this.state.selected); + } } break; case "ArrowLeft": - if (row?.props.member.open) { + if (hasChildren && open) { this.toggle(this.state.selected); } else { const parentRow = rows @@ -386,6 +411,15 @@ define(function(require, exports, module) { break; } + if (keyEligibleForFirstLetterNavigation) { + const next = rows + .slice(index + 1) + .find(r => r.props.member.name.startsWith(event.key)); + if (next) { + this.selectRow(next, { alignTo: "bottom" }); + } + } + // Focus should always remain on the tree container itself. this.treeRef.current.focus(); event.preventDefault(); @@ -673,6 +707,7 @@ define(function(require, exports, module) { role: "tree", ref: this.treeRef, tabIndex: 0, + onFocus: this.onFocus, onKeyDown: this.onKeyDown, onContextMenu: onContextMenuTree && onContextMenuTree.bind(this), onClick: () => {