From ef6637ffae36b30abf91c6cd78dcd2fa7a3f59b5 Mon Sep 17 00:00:00 2001 From: varkha Date: Fri, 26 Feb 2016 14:12:27 -0800 Subject: [PATCH] Enables hot-tracking for overflow extension buttons in the app menu BUG=583559 Review URL: https://codereview.chromium.org/1661673004 Cr-Commit-Position: refs/heads/master@{#377999} --- ash/ash.gyp | 1 + .../browser/ui/libgtk2ui/native_theme_gtk2.cc | 7 ++- .../browser/ui/libgtk2ui/native_theme_gtk2.h | 2 +- chrome/browser/ui/views/toolbar/app_menu.cc | 48 +++++++++++---- .../ui/views/toolbar/toolbar_action_view.cc | 5 ++ .../ui/views/toolbar/toolbar_action_view.h | 1 + .../toolbar_action_view_interactive_uitest.cc | 8 +-- ui/native_theme/common_theme.cc | 13 +++- ui/native_theme/common_theme.h | 3 +- ui/native_theme/native_theme.cc | 6 ++ ui/native_theme/native_theme.h | 5 +- ui/native_theme/native_theme_aura.cc | 4 +- ui/native_theme/native_theme_aura.h | 2 +- ui/native_theme/native_theme_base.cc | 4 +- ui/native_theme/native_theme_base.h | 2 +- ui/native_theme/native_theme_mac.h | 2 +- ui/native_theme/native_theme_mac.mm | 2 +- ui/native_theme/native_theme_win.cc | 2 +- ui/views/controls/button/custom_button.cc | 8 +-- ui/views/controls/button/custom_button.h | 5 +- ui/views/controls/menu/menu_controller.cc | 60 ++++++++++--------- ui/views/controls/menu/menu_controller.h | 13 ++-- .../controls/menu/menu_controller_unittest.cc | 4 +- .../menu/menu_scroll_view_container.cc | 1 - 24 files changed, 134 insertions(+), 74 deletions(-) diff --git a/ash/ash.gyp b/ash/ash.gyp index e39e01965da57f..cd1e630ce7543e 100644 --- a/ash/ash.gyp +++ b/ash/ash.gyp @@ -982,6 +982,7 @@ '../ui/gfx/gfx.gyp:gfx_vector_icons', '../ui/keyboard/keyboard.gyp:keyboard', '../ui/message_center/message_center.gyp:message_center', + '../ui/native_theme/native_theme.gyp:native_theme', '../ui/platform_window/stub/stub_window.gyp:stub_window', '../ui/resources/ui_resources.gyp:ui_resources', '../ui/strings/ui_strings.gyp:ui_strings', diff --git a/chrome/browser/ui/libgtk2ui/native_theme_gtk2.cc b/chrome/browser/ui/libgtk2ui/native_theme_gtk2.cc index 371e13dd271ee0..d9dd353a5e0dcf 100644 --- a/chrome/browser/ui/libgtk2ui/native_theme_gtk2.cc +++ b/chrome/browser/ui/libgtk2ui/native_theme_gtk2.cc @@ -199,7 +199,7 @@ void NativeThemeGtk2::PaintMenuItemBackground( SkCanvas* canvas, State state, const gfx::Rect& rect, - const MenuListExtraParams& menu_list) const { + const MenuItemExtraParams& menu_item) const { SkColor color; SkPaint paint; switch (state) { @@ -217,6 +217,11 @@ void NativeThemeGtk2::PaintMenuItemBackground( NOTREACHED() << "Invalid state " << state; break; } + if (menu_item.corner_radius > 0) { + const SkScalar radius = SkIntToScalar(menu_item.corner_radius); + canvas->drawRoundRect(gfx::RectToSkRect(rect), radius, radius, paint); + return; + } canvas->drawRect(gfx::RectToSkRect(rect), paint); } diff --git a/chrome/browser/ui/libgtk2ui/native_theme_gtk2.h b/chrome/browser/ui/libgtk2ui/native_theme_gtk2.h index ddb83ebf4e02d6..042a87e6147acc 100644 --- a/chrome/browser/ui/libgtk2ui/native_theme_gtk2.h +++ b/chrome/browser/ui/libgtk2ui/native_theme_gtk2.h @@ -30,7 +30,7 @@ class NativeThemeGtk2 : public ui::NativeThemeBase { SkCanvas* canvas, State state, const gfx::Rect& rect, - const MenuListExtraParams& menu_list) const override; + const MenuItemExtraParams& menu_item) const override; // Gets a ChromeGtkFrame theme color; returns true on success. Always returns // false in GTK3. diff --git a/chrome/browser/ui/views/toolbar/app_menu.cc b/chrome/browser/ui/views/toolbar/app_menu.cc index 916ebad4a9e117..487e2c4101791e 100644 --- a/chrome/browser/ui/views/toolbar/app_menu.cc +++ b/chrome/browser/ui/views/toolbar/app_menu.cc @@ -46,6 +46,7 @@ #include "third_party/skia/include/core/SkPaint.h" #include "ui/base/l10n/l10n_util.h" #include "ui/base/layout.h" +#include "ui/base/material_design/material_design_controller.h" #include "ui/base/resource/resource_bundle.h" #include "ui/gfx/canvas.h" #include "ui/gfx/font_list.h" @@ -131,10 +132,20 @@ class FullscreenButton : public ImageButton { class InMenuButtonBackground : public views::Background { public: enum ButtonType { + // A rectangular button with no neighbor on the left. LEFT_BUTTON, + + // A rectangular button with neighbors on both sides. CENTER_BUTTON, + + // A rectangular button with no neighbor on the right. RIGHT_BUTTON, + + // A rectangular button that is not a member in a group. SINGLE_BUTTON, + + // A button with no group neighbors and a rounded background. + ROUNDED_BUTTON, }; explicit InMenuButtonBackground(ButtonType type) @@ -161,14 +172,15 @@ class InMenuButtonBackground : public views::Background { int h = view->height(); // Normal buttons get a border drawn on the right side and the rest gets - // filled in. The left button however does not get a line to combine - // buttons. - if (type_ != RIGHT_BUTTON) { + // filled in. The left or rounded buttons however do not get a line to + // combine buttons. + gfx::Rect bounds(view->GetLocalBounds()); + if (type_ != RIGHT_BUTTON && type_ != ROUNDED_BUTTON) { canvas->FillRect(gfx::Rect(0, 0, 1, h), BorderColor(view, views::Button::STATE_NORMAL)); + bounds.Inset(gfx::Insets(0, 1, 0, 0)); } - gfx::Rect bounds(view->GetLocalBounds()); bounds.set_x(view->GetMirroredXForRect(bounds)); DrawBackground(canvas, view, bounds, state); } @@ -213,11 +225,15 @@ class InMenuButtonBackground : public views::Background { views::Button::ButtonState state) const { if (state == views::Button::STATE_HOVERED || state == views::Button::STATE_PRESSED) { + ui::NativeTheme::ExtraParams params; + if (type_ == ROUNDED_BUTTON) { + // Consistent with a hover corner radius (kInkDropSmallCornerRadius). + const int kBackgroundCornerRadius = 2; + params.menu_item.corner_radius = kBackgroundCornerRadius; + } view->GetNativeTheme()->Paint(canvas->sk_canvas(), ui::NativeTheme::kMenuItemBackground, - ui::NativeTheme::kHovered, - bounds, - ui::NativeTheme::ExtraParams()); + ui::NativeTheme::kHovered, bounds, params); } } @@ -1137,11 +1153,21 @@ void AppMenu::PopulateMenu(MenuItemView* parent, MenuModel* model) { case IDC_EXTENSIONS_OVERFLOW_MENU: { scoped_ptr extension_toolbar( new ExtensionToolbarMenuView(browser_, this)); - extension_toolbar_ = extension_toolbar.get(); - if (extension_toolbar->ShouldShow()) - item->AddChildView(extension_toolbar.release()); - else + if (!extension_toolbar->ShouldShow()) { item->SetVisible(false); + extension_toolbar_ = nullptr; + break; + } + if (ui::MaterialDesignController::IsModeMaterial()) { + for (int i = 0; i < extension_toolbar->contents()->child_count(); + ++i) { + View* action_view = extension_toolbar->contents()->child_at(i); + action_view->set_background(new InMenuButtonBackground( + InMenuButtonBackground::ROUNDED_BUTTON)); + } + } + extension_toolbar_ = extension_toolbar.get(); + item->AddChildView(extension_toolbar.release()); break; } diff --git a/chrome/browser/ui/views/toolbar/toolbar_action_view.cc b/chrome/browser/ui/views/toolbar/toolbar_action_view.cc index 40e9ee8617d411..37f514008e2685 100644 --- a/chrome/browser/ui/views/toolbar/toolbar_action_view.cc +++ b/chrome/browser/ui/views/toolbar/toolbar_action_view.cc @@ -126,6 +126,11 @@ SkColor ToolbarActionView::GetInkDropBaseColor() const { ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON); } +bool ToolbarActionView::ShouldShowInkDropHover() const { + return !delegate_->ShownInsideMenu() && + views::MenuButton::ShouldShowInkDropHover(); +} + content::WebContents* ToolbarActionView::GetCurrentWebContents() const { return delegate_->GetCurrentWebContents(); } diff --git a/chrome/browser/ui/views/toolbar/toolbar_action_view.h b/chrome/browser/ui/views/toolbar/toolbar_action_view.h index 0524dec8f158e3..08d2b61b5922ed 100644 --- a/chrome/browser/ui/views/toolbar/toolbar_action_view.h +++ b/chrome/browser/ui/views/toolbar/toolbar_action_view.h @@ -79,6 +79,7 @@ class ToolbarActionView : public views::MenuButton, void AddInkDropLayer(ui::Layer* ink_drop_layer) override; void RemoveInkDropLayer(ui::Layer* ink_drop_layer) override; SkColor GetInkDropBaseColor() const override; + bool ShouldShowInkDropHover() const override; // ToolbarActionViewDelegateViews: content::WebContents* GetCurrentWebContents() const override; diff --git a/chrome/browser/ui/views/toolbar/toolbar_action_view_interactive_uitest.cc b/chrome/browser/ui/views/toolbar/toolbar_action_view_interactive_uitest.cc index 8c77e14ef1dd32..8a7e081039474f 100644 --- a/chrome/browser/ui/views/toolbar/toolbar_action_view_interactive_uitest.cc +++ b/chrome/browser/ui/views/toolbar/toolbar_action_view_interactive_uitest.cc @@ -72,12 +72,8 @@ void ActivateOverflowedActionWithKeyboard(Browser* browser, gfx::NativeWindow native_window = views::MenuController::GetActiveInstance()->owner()->GetNativeWindow(); - // Send two key down events followed by the return key. - // The two key down events target the toolbar action in the app menu. - // TODO(devlin): Shouldn't this be one key down event? - ui_controls::SendKeyPress(native_window, - ui::VKEY_DOWN, - false, false, false, false); + // Send a key down event followed by the return key. + // The key down event targets the toolbar action in the app menu. ui_controls::SendKeyPress(native_window, ui::VKEY_DOWN, false, false, false, false); diff --git a/ui/native_theme/common_theme.cc b/ui/native_theme/common_theme.cc index 2c1dd9af988de3..50dba800b87ff4 100644 --- a/ui/native_theme/common_theme.cc +++ b/ui/native_theme/common_theme.cc @@ -460,9 +460,11 @@ void CommonThemePaintMenuBackground(SkCanvas* canvas, const gfx::Rect& rect) { canvas->drawRect(gfx::RectToSkRect(rect), paint); } -void CommonThemePaintMenuItemBackground(SkCanvas* canvas, - NativeTheme::State state, - const gfx::Rect& rect) { +void CommonThemePaintMenuItemBackground( + SkCanvas* canvas, + NativeTheme::State state, + const gfx::Rect& rect, + const NativeTheme::MenuItemExtraParams& menu_item) { SkPaint paint; switch (state) { case NativeTheme::kNormal: @@ -478,6 +480,11 @@ void CommonThemePaintMenuItemBackground(SkCanvas* canvas, NOTREACHED() << "Invalid state " << state; break; } + if (menu_item.corner_radius > 0) { + const SkScalar radius = SkIntToScalar(menu_item.corner_radius); + canvas->drawRoundRect(gfx::RectToSkRect(rect), radius, radius, paint); + return; + } canvas->drawRect(gfx::RectToSkRect(rect), paint); } diff --git a/ui/native_theme/common_theme.h b/ui/native_theme/common_theme.h index b30b68d8686fcf..ca40765c67ca9e 100644 --- a/ui/native_theme/common_theme.h +++ b/ui/native_theme/common_theme.h @@ -45,7 +45,8 @@ void NATIVE_THEME_EXPORT CommonThemePaintMenuBackground(SkCanvas* canvas, void NATIVE_THEME_EXPORT CommonThemePaintMenuItemBackground( SkCanvas* canvas, NativeTheme::State state, - const gfx::Rect& rect); + const gfx::Rect& rect, + const NativeTheme::MenuItemExtraParams& menu_item); // Creates a gfx::Canvas wrapping an SkCanvas. scoped_ptr NATIVE_THEME_EXPORT CommonThemeCreateCanvas( diff --git a/ui/native_theme/native_theme.cc b/ui/native_theme/native_theme.cc index f15f144b8c0874..96b0072ce706ec 100644 --- a/ui/native_theme/native_theme.cc +++ b/ui/native_theme/native_theme.cc @@ -4,10 +4,16 @@ #include "ui/native_theme/native_theme.h" +#include + #include "ui/native_theme/native_theme_observer.h" namespace ui { +NativeTheme::ExtraParams::ExtraParams() { + memset(this, 0, sizeof(*this)); +} + void NativeTheme::SetScrollbarColors(unsigned inactive_color, unsigned active_color, unsigned track_color) { diff --git a/ui/native_theme/native_theme.h b/ui/native_theme/native_theme.h index b09dd15b1c1717..f4dbd690a7edff 100644 --- a/ui/native_theme/native_theme.h +++ b/ui/native_theme/native_theme.h @@ -132,6 +132,7 @@ class NATIVE_THEME_EXPORT NativeTheme { struct MenuItemExtraParams { bool is_selected; + int corner_radius; }; struct MenuListExtraParams { @@ -196,7 +197,9 @@ class NATIVE_THEME_EXPORT NativeTheme { int classic_state; // Used on Windows when uxtheme is not available. }; - union ExtraParams { + union NATIVE_THEME_EXPORT ExtraParams { + ExtraParams(); + ButtonExtraParams button; InnerSpinButtonExtraParams inner_spin; MenuArrowExtraParams menu_arrow; diff --git a/ui/native_theme/native_theme_aura.cc b/ui/native_theme/native_theme_aura.cc index 223969f44b34ff..0afdd1191149f0 100644 --- a/ui/native_theme/native_theme_aura.cc +++ b/ui/native_theme/native_theme_aura.cc @@ -130,8 +130,8 @@ void NativeThemeAura::PaintMenuItemBackground( SkCanvas* canvas, State state, const gfx::Rect& rect, - const MenuListExtraParams& menu_list) const { - CommonThemePaintMenuItemBackground(canvas, state, rect); + const MenuItemExtraParams& menu_item) const { + CommonThemePaintMenuItemBackground(canvas, state, rect, menu_item); } void NativeThemeAura::PaintArrowButton(SkCanvas* canvas, diff --git a/ui/native_theme/native_theme_aura.h b/ui/native_theme/native_theme_aura.h index d9503f6ab2eaa7..85ce5bc708dbdf 100644 --- a/ui/native_theme/native_theme_aura.h +++ b/ui/native_theme/native_theme_aura.h @@ -29,7 +29,7 @@ class NATIVE_THEME_EXPORT NativeThemeAura : public NativeThemeBase { SkCanvas* canvas, State state, const gfx::Rect& rect, - const MenuListExtraParams& menu_list) const override; + const MenuItemExtraParams& menu_item) const override; void PaintArrowButton(SkCanvas* gc, const gfx::Rect& rect, Part direction, diff --git a/ui/native_theme/native_theme_base.cc b/ui/native_theme/native_theme_base.cc index 1a7837ef3ebc61..4da1497e40a2ec 100644 --- a/ui/native_theme/native_theme_base.cc +++ b/ui/native_theme/native_theme_base.cc @@ -213,7 +213,7 @@ void NativeThemeBase::Paint(SkCanvas* canvas, PaintMenuPopupBackground(canvas, rect.size(), extra.menu_background); break; case kMenuItemBackground: - PaintMenuItemBackground(canvas, state, rect, extra.menu_list); + PaintMenuItemBackground(canvas, state, rect, extra.menu_item); break; case kProgressBar: PaintProgressBar(canvas, state, rect, extra.progress_bar); @@ -792,7 +792,7 @@ void NativeThemeBase::PaintMenuItemBackground( SkCanvas* canvas, State state, const gfx::Rect& rect, - const MenuListExtraParams& menu_list) const { + const MenuItemExtraParams& menu_item) const { // By default don't draw anything over the normal background. } diff --git a/ui/native_theme/native_theme_base.h b/ui/native_theme/native_theme_base.h index 5b91753426c3bb..a8677d3bf30e1c 100644 --- a/ui/native_theme/native_theme_base.h +++ b/ui/native_theme/native_theme_base.h @@ -108,7 +108,7 @@ class NATIVE_THEME_EXPORT NativeThemeBase : public NativeTheme { SkCanvas* canvas, State state, const gfx::Rect& rect, - const MenuListExtraParams& menu_list) const; + const MenuItemExtraParams& menu_item) const; virtual void PaintSliderTrack( SkCanvas* canvas, diff --git a/ui/native_theme/native_theme_mac.h b/ui/native_theme/native_theme_mac.h index ca9ac071b152ef..6fe3f61d1c4740 100644 --- a/ui/native_theme/native_theme_mac.h +++ b/ui/native_theme/native_theme_mac.h @@ -28,7 +28,7 @@ class NATIVE_THEME_EXPORT NativeThemeMac : public NativeThemeBase { SkCanvas* canvas, State state, const gfx::Rect& rect, - const MenuListExtraParams& menu_list) const override; + const MenuItemExtraParams& menu_item) const override; private: NativeThemeMac(); diff --git a/ui/native_theme/native_theme_mac.mm b/ui/native_theme/native_theme_mac.mm index e81c60c2f4d8ed..f9627ff3cb75d4 100644 --- a/ui/native_theme/native_theme_mac.mm +++ b/ui/native_theme/native_theme_mac.mm @@ -245,7 +245,7 @@ SkColor NSSystemColorToSkColor(NSColor* color) { SkCanvas* canvas, State state, const gfx::Rect& rect, - const MenuListExtraParams& menu_list) const { + const MenuItemExtraParams& menu_item) const { SkPaint paint; switch (state) { case NativeTheme::kNormal: diff --git a/ui/native_theme/native_theme_win.cc b/ui/native_theme/native_theme_win.cc index be5109a7aedfb7..745af69ab164da 100644 --- a/ui/native_theme/native_theme_win.cc +++ b/ui/native_theme/native_theme_win.cc @@ -257,7 +257,7 @@ void NativeThemeWin::Paint(SkCanvas* canvas, CommonThemePaintMenuBackground(canvas, rect); return; case kMenuItemBackground: - CommonThemePaintMenuItemBackground(canvas, state, rect); + CommonThemePaintMenuItemBackground(canvas, state, rect, extra.menu_item); return; default: break; diff --git a/ui/views/controls/button/custom_button.cc b/ui/views/controls/button/custom_button.cc index 11ea15bb9a72c5..5cb521b9ebcb66 100644 --- a/ui/views/controls/button/custom_button.cc +++ b/ui/views/controls/button/custom_button.cc @@ -399,6 +399,10 @@ bool CustomButton::ShouldEnterHoveredState() { return check_mouse_position && IsMouseHovered(); } +bool CustomButton::ShouldShowInkDropHover() const { + return enabled() && IsMouseHovered() && !InDrag(); +} + //////////////////////////////////////////////////////////////////////////////// // CustomButton, View overrides (protected): @@ -425,8 +429,4 @@ void CustomButton::OnClickCanceled(const ui::Event& event) { Button::OnClickCanceled(event); } -bool CustomButton::ShouldShowInkDropHover() const { - return enabled() && IsMouseHovered() && !InDrag(); -} - } // namespace views diff --git a/ui/views/controls/button/custom_button.h b/ui/views/controls/button/custom_button.h index 1ec88a00b73dca..dfd0384c4340fe 100644 --- a/ui/views/controls/button/custom_button.h +++ b/ui/views/controls/button/custom_button.h @@ -122,6 +122,9 @@ class VIEWS_EXPORT CustomButton : public Button, public gfx::AnimationDelegate { // we simply return IsTriggerableEvent(event). virtual bool ShouldEnterPushedState(const ui::Event& event); + // Returns true if hover effect should be visible. + virtual bool ShouldShowInkDropHover() const; + void set_has_ink_drop_action_on_click(bool has_ink_drop_action_on_click) { has_ink_drop_action_on_click_ = has_ink_drop_action_on_click; } @@ -151,8 +154,6 @@ class VIEWS_EXPORT CustomButton : public Button, public gfx::AnimationDelegate { } private: - bool ShouldShowInkDropHover() const; - ButtonState state_; gfx::ThrobAnimation hover_animation_; diff --git a/ui/views/controls/menu/menu_controller.cc b/ui/views/controls/menu/menu_controller.cc index a3af4c13b5876b..b271d959e78cb6 100644 --- a/ui/views/controls/menu/menu_controller.cc +++ b/ui/views/controls/menu/menu_controller.cc @@ -53,16 +53,6 @@ using base::Time; using base::TimeDelta; using ui::OSExchangeData; -// Period of the scroll timer (in milliseconds). -static const int kScrollTimerMS = 30; - -// Amount of time from when the drop exits the menu and the menu is hidden. -static const int kCloseOnExitTime = 1200; - -// If a context menu is invoked by touch, we shift the menu by this offset so -// that the finger does not obscure the menu. -static const int kCenteredContextMenuYOffset = -15; - namespace views { namespace { @@ -70,7 +60,17 @@ namespace { // When showing context menu on mouse down, the user might accidentally select // the menu item on the subsequent mouse up. To prevent this, we add the // following delay before the user is able to select an item. -static int menu_selection_hold_time_ms = kMinimumMsPressedToActivate; +int menu_selection_hold_time_ms = kMinimumMsPressedToActivate; + +// Period of the scroll timer (in milliseconds). +const int kScrollTimerMS = 30; + +// Amount of time from when the drop exits the menu and the menu is hidden. +const int kCloseOnExitTime = 1200; + +// If a context menu is invoked by touch, we shift the menu by this offset so +// that the finger does not obscure the menu. +const int kCenteredContextMenuYOffset = -15; // The spacing offset for the bubble tip. const int kBubbleTipSizeLeftRight = 12; @@ -97,7 +97,7 @@ bool TitleMatchesMnemonic(MenuItemView* menu, base::char16 key) { } // Returns the first descendant of |view| that is hot tracked. -static CustomButton* GetFirstHotTrackedView(View* view) { +CustomButton* GetFirstHotTrackedView(View* view) { if (!view) return NULL; CustomButton* button = CustomButton::AsCustomButton(view); @@ -119,7 +119,7 @@ static CustomButton* GetFirstHotTrackedView(View* view) { // the first view (if |forward| is false, iterating starts at the last view). If // |forward| is true the children are considered first to last, otherwise last // to first. -static View* GetFirstFocusableView(View* view, int start, bool forward) { +View* GetFirstFocusableView(View* view, int start, bool forward) { if (forward) { for (int i = start == -1 ? 0 : start; i < view->child_count(); ++i) { View* deepest = GetFirstFocusableView(view->child_at(i), -1, forward); @@ -137,15 +137,13 @@ static View* GetFirstFocusableView(View* view, int start, bool forward) { } // Returns the first child of |start| that is focusable. -static View* GetInitialFocusableView(View* start, bool forward) { +View* GetInitialFocusableView(View* start, bool forward) { return GetFirstFocusableView(start, -1, forward); } // Returns the next view after |start_at| that is focusable. Returns NULL if // there are no focusable children of |ancestor| after |start_at|. -static View* GetNextFocusableView(View* ancestor, - View* start_at, - bool forward) { +View* GetNextFocusableView(View* ancestor, View* start_at, bool forward) { DCHECK(ancestor->Contains(start_at)); View* parent = start_at; do { @@ -719,6 +717,9 @@ void MenuController::OnMouseMoved(SubmenuView* source, MenuHostRootView* root_view = GetRootView(source, event.location()); if (root_view) root_view->ProcessMouseMoved(event); + // TODO(varkha): It is possible that another child CustomButton has become + // hot-tracked as a result of this event. We need to track it for accurate + // hot-tracking when both mouse and keyboard are used to navigate the menu. HandleMouseLocation(source, event.location()); } @@ -2086,8 +2087,7 @@ void MenuController::IncrementSelection( // select the first menu item that is visible and enabled. if (item->GetSubmenu()->GetMenuItemCount()) { MenuItemView* to_select = FindInitialSelectableMenuItem(item, direction); - if (to_select) - SetSelection(to_select, SELECTION_DEFAULT); + SetInitialHotTrackedView(to_select, direction); return; } } @@ -2122,14 +2122,7 @@ void MenuController::IncrementSelection( if (parent->GetSubmenu()->GetMenuItemAt(i) == item) { MenuItemView* to_select = FindNextSelectableMenuItem(parent, i, direction); - if (!to_select) - break; - SetSelection(to_select, SELECTION_DEFAULT); - View* to_make_hot = GetInitialFocusableView( - to_select, direction == INCREMENT_SELECTION_DOWN); - CustomButton* button_hot = CustomButton::AsCustomButton(to_make_hot); - if (button_hot) - button_hot->SetHotTracked(true); + SetInitialHotTrackedView(to_select, direction); break; } } @@ -2611,4 +2604,17 @@ void MenuController::HandleMouseLocation(SubmenuView* source, } } +void MenuController::SetInitialHotTrackedView( + MenuItemView* item, + SelectionIncrementDirectionType direction) { + if (!item) + return; + SetSelection(item, SELECTION_DEFAULT); + View* hot_view = + GetInitialFocusableView(item, direction == INCREMENT_SELECTION_DOWN); + CustomButton* hot_button = CustomButton::AsCustomButton(hot_view); + if (hot_button) + hot_button->SetHotTracked(true); +} + } // namespace views diff --git a/ui/views/controls/menu/menu_controller.h b/ui/views/controls/menu/menu_controller.h index 2b743f72e1c2f9..839b19fa6d64fe 100644 --- a/ui/views/controls/menu/menu_controller.h +++ b/ui/views/controls/menu/menu_controller.h @@ -499,11 +499,10 @@ class VIEWS_EXPORT MenuController : public WidgetObserver { void SelectByChar(base::char16 key); // For Windows and Aura we repost an event which dismisses the |source| menu. - // The menu is also canceled dependent on the target of the event. The event - // is then reprocessed to cause its result if the menu had not been present. - // On non-aura Windows, a new mouse event is generated and posted to - // the window (if there is one) at the location of the event. On - // aura, the event is reposted on the RootWindow. + // The menu may also be canceled depending on the target of the event. |event| + // is then processed without the menu present. On non-aura Windows, a new + // mouse event is generated and posted to the window (if there is one) at the + // location of the event. On aura, the event is reposted on the RootWindow. void RepostEventAndCancel(SubmenuView* source, const ui::LocatedEvent* event); // Sets the drop target to new_item. @@ -558,6 +557,10 @@ class VIEWS_EXPORT MenuController : public WidgetObserver { void HandleMouseLocation(SubmenuView* source, const gfx::Point& mouse_location); + // Sets hot-tracked state to the first focusable descendant view of |item|. + void SetInitialHotTrackedView(MenuItemView* item, + SelectionIncrementDirectionType direction); + // The active instance. static MenuController* active_instance_; diff --git a/ui/views/controls/menu/menu_controller_unittest.cc b/ui/views/controls/menu/menu_controller_unittest.cc index a25610c50804af..4030092e556a0b 100644 --- a/ui/views/controls/menu/menu_controller_unittest.cc +++ b/ui/views/controls/menu/menu_controller_unittest.cc @@ -876,7 +876,7 @@ TEST_F(MenuControllerTest, AsynchronousRepostEvent) { false, false, &mouse_event_flags); EXPECT_EQ(run_result, nullptr); - // Show a sub menu to targert with a pointer selection. However have the event + // Show a sub menu to target with a pointer selection. However have the event // occur outside of the bounds of the entire menu. SubmenuView* sub_menu = item->GetSubmenu(); sub_menu->ShowAt(owner(), item->bounds(), false); @@ -975,7 +975,7 @@ TEST_F(MenuControllerTest, AsynchronousRepostEventDeletesController) { false, false, &mouse_event_flags); EXPECT_EQ(run_result, nullptr); - // Show a sub menu to targert with a pointer selection. However have the event + // Show a sub menu to target with a pointer selection. However have the event // occur outside of the bounds of the entire menu. SubmenuView* sub_menu = item->GetSubmenu(); sub_menu->ShowAt(owner(), item->bounds(), true); diff --git a/ui/views/controls/menu/menu_scroll_view_container.cc b/ui/views/controls/menu/menu_scroll_view_container.cc index fa71a83dcfc2a2..381bef70f6f814 100644 --- a/ui/views/controls/menu/menu_scroll_view_container.cc +++ b/ui/views/controls/menu/menu_scroll_view_container.cc @@ -76,7 +76,6 @@ class MenuScrollButton : public View { // The background. gfx::Rect item_bounds(0, 0, width(), height()); NativeTheme::ExtraParams extra; - extra.menu_item.is_selected = false; GetNativeTheme()->Paint(canvas->sk_canvas(), NativeTheme::kMenuItemBackground, NativeTheme::kNormal, item_bounds, extra);