Maintain focused column in chrome://settings/searchEngines

chrome/browser/resources/options/inline_editable_list.js

 // Copyright (c) 2012 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.
 
 cr.define('options', function() {
   /** @const */ var DeletableItem = options.DeletableItem;
   /** @const */ var DeletableItemList = options.DeletableItemList;
 
   /**
    * Creates a new list item with support for inline editing.
@@ skipping 13 matching lines @@
    */
   InlineEditableItem.decorate = function(el) {
     el.__proto__ = InlineEditableItem.prototype;
     el.decorate();
   };
 
   InlineEditableItem.prototype = {
     __proto__: DeletableItem.prototype,
 
     /**
+     * Index of currently focused column, or -1 for none.
+     * @type {number}
+     */
+    focusedColumnIndex: -1,

[bondd, 2014/10/29 21:14:21]

Is there a style recommendation for whether this should this go before or after
the private member variables?

[Dan Beam, 2014/10/29 22:39:07]

why is it not private?  because it's edited by the list?

[bondd, 2014/10/29 23:45:25]

Yes, it's edited by the list to tell the item which column was focused in the
previously focused item. Item can't figure it out by itself because it can't
(shouldn't) look into its siblings to see which column they had focused.

+
+    /**
      * Whether or not this item can be edited.
      * @type {boolean}
      * @private
      */
     editable_: true,
 
     /**
      * Whether or not this is a placeholder for adding a new item.
      * @type {boolean}
      * @private
@@ skipping 23 matching lines @@
      */
     editClickTarget_: null,
 
     /** @override */
     decorate: function() {
       DeletableItem.prototype.decorate.call(this);
 
       this.editFields_ = [];
       this.addEventListener('mousedown', this.handleMouseDown_);
       this.addEventListener('keydown', this.handleKeyDown_);
-      this.addEventListener('leadChange', this.handleLeadChange_);
+      this.addEventListener('focusin', this.handleFocusIn_);
     },
 
     /** @override */
     selectionChanged: function() {
       this.updateEditState();
     },
 
     /**
      * Called when this element gains or loses 'lead' status. Updates editing
      * mode accordingly.
-     * @private
      */
-    handleLeadChange_: function() {
+    updateLeadState: function() {
       // Add focusability before call to updateEditState.
       if (this.lead) {
         this.setEditableValuesFocusable(true);
         this.setCloseButtonFocusable(true);
       }
 
       this.updateEditState();
 
       // Remove focusability after call to updateEditState.
       this.setStaticValuesFocusable(false);
@@ skipping 25 matching lines @@
       if (editing)
         this.setAttribute('editing', '');
       else
         this.removeAttribute('editing');
 
       if (editing) {
         this.editCancelled_ = false;
 
         cr.dispatchSimpleEvent(this, 'edit', true);
 
-        var focusElement = this.editClickTarget_ || this.initialFocusElement;
-        this.editClickTarget_ = null;
+        var focusElement = this.getEditFocusElement_();
         if (focusElement)
           this.focusAndMaybeSelect_(focusElement);
       } else {
         if (!this.editCancelled_ && this.hasBeenEdited &&
             this.currentInputIsValid) {
           if (this.isPlaceholder)
             this.parentNode.focusPlaceholder = true;
 
           this.updateStaticValues_();
           cr.dispatchSimpleEvent(this, 'commitedit', true);
         } else {
           this.resetEditableValues_();
           cr.dispatchSimpleEvent(this, 'canceledit', true);
         }
       }
     },
 
     /**
+     * Return editable element that should be focused, or null for none.
+     * @private
+     */
+    getEditFocusElement_: function() {
+      // If an edit field was clicked on then use the clicked element.
+      if (this.editClickTarget_) {
+        var result = this.editClickTarget_;
+        this.editClickTarget_ = null;
+        return result;
+      }
+
+      // If focusedColumnIndex is valid then use the element in that column.
+      if (this.focusedColumnIndex != -1) {
+        var nearestColumn =
+            this.getNearestColumnByIndex_(this.focusedColumnIndex);
+        if (nearestColumn)
+          return nearestColumn;
+      }
+
+      // It's possible that focusedColumnIndex hasn't been updated yet.
+      // Check getFocusedColumnIndex_ directly.
+      // This can't completely replace the above focusedColumnIndex check
+      // because InlineEditableItemList may have set focusedColumnIndex to a
+      // different value.
+      var columnIndex = this.getFocusedColumnIndex_();
+      if (columnIndex != -1) {
+        var nearestColumn = this.getNearestColumnByIndex_(columnIndex);
+        if (nearestColumn)
+          return nearestColumn;
+      }
+
+      // Everything else failed so return the default.
+      return this.initialFocusElement;
+    },
+
+    /**
      * Focus on the specified element, and select the editable text in it
      * if possible.
      * @param {!Element} control An element to be focused.
      * @private
      */
     focusAndMaybeSelect_: function(control) {
       control.focus();
       if (control.tagName == 'INPUT')
         control.select();
     },
@@ skipping 170 matching lines @@
         if (this.editable)
           control.staticVersion.tabIndex = -1;
         control.staticVersion.editableVersion = control;
         control.staticVersion.addEventListener('focus',
                                                this.handleFocus.bind(this));
       }
       this.editFields_.push(control);
     },
 
     /**
+     * Set the column index for a child element of this InlineEditableItem.
+     * Only elements with a column index will be keyboard focusable, e.g. by
+     * pressing the tab key.
+     * @param {Element} element Element whose column index to set. Method does
+     *     nothing if element is null.
+     * @param {number} columnIndex The new column index to set on the element.
+     *     -1 removes the column index.
+     */
+    setFocusableColumnIndex: function(element, columnIndex) {
+      if (!element)
+        return;
+
+      if (columnIndex >= 0)
+        element.setAttribute('inlineeditable-column', columnIndex);
+      else
+        element.removeAttribute('inlineeditable-column');
+    },
+
+    /**
      * Resets the editable version of any controls created by createEditable*
      * to match the static text.
      * @private
      */
     resetEditableValues_: function() {
       var editFields = this.editFields_;
       for (var i = 0; i < editFields.length; i++) {
         var staticLabel = editFields[i].staticVersion;
         if (!staticLabel && !this.isPlaceholder)
           continue;
@@ skipping 21 matching lines @@
         if (!staticLabel)
           continue;
 
         if (editFields[i].tagName == 'INPUT')
           staticLabel.textContent = editFields[i].value;
         // Add more tag types here as new createEditable* methods are added.
       }
     },
 
     /**
+     * Returns the index of the column that currently has focus, or -1 if no
+     * column has focus.
+     * @return {number}
+     * @private
+     */
+    getFocusedColumnIndex_: function() {
+      var element = document.activeElement;
+      if (element.editableVersion)
+        element = element.editableVersion;
+
+      if (element.hasAttribute('inlineeditable-column'))
+        return element.getAttribute('inlineeditable-column');

[Dan Beam, 2014/10/29 22:39:07]

this returns a string, not a number

[bondd, 2014/10/29 23:45:25]

Done. Is there a better way to do this, maybe by storing the attribute as a
number somehow? This seems to be the recommended way to do things, but maybe I
am missing something.

[Dan Beam, 2014/10/30 19:34:45]

no, setAttribute() always stringifies.  you could save it in JS only (e.g. just
element.editColumn_) but you'd have to change the way that you querySelector()
for [inlineeditable-column] (which would probably be fine).

[bondd, 2014/10/30 20:34:06]

So is it fine the way it is now in patch set #2 (with the parseInt call), or
would you like me to change this somehow?

+      return -1;
+    },
+
+    /**
+     * Returns the element from the column that has the largest index where:
+     * where:
+     *   + index <= startIndex, and
+     *   + the element exists, and
+     *   + the element is not disabled
+     * @return {Element}
+     * @private
+     */
+    getNearestColumnByIndex_: function(startIndex) {
+      for (var i = startIndex; i >= 0; --i) {
+        var el = this.querySelector('[inlineeditable-column="' + i + '"]');
+        if (el && !el.disabled)
+          return el;
+      }
+      return null;
+    },
+
+    /**
      * Called when a key is pressed. Handles committing and canceling edits.
      * @param {Event} e The key down event.
      * @private
      */
     handleKeyDown_: function(e) {
       if (!this.editing)
         return;
 
       var endEdit = false;
       var handledKey = true;
@@ skipping 47 matching lines @@
           // from currently selected edit field.
           e.stopPropagation();
           e.preventDefault();
         }
         return;
       }
 
       if (editClickField && !editClickField.disabled)
         this.editClickTarget_ = editClickField;
     },
+
+    /**
+     * Called when this InlineEditableItem or any of its children are given
+     * focus. Updates focusedColumnIndex with the index of the newly focused
+     * column, or -1 if the focused element does not have a column index.
+     * @param {Event} e The focusin event.
+     * @private
+     */
+    handleFocusIn_: function(e) {
+      var target = e.target;
+      if (target.editableVersion)
+        target = target.editableVersion;
+
+      if (target.hasAttribute('inlineeditable-column'))
+        this.focusedColumnIndex = target.getAttribute('inlineeditable-column');

[Dan Beam, 2014/10/29 22:39:07]

this is not a number, it's a string

[bondd, 2014/10/29 23:45:25]

Done.

+      else
+        this.focusedColumnIndex = -1;
+    },
   };
 
   /**
    * Takes care of committing changes to inline editable list items when the
    * window loses focus.
    */
   function handleWindowBlurs() {
     window.addEventListener('blur', function(e) {
       var itemAncestor = findAncestor(document.activeElement, function(node) {
         return node instanceof InlineEditableItem;
@@ skipping 36 matching lines @@
      * @param {Event} e The change event.
      * @private
      */
     handleListFocusChange_: function(e) {
       var leadItem = this.getListItemByIndex(this.selectionModel.leadIndex);
       if (leadItem) {
         if (e.newValue) {
           // Add focusability before making other changes.
           leadItem.setEditableValuesFocusable(true);
           leadItem.setCloseButtonFocusable(true);
+          leadItem.focusedColumnIndex = -1;
           leadItem.updateEditState();
           // Remove focusability after making other changes.
           leadItem.setStaticValuesFocusable(false);
         } else {
           // Add focusability before making other changes.
           leadItem.setStaticValuesFocusable(true);
           leadItem.setCloseButtonFocusable(true);
           leadItem.editing = false;
           // Remove focusability after making other changes.
           if (!leadItem.isPlaceholder)
             leadItem.setEditableValuesFocusable(false);
         }
       }
     },
 
     /**
+     * Handles a change of the lead item from the selection model.
+     * @param {Event} e The property change event.

[Dan Beam, 2014/10/29 22:39:07]

don't duplicate doc if this is an @override

[bondd, 2014/10/29 23:45:25]

Done.

+     * @override
+     */
+    handleLeadChange: function(e) {
+      DeletableItemList.prototype.handleLeadChange.call(this, e);
+
+      var focusedColumnIndex = -1;
+      if (e.oldValue != -1) {
+        var element = this.getListItemByIndex(e.oldValue);
+        if (element) {
+          focusedColumnIndex = element.focusedColumnIndex;
+          element.updateLeadState();
+        }
+      }
+
+      if (e.newValue != -1) {
+        var element = this.getListItemByIndex(e.newValue);
+        if (element) {
+          element.focusedColumnIndex = focusedColumnIndex;
+          element.updateLeadState();
+        }
+      }
+    },
+
+    /**
      * Called after the DataModel for the list has been set.
      * @override
      */
     onSetDataModelComplete: function() {
       DeletableItemList.prototype.onSetDataModelComplete.call(this);
 
       var firstItem = this.getListItemByIndex(0);
       if (firstItem) {
         firstItem.setStaticValuesFocusable(true);
         firstItem.setCloseButtonFocusable(true);
@@ skipping 11 matching lines @@
       return true;
     },
   };
 
   // Export
   return {
     InlineEditableItem: InlineEditableItem,
     InlineEditableItemList: InlineEditableItemList,
   };
 });