DevTools: Navigate TabbedPane with arrow keys

third_party/WebKit/Source/devtools/front_end/ui/TabbedPane.js

Index: third_party/WebKit/Source/devtools/front_end/ui/TabbedPane.js
diff --git a/third_party/WebKit/Source/devtools/front_end/ui/TabbedPane.js b/third_party/WebKit/Source/devtools/front_end/ui/TabbedPane.js
index a578b774e1ce12d82f7b5394b17dde3d361e68e0..4b8cca8cbb155903eaa288228421d2c06873b8c2 100644
--- a/third_party/WebKit/Source/devtools/front_end/ui/TabbedPane.js
+++ b/third_party/WebKit/Source/devtools/front_end/ui/TabbedPane.js
@@ -43,6 +43,7 @@ UI.TabbedPane = class extends UI.VBox {
     this._tabSlider = createElementWithClass('div', 'tabbed-pane-tab-slider');
     this._tabsElement = this._headerContentsElement.createChild('div', 'tabbed-pane-header-tabs');
     this._tabsElement.setAttribute('role', 'tablist');
+    this._tabsElement.addEventListener('keydown', this._keyDown.bind(this), false);
     this._contentElement = this.contentElement.createChild('div', 'tabbed-pane-content');
     this._contentElement.setAttribute('role', 'tabpanel');
     this._contentElement.createChild('content');
@@ -217,6 +218,7 @@ UI.TabbedPane = class extends UI.VBox {
     this.closeTabs([id], userGesture);
   }
 
+
   /**
    * @param {!Array.<string>} ids
    * @param {boolean=} userGesture
@@ -306,6 +308,12 @@ UI.TabbedPane = class extends UI.VBox {
     });
   }
 
+  _viewHasFocus() {
+    if (this.visibleView)
+      return this.visibleView.hasFocus();
+    return this.contentElement === this.contentElement.getComponentRoot().activeElement;
+  }
+
   /**
    * @param {string} id
    * @param {boolean=} userGesture
@@ -314,7 +322,7 @@ UI.TabbedPane = class extends UI.VBox {
   selectTab(id, userGesture) {
     if (this._currentTabLocked)
       return false;
-    var focused = this.hasFocus();
+    var focused = this._viewHasFocus();
     var tab = this._tabsById.get(id);
     if (!tab)
       return false;
@@ -543,6 +551,7 @@ UI.TabbedPane = class extends UI.VBox {
     var dropDownContainer = createElementWithClass('div', 'tabbed-pane-header-tabs-drop-down-container');
     var chevronIcon = UI.Icon.create('largeicon-chevron', 'chevron-icon');
     dropDownContainer.appendChild(chevronIcon);
+    dropDownContainer.addEventListener('click', this._onDropDownMouseDown.bind(this));
     dropDownContainer.addEventListener('mousedown', this._onDropDownMouseDown.bind(this));
     return dropDownContainer;
   }
@@ -553,7 +562,8 @@ UI.TabbedPane = class extends UI.VBox {
   _onDropDownMouseDown(event) {
     if (event.which !== 1)
       return;
-    var menu = new UI.ContextMenu(event);
+    var rect = this._dropDownButton.getBoundingClientRect();
+    var menu = new UI.ContextMenu(event, false, rect.left, rect.bottom);
     for (var i = 0; i < this._tabs.length; ++i) {
       var tab = this._tabs[i];
       if (tab._shown)
@@ -756,6 +766,7 @@ UI.TabbedPane = class extends UI.VBox {
    * @param {!UI.TabbedPaneTab} tab
    */
   _showTab(tab) {
+    tab.tabElement.tabIndex = 0;
     tab.tabElement.classList.add('selected');
     UI.ARIAUtils.setSelected(tab.tabElement, true);
     tab.view.show(this.element);
@@ -781,6 +792,7 @@ UI.TabbedPane = class extends UI.VBox {
    * @param {!UI.TabbedPaneTab} tab
    */
   _hideTab(tab) {
+    tab.tabElement.removeAttribute('tabIndex');
     tab.tabElement.classList.remove('selected');
     tab.tabElement.setAttribute('aria-selected', 'false');
     tab.view.detach();
@@ -842,6 +854,42 @@ UI.TabbedPane = class extends UI.VBox {
     this._allowTabReorder = allow;
     this._automaticReorder = automatic;
   }
+
+  /**
+   * @param {!Event} event
+   */
+  _keyDown(event) {
+    if (!this._currentTab)
+      return;
+    var nextTabElement = null;
+    switch (event.keyCode) {
+      case UI.KeyboardShortcut.Keys.Up.code:
+      case UI.KeyboardShortcut.Keys.Left.code:
+        nextTabElement = this._currentTab.tabElement.previousElementSibling;

[aboxhall, 2017/04/17 23:03:36]

Sorry for not noticing this the first time around, but this looks potentially
brittle. Obviously though, I don't know the code as well as you. Would it make
sense to do this in terms of `this._tabs` instead? i.e. wrap around the array,
check if the "next" tab is showing, if not, click the drop down?

[einbinder, 2017/04/18 18:45:22]

It would be nicer to use this._tabs, but this._tabs doesn't correspond to the
DOM order. Depending on the last tab shown, or if the user drags and drops tab
elements, the DOM order can be out of sync with this._tabs.

+        if (!nextTabElement && !this._dropDownButton.parentElement)
+          nextTabElement = this._currentTab.tabElement.parentElement.lastElementChild;
+        break;
+      case UI.KeyboardShortcut.Keys.Down.code:
+      case UI.KeyboardShortcut.Keys.Right.code:
+        nextTabElement = this._currentTab.tabElement.nextElementSibling;
+        if (!nextTabElement && !this._dropDownButton.parentElement)
+          nextTabElement = this._currentTab.tabElement.parentElement.firstElementChild;
+        break;
+      case UI.KeyboardShortcut.Keys.Enter.code:
+      case UI.KeyboardShortcut.Keys.Space.code:
+        this._currentTab.view.focus();
+        return;
+      default:
+        return;
+    }
+    if (!nextTabElement) {
+      this._dropDownButton.click();
+      return;
+    }
+    var tab = this._tabs.find(tab => tab.tabElement === nextTabElement);
+    this.selectTab(tab.id, true);
+    nextTabElement.focus();

[aboxhall, 2017/04/17 23:03:36]

Why not make focusing the tab part of selectTab()? If it got selected from a
mouse click it'll be focused anyway, right?

[einbinder, 2017/04/18 18:45:21]

selectTab is also called for things like Ctrl+] shortcut, when the current tab
is closed, or when a view is revealed.

+  }
 };
 
 /** @enum {symbol} */
@@ -1026,7 +1074,6 @@ UI.TabbedPaneTab = class {
   _createTabElement(measuring) {
     var tabElement = createElementWithClass('div', 'tabbed-pane-header-tab');
     tabElement.id = 'tab-' + this._id;
-    tabElement.tabIndex = -1;
     UI.ARIAUtils.markAsTab(tabElement);
     UI.ARIAUtils.setSelected(tabElement, false);
     tabElement.selectTabForTest = this._tabbedPane.selectTab.bind(this._tabbedPane, this.id, true);