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

chrome/browser/resources/options/search_engine_manager.js

Index: chrome/browser/resources/options/search_engine_manager.js
diff --git a/chrome/browser/resources/options/search_engine_manager.js b/chrome/browser/resources/options/search_engine_manager.js
index 7a56bad1e6731bc2a54404d966c5ab0e377325e0..bbbb1999026f301494809c17499539b867d980ca 100644
--- a/chrome/browser/resources/options/search_engine_manager.js
+++ b/chrome/browser/resources/options/search_engine_manager.js
@@ -37,23 +37,19 @@ cr.define('options', function() {
           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');
     },
 
     /**
@@ -62,16 +58,11 @@ cr.define('options', function() {
      * @param {Array} engineList List of available search engines.
      */
     updateSearchEngineList_: function(engineList) {
-      this.list_.dataModel = new ArrayDataModel(engineList);
-    },
-
-    /**
-     * 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'];
+      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;
     },
 
     /**
@@ -80,11 +71,7 @@ cr.define('options', function() {
      * @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']);
     },
@@ -94,6 +81,11 @@ cr.define('options', function() {
     SearchEngineManager.getInstance().updateSearchEngineList_(engineList);
   };
 
+  SearchEngineManager.validityCheckCallback = function(validity, modelIndex) {
+    SearchEngineManager.getInstance().list_.validationComplete(validity,
+                                                               modelIndex);
+  };
+
   // Export
   return {
     SearchEngineManager: SearchEngineManager