DOMUI Prefs: Refactor deletable lists

Deletable list items are now subclasses instead of wrapped items, which gives implementations greater control over things like class assignment, and removes a level of indirection for some kinds of access.

BUG=None
TEST=Deletable list items in DOMUI prefs should still work.

Review URL: http://codereview.chromium.org/6035002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@69781 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 90 insertions, 79 deletions

diff --git a/chrome/browser/resources/options/browser_options_page.css b/chrome/browser/resources/options/browser_options_page.css
index 7ac8602..30f93ce 100644
--- a/chrome/browser/resources/options/browser_options_page.css
+++ b/chrome/browser/resources/options/browser_options_page.css
@@ -8,6 +8,11 @@
   padding: 5px 10px;
 }
 
+#startupPagesShortList .favicon-cell,
+#startupPagesFullList .favicon-cell {
+  -webkit-box-flex: 1;
+}
+
 #customHomePageGroup {
   display: -webkit-box;
   -webkit-box-orient: horizontal;
diff --git a/chrome/browser/resources/options/browser_options_startup_page_list.js b/chrome/browser/resources/options/browser_options_startup_page_list.js
index f63aa8b..8265e13 100644
--- a/chrome/browser/resources/options/browser_options_startup_page_list.js
+++ b/chrome/browser/resources/options/browser_options_startup_page_list.js
@@ -4,7 +4,7 @@
 
 cr.define('options.browser_options', function() {
   const DeletableItemList = options.DeletableItemList;
-  const ListItem = cr.ui.ListItem;
+  const DeletableItem = options.DeletableItem;
 
   /**
    * Creates a new startup page list item.
@@ -29,11 +29,11 @@ cr.define('options.browser_options', function() {
   };
 
   StartupPageListItem.prototype = {
-    __proto__: ListItem.prototype,
+    __proto__: DeletableItem.prototype,
 
     /** @inheritDoc */
     decorate: function() {
-      ListItem.prototype.decorate.call(this);
+      DeletableItem.prototype.decorate.call(this);
 
       var titleEl = this.ownerDocument.createElement('span');
       titleEl.className = 'title';
@@ -43,7 +43,7 @@ cr.define('options.browser_options', function() {
                                           this.pageInfo_['url']);
       titleEl.title = this.pageInfo_['tooltip'];
 
-      this.appendChild(titleEl);
+      this.contentElement.appendChild(titleEl);
     },
   };
 
@@ -53,7 +53,7 @@ cr.define('options.browser_options', function() {
     __proto__: DeletableItemList.prototype,
 
     /** @inheritDoc */
-    createItemContents: function(pageInfo) {
+    createItem: function(pageInfo) {
       return new StartupPageListItem(pageInfo);
     },
 
diff --git a/chrome/browser/resources/options/content_settings.css b/chrome/browser/resources/options/content_settings.css
index 5e1bffe..96127ee 100644
--- a/chrome/browser/resources/options/content_settings.css
+++ b/chrome/browser/resources/options/content_settings.css
@@ -10,8 +10,10 @@ found in the LICENSE file.
 }
 
 .exceptionPattern {
-  display: -webkit-box;
   -webkit-box-flex: 1;
+  display: -webkit-box;
+  overflow: hidden;
+  white-space: nowrap;
 }
 
 .exceptionSetting {
diff --git a/chrome/browser/resources/options/content_settings_exceptions_area.js b/chrome/browser/resources/options/content_settings_exceptions_area.js
index df0f5e0..ac20705 100644
--- a/chrome/browser/resources/options/content_settings_exceptions_area.js
+++ b/chrome/browser/resources/options/content_settings_exceptions_area.js
@@ -3,10 +3,9 @@
 // found in the LICENSE file.
 
 cr.define('options.contentSettings', function() {
-  const List = cr.ui.List;
-  const ListItem = cr.ui.ListItem;
-  const ArrayDataModel = cr.ui.ArrayDataModel;
   const DeletableItemList = options.DeletableItemList;
+  const DeletableItem = options.DeletableItem;
+  const ArrayDataModel = cr.ui.ArrayDataModel;
 
   /**
    * Creates a new exceptions list item.
@@ -17,7 +16,7 @@ cr.define('options.contentSettings', function() {
    * @param {Object} exception A dictionary that contains the data of the
    *     exception.
    * @constructor
-   * @extends {cr.ui.ListItem}
+   * @extends {options.DeletableItem}
    */
   function ExceptionsListItem(contentType, mode, enableAskOption, exception) {
     var el = cr.doc.createElement('div');
@@ -32,13 +31,13 @@ cr.define('options.contentSettings', function() {
   }
 
   ExceptionsListItem.prototype = {
-    __proto__: ListItem.prototype,
+    __proto__: DeletableItem.prototype,
 
     /**
      * Called when an element is decorated as a list item.
      */
     decorate: function() {
-      ListItem.prototype.decorate.call(this);
+      DeletableItem.prototype.decorate.call(this);
 
       // Labels for display mode. |pattern| will be null for the 'add new
       // exception' row.
@@ -46,20 +45,20 @@ cr.define('options.contentSettings', function() {
         var patternLabel = cr.doc.createElement('span');
         patternLabel.textContent = this.pattern;
         patternLabel.className = 'exceptionPattern';
-        this.appendChild(patternLabel);
+        this.contentElement.appendChild(patternLabel);
         this.patternLabel = patternLabel;
 
         var settingLabel = cr.doc.createElement('span');
         settingLabel.textContent = this.settingForDisplay();
         settingLabel.className = 'exceptionSetting';
-        this.appendChild(settingLabel);
+        this.contentElement.appendChild(settingLabel);
         this.settingLabel = settingLabel;
       }
 
       // Elements for edit mode.
       var input = cr.doc.createElement('input');
       input.type = 'text';
-      this.appendChild(input);
+      this.contentElement.appendChild(input);
       input.className = 'exceptionPattern hidden';
 
       var select = cr.doc.createElement('select');
@@ -85,7 +84,7 @@ cr.define('options.contentSettings', function() {
       optionBlock.textContent = templateData.blockException;
       select.appendChild(optionBlock);
 
-      this.appendChild(select);
+      this.contentElement.appendChild(select);
       select.className = 'exceptionSetting hidden';
 
       // Used to track whether the URL pattern in the input is valid.
@@ -431,7 +430,7 @@ cr.define('options.contentSettings', function() {
      * Creates an item to go in the list.
      * @param {Object} entry The element from the data model for this row.
      */
-    createItemContents: function(entry) {
+    createItem: function(entry) {
       if (entry) {
         return new ExceptionsListItem(this.contentType,
                                       this.mode,
@@ -441,7 +440,7 @@ cr.define('options.contentSettings', function() {
         var addRowItem = new ExceptionsAddRowListItem(this.contentType,
                                                       this.mode,
                                                       this.enableAskOption);
-        addRowItem.undeletable = true;
+        addRowItem.deletable = false;
         return addRowItem;
       }
     },
@@ -469,7 +468,7 @@ cr.define('options.contentSettings', function() {
     patternValidityCheckComplete: function(pattern, valid) {
       var listItems = this.items;
       for (var i = 0; i < listItems.length; i++) {
-        var listItem = listItems[i].contentItem;
+        var listItem = listItems[i];
         // Don't do anything for messages for the item if it is not the intended
         // recipient, or if the response is stale (i.e. the input value has
         // changed since we sent the request to analyze it).
@@ -501,7 +500,7 @@ cr.define('options.contentSettings', function() {
 
     /** @inheritDoc */
     deleteItemAtIndex: function(index) {
-      var listItem = this.getListItemByIndex(index).contentItem;
+      var listItem = this.getListItemByIndex(index);
       if (listItem.undeletable) {
         console.log('Tried to delete an undeletable row.');
         return;
diff --git a/chrome/browser/resources/options/deletable_item_list.js b/chrome/browser/resources/options/deletable_item_list.js
index 0884e38..6781c50 100644
--- a/chrome/browser/resources/options/deletable_item_list.js
+++ b/chrome/browser/resources/options/deletable_item_list.js
@@ -7,14 +7,12 @@ cr.define('options', function() {
   const ListItem = cr.ui.ListItem;
 
   /**
-   * Wraps a list item to make it deletable, adding a button that will trigger a
-   * call to deleteItemAtIndex(index) in the list.
-   * @param {!ListItem} baseItem The list element to wrap.
+   * Creates a deletable list item, which has a button that will trigger a call
+   * to deleteItemAtIndex(index) in the list.
    */
-  function DeletableListItem(baseItem) {
+  function DeletableItem(value) {
     var el = cr.doc.createElement('div');
-    el.baseItem_ = baseItem;
-    DeletableListItem.decorate(el);
+    DeletableItem.decorate(el);
     return el;
   }
 
@@ -22,49 +20,68 @@ cr.define('options', function() {
    * Decorates an element as a deletable list item.
    * @param {!HTMLElement} el The element to decorate.
    */
-  DeletableListItem.decorate = function(el) {
-    el.__proto__ = DeletableListItem.prototype;
+  DeletableItem.decorate = function(el) {
+    el.__proto__ = DeletableItem.prototype;
     el.decorate();
   };
 
-  DeletableListItem.prototype = {
+  DeletableItem.prototype = {
     __proto__: ListItem.prototype,
 
     /**
-     * The list item being wrapped to make it deletable.
-     * @type {!ListItem}
+     * The element subclasses should populate with content.
+     * @type {HTMLElement}
      * @private
      */
-    baseItem_: null,
+    contentElement_: null,
+
+    /**
+     * The close button element.
+     * @type {HTMLElement}
+     * @private
+     */
+    closeButtonElement_: null,
+
+    /**
+     * Whether or not this item can be deleted.
+     * @type {boolean}
+     * @private
+     */
+    deletable_: true,
 
     /** @inheritDoc */
     decorate: function() {
       ListItem.prototype.decorate.call(this);
 
-      this.baseItem_.classList.add('deletable-item');
-      this.appendChild(this.baseItem_);
+      this.classList.add('deletable-item');
 
-      var closeButtonEl = this.ownerDocument.createElement('button');
-      closeButtonEl.className = 'close-button';
-      closeButtonEl.disabled = this.baseItem_.undeletable;
-      closeButtonEl.addEventListener('mousedown', this.handleMouseDownOnClose_);
-      this.appendChild(closeButtonEl);
-    },
+      this.contentElement_ = this.ownerDocument.createElement('div');
+      this.appendChild(this.contentElement_);
 
-    /** @inheritDoc */
-    selectionChanged: function() {
-      // Forward the selection state to the |baseItem_|.
-      // TODO(jhawkins): This is terrible.
-      this.baseItem_.selected = this.selected;
-      this.baseItem_.selectionChanged();
+      this.closeButtonElement_ = this.ownerDocument.createElement('button');
+      this.closeButtonElement_.className = 'close-button';
+      this.closeButtonElement_.addEventListener('mousedown',
+                                                this.handleMouseDownOnClose_);
+      this.appendChild(this.closeButtonElement_);
     },
 
     /**
-     * Returns the list item being wrapped to make it deletable.
-     * @return {!ListItem} The list item being wrapped
+     * Returns the element subclasses should add content to.
+     * @return {HTMLElement} The element subclasses should popuplate.
      */
-    get contentItem() {
-      return this.baseItem_;
+    get contentElement() {
+      return this.contentElement_;
+    },
+
+    /* Gets/sets the deletable property. An item that is not deletable doesn't
+     * show the delete button (although space is still reserved for it).
+     */
+    get deletable() {
+      return this.deletable_;
+    },
+    set deletable(value) {
+      this.deletable_ = value;
+      this.closeButtonElement_.disabled = !value;
     },
 
     /**
@@ -90,22 +107,6 @@ cr.define('options', function() {
       this.addEventListener('click', this.handleClick_);
     },
 
-    /** @inheritDoc */
-    createItem: function(value) {
-      var baseItem = this.createItemContents(value);
-      return new DeletableListItem(baseItem);
-    },
-
-    /**
-     * Creates a new list item to use as the main row contents for |value|.
-     * Subclasses should override this instead of createItem.
-     * @param {*} value The value to use for the item.
-     * @return {!ListItem} The newly created list item.
-     */
-    createItemContents: function(value) {
-      List.prototype.createItem.call(this, value);
-    },
-
     /**
      * Callback for onclick events.
      * @param {Event} e The click event object.
@@ -133,6 +134,7 @@ cr.define('options', function() {
   };
 
   return {
-    DeletableItemList: DeletableItemList
+    DeletableItemList: DeletableItemList,
+    DeletableItem: DeletableItem,
   };
 });
diff --git a/chrome/browser/resources/options/options_page.css b/chrome/browser/resources/options/options_page.css
index 8ba68ca..b9a38f2 100644
--- a/chrome/browser/resources/options/options_page.css
+++ b/chrome/browser/resources/options/options_page.css
@@ -405,6 +405,9 @@ list > .heading:hover {
 
 list .deletable-item {
   -webkit-box-align: center;
+}
+
+list .deletable-item > :first-child {
   -webkit-box-flex: 1;
   display: -webkit-box;
 }
diff --git a/chrome/browser/resources/options/password_manager_list.js b/chrome/browser/resources/options/password_manager_list.js
index 6b057cd..593a9bc 100644
--- a/chrome/browser/resources/options/password_manager_list.js
+++ b/chrome/browser/resources/options/password_manager_list.js
@@ -5,8 +5,8 @@
 cr.define('options.passwordManager', function() {
   const ArrayDataModel = cr.ui.ArrayDataModel;
   const DeletableItemList = options.DeletableItemList;
+  const DeletableItem = options.DeletableItem;
   const List = cr.ui.List;
-  const ListItem = cr.ui.ListItem;
 
   /**
    * Creates a new passwords list item.
@@ -24,11 +24,11 @@ cr.define('options.passwordManager', function() {
   }
 
   PasswordListItem.prototype = {
-    __proto__: ListItem.prototype,
+    __proto__: DeletableItem.prototype,
 
     /** @inheritDoc */
     decorate: function() {
-      ListItem.prototype.decorate.call(this);
+      DeletableItem.prototype.decorate.call(this);
 
       // The URL of the site.
       var urlLabel = this.ownerDocument.createElement('div');
@@ -36,13 +36,13 @@ cr.define('options.passwordManager', function() {
       urlLabel.classList.add('favicon-cell');
       urlLabel.textContent = this.url;
       urlLabel.style.backgroundImage = url('chrome://favicon/' + this.url);
-      this.appendChild(urlLabel);
+      this.contentElement.appendChild(urlLabel);
 
       // The stored username.
       var usernameLabel = this.ownerDocument.createElement('div');
       usernameLabel.className = 'name';
       usernameLabel.textContent = this.username;
-      this.appendChild(usernameLabel);
+      this.contentElement.appendChild(usernameLabel);
 
       // The stored password.
       var passwordInputDiv = this.ownerDocument.createElement('div');
@@ -61,7 +61,7 @@ cr.define('options.passwordManager', function() {
       buttonSpan.addEventListener('click', this.onClick_, true);
       passwordInputDiv.appendChild(buttonSpan);
 
-      this.appendChild(passwordInputDiv);
+      this.contentElement.appendChild(passwordInputDiv);
     },
 
     /** @inheritDoc */
@@ -128,7 +128,7 @@ cr.define('options.passwordManager', function() {
    * Creates a new PasswordExceptions list item.
    * @param {Array} entry A pair of the form [url, username].
    * @constructor
-   * @extends {cr.ui.ListItem}
+   * @extends {Deletable.ListItem}
    */
   function PasswordExceptionsListItem(entry) {
     var el = cr.doc.createElement('div');
@@ -140,13 +140,13 @@ cr.define('options.passwordManager', function() {
   }
 
   PasswordExceptionsListItem.prototype = {
-    __proto__: ListItem.prototype,
+    __proto__: DeletableItem.prototype,
 
     /**
      * Call when an element is decorated as a list item.
      */
     decorate: function() {
-      ListItem.prototype.decorate.call(this);
+      DeletableItem.prototype.decorate.call(this);
 
       // The URL of the site.
       var urlLabel = this.ownerDocument.createElement('div');
@@ -154,7 +154,7 @@ cr.define('options.passwordManager', function() {
       urlLabel.classList.add('favicon-cell');
       urlLabel.textContent = this.url;
       urlLabel.style.backgroundImage = url('chrome://favicon/' + this.url);
-      this.appendChild(urlLabel);
+      this.contentElement.appendChild(urlLabel);
     },
 
     /**
@@ -180,7 +180,7 @@ cr.define('options.passwordManager', function() {
     __proto__: DeletableItemList.prototype,
 
     /** @inheritDoc */
-    createItemContents: function(entry) {
+    createItem: function(entry) {
       return new PasswordListItem(entry);
     },
 
@@ -208,7 +208,7 @@ cr.define('options.passwordManager', function() {
     __proto__: DeletableItemList.prototype,
 
     /** @inheritDoc */
-    createItemContents: function(entry) {
+    createItem: function(entry) {
       return new PasswordExceptionsListItem(entry);
     },