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

Issue 2622603003: Hide icons in message center on lock screen (Closed)

Created:
3 years, 11 months ago by yoshiki
Modified:
3 years, 10 months ago
Reviewers:
sky, dewittj
CC:
chromium-reviews, Peter Beverloo, tfarina, mlamouri+watch-notifications_chromium.org, awdf+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Hide icons in message center on lock screen This patch removes all the icons in the message center when the screen is locked. This patch also sets the height of the button bar to keep the height even when all the icons are invisible in the message center. BUG=674410 TEST=manually Review-Url: https://codereview.chromium.org/2622603003 Cr-Commit-Position: refs/heads/master@{#447679} Committed: https://chromium.googlesource.com/chromium/src/+/bc9a6161faa2b8261a38df8f0d640cb42a7e26d0

Patch Set 1 #

Total comments: 1

Patch Set 2 : Addressed comment and add tests #

Total comments: 2

Patch Set 3 : Use default argument #

Total comments: 2

Patch Set 4 : Mentioned about height parameter #

Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -6 lines) Patch
M ui/message_center/views/message_center_button_bar.h View 1 2 chunks +4 lines, -0 lines 0 comments Download
M ui/message_center/views/message_center_button_bar.cc View 1 2 3 chunks +20 lines, -1 line 0 comments Download
M ui/message_center/views/message_center_view.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M ui/message_center/views/message_center_view.cc View 1 1 chunk +6 lines, -1 line 0 comments Download
M ui/message_center/views/message_center_view_unittest.cc View 1 4 chunks +24 lines, -0 lines 0 comments Download
M ui/views/layout/grid_layout.h View 1 2 3 1 chunk +3 lines, -2 lines 0 comments Download
M ui/views/layout/grid_layout.cc View 1 2 1 chunk +5 lines, -2 lines 0 comments Download

Messages

Total messages: 42 (33 generated)
yoshiki
Justin, PTAL. Thanks.
3 years, 11 months ago (2017-01-10 17:10:57 UTC) #7
dewittj
can you please add a test? (perhaps to message_center_view_unittest) otherwise code lgtm https://codereview.chromium.org/2622603003/diff/1/ui/message_center/views/message_center_view.cc File ui/message_center/views/message_center_view.cc ...
3 years, 11 months ago (2017-01-12 18:29:57 UTC) #8
yoshiki
sky@: could you take a look at the changes under ui/layout? Thanks
3 years, 11 months ago (2017-01-18 09:17:41 UTC) #14
sky
https://codereview.chromium.org/2622603003/diff/20001/ui/views/layout/grid_layout.h File ui/views/layout/grid_layout.h (right): https://codereview.chromium.org/2622603003/diff/20001/ui/views/layout/grid_layout.h#newcode136 ui/views/layout/grid_layout.h:136: void StartRowWithHeight(float vertical_resize, int column_set_id, int height); I think ...
3 years, 11 months ago (2017-01-18 16:55:03 UTC) #15
yoshiki
Sky, PTAL. Thanks https://codereview.chromium.org/2622603003/diff/20001/ui/views/layout/grid_layout.h File ui/views/layout/grid_layout.h (right): https://codereview.chromium.org/2622603003/diff/20001/ui/views/layout/grid_layout.h#newcode136 ui/views/layout/grid_layout.h:136: void StartRowWithHeight(float vertical_resize, int column_set_id, int ...
3 years, 10 months ago (2017-02-01 09:47:43 UTC) #30
sky
LGTM https://codereview.chromium.org/2622603003/diff/80001/ui/views/layout/grid_layout.h File ui/views/layout/grid_layout.h (right): https://codereview.chromium.org/2622603003/diff/80001/ui/views/layout/grid_layout.h#newcode132 ui/views/layout/grid_layout.h:132: // Starts a new row with the specified ...
3 years, 10 months ago (2017-02-01 16:33:51 UTC) #31
yoshiki
Thanks! https://codereview.chromium.org/2622603003/diff/80001/ui/views/layout/grid_layout.h File ui/views/layout/grid_layout.h (right): https://codereview.chromium.org/2622603003/diff/80001/ui/views/layout/grid_layout.h#newcode132 ui/views/layout/grid_layout.h:132: // Starts a new row with the specified ...
3 years, 10 months ago (2017-02-01 18:01:06 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2622603003/100001
3 years, 10 months ago (2017-02-02 01:40:01 UTC) #39
commit-bot: I haz the power
3 years, 10 months ago (2017-02-02 01:49:47 UTC) #42
Message was sent while issue was closed.
Committed patchset #4 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/bc9a6161faa2b8261a38df8f0d64...

Powered by Google App Engine
This is Rietveld 408576698