DevTools: Focus background in Toolbars

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

Index: third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js
diff --git a/third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js b/third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js
index edef86920d8e16ca16ecc22b440b22ee97bff2a4..624a8df4121f19fb56c793a137fd3290ea7312b1 100644
--- a/third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js
+++ b/third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js
@@ -784,6 +784,8 @@ UI._windowBlurred = function(document, event) {
 UI._focusChanged = function(event) {
   var document = event.target && event.target.ownerDocument;
   var element = document ? document.deepActiveElement() : null;
+  if (document && document.body)
+    document.body.classList.toggle('keyboard-focus', UI._keyboardFocus);

[pfeldman, 2017/03/27 21:17:35]

What is this about?

[einbinder, 2017/03/27 23:12:59]

To distinguish between tabbing into an element vs clicking on it.

[pfeldman, 2017/03/28 17:44:35]

Somehow I don't like storing the 'last focus was gained by means of key event'
bit of information by means of the class on the document body. How do you use
it? Does it pierce shadow DOM?

   UI.Widget.focusWidgetForNode(element);
 };
 
@@ -1182,6 +1184,10 @@ UI.LongClickController = class extends Common.Object {
 UI.initializeUIUtils = function(document, themeSetting) {
   document.defaultView.addEventListener('focus', UI._windowFocused.bind(UI, document), false);
   document.defaultView.addEventListener('blur', UI._windowBlurred.bind(UI, document), false);
+  document.addEventListener('keydown', event => {
+    UI._keyboardFocus = true;
+    document.defaultView.requestAnimationFrame(() => UI._keyboardFocus = false);

[pfeldman, 2017/03/27 21:17:35]

Seems like technical debt.

[einbinder, 2017/03/27 23:12:59]

It is a hack, but it felt like the cleanest hack.

[pfeldman, 2017/03/28 03:23:38]

Let me try to narrate this code:

- every time key is down, for example, 'tab', you are settingUI._keyboardFocus
to true for exactly one render frame.
- if within this render frame, focus changed event is dispatched, you set
'keyboard-focus' class on body
- after that, until another key is down, this 'keyboard-focus' class stays on.

Few things I don't understand here:
- what if i do multiple mouse clicks after that key down? You say it helps
disambiguating clicks from tabs, but I don't see now.
- focus event bubbling does not correlate with render frames, you mean to simply
reset the flag after keydown is dispatched via microtask
(Promise.resolve().then(() => UI._keyboardFocus = false)?

[einbinder, 2017/03/28 08:28:39]

The keyboard-focus class stays on until the focus is changed, not until the next
key is pressed.
If the mouse clicks cause the focus to change, then the keyboard-focus class
will be removed.
It doesn't perfectly match either. We have code that might cause focus to change
after a Promise.resolve. I feel that a key event happening in the same render
frame has focus change is a good indication that the key event caused the focus
change.

[pfeldman, 2017/03/28 17:44:35]

This blurs the contract you are introducing. Code could do a setTimeout or RAF
or protocol roundtrip or whatever. I would understand marking a focus even
object with the "keyboardFocus" marker, the result clients would be responsible
for.

+  }, true);
   document.addEventListener('focus', UI._focusChanged.bind(UI), true);
 
   if (!UI.themeSupport)
@@ -1413,14 +1419,6 @@ UI.appendStyle = function(node, cssFile) {
       this.checkboxElement.style.borderColor = color;
     },
 
-    /**
-     * @param {boolean} focus
-     * @this {Element}
-     */
-    set visualizeFocus(focus) {

[einbinder, 2017/03/14 09:14:36]

This was used to selectively add focus rings to checkboxes. I don't think there
is ever a time we don't want a focus ring on checkboxes.

-      this.checkboxElement.classList.toggle('dt-checkbox-visualize-focus', focus);
-    },
-
     __proto__: HTMLLabelElement.prototype
   });