DOMUI: Make Autofill profile lists multi-select and implementing deleting

DOMUI: Make Autofill profile lists multi-select and implementing deleting
multiple list items.

BUG=66956
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=70644

[stuartmorgan, 2011-01-06 19:27:33]

http://codereview.chromium.org/6029013/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/deletable_item_list.js (right):

http://codereview.chromium.org/6029013/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/deletable_item_list.js:126:
this.deleteItemAtIndex(selected[j]);
Won't this do the wrong thing when clicking the close button on a
hovered-but-not-selected row?

It seems like the logic is going to have to be:
- If the target is selected, delete all the selected rows
- If the target isn't selected, just delete the target.

Hopefully that won't end up being really confusing...

http://codereview.chromium.org/6029013/diff/1/chrome/browser/resources/shared...
File chrome/browser/resources/shared/js/cr/ui/list.js (right):

http://codereview.chromium.org/6029013/diff/1/chrome/browser/resources/shared...
chrome/browser/resources/shared/js/cr/ui/list.js:274:
this.addEventListener('mouseup', this.handleMouseDownUp_, false);
Isn't false the default? Why do we need this here but not on the other
listeners?

[James Hawkins, 2011-01-06 20:06:43]

http://codereview.chromium.org/6029013/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/deletable_item_list.js (right):

http://codereview.chromium.org/6029013/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/deletable_item_list.js:126:
this.deleteItemAtIndex(selected[j]);
On 2011/01/06 19:27:33, stuartmorgan wrote:
> Won't this do the wrong thing when clicking the close button on a
> hovered-but-not-selected row?
> 
> It seems like the logic is going to have to be:
> - If the target is selected, delete all the selected rows
> - If the target isn't selected, just delete the target.
> 
> Hopefully that won't end up being really confusing...

Yep, this does the wrong thing in this case.  Fixed.

http://codereview.chromium.org/6029013/diff/1/chrome/browser/resources/shared...
File chrome/browser/resources/shared/js/cr/ui/list.js (right):

http://codereview.chromium.org/6029013/diff/1/chrome/browser/resources/shared...
chrome/browser/resources/shared/js/cr/ui/list.js:274:
this.addEventListener('mouseup', this.handleMouseDownUp_, false);
On 2011/01/06 19:27:33, stuartmorgan wrote:
> Isn't false the default? Why do we need this here but not on the other
> listeners?

Removing per off-list conversation.

[csilv, 2011-01-06 20:49:08]

LGTM per in-person discussion.

[stuartmorgan, 2011-01-06 20:59:34]

http://codereview.chromium.org/6029013/diff/5001/chrome/browser/resources/opt...
File chrome/browser/resources/options/deletable_item_list.js (right):

http://codereview.chromium.org/6029013/diff/5001/chrome/browser/resources/opt...
chrome/browser/resources/options/deletable_item_list.js:127: // is in the list
of selected items. Only delete this item in that case.
The comment (and the logic) are backward here. If the list item *isn't*
selected, we should delete only that item.

[James Hawkins, 2011-01-06 21:01:54]

http://codereview.chromium.org/6029013/diff/5001/chrome/browser/resources/opt...
File chrome/browser/resources/options/deletable_item_list.js (right):

http://codereview.chromium.org/6029013/diff/5001/chrome/browser/resources/opt...
chrome/browser/resources/options/deletable_item_list.js:127: // is in the list
of selected items. Only delete this item in that case.
On 2011/01/06 20:59:34, stuartmorgan wrote:
> The comment (and the logic) are backward here. If the list item *isn't*
> selected, we should delete only that item.

Done.

[James Hawkins, 2011-01-06 21:03:40]

http://codereview.chromium.org/6029013/diff/5001/chrome/browser/resources/opt...
File chrome/browser/resources/options/deletable_item_list.js (right):

http://codereview.chromium.org/6029013/diff/5001/chrome/browser/resources/opt...
chrome/browser/resources/options/deletable_item_list.js:127: // is in the list
of selected items. Only delete this item in that case.
On 2011/01/06 21:01:54, James Hawkins wrote:
> On 2011/01/06 20:59:34, stuartmorgan wrote:
> > The comment (and the logic) are backward here. If the list item *isn't*
> > selected, we should delete only that item.
> 
> Done.

Actually fixed now.

[stuartmorgan, 2011-01-06 21:06:57]

LGTM, but we should be sure to watch for confusion about how this works and
evaluate if we need to change the heuristic for what to delete.