MD Settings: adjust focus-outline behaviors on search engine iron-list.

chrome/browser/resources/settings/search_engines_page/search_engine_entry.js

Index: chrome/browser/resources/settings/search_engines_page/search_engine_entry.js
diff --git a/chrome/browser/resources/settings/search_engines_page/search_engine_entry.js b/chrome/browser/resources/settings/search_engines_page/search_engine_entry.js
index 5e026d6e06927841b27e5c5c828164514f8ebea0..445a5d1c01d8af2fca35e29fbb5013c01fc366ea 100644
--- a/chrome/browser/resources/settings/search_engines_page/search_engine_entry.js
+++ b/chrome/browser/resources/settings/search_engines_page/search_engine_entry.js
@@ -33,9 +33,40 @@ Polymer({
     showEditSearchEngineDialog_: Boolean,
   },
 
+  /** @private {!boolean} */
+  focusedByKey_: false,
+
   /** @private {settings.SearchEnginesBrowserProxy} */
   browserProxy_: null,
 
+  attached: function() {
+    // Flag that `this` is focused by keyboard, so mouse click doesn't apply
+    // the .no-outline class.
+    this.addEventListener('keyup', function(e) {

[hcarmona, 2017/02/16 22:33:06]

can we use listeners instead?

listeners: {
  'keyup': 'onKeyUp_',
  'keydown': 'onKeyDown_',
  'mousedown': 'onMouseDown_',
  'blur': 'onBlur_',
},

[Dan Beam, 2017/02/16 22:52:48]

+1

[scottchen, 2017/02/16 23:01:53]

Acknowledged, will try that on the new behavior I just uploaded with the new
patch.

+      // If focusing on icon, then it doesn't count as the row being focused.
+      if(!this.$$('paper-icon-button').focused)
+        this.focusedByKey_ = true;
+    }.bind(this));
+
+    // Unflag when moving away via keyboard (e.g. tabbing onto its children).
+    this.addEventListener('keydown', function(e) {
+      this.focusedByKey_ = false;
+    }.bind(this));
+
+    // When clicking on a row, do not show focus outline if the element wasn't
+    // already in focus.
+    this.addEventListener('mousedown', function(e){
+      if(!this.focusedByKey_)

[hcarmona, 2017/02/16 22:33:06]

What are you trying to prevent here? Is this to make sure the focus ring doesn't
go away if the user tabbed here then clicked?

[Dan Beam, 2017/02/16 22:52:48]

see: action_link.js

[scottchen, 2017/02/16 23:01:54]

Yeah hcarmona@ it's to prevent exactly that.
dbeam@ I have a brief explanation in the CL description why we can't do it the
same way as action-link.js; I looked at it and investigated for a while already
(see caveat #2).

As a general note, is there a better place to put additional explanation of the
CL's code, besides the CL description (is it better to send it with the first
publish email?) I'm wondering because I've experienced a couple times where the
notes in the CL description seems to be not seen by the reviewers.

+        this.classList.add('no-outline');
+    }.bind(this));
+
+    // Reset when moving away from the row entirely.
+    this.addEventListener('blur', function() {
+      this.classList.remove('no-outline');
+      this.focusedByKey_ = false;
+    }.bind(this));
+  },
+
   /** @override */
   created: function() {
     this.browserProxy_ = settings.SearchEnginesBrowserProxyImpl.getInstance();