Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2070623003:
[MacViews] Show combobox menu popup at mouse press (Closed)
Description[MacViews] Show combobox menu popup at mouse press.
Create a PlatformStyle variable for the combobox button's NotifyAction
value. The variable is set to NOTIFY_ON_PRESS on Mac, and
NOTIFY_ON_RELEASE for other platforms. Placed TransparentButton's
OnMousePressed() override behind #if defined guards for Mac.
BUG=602912
Committed: https://crrev.com/a7b6e52dc0b0259c67dac9ded5c385739d36c7fc
Cr-Commit-Position: refs/heads/master@{#404012}
Patch Set 1 #Patch Set 2 : nit #
Total comments: 4
Patch Set 3 : Fix for tapted #Patch Set 4 : comment #
Total comments: 2
Patch Set 5 : Revert override #Patch Set 6 : added #if defined #
Total comments: 3
Patch Set 7 : Combobox test #Patch Set 8 #Patch Set 9 : nit #
Total comments: 13
Patch Set 10 : Fix for tapted #Patch Set 11 #Patch Set 12 : nit #
Total comments: 2
Patch Set 13 : invert #
Total comments: 2
Patch Set 14 : fix for sky #Patch Set 15 : Fixed test #
Messages
Total messages: 43 (16 generated)
Description was changed from ========== [MacViews] Show combobox menu popup at mouse press BUG= ========== to ========== [MacViews] Show combobox menu popup at mouse press BUG=602912 ==========
spqchan@chromium.org changed reviewers: + tapted@chromium.org
PTAL
https://codereview.chromium.org/2070623003/diff/20001/ui/views/controls/combo... File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/2070623003/diff/20001/ui/views/controls/combo... ui/views/controls/combobox/combobox.cc:113: set_notify_action(NOTIFY_ON_PRESS); We need to restrict this just to Mac - I think the way to go about it is to add CustomButton::NotifyAction PlatformStyle::kMenuActivationAction = CustomButton::NOTIFY_ON_PRESS; in platform_style_mac.mm (and = CustomButton::NOTIFY_ON_RELEASE in platform_style.cc within the !mac guards) https://codereview.chromium.org/2070623003/diff/20001/ui/views/controls/combo... ui/views/controls/combobox/combobox.cc:118: parent()->RequestFocus(); This actually isn't needed, CustomButton has a set_request_focus_on_press(bool) method which TransparentButton can call in the constructor. Conveniently, that setter is disabled on Mac - and that's exactly what we want, since activating a native combobox doesn't transfer focus to the combobox control. So I think we can get rid of the OnMousePressed override completely (regardless of platform) by adding that set_request_focus_on_press call (make sure you say something about this in the CL description, since it's subtle)
Description was changed from ========== [MacViews] Show combobox menu popup at mouse press BUG=602912 ========== to ========== [MacViews] Show combobox menu popup at mouse press. Replaced BUG=602912 ==========
Description was changed from ========== [MacViews] Show combobox menu popup at mouse press. Replaced BUG=602912 ========== to ========== [MacViews] Show combobox menu popup at mouse press. Replaced TransparentButton's OnMousePressed() override with set_request_focus_on_press(true); BUG=602912 ==========
PTAL https://codereview.chromium.org/2070623003/diff/20001/ui/views/controls/combo... File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/2070623003/diff/20001/ui/views/controls/combo... ui/views/controls/combobox/combobox.cc:113: set_notify_action(NOTIFY_ON_PRESS); On 2016/06/16 00:05:13, tapted wrote: > We need to restrict this just to Mac - I think the way to go about it is to add > > CustomButton::NotifyAction PlatformStyle::kMenuActivationAction = > CustomButton::NOTIFY_ON_PRESS; > > in platform_style_mac.mm > > (and = CustomButton::NOTIFY_ON_RELEASE in platform_style.cc within the !mac > guards) Done. https://codereview.chromium.org/2070623003/diff/20001/ui/views/controls/combo... ui/views/controls/combobox/combobox.cc:118: parent()->RequestFocus(); On 2016/06/16 00:05:13, tapted wrote: > This actually isn't needed, CustomButton has a set_request_focus_on_press(bool) > method which TransparentButton can call in the constructor. > > Conveniently, that setter is disabled on Mac - and that's exactly what we want, > since activating a native combobox doesn't transfer focus to the combobox > control. > > So I think we can get rid of the OnMousePressed override completely (regardless > of platform) by adding that set_request_focus_on_press call > > (make sure you say something about this in the CL description, since it's > subtle) Done.
lgtm %nit and an updated CL description it should mention the change to PlatformStyle. Also (for all CLs generally), there should be a blank line between the repeated one-line summary and the rest of the description. (i.e. break after " mouse press." with a double \n) https://codereview.chromium.org/2070623003/diff/60001/ui/views/style/platform... File ui/views/style/platform_style_mac.mm (right): https://codereview.chromium.org/2070623003/diff/60001/ui/views/style/platform... ui/views/style/platform_style_mac.mm:26: nit: remove blank line
lgtm %nit and an updated CL description it should mention the change to PlatformStyle. Also (for all CLs generally), there should be a blank line between the repeated one-line summary and the rest of the description. (i.e. break after " mouse press." with a double \n)
oops not lgtm yet https://codereview.chromium.org/2070623003/diff/60001/ui/views/controls/combo... File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/2070623003/diff/60001/ui/views/controls/combo... ui/views/controls/combobox/combobox.cc:114: set_request_focus_on_press(true); oh hang on a sec.. I just noticed the OnMousePressed override does *parent()*->RequestFocus but that's not what set_request_focus_on_press will do - doh. So I think we need to keep that override. But: - with the RequestFocus() call between #if defined(OS_MACOSX) - with the call to super::OnMousePressed
oops not lgtm yet
Description was changed from ========== [MacViews] Show combobox menu popup at mouse press. Replaced TransparentButton's OnMousePressed() override with set_request_focus_on_press(true); BUG=602912 ========== to ========== [MacViews] Show combobox menu popup at mouse press. Create a PlatformStyle variable for the combobox button's NotifyAction value. The variable is set to NOTIFY_ON_PRESS on Mac, and NOTIFY_ON_RELEASE for other platforms. Placed TransparentButton's OnMousePressed() override behind #if defined guards for Mac. BUG=602912 ==========
On 2016/06/22 04:23:04, tapted wrote: > oops not lgtm yet PTAL Sounds good. I fixed the nit and I added the override back in. I think it's more clean if we put the entire override in between #if defined(OS_MACOSX). WDYT?
https://codereview.chromium.org/2070623003/diff/100001/ui/views/controls/comb... File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/2070623003/diff/100001/ui/views/controls/comb... ui/views/controls/combobox/combobox.cc:118: bool OnMousePressed(const ui::MouseEvent& mouse_event) override { nit: comment here like // Override OnMousePressed() to transfer focus the the parent() on a click, except on Mac which doesn't transfer focus when buttons are clicked. there might be an alternative that involves setting request_focus_on_press, then checking CustomButton::request_focus_on_press() inside TransparentButton::OnMousePressed(), but then there's the issue of whether OnMousePressed should invoke super::OnMousePressed at all, since it will interpret request_focus_on_press a second time. We could also add a PlatformStyle setting for this, and update CustomButton::set_request_focus_on_press.. perhaps raise this with sky@ I think this is OK, since TransparentButton is a little odd anyway. And, e.g., having these child views might be messing up a11y hit tests, as well as intercepting these mouse presses. But! This is all pretty subtle - more subtle than I first realised - sorry!. We should update TEST_F(ComboboxTest, NotifyOnClickWithMouse) { to test the platform specific stuff. It currently uses PerformClick() which does both a mouse press and a mouse up. I think we can rewite PerformClick as { PerformMousePress(); PerformMouseRelease(); } then have NotifyOnClickWithMouse call PerformMouse/Release with checks in between for Mac and not-Mac to validate the platform differences.
PTAL. Sorry for the delay :) https://codereview.chromium.org/2070623003/diff/100001/ui/views/controls/comb... File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/2070623003/diff/100001/ui/views/controls/comb... ui/views/controls/combobox/combobox.cc:118: bool OnMousePressed(const ui::MouseEvent& mouse_event) override { On 2016/06/23 00:54:29, tapted wrote: > nit: comment here like > // Override OnMousePressed() to transfer focus the the parent() on a click, > except on Mac which doesn't transfer focus when buttons are clicked. > > there might be an alternative that involves setting request_focus_on_press, then > checking CustomButton::request_focus_on_press() inside > TransparentButton::OnMousePressed(), but then there's the issue of whether > OnMousePressed should invoke super::OnMousePressed at all, since it will > interpret request_focus_on_press a second time. > > We could also add a PlatformStyle setting for this, and update > CustomButton::set_request_focus_on_press.. > > perhaps raise this with sky@ > > I think this is OK, since TransparentButton is a little odd anyway. And, e.g., > having these child views might be messing up a11y hit tests, as well as > intercepting these mouse presses. > > > But! This is all pretty subtle - more subtle than I first realised - sorry!. We > should update > > TEST_F(ComboboxTest, NotifyOnClickWithMouse) { > > to test the platform specific stuff. It currently uses PerformClick() which does > both a mouse press and a mouse up. I think we can rewite PerformClick as { > PerformMousePress(); > PerformMouseRelease(); > } > > then have NotifyOnClickWithMouse call PerformMouse/Release with checks in > between for Mac and not-Mac to validate the platform differences. Added the comment in. I'm a bit confused with what you mean, should we leave parent()->RequestFocus() as it is or not? I updated the test. I noticed that other tests also check for the menu after PerformClick() is called. Should we make changes to them as well?
lgtm - hopefully sky will like it too https://codereview.chromium.org/2070623003/diff/100001/ui/views/controls/comb... File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/2070623003/diff/100001/ui/views/controls/comb... ui/views/controls/combobox/combobox.cc:118: bool OnMousePressed(const ui::MouseEvent& mouse_event) override { On 2016/06/28 17:05:59, spqchan wrote: > On 2016/06/23 00:54:29, tapted wrote: > > nit: comment here like > > // Override OnMousePressed() to transfer focus the the parent() on a click, > > except on Mac which doesn't transfer focus when buttons are clicked. > > > > there might be an alternative that involves setting request_focus_on_press, > then > > checking CustomButton::request_focus_on_press() inside > > TransparentButton::OnMousePressed(), but then there's the issue of whether > > OnMousePressed should invoke super::OnMousePressed at all, since it will > > interpret request_focus_on_press a second time. > > > > We could also add a PlatformStyle setting for this, and update > > CustomButton::set_request_focus_on_press.. > > > > perhaps raise this with sky@ > > > > I think this is OK, since TransparentButton is a little odd anyway. And, e.g., > > having these child views might be messing up a11y hit tests, as well as > > intercepting these mouse presses. > > > > > > But! This is all pretty subtle - more subtle than I first realised - sorry!. > We > > should update > > > > TEST_F(ComboboxTest, NotifyOnClickWithMouse) { > > > > to test the platform specific stuff. It currently uses PerformClick() which > does > > both a mouse press and a mouse up. I think we can rewite PerformClick as { > > PerformMousePress(); > > PerformMouseRelease(); > > } > > > > then have NotifyOnClickWithMouse call PerformMouse/Release with checks in > > between for Mac and not-Mac to validate the platform differences. > > Added the comment in. I'm a bit confused with what you mean, should we leave > parent()->RequestFocus() as it is or not? Yes - it should stay (just not on Mac). Sorry - had a typo in my sample comment. > I updated the test. I noticed that other tests also check for the menu after > PerformClick() is called. Should we make changes to them as well? I think PerformClick() is OK coverage for most - we don't want to litter the test file with #ifdefs https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... ui/views/controls/combobox/combobox.cc:1: // Copyright (c) 2012 The Chromium Authors. All rights reserved. cl description nit: wordwrap at <80 chars https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... ui/views/controls/combobox/combobox.cc:118: // Override OnMousePressed() to transfer focus on the parent() on a click "on the parent() on a click" -> "to the parent() on a click." "on Mac," -> "Except on Mac," https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... ui/views/controls/combobox/combobox.cc:122: CustomButton::OnMousePressed(mouse_event); nit: remove this bit (although I think it's fine, there's no longer a reason to make this change and there could be something subtle). https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... File ui/views/controls/combobox/combobox_unittest.cc (right): https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... ui/views/controls/combobox/combobox_unittest.cc:597: gfx::Point right_point(combobox_->x() + combobox_->width() - 1, nit: const gfx::Point right_point https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... ui/views/controls/combobox/combobox_unittest.cc:604: #if defined(OS_MACOSX) I'd structure this something like the following. This way we do the same thing on all platforms, just have different expectations. // On Mac, actions occur on mouse down. Otherwise mouse up. #if defined(OS_MACOSX) const int kActOnMouseDown = 1; #else const int kActOnMouseDown = 0; #endif PerformMousePress(right_point); EXPECT_EQ(kActOnMouseDown, menu_show_count); PerformMouseRelease(right_point); EXPECT_EQ(1, menu_show_count); ... PerformMousePress(left_point); EXPECT_EQ(kActOnMouseDown, listener.actions_performed()); PerformMouseRelease(left_point); EXPECT_EQ(1, listener.actions_performed()); https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... ui/views/controls/combobox/combobox_unittest.cc:616: gfx::Point left_point( nit: const
Description was changed from ========== [MacViews] Show combobox menu popup at mouse press. Create a PlatformStyle variable for the combobox button's NotifyAction value. The variable is set to NOTIFY_ON_PRESS on Mac, and NOTIFY_ON_RELEASE for other platforms. Placed TransparentButton's OnMousePressed() override behind #if defined guards for Mac. BUG=602912 ========== to ========== [MacViews] Show combobox menu popup at mouse press. Create a PlatformStyle variable for the combobox button's NotifyAction value. The variable is set to NOTIFY_ON_PRESS on Mac, and NOTIFY_ON_RELEASE for other platforms. Placed TransparentButton's OnMousePressed() override behind #if defined guards for Mac. BUG=602912 ==========
spqchan@chromium.org changed reviewers: + sky@chromium.org
Thanks! +sky What are your thoughts on combobox.cc? https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... ui/views/controls/combobox/combobox.cc:1: // Copyright (c) 2012 The Chromium Authors. All rights reserved. On 2016/06/29 12:02:55, tapted wrote: > cl description nit: wordwrap at <80 chars Done. https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... ui/views/controls/combobox/combobox.cc:118: // Override OnMousePressed() to transfer focus on the parent() on a click On 2016/06/29 12:02:55, tapted wrote: > "on the parent() on a click" -> "to the parent() on a click." > "on Mac," -> "Except on Mac," Done. https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... ui/views/controls/combobox/combobox.cc:122: CustomButton::OnMousePressed(mouse_event); On 2016/06/29 12:02:55, tapted wrote: > nit: remove this bit (although I think it's fine, there's no longer a reason to > make this change and there could be something subtle). We need this actually because CustomButton::MousePressed calls NotifyClick() https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... File ui/views/controls/combobox/combobox_unittest.cc (right): https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... ui/views/controls/combobox/combobox_unittest.cc:597: gfx::Point right_point(combobox_->x() + combobox_->width() - 1, On 2016/06/29 12:02:55, tapted wrote: > nit: const gfx::Point right_point Done. https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... ui/views/controls/combobox/combobox_unittest.cc:604: #if defined(OS_MACOSX) On 2016/06/29 12:02:55, tapted wrote: > I'd structure this something like the following. This way we do the same thing > on all platforms, just have different expectations. > > // On Mac, actions occur on mouse down. Otherwise mouse up. > #if defined(OS_MACOSX) > const int kActOnMouseDown = 1; > #else > const int kActOnMouseDown = 0; > #endif > > PerformMousePress(right_point); > EXPECT_EQ(kActOnMouseDown, menu_show_count); > PerformMouseRelease(right_point); > EXPECT_EQ(1, menu_show_count); > > ... > > PerformMousePress(left_point); > EXPECT_EQ(kActOnMouseDown, listener.actions_performed()); > PerformMouseRelease(left_point); > EXPECT_EQ(1, listener.actions_performed()); > Done. https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... ui/views/controls/combobox/combobox_unittest.cc:616: gfx::Point left_point( On 2016/06/29 12:02:55, tapted wrote: > nit: const Done.
https://codereview.chromium.org/2070623003/diff/220001/ui/views/controls/comb... File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/2070623003/diff/220001/ui/views/controls/comb... ui/views/controls/combobox/combobox.cc:117: #if defined(OS_MACOSX) This ifdef means we'll no longer get the old behavior on non-macos. Also, for mac, are you sure you want to change focus at all?
https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... ui/views/controls/combobox/combobox.cc:122: CustomButton::OnMousePressed(mouse_event); On 2016/06/29 18:29:14, spqchan wrote: > On 2016/06/29 12:02:55, tapted wrote: > > nit: remove this bit (although I think it's fine, there's no longer a reason > to > > make this change and there could be something subtle). > > We need this actually because CustomButton::MousePressed calls NotifyClick() Ah! So, I think we can remove this once the #ifdef is inverted :). https://codereview.chromium.org/2070623003/diff/220001/ui/views/controls/comb... File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/2070623003/diff/220001/ui/views/controls/comb... ui/views/controls/combobox/combobox.cc:117: #if defined(OS_MACOSX) On 2016/06/29 21:35:08, sky wrote: > This ifdef means we'll no longer get the old behavior on non-macos. Also, for > mac, are you sure you want to change focus at all? oh! yeah - this #ifdef needs to be inverted. (somehow I always saw an extra '!' here already :/).
On 2016/06/29 23:24:43, tapted wrote: > https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... > File ui/views/controls/combobox/combobox.cc (right): > > https://codereview.chromium.org/2070623003/diff/160001/ui/views/controls/comb... > ui/views/controls/combobox/combobox.cc:122: > CustomButton::OnMousePressed(mouse_event); > On 2016/06/29 18:29:14, spqchan wrote: > > On 2016/06/29 12:02:55, tapted wrote: > > > nit: remove this bit (although I think it's fine, there's no longer a reason > > to > > > make this change and there could be something subtle). > > > > We need this actually because CustomButton::MousePressed calls NotifyClick() > > Ah! So, I think we can remove this once the #ifdef is inverted :). > > https://codereview.chromium.org/2070623003/diff/220001/ui/views/controls/comb... > File ui/views/controls/combobox/combobox.cc (right): > > https://codereview.chromium.org/2070623003/diff/220001/ui/views/controls/comb... > ui/views/controls/combobox/combobox.cc:117: #if defined(OS_MACOSX) > On 2016/06/29 21:35:08, sky wrote: > > This ifdef means we'll no longer get the old behavior on non-macos. Also, for > > mac, are you sure you want to change focus at all? > > oh! yeah - this #ifdef needs to be inverted. (somehow I always saw an extra '!' > here already :/). Argh. Sorry, I misunderstood you =( I made the inversion
small nit on naming. https://codereview.chromium.org/2070623003/diff/240001/ui/views/style/platfor... File ui/views/style/platform_style.h (right): https://codereview.chromium.org/2070623003/diff/240001/ui/views/style/platfor... ui/views/style/platform_style.h:47: static const CustomButton::NotifyAction kMenuActivationAction; kMenuNotifyActivationAction?
PTAL https://codereview.chromium.org/2070623003/diff/240001/ui/views/style/platfor... File ui/views/style/platform_style.h (right): https://codereview.chromium.org/2070623003/diff/240001/ui/views/style/platfor... ui/views/style/platform_style.h:47: static const CustomButton::NotifyAction kMenuActivationAction; On 2016/06/30 16:24:46, sky wrote: > kMenuNotifyActivationAction? Done.
LGTM
The CQ bit was checked by spqchan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org Link to the patchset: https://codereview.chromium.org/2070623003/#ps260001 (title: "fix for sky")
thanks!
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: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_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 spqchan@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: linux_chromium_chromeos_ozone_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 spqchan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org, sky@chromium.org Link to the patchset: https://codereview.chromium.org/2070623003/#ps280001 (title: "Fixed test")
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 ========== [MacViews] Show combobox menu popup at mouse press. Create a PlatformStyle variable for the combobox button's NotifyAction value. The variable is set to NOTIFY_ON_PRESS on Mac, and NOTIFY_ON_RELEASE for other platforms. Placed TransparentButton's OnMousePressed() override behind #if defined guards for Mac. BUG=602912 ========== to ========== [MacViews] Show combobox menu popup at mouse press. Create a PlatformStyle variable for the combobox button's NotifyAction value. The variable is set to NOTIFY_ON_PRESS on Mac, and NOTIFY_ON_RELEASE for other platforms. Placed TransparentButton's OnMousePressed() override behind #if defined guards for Mac. BUG=602912 ==========
Message was sent while issue was closed.
Committed patchset #15 (id:280001)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== [MacViews] Show combobox menu popup at mouse press. Create a PlatformStyle variable for the combobox button's NotifyAction value. The variable is set to NOTIFY_ON_PRESS on Mac, and NOTIFY_ON_RELEASE for other platforms. Placed TransparentButton's OnMousePressed() override behind #if defined guards for Mac. BUG=602912 ========== to ========== [MacViews] Show combobox menu popup at mouse press. Create a PlatformStyle variable for the combobox button's NotifyAction value. The variable is set to NOTIFY_ON_PRESS on Mac, and NOTIFY_ON_RELEASE for other platforms. Placed TransparentButton's OnMousePressed() override behind #if defined guards for Mac. BUG=602912 Committed: https://crrev.com/a7b6e52dc0b0259c67dac9ded5c385739d36c7fc Cr-Commit-Position: refs/heads/master@{#404012} ==========
Message was sent while issue was closed.
Patchset 15 (id:??) landed as https://crrev.com/a7b6e52dc0b0259c67dac9ded5c385739d36c7fc Cr-Commit-Position: refs/heads/master@{#404012} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
