DeletableItemList: Make 'X' button delete only the one list item that contains it.

chrome/browser/resources/options/deletable_item_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 List = cr.ui.List;
   /** @const */ var ListItem = cr.ui.ListItem;
 
   /**
    * Creates a deletable list item, which has a button that will trigger a call
@@ skipping 118 matching lines @@
      * Callback for onclick events.
      * @param {Event} e The click event object.
      * @private
      */
     handleClick_: function(e) {
       if (this.disabled)
         return;
 
       var target = e.target;
       if (target.classList.contains('row-delete-button')) {
+        // Because the close buttons do not give any visual indication that they
+        // would behave differently when multiple items are selected, delete
+        // only the one list item that contains the close button being clicked,
+        // regardless of how many other items are selected. If the user wants to
+        // delete a lot of stuff at once, he would have to press the delete key.

[Evan Stade, 2014/07/31 15:09:26]

I think this comment is too much. Better to make this part of the CL
description.

[engedy, 2014/07/31 16:36:53]

Moved it to the CL description, and replaced it with a much shorter one. WDYT? I
am happy to remove it entire if you still do not like it.

         var listItem = this.getListItemAncestor(target);
-        var selected = this.selectionModel.selectedIndexes;
-
-        // Check if the list item that contains the close button being clicked
-        // is not in the list of selected items. Only delete this item in that
-        // case.
         var idx = this.getIndexOfListItem(listItem);
-        if (selected.indexOf(idx) == -1) {
-          this.deleteItemAtIndex(idx);
-        } else {
-          this.deleteSelectedItems_();
-        }
+        this.deleteItemAtIndex(idx);
+        this.selectionModel.clear();

[engedy, 2014/07/31 09:30:22]

What is the motivation for clearing the selection? (Even though it happens in
most all cases automatically even without this line, as we rebuild the list of
items.)

[Evan Stade, 2014/07/31 15:09:27]

It's my impression that that is the expected behavior --- any time I left click
without holding ctrl or shift, the selection will be cleared.

also based on your comment, it sounds like consistency is another good reason

[engedy, 2014/07/31 16:36:53]

Sorry, turns out my above statement was incorrect:
 1.) We currently have some lists where the selection does not go away when an
item gets deleted (e.g. Content Settings exceptions).
 2.) I have just discovered lines 102--113 in this file, which implements the
complete opposite of what we have just discussed. :-)

Actually, I now think that *not* clearing the selection here would actually make
sense. This way, the 'X' buttons would consistently have absolutely nothing to
do with the current selection: the selection would not affect which item is
deleted, and the selection would be not affected by deletions. I think the
current visual treatment does not contradict this too much.

Then, if a subclass implements deletion in such a way that the underlying model
/ selection model is reset, that would still be consistently done so. (And might
even be considered an erroneous implementation.)

WDYT?

       }
     },
 
     /**
      * Callback for keydown events.
      * @param {Event} e The keydown event object.
      * @private
      */
     handleKeyDown_: function(e) {
       // Map delete (and backspace on Mac) to item deletion (unless focus is
@@ skipping 28 matching lines @@
      */
     deleteItemAtIndex: function(index) {
     },
   };
 
   return {
     DeletableItemList: DeletableItemList,
     DeletableItem: DeletableItem,
   };
 });