Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2291193003:
arc: Fix mouse scroll on notification (Closed)
|
Created:
4 years, 3 months ago by xiyuan Modified:
4 years, 3 months ago Reviewers:
yoshiki CC:
chromium-reviews, elijahtaylor+arcwatch_chromium.org, hidehiko+watch_chromium.org, lhchavez+watch_chromium.org, yusukes+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptionarc: Fix mouse scroll on notification
Forward ScrollEvent/MouseWheelEvent to the hosting widget so that
it is processed and reaches the containing ScrollView.
Also not forward touch events to ArcCustomNotificationView since
a View is not supposed to receive touch events. This gets rid of
the NOTREACHED() warning in chrome log.
BUG=b/31115616
BUG=642501
Committed: https://crrev.com/c00a606faa2cd8099f9d88abe08e93c7418fb1fb
Cr-Commit-Position: refs/heads/master@{#415708}
Patch Set 1 #Patch Set 2 : handle mouse wheel and move forwarding to widget code into EventForwarder #
Total comments: 2
Patch Set 3 : comment on why not forward touches #Messages
Total messages: 24 (15 generated)
Description was changed from ========== arc: Fix mouse scroll on notification Forward ScrollEvent to the hosting widget so that it is processed and reaches the containing ScrollView. Also not forward touch events to ArcCustomNotificationView since a View is not supposed to receive touch events. This gets rid of the NOTREACHED() warning in chrome log. BUG=b/31115616 ========== to ========== arc: Fix mouse scroll on notification Forward ScrollEvent to the hosting widget so that it is processed and reaches the containing ScrollView. Also not forward touch events to ArcCustomNotificationView since a View is not supposed to receive touch events. This gets rid of the NOTREACHED() warning in chrome log. BUG=b/31115616 BUG=642501 ==========
The CQ bit was checked by xiyuan@chromium.org to run a CQ dry run
xiyuan@chromium.org changed reviewers: + yoshiki@chromium.org
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
Thank you for the quick review!
Description was changed from ========== arc: Fix mouse scroll on notification Forward ScrollEvent to the hosting widget so that it is processed and reaches the containing ScrollView. Also not forward touch events to ArcCustomNotificationView since a View is not supposed to receive touch events. This gets rid of the NOTREACHED() warning in chrome log. BUG=b/31115616 BUG=642501 ========== to ========== arc: Fix mouse scroll on notification Forward ScrollEvent/MouseWheelEvent to the hosting widget so that it is processed and reaches the containing ScrollView. Also not forward touch events to ArcCustomNotificationView since a View is not supposed to receive touch events. This gets rid of the NOTREACHED() warning in chrome log. BUG=b/31115616 BUG=642501 ==========
Mind taking another look ? Just realized that I forgot to handle MouseWheelEvent which is separate from ScrollEvent. Added code to handle that. Also move those code into EventForwarder to keep the relevant together. Thanks.
The CQ bit was checked by xiyuan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM with nit. Thanks! https://codereview.chromium.org/2291193003/diff/20001/ui/arc/notification/arc... File ui/arc/notification/arc_custom_notification_view.cc (right): https://codereview.chromium.org/2291193003/diff/20001/ui/arc/notification/arc... ui/arc/notification/arc_custom_notification_view.cc:47: else if (!event->IsTouchEvent()) nit: as we discussed in chat, please add a comment of why you're skipping only touch event.
https://codereview.chromium.org/2291193003/diff/20001/ui/arc/notification/arc... File ui/arc/notification/arc_custom_notification_view.cc (right): https://codereview.chromium.org/2291193003/diff/20001/ui/arc/notification/arc... ui/arc/notification/arc_custom_notification_view.cc:47: else if (!event->IsTouchEvent()) On 2016/08/31 18:04:37, yoshiki wrote: > nit: as we discussed in chat, please add a comment of why you're skipping only > touch event. Done.
The CQ bit was checked by xiyuan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from yoshiki@chromium.org Link to the patchset: https://codereview.chromium.org/2291193003/#ps40001 (title: "comment on why not forward touches")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== arc: Fix mouse scroll on notification Forward ScrollEvent/MouseWheelEvent to the hosting widget so that it is processed and reaches the containing ScrollView. Also not forward touch events to ArcCustomNotificationView since a View is not supposed to receive touch events. This gets rid of the NOTREACHED() warning in chrome log. BUG=b/31115616 BUG=642501 ========== to ========== arc: Fix mouse scroll on notification Forward ScrollEvent/MouseWheelEvent to the hosting widget so that it is processed and reaches the containing ScrollView. Also not forward touch events to ArcCustomNotificationView since a View is not supposed to receive touch events. This gets rid of the NOTREACHED() warning in chrome log. BUG=b/31115616 BUG=642501 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== arc: Fix mouse scroll on notification Forward ScrollEvent/MouseWheelEvent to the hosting widget so that it is processed and reaches the containing ScrollView. Also not forward touch events to ArcCustomNotificationView since a View is not supposed to receive touch events. This gets rid of the NOTREACHED() warning in chrome log. BUG=b/31115616 BUG=642501 ========== to ========== arc: Fix mouse scroll on notification Forward ScrollEvent/MouseWheelEvent to the hosting widget so that it is processed and reaches the containing ScrollView. Also not forward touch events to ArcCustomNotificationView since a View is not supposed to receive touch events. This gets rid of the NOTREACHED() warning in chrome log. BUG=b/31115616 BUG=642501 Committed: https://crrev.com/c00a606faa2cd8099f9d88abe08e93c7418fb1fb Cr-Commit-Position: refs/heads/master@{#415708} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/c00a606faa2cd8099f9d88abe08e93c7418fb1fb Cr-Commit-Position: refs/heads/master@{#415708} |
