Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(129)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 324853004: Password bubble: Add an "active" icon state, and a new disabled icon. (Closed)

Created:
6 years, 6 months ago by Mike West
Modified:
6 years, 6 months ago
Reviewers:
vabr (Chromium), oshima
CC:
chromium-reviews, tfarina, gcasto+watchlist_chromium.org, mkwst+watchlist_chromium.org, oshima+watch_chromium.org
Visibility:
Public.

Description

Password bubble: Add an "active" icon state, and a new disabled icon. BUG=261628 R=oshima@chromium.org, vabr@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=276284

Patch Set 1 #

Total comments: 3

Patch Set 2 : Sorted. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+56 lines, -18 lines) Patch
D chrome/app/theme/default_100_percent/common/blocked_save_passwords.png View Binary file 0 comments Download
D chrome/app/theme/default_100_percent/common/save_password.png View Binary file 0 comments Download
A chrome/app/theme/default_100_percent/common/save_password_active.png View Binary file 0 comments Download
A chrome/app/theme/default_100_percent/common/save_password_disabled_active.png View Binary file 0 comments Download
A chrome/app/theme/default_100_percent/common/save_password_disabled_inactive.png View Binary file 0 comments Download
A chrome/app/theme/default_100_percent/common/save_password_inactive.png View Binary file 0 comments Download
D chrome/app/theme/default_200_percent/common/blocked_save_passwords.png View Binary file 0 comments Download
D chrome/app/theme/default_200_percent/common/save_password.png View Binary file 0 comments Download
A chrome/app/theme/default_200_percent/common/save_password_active.png View Binary file 0 comments Download
A chrome/app/theme/default_200_percent/common/save_password_disabled_active.png View Binary file 0 comments Download
A chrome/app/theme/default_200_percent/common/save_password_disabled_inactive.png View Binary file 0 comments Download
A chrome/app/theme/default_200_percent/common/save_password_inactive.png View Binary file 0 comments Download
M chrome/app/theme/theme_resources.grd View 1 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/ui/passwords/manage_passwords_icon.h View 2 chunks +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/passwords/manage_passwords_icon.cc View 1 chunk +9 lines, -1 line 0 comments Download
M chrome/browser/ui/passwords/manage_passwords_ui_controller.cc View 1 chunk +1 line, -3 lines 0 comments Download
M chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h View 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc View 3 chunks +13 lines, -6 lines 0 comments Download
M chrome/browser/ui/views/passwords/manage_passwords_icon_view.cc View 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/passwords/manage_passwords_icon_view_browsertest.cc View 1 chunk +19 lines, -3 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Mike West
vabr: Mind taking a look at the password code? Screenshots forthcoming. oshima: Can you take ...
6 years, 6 months ago (2014-06-10 12:25:44 UTC) #1
Mike West
https://folio.googleplex.com/mkwst/PasswordBubble/InactiveActive
6 years, 6 months ago (2014-06-10 12:30:30 UTC) #2
vabr (Chromium)
//chrome/browser/ui/* LGTM, with 2 optional nits Thanks! Vaclav https://codereview.chromium.org/324853004/diff/1/chrome/browser/ui/passwords/manage_passwords_icon.h File chrome/browser/ui/passwords/manage_passwords_icon.h (right): https://codereview.chromium.org/324853004/diff/1/chrome/browser/ui/passwords/manage_passwords_icon.h#newcode20 chrome/browser/ui/passwords/manage_passwords_icon.h:20: void ...
6 years, 6 months ago (2014-06-10 12:51:11 UTC) #3
oshima
https://codereview.chromium.org/324853004/diff/1/chrome/app/theme/theme_resources.grd File chrome/app/theme/theme_resources.grd (right): https://codereview.chromium.org/324853004/diff/1/chrome/app/theme/theme_resources.grd#newcode26 chrome/app/theme/theme_resources.grd:26: SAME CONDITIONALS. --> can you sort in alphabetical order?
6 years, 6 months ago (2014-06-10 18:51:05 UTC) #4
Mike West
On 2014/06/10 at 18:51:05, oshima wrote: > https://codereview.chromium.org/324853004/diff/1/chrome/app/theme/theme_resources.grd > File chrome/app/theme/theme_resources.grd (right): > > https://codereview.chromium.org/324853004/diff/1/chrome/app/theme/theme_resources.grd#newcode26 ...
6 years, 6 months ago (2014-06-10 18:58:33 UTC) #5
oshima
On 2014/06/10 18:58:33, Mike West wrote: > On 2014/06/10 at 18:51:05, oshima wrote: > > ...
6 years, 6 months ago (2014-06-10 19:16:46 UTC) #6
Mike West
Committed patchset #2 manually as r276284 (presubmit successful).
6 years, 6 months ago (2014-06-11 06:49:26 UTC) #7
Mike West
6 years, 6 months ago (2014-06-11 06:49:54 UTC) #8
Message was sent while issue was closed. 
Landed this manually, as rietveld does not play well with binary files.

Powered by Google App Engine
This is Rietveld 408576698