Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2880623002:
[ash-md] Fixed MenuButton focus appearing after dismissing menus. (Closed)
Description[ash-md] Fixed MenuButton focus appearing after dismissing menus.
The InkDrop hover was not properly being set when menus were being
shown/dismissed and the event handler target was changing.
BUG=719399
TEST=views_unittests --gtest_filter=*MenuButtonTest*
Review-Url: https://codereview.chromium.org/2880623002
Cr-Commit-Position: refs/heads/master@{#472716}
Committed: https://chromium.googlesource.com/chromium/src/+/0a2a69a314c707ddfb83387690242dc86b088eed
Patch Set 1 #Patch Set 2 : Added check for null Widget. #
Total comments: 1
Patch Set 3 : Reworked solution to use CustomButton::ShouldEnterHoveredState(). #
Messages
Total messages: 31 (21 generated)
The CQ bit was checked by bruthig@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: Try jobs failed on following builders: linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by bruthig@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: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
bruthig@chromium.org changed reviewers: + sky@chromium.org, yoshiki@chromium.org
sky@, can you take a quick look and let me know if you have any concerns with the approach? yoshiki@, I thought you might have some opinions here too as this change is related to https://codereview.chromium.org/1297373004. Can you let me know if see any averse side effects of the approach? BTW I need to investigate the try-bot failures and see if a test is doable too but I'd appreciate your early feedback
https://codereview.chromium.org/2880623002/diff/20001/ui/views/controls/butto... File ui/views/controls/button/menu_button.cc (right): https://codereview.chromium.org/2880623002/diff/20001/ui/views/controls/butto... ui/views/controls/button/menu_button.cc:406: if (GetWidget()) { What about dispatching the event at the time the menu is shown? Doing it later seems rather after the fact and potentially problematic.
The CQ bit was checked by bruthig@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.
Description was changed from ========== [ash-md] Fixed MenuButton focus appearing after dismissing menus. Mouse events are not dispatched to MenuButtons after the menu is shown and this caused the hover affordance to be shown incorrectly when the menu is dismissed and the mouse is no longer in the MenuButton's bounds. This change forces synthetic mouse move events to be dispatched to the Window/Widget that owns the MenuButton after the menu is dismissed. BUG=719399 ========== to ========== [ash-md] Fixed MenuButton focus appearing after dismissing menus. The InkDrop hover was not properly being set when menus were being shown/dismissed and the event handler target was changing. BUG=719399 TEST=views_unittests --gtest_filter=*MenuButtonTest* ==========
sky@, can you take another look? I've completely changed the approach here to re-use CustomButton::ShouldEnterHoveredState().
Nice! LGTM
The CQ bit was checked by bruthig@chromium.org
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
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by bruthig@chromium.org
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
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by bruthig@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495076410941540,
"parent_rev": "fee367ad5f048733aba40a3a7b64ddb7967355dc", "commit_rev":
"0a2a69a314c707ddfb83387690242dc86b088eed"}
Message was sent while issue was closed.
Description was changed from ========== [ash-md] Fixed MenuButton focus appearing after dismissing menus. The InkDrop hover was not properly being set when menus were being shown/dismissed and the event handler target was changing. BUG=719399 TEST=views_unittests --gtest_filter=*MenuButtonTest* ========== to ========== [ash-md] Fixed MenuButton focus appearing after dismissing menus. The InkDrop hover was not properly being set when menus were being shown/dismissed and the event handler target was changing. BUG=719399 TEST=views_unittests --gtest_filter=*MenuButtonTest* Review-Url: https://codereview.chromium.org/2880623002 Cr-Commit-Position: refs/heads/master@{#472716} Committed: https://chromium.googlesource.com/chromium/src/+/0a2a69a314c707ddfb8338769024... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/0a2a69a314c707ddfb8338769024... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
