Improve keyboard navigation on 'Manage search engines' options page

ui/webui/resources/js/cr/ui/list.js

Index: ui/webui/resources/js/cr/ui/list.js
diff --git a/ui/webui/resources/js/cr/ui/list.js b/ui/webui/resources/js/cr/ui/list.js
index 720f0704ee2d43203d0cc1f73242aaaca512882b..b8da10daa7e114a573887a55ddfe38301c44c45e 100644
--- a/ui/webui/resources/js/cr/ui/list.js
+++ b/ui/webui/resources/js/cr/ui/list.js
@@ -145,13 +145,20 @@ cr.define('cr.ui', function() {
         }
 
         this.redraw();
-      }
+        this.onSetDataModelComplete();
+     }

[Dan Beam, 2014/10/18 01:35:45]

indent off

[bondd, 2014/10/28 22:31:27]

Done.

     },
 
     get dataModel() {
       return this.dataModel_;
     },
 
+    /**
+     * May be overridden by subclasses to implement specialized behavior once
+     * dataModel has been set. 

[Dan Beam, 2014/10/18 01:35:44]

nit: Override to be notified when |this.dataModel| is set.

or something more direct.  it's pretty clear that this is optionally overridden
as it's not abstract.

[bondd, 2014/10/28 22:31:27]

Done.

+     */
+    onSetDataModelComplete: function() {
+    },
 
     /**
      * Cached item for measuring the default item size by measureItem().
@@ -173,7 +180,7 @@ cr.define('cr.ui', function() {
 
       if (!this.boundHandleOnChange_) {
         this.boundHandleOnChange_ = this.handleOnChange_.bind(this);
-        this.boundHandleLeadChange_ = this.handleLeadChange_.bind(this);
+        this.boundHandleLeadChange_ = this.handleLeadChange.bind(this);
       }
 
       if (oldSm) {
@@ -586,9 +593,8 @@ cr.define('cr.ui', function() {
     /**
      * Handles a change of the lead item from the selection model.
      * @param {Event} pe The property change event.
-     * @private
      */
-    handleLeadChange_: function(pe) {
+    handleLeadChange: function(pe) {

[bondd, 2014/10/17 01:23:15]

I made this public because I need to hook it in InlineEditableItemList. It seems
private vs. public for handlers in this class is a bit random: for example
handleDragStart, handleKeyDown, handleMouseDown are public, while
handleDoubleClick_ and handlePointerDownUp_ are private. It seems to follow
convention that if I want to override handleLeadChange I should just make it
public here. Or am I missing something?

[Dan Beam, 2014/10/18 01:35:45]

why can't the derived class just listen to 'leadChange' events?  

it seems like both construction and decoration ensure the base class subscribes
to events first, basically giving you the exact same code (functionally).

this is what the current code does (but both the derived and base classes have
methods named handleLeadChange_, which can lead to some quirky results).

[bondd, 2014/10/29 21:12:39]

Maybe you are confused about the inheritance here (or hey, maybe I am). Current
code listens to 'leadChange' in InlineEditableItem (which is not derived from
this List class). I need to hook handleLeadChange in InlineEditableItemList
(which is derived from this List class). Current code
InlineEditableItem.handleLeadChange_ doesn't overlap with this one, because
neither class inherits from the other.

Based on that info, does it still make sense to listen for 'leadChange' in
InlineEditableItemList vs. making handleLeadChange protected?

       var element;
       if (pe.oldValue != -1) {
         if ((element = this.getListItemByIndex(pe.oldValue)))