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

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview 'settings-search-engine-entry' is a component for showing a
  * search engine with its name, domain and query URL.
  */
 Polymer({
   is: 'settings-search-engine-entry',
@@ skipping 15 matching lines @@
       type: Boolean,
       computed: 'computeShowDots_(engine.canBeDefault,' +
                                  'engine.canBeEdited,' +
                                  'engine.canBeRemoved)',
     },
 
     /** @private {boolean} */
     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();
   },
 
   /** @private */
   closePopupMenu_: function() {
     this.$$('dialog[is=cr-action-menu]').close();
   },
 
@@ skipping 58 matching lines @@
       }.bind(this));
     }.bind(this));
   },
 
   /** @private */
   onMakeDefaultTap_: function() {
     this.closePopupMenu_();
     this.browserProxy_.setDefaultSearchEngine(this.engine.modelIndex);
   },
 });