DOMUI Prefs: Replace search engine edit overlay with inline editing.

chrome/browser/resources/options/search_engine_manager.js

 // Copyright (c) 2010 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 OptionsPage = options.OptionsPage;
   const ArrayDataModel = cr.ui.ArrayDataModel;
   const ListSingleSelectionModel = cr.ui.ListSingleSelectionModel;
 
   /**
@@ skipping 19 matching lines @@
       this.list_ = $('searchEngineList')
       options.search_engines.SearchEngineList.decorate(this.list_);
       var selectionModel = new ListSingleSelectionModel;
       this.list_.selectionModel = selectionModel;
       this.list_.autoExpands = true;
 
       selectionModel.addEventListener('change',
           this.selectionChanged_.bind(this));
 
       var self = this;
-      $('addSearchEngineButton').onclick = function(event) {
-        chrome.send('editSearchEngine', ["-1"]);
-        OptionsPage.showOverlay('editSearchEngineOverlay');
-      };
-      $('editSearchEngineButton').onclick = function(event) {
-        chrome.send('editSearchEngine', [self.selectedModelIndex_]);
-        OptionsPage.showOverlay('editSearchEngineOverlay');
+      // This is a temporary hack to allow the "Make Default" button to

[arv (Not doing code reviews), 2011/01/11 23:29:29]

Why are we removing the selection again?

[stuartmorgan, 2011/01/12 00:26:51]

In the DOMUI pref list model, selection == edit. The options for inline-editable
lists are:
1) Leave the row in edit mode when the list is no longer focused; this feels
really weird
2) Make edit require both selection and list focus.
3) Remove selection when unfocusing a list.

Evan implemented 3 in exceptions, and I was being consistent with that. Once we
everything is implemented with the new base class it will be easy enough to
switch to 2 if we decide that's better.

(As we've added more of the new mock behaviors to the lists more interaction
edge cases are showing up that weren't obvious for the mocks, so my plan is to
move forward getting all the capabilities implemented and then we can test
everything together and discuss with the UX folks as necessary.)

+      // continue working despite the new list behavior of removing selection
+      // on focus loss.
+      // Once drag-and-drop is supported, so items can be moved into the default
+      // section, this button will go away entirely.
+      $('makeDefaultSearchEngineButton').onmousedown = function(event) {
+        self.pendingDefaultEngine_ = self.list_.selectedItem;
       };
       $('makeDefaultSearchEngineButton').onclick = function(event) {
         chrome.send('managerSetDefaultSearchEngine',
-                    [self.selectedModelIndex_]);
+                    [self.pendingDefaultEngine_['modelIndex']]);
+        self.pendingDefaultEngine_ = null;
       };
-
-      // Remove Windows-style accelerators from button labels.
-      // TODO(stuartmorgan): Remove this once the strings are updated.
-      $('addSearchEngineButton').textContent =
-          localStrings.getStringWithoutAccelerator('addSearchEngineButton');
     },
 
     /**
      * Updates the search engine list with the given entries.
      * @private
      * @param {Array} engineList List of available search engines.
      */
     updateSearchEngineList_: function(engineList) {
-      this.list_.dataModel = new ArrayDataModel(engineList);
+      var model = new ArrayDataModel(engineList);
+      var placeholder = {};
+      placeholder['modelIndex'] = '-1';

[arv (Not doing code reviews), 2011/01/11 23:29:29]

Should this be a number instead of a string?

[arv (Not doing code reviews), 2011/01/11 23:29:29]

var placeHolder = {
  'modelIndex': -1
};

or just

model.push({
  'modelIndex': -1
});

[stuartmorgan, 2011/01/12 00:26:51]

modelIndex only really used as a chrome.send argument, so leaving it as a string
simplifies everything.

[stuartmorgan, 2011/01/12 00:26:51]

Done.

+      model.push(placeholder);
+      this.list_.dataModel = model;
     },
 
     /**
-     * Returns the currently selected list item's underlying model index.
-     * @private
-     */
-    get selectedModelIndex_() {
-      var listIndex = this.list_.selectionModel.selectedIndex;
-      return this.list_.dataModel.item(listIndex)['modelIndex'];
-    },
-
-    /**
      * Callback from the selection model when the selection changes.
      * @private
      * @param {!cr.Event} e Event with change info.
      */
     selectionChanged_: function(e) {
-      var selectedIndex = this.list_.selectionModel.selectedIndex;
-      var engine = selectedIndex != -1 ?
-          this.list_.dataModel.item(selectedIndex) : null;
-
-      $('editSearchEngineButton').disabled = engine == null;
+      var engine = this.list_.selectedItem || this.pendingDefaultEngine_;
       $('makeDefaultSearchEngineButton').disabled =
           !(engine && engine['canBeDefault']);
     },
   };
 
   SearchEngineManager.updateSearchEngineList = function(engineList) {
     SearchEngineManager.getInstance().updateSearchEngineList_(engineList);
   };
 
+  SearchEngineManager.validityCheckCallback = function(validity, modelIndex) {
+    SearchEngineManager.getInstance().list_.validationComplete(validity,
+                                                               modelIndex);
+  };
+
   // Export
   return {
     SearchEngineManager: SearchEngineManager
   };
 
 });