DevTools: Focus background in Toolbars

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

 /*
  * Copyright (C) 2011 Google Inc.  All rights reserved.
  * Copyright (C) 2006, 2007, 2008 Apple Inc.  All rights reserved.
  * Copyright (C) 2007 Matt Lilek (pewtermoose@gmail.com).
  * Copyright (C) 2009 Joseph Pecoraro
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
@@ skipping 766 matching lines @@
   if (event.target.document.nodeType === Node.DOCUMENT_NODE)
     document.body.classList.add('inactive');
 };
 
 /**
  * @param {!Event} 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);
 };
 
 /**
  * @unrestricted
  */
 UI.ElementFocusRestorer = class {
   /**
    * @param {!Element} element
    */
@@ skipping 378 matching lines @@
   }
 };
 
 /**
  * @param {!Document} document
  * @param {!Common.Setting} themeSetting
  */
 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)
     UI.themeSupport = new UI.ThemeSupport(themeSetting);
   UI.themeSupport.applyTheme(document);
 
   var body = /** @type {!Element} */ (document.body);
   UI.appendStyle(body, 'ui/inspectorStyle.css');
   UI.GlassPane.setContainer(/** @type {!Element} */ (document.body));
 };
@@ skipping 211 matching lines @@
 
     /**
      * @param {string} color
      * @this {Element}
      */
     set borderColor(color) {
       this.checkboxElement.classList.add('dt-checkbox-themed');
       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
   });
 
   UI.registerCustomElement('label', 'dt-icon-label', {
     /**
      * @this {Element}
      */
     createdCallback: function() {
       var root = UI.createShadowRootWithCoreStyles(this);
       this._iconElement = UI.Icon.create();
@@ skipping 630 matching lines @@
    */
   constructor(message, okCallback, cancelCallback) {
     super(true);
     this.registerRequiredCSS('ui/confirmDialog.css');
     this.contentElement.createChild('div', 'message').createChild('span').textContent = message;
     var buttonsBar = this.contentElement.createChild('div', 'button');
     buttonsBar.appendChild(UI.createTextButton(Common.UIString('Ok'), okCallback));
     buttonsBar.appendChild(UI.createTextButton(Common.UIString('Cancel'), cancelCallback));
   }
 };