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

Issue 2966673002: Add ripple effect to action in expand button. (Closed)

Created:
3 years, 5 months ago by tetsui
Modified:
3 years, 5 months ago
Reviewers:
yoshiki, fukino
CC:
chromium-reviews, Peter Beverloo, mlamouri+watch-notifications_chromium.org, awdf+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Add ripple effect to action in expand button. In new-style notification, click inside the expand button in the header could not trigger ripple effect animation. This CL forwards mouse and key events to NotificationHeaderView. The CL also disables ripple effect when the expand button is disabled. BUG=736996 TEST=manual Review-Url: https://codereview.chromium.org/2966673002 Cr-Commit-Position: refs/heads/master@{#484045} Committed: https://chromium.googlesource.com/chromium/src/+/984a15c405577aff40ac40636b010231c1009f33

Patch Set 1 #

Total comments: 6

Patch Set 2 : Resolve review comments. #

Patch Set 3 : Remove unnecessary include. #

Patch Set 4 : Rebased. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -13 lines) Patch
M ui/message_center/views/notification_header_view.h View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M ui/message_center/views/notification_header_view.cc View 1 2 3 3 chunks +46 lines, -9 lines 0 comments Download
M ui/message_center/views/notification_view_md.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 24 (16 generated)
tetsui
Please take a look. Thanks!
3 years, 5 months ago (2017-06-30 04:21:27 UTC) #2
yoshiki
https://codereview.chromium.org/2966673002/diff/1/ui/message_center/views/notification_header_view.h File ui/message_center/views/notification_header_view.h (right): https://codereview.chromium.org/2966673002/diff/1/ui/message_center/views/notification_header_view.h#newcode21 ui/message_center/views/notification_header_view.h:21: class ExpandButton; We should avoid forward declaring a methods ...
3 years, 5 months ago (2017-07-03 06:15:47 UTC) #7
yoshiki
https://codereview.chromium.org/2966673002/diff/1/ui/message_center/views/notification_header_view.cc File ui/message_center/views/notification_header_view.cc (right): https://codereview.chromium.org/2966673002/diff/1/ui/message_center/views/notification_header_view.cc#newcode46 ui/message_center/views/notification_header_view.cc:46: class ExpandButton : public views::ImageButton, public views::ButtonListener { BTW, ...
3 years, 5 months ago (2017-07-03 06:26:49 UTC) #8
tetsui
https://codereview.chromium.org/2966673002/diff/1/ui/message_center/views/notification_header_view.cc File ui/message_center/views/notification_header_view.cc (right): https://codereview.chromium.org/2966673002/diff/1/ui/message_center/views/notification_header_view.cc#newcode46 ui/message_center/views/notification_header_view.cc:46: class ExpandButton : public views::ImageButton, public views::ButtonListener { On ...
3 years, 5 months ago (2017-07-03 07:10:54 UTC) #9
yoshiki
lgtm
3 years, 5 months ago (2017-07-04 02:06:22 UTC) #18
tetsui
Thank you for the reviewing!
3 years, 5 months ago (2017-07-04 02:07:21 UTC) #19
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/2966673002/60001
3 years, 5 months ago (2017-07-04 02:07:43 UTC) #21
commit-bot: I haz the power
3 years, 5 months ago (2017-07-04 03:11:30 UTC) #24
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/984a15c405577aff40ac40636b01...

Powered by Google App Engine
This is Rietveld 408576698