From bbddcc4a938adbd38ec1a5a3ff07e54e389d9391 Mon Sep 17 00:00:00 2001 From: Jack Lynch Date: Mon, 9 Dec 2019 09:06:10 -0800 Subject: [PATCH] Enable keyboard navigation of threads pane This CL makes the threads pane navigable by keyboard. Based on feedback received in [1], I am managing selection within ThreadsSidebarPane and using ListControl's selection to manage focus. Separating the ListControl selection from the currently active thread neccessitated a change to a method in DebuggerTestRunner that changes threads in order to avoid breaking tests. Previously +1'd in the old repo at https://chromium-review.googlesource.com/c/chromium/src/+/1707832 Video of navigation: https://i.imgur.com/A4eWj2u.mp4 [1] https://chromium-review.googlesource.com/c/chromium/src/+/1690089/17#message-3e9e158cce4628b01398d9cf84e37550b55b14c6 Bug: 963183 Change-Id: Ib14f1e4e567f0a70976b587528de8f83c15f0d4a Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1888991 Commit-Queue: Jack Lynch Reviewed-by: Lorne Mitchell --- front_end/sources/ThreadsSidebarPane.js | 31 +++++++++++++++---- front_end/sources/threadsSidebarPane.css | 5 ++- .../sources_test_runner/DebuggerTestRunner.js | 3 +- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/front_end/sources/ThreadsSidebarPane.js b/front_end/sources/ThreadsSidebarPane.js index b8e0cd17b3..0d90dc0904 100644 --- a/front_end/sources/ThreadsSidebarPane.js +++ b/front_end/sources/ThreadsSidebarPane.js @@ -14,6 +14,8 @@ Sources.ThreadsSidebarPane = class extends UI.VBox { this._items = new UI.ListModel(); /** @type {!UI.ListControl} */ this._list = new UI.ListControl(this._items, this, UI.ListMode.NonViewport); + const currentTarget = UI.context.flavor(SDK.Target); + this._selectedModel = !!currentTarget ? currentTarget.model(SDK.DebuggerModel) : null; this.contentElement.appendChild(this._list.element); UI.context.addFlavorChangeListener(SDK.Target, this._targetFlavorChanged, this); @@ -37,6 +39,14 @@ Sources.ThreadsSidebarPane = class extends UI.VBox { const title = element.createChild('div', 'thread-item-title'); const pausedState = element.createChild('div', 'thread-item-paused-state'); element.appendChild(UI.Icon.create('smallicon-thick-right-arrow', 'selected-thread-icon')); + element.tabIndex = -1; + self.onInvokeElement(element, event => { + UI.context.setFlavor(SDK.Target, debuggerModel.target()); + event.consume(true); + }); + const isSelected = UI.context.flavor(SDK.Target) === debuggerModel.target(); + element.classList.toggle('selected', isSelected); + UI.ARIAUtils.setSelected(element, isSelected); function updateTitle() { const executionContext = debuggerModel.runtimeModel().defaultExecutionContext(); @@ -96,13 +106,14 @@ Sources.ThreadsSidebarPane = class extends UI.VBox { */ selectedItemChanged(from, to, fromElement, toElement) { if (fromElement) { - fromElement.classList.remove('selected'); + fromElement.tabIndex = -1; } if (toElement) { - toElement.classList.add('selected'); - } - if (to) { - UI.context.setFlavor(SDK.Target, to.target()); + this.setDefaultFocusedElement(toElement); + toElement.tabIndex = 0; + if (this.hasFocus()) { + toElement.focus(); + } } } @@ -140,10 +151,18 @@ Sources.ThreadsSidebarPane = class extends UI.VBox { * @param {!Common.Event} event */ _targetFlavorChanged(event) { + const hadFocus = this.hasFocus(); const target = /** @type {!SDK.Target} */ (event.data); const debuggerModel = target.model(SDK.DebuggerModel); if (debuggerModel) { - this._list.selectItem(debuggerModel); + this._list.refreshItem(debuggerModel); + } + if (!!this._selectedModel) { + this._list.refreshItem(this._selectedModel); + } + this._selectedModel = debuggerModel; + if (hadFocus) { + this.focus(); } } }; diff --git a/front_end/sources/threadsSidebarPane.css b/front_end/sources/threadsSidebarPane.css index 6863de6d1e..c10ec44d56 100644 --- a/front_end/sources/threadsSidebarPane.css +++ b/front_end/sources/threadsSidebarPane.css @@ -21,6 +21,10 @@ background-color: #eee; } +.thread-item[data-keyboard-focus="true"] { + background-color: var(--focus-bg-color); +} + .thread-item-title, .thread-item-paused-state { text-overflow: ellipsis; @@ -44,4 +48,3 @@ .thread-item.selected .selected-thread-icon { display: block; } - diff --git a/front_end/sources_test_runner/DebuggerTestRunner.js b/front_end/sources_test_runner/DebuggerTestRunner.js index 62fefd312a..941bf92066 100644 --- a/front_end/sources_test_runner/DebuggerTestRunner.js +++ b/front_end/sources_test_runner/DebuggerTestRunner.js @@ -718,8 +718,7 @@ SourcesTestRunner.waitForExecutionContextInTarget = function(target, callback) { }; SourcesTestRunner.selectThread = function(target) { - const threadsPane = self.runtime.sharedInstance(Sources.ThreadsSidebarPane); - threadsPane._list.selectItem(target.model(SDK.DebuggerModel)); + UI.context.setFlavor(SDK.Target, target); }; SourcesTestRunner.evaluateOnCurrentCallFrame = function(code) {