Issue 6317005: DOMUI Prefs: Add delete/backspace handling to deletable item lists

Issue 6317005: DOMUI Prefs: Add delete/backspace handling to deletable item lists (Closed)

Created:
9 years, 11 months ago by stuartmorgan

Modified:
9 years, 7 months ago

Reviewers:
arv (Not doing code reviews), James Hawkins

CC:
chromium-reviews

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

Description

DOMUI Prefs: Add delete/backspace handling to deletable item lists Also changes the close button to always show for the lead element (even in a non-editable list), and fixes a bug in close-button-based multiple deletion where the deletable property wasn't being checked. BUG=67027 TEST=List items should be deletable with the delete or backspace key. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=71706

Patch Set 1 #

Patch Set 2 : Change when the close button is shown" #

Patch Set 3 : Make backspace Mac-only #

Total comments: 2

Patch Set 4 : Improve comment #

Total comments: 4

Created: 9 years, 11 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+34 lines, -5 lines)			Patch
	M	chrome/browser/resources/options/deletable_item_list.js	View	1 2 3	2 chunks	+33 lines, -4 lines	4 comments	Download
	M	chrome/browser/resources/options/options_page.css	View	1	1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

stuartmorgan

Updated to make backspace Mac-only, per discussion.

9 years, 11 months ago (2011-01-18 19:30:18 UTC) #2

James Hawkins

http://codereview.chromium.org/6317005/diff/6001/chrome/browser/resources/options/deletable_item_list.js File chrome/browser/resources/options/deletable_item_list.js (right): http://codereview.chromium.org/6317005/diff/6001/chrome/browser/resources/options/deletable_item_list.js#newcode147 chrome/browser/resources/options/deletable_item_list.js:147: e.target.tagName != 'INPUT') { Can you explain this check ...

9 years, 11 months ago (2011-01-18 19:32:23 UTC) #3

stuartmorgan

9 years, 11 months ago (2011-01-18 19:34:51 UTC) #4

arv (Not doing code reviews)

http://codereview.chromium.org/6317005/diff/1002/chrome/browser/resources/options/deletable_item_list.js File chrome/browser/resources/options/deletable_item_list.js (right): http://codereview.chromium.org/6317005/diff/1002/chrome/browser/resources/options/deletable_item_list.js#newcode145 chrome/browser/resources/options/deletable_item_list.js:145: // Map delete (and backspapce on Mac) to item ...

9 years, 11 months ago (2011-01-18 19:56:24 UTC) #6

stuartmorgan

9 years, 11 months ago (2011-01-18 21:55:54 UTC) #7

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

http://codereview.chromium.org/6317005/diff/1002/chrome/browser/resources/opt...
chrome/browser/resources/options/deletable_item_list.js:145: // Map delete (and
backspapce on Mac) to item deletion (unless focus is
On 2011/01/18 19:56:24, arv wrote:
> typo

Oops, I'll fix that next time I'm in the file.

http://codereview.chromium.org/6317005/diff/1002/chrome/browser/resources/opt...
chrome/browser/resources/options/deletable_item_list.js:147: if ((e.keyCode ==
46 || (e.keyCode == 8 && cr.isMac)) &&
On 2011/01/18 19:56:24, arv wrote:
> I thought Mac used Command-Backspace (Meta-Backspace)

For menu items, yes. For interacting with tables and things, which is what this
UI is comparable to, no.

Expand Messages | Collapse Messages