Issue 6241010: DOMUI: Use a styled button instead of an image for the password list show/hide

Issue 6241010: DOMUI: Use a styled button instead of an image for the password list show/hide (Closed)

Created:
9 years, 11 months ago by James Hawkins

Modified:
9 years, 7 months ago

Reviewers:
stuartmorgan, arv (Not doing code reviews)

CC:
chromium-reviews, arv (Not doing code reviews)

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

Visibility:
Public.

Description

DOMUI: Use a styled button instead of an image for the password list show/hide button. BUG=69145 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=72057

Patch Set 2 : Review fixes. #

Created: 9 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+83 lines, -83 lines)			Patch
M	chrome/app/generated_resources.grd	View	1	1 chunk	+6 lines, -12 lines	0 comments	Download
M	chrome/browser/dom_ui/options/password_manager_handler.cc	View		1 chunk	+4 lines, -0 lines	0 comments	Download
M	chrome/browser/resources/options/options.html	View		1 chunk	+0 lines, -1 line	0 comments	Download
M	chrome/browser/resources/options/options_page.css	View		2 chunks	+3 lines, -2 lines	0 comments	Download
D	chrome/browser/resources/options/password_manager.css	View		1 chunk	+0 lines, -38 lines	0 comments	Download
M	chrome/browser/resources/options/password_manager_list.css	View		1 chunk	+47 lines, -4 lines	0 comments	Download
M	chrome/browser/resources/options/password_manager_list.js	View	1	4 chunks	+21 lines, -14 lines	0 comments	Download
M	chrome/browser/resources/options/search_engine_manager.css	View	1	1 chunk	+2 lines, -12 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

stuartmorgan

LGTM http://codereview.chromium.org/6241010/diff/1/chrome/browser/resources/options/password_manager_list.css File chrome/browser/resources/options/password_manager_list.css (right): http://codereview.chromium.org/6241010/diff/1/chrome/browser/resources/options/password_manager_list.css#newcode17 chrome/browser/resources/options/password_manager_list.css:17: width: 50px; Can you make sure the colors ...

9 years, 11 months ago (2011-01-21 00:05:05 UTC) #2

James Hawkins

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

http://codereview.chromium.org/6241010/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/password_manager_list.css (right):

http://codereview.chromium.org/6241010/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/password_manager_list.css:17: width: 50px;
On 2011/01/21 00:05:05, stuartmorgan wrote:
> Can you make sure the colors and border radius match the style of the Make
> Default buttons in the search engine manager page? I intended them to more or
> less match these buttons.

Done.

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

http://codereview.chromium.org/6241010/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/password_manager_list.js:35: urlLabel.className
= 'favicon-cell url';
On 2011/01/21 00:05:05, stuartmorgan wrote:
> Two classList.add lines instead?

Done.

http://codereview.chromium.org/6241010/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/password_manager_list.js:60: button.className =
'hidden password-button';
On 2011/01/21 00:05:05, stuartmorgan wrote:
> Same

Done.

Expand Messages | Collapse Messages