From 894ae1b3f5f0c0322f083d240db04d11cf28e885 Mon Sep 17 00:00:00 2001 From: Heilig Benedek Date: Mon, 29 Oct 2018 19:08:47 +0100 Subject: [PATCH] refactor: Improve accessibility of menus (#15302) * refactor: improve menubar keyboard accessibility * fix: create a temporary widget for tray icon context menu * fix: focus menu bar with Alt when autohide is off * fix: make menu bar focus work more like the native menus * fix: only focus menu bar if it's not already focused * fix: track accelerator registration to avoid duplicates * docs: add docs for & notation in app menu item names * fix: only try to activate accelerator if it's registered * fix: add friend to monitor window focus change * style: add include --- atom/browser/ui/views/menu_bar.cc | 159 +++++++++++++++++++-- atom/browser/ui/views/menu_bar.h | 44 ++++-- atom/browser/ui/views/menu_delegate.cc | 26 +++- atom/browser/ui/views/menu_delegate.h | 18 ++- atom/browser/ui/views/root_view.cc | 45 ++++-- atom/browser/ui/views/root_view.h | 4 + atom/browser/ui/views/submenu_button.cc | 16 ++- atom/browser/ui/views/submenu_button.h | 3 + atom/browser/ui/win/notify_icon.cc | 30 +++- atom/browser/ui/win/notify_icon.h | 11 +- atom/common/keyboard_util.cc | 178 ++++++++++++------------ atom/common/keyboard_util.h | 5 + docs/api/menu.md | 6 + 13 files changed, 405 insertions(+), 140 deletions(-) diff --git a/atom/browser/ui/views/menu_bar.cc b/atom/browser/ui/views/menu_bar.cc index 332a42b513a1e..39e1614dc5ce9 100644 --- a/atom/browser/ui/views/menu_bar.cc +++ b/atom/browser/ui/views/menu_bar.cc @@ -5,13 +5,16 @@ #include "atom/browser/ui/views/menu_bar.h" #include +#include +#include #include -#include "atom/browser/ui/views/menu_delegate.h" #include "atom/browser/ui/views/submenu_button.h" +#include "atom/common/keyboard_util.h" #include "ui/base/models/menu_model.h" #include "ui/views/background.h" #include "ui/views/layout/box_layout.h" +#include "ui/views/widget/widget.h" #if defined(USE_X11) #include "chrome/browser/ui/libgtkui/gtk_util.h" @@ -32,17 +35,36 @@ const SkColor kDefaultColor = SkColorSetARGB(255, 233, 233, 233); const char MenuBar::kViewClassName[] = "ElectronMenuBar"; -MenuBar::MenuBar(views::View* window) - : background_color_(kDefaultColor), window_(window) { +MenuBarColorUpdater::MenuBarColorUpdater(MenuBar* menu_bar) + : menu_bar_(menu_bar) {} + +MenuBarColorUpdater::~MenuBarColorUpdater() {} + +void MenuBarColorUpdater::OnDidChangeFocus(views::View* focused_before, + views::View* focused_now) { + if (menu_bar_) { + // if we've changed window focus, update menu bar colors + const auto had_focus = menu_bar_->has_focus_; + menu_bar_->has_focus_ = focused_now != nullptr; + if (menu_bar_->has_focus_ != had_focus) + menu_bar_->UpdateViewColors(); + } +} + +MenuBar::MenuBar(RootView* window) + : background_color_(kDefaultColor), + window_(window), + color_updater_(new MenuBarColorUpdater(this)) { RefreshColorCache(); UpdateViewColors(); + SetFocusBehavior(FocusBehavior::ALWAYS); SetLayoutManager( std::make_unique(views::BoxLayout::kHorizontal)); - window_->GetFocusManager()->AddFocusChangeListener(this); + window_->GetFocusManager()->AddFocusChangeListener(color_updater_.get()); } MenuBar::~MenuBar() { - window_->GetFocusManager()->RemoveFocusChangeListener(this); + window_->GetFocusManager()->RemoveFocusChangeListener(color_updater_.get()); } void MenuBar::SetMenu(AtomMenuModel* model) { @@ -96,6 +118,116 @@ bool MenuBar::GetMenuButtonFromScreenPoint(const gfx::Point& screenPoint, return false; } +void MenuBar::OnBeforeExecuteCommand() { + RemovePaneFocus(); + window_->RestoreFocus(); +} + +void MenuBar::OnMenuClosed() { + SetAcceleratorVisibility(true); +} + +bool MenuBar::AcceleratorPressed(const ui::Accelerator& accelerator) { + views::View* focused_view = GetFocusManager()->GetFocusedView(); + if (!ContainsForFocusSearch(this, focused_view)) + return false; + + switch (accelerator.key_code()) { + case ui::VKEY_MENU: + case ui::VKEY_ESCAPE: { + RemovePaneFocus(); + window_->RestoreFocus(); + return true; + } + case ui::VKEY_LEFT: + GetFocusManager()->AdvanceFocus(true); + return true; + case ui::VKEY_RIGHT: + GetFocusManager()->AdvanceFocus(false); + return true; + case ui::VKEY_HOME: + GetFocusManager()->SetFocusedViewWithReason( + GetFirstFocusableChild(), views::FocusManager::kReasonFocusTraversal); + return true; + case ui::VKEY_END: + GetFocusManager()->SetFocusedViewWithReason( + GetLastFocusableChild(), views::FocusManager::kReasonFocusTraversal); + return true; + default: { + auto children = GetChildrenInZOrder(); + for (int i = 0, n = children.size(); i < n; ++i) { + auto* button = static_cast(children[i]); + bool shifted = false; + auto keycode = + atom::KeyboardCodeFromCharCode(button->accelerator(), &shifted); + + if (keycode == accelerator.key_code()) { + const gfx::Point p(0, 0); + auto event = accelerator.ToKeyEvent(); + OnMenuButtonClicked(button, p, &event); + return true; + } + } + + return false; + } + } +} + +bool MenuBar::SetPaneFocus(views::View* initial_focus) { + bool result = views::AccessiblePaneView::SetPaneFocus(initial_focus); + + if (result) { + auto children = GetChildrenInZOrder(); + std::set reg; + for (int i = 0, n = children.size(); i < n; ++i) { + auto* button = static_cast(children[i]); + bool shifted = false; + auto keycode = + atom::KeyboardCodeFromCharCode(button->accelerator(), &shifted); + + // We want the menu items to activate if the user presses the accelerator + // key, even without alt, since we are now focused on the menu bar + if (keycode != ui::VKEY_UNKNOWN && reg.find(keycode) != reg.end()) { + reg.insert(keycode); + focus_manager()->RegisterAccelerator( + ui::Accelerator(keycode, ui::EF_NONE), + ui::AcceleratorManager::kNormalPriority, this); + } + } + + // We want to remove focus / hide menu bar when alt is pressed again + focus_manager()->RegisterAccelerator( + ui::Accelerator(ui::VKEY_MENU, ui::EF_ALT_DOWN), + ui::AcceleratorManager::kNormalPriority, this); + } + + return result; +} + +void MenuBar::RemovePaneFocus() { + views::AccessiblePaneView::RemovePaneFocus(); + SetAcceleratorVisibility(false); + + auto children = GetChildrenInZOrder(); + std::set unreg; + for (int i = 0, n = children.size(); i < n; ++i) { + auto* button = static_cast(children[i]); + bool shifted = false; + auto keycode = + atom::KeyboardCodeFromCharCode(button->accelerator(), &shifted); + + if (keycode != ui::VKEY_UNKNOWN && unreg.find(keycode) != unreg.end()) { + unreg.insert(keycode); + focus_manager()->UnregisterAccelerator( + ui::Accelerator(keycode, ui::EF_NONE), this); + } + } + + focus_manager()->UnregisterAccelerator( + ui::Accelerator(ui::VKEY_MENU, ui::EF_ALT_DOWN), this); +} + const char* MenuBar::GetClassName() const { return kViewClassName; } @@ -119,9 +251,16 @@ void MenuBar::OnMenuButtonClicked(views::MenuButton* source, return; } + GetFocusManager()->SetFocusedViewWithReason( + source, views::FocusManager::kReasonFocusTraversal); + // Deleted in MenuDelegate::OnMenuClosed MenuDelegate* menu_delegate = new MenuDelegate(this); - menu_delegate->RunMenu(menu_model_->GetSubmenuModelAt(id), source); + menu_delegate->RunMenu(menu_model_->GetSubmenuModelAt(id), source, + event != nullptr && event->IsKeyEvent() + ? ui::MENU_SOURCE_KEYBOARD + : ui::MENU_SOURCE_MOUSE); + menu_delegate->AddObserver(this); } void MenuBar::RefreshColorCache(const ui::NativeTheme* theme) { @@ -151,14 +290,6 @@ void MenuBar::OnNativeThemeChanged(const ui::NativeTheme* theme) { UpdateViewColors(); } -void MenuBar::OnDidChangeFocus(View* focused_before, View* focused_now) { - // if we've changed focus, update our view - const auto had_focus = has_focus_; - has_focus_ = focused_now != nullptr; - if (has_focus_ != had_focus) - UpdateViewColors(); -} - void MenuBar::RebuildChildren() { RemoveAllChildViews(true); for (int i = 0, n = GetItemCount(); i < n; ++i) { diff --git a/atom/browser/ui/views/menu_bar.h b/atom/browser/ui/views/menu_bar.h index 2b10ba3e3bf2c..a49ec58ac1413 100644 --- a/atom/browser/ui/views/menu_bar.h +++ b/atom/browser/ui/views/menu_bar.h @@ -5,7 +5,12 @@ #ifndef ATOM_BROWSER_UI_VIEWS_MENU_BAR_H_ #define ATOM_BROWSER_UI_VIEWS_MENU_BAR_H_ +#include + #include "atom/browser/ui/atom_menu_model.h" +#include "atom/browser/ui/views/menu_delegate.h" +#include "atom/browser/ui/views/root_view.h" +#include "ui/views/accessible_pane_view.h" #include "ui/views/controls/button/menu_button_listener.h" #include "ui/views/focus/focus_manager.h" #include "ui/views/view.h" @@ -16,15 +21,27 @@ class MenuButton; namespace atom { -class MenuDelegate; +class MenuBarColorUpdater : public views::FocusChangeListener { + public: + explicit MenuBarColorUpdater(MenuBar* menu_bar); + ~MenuBarColorUpdater() override; + + void OnDidChangeFocus(views::View* focused_before, + views::View* focused_now) override; + void OnWillChangeFocus(views::View* focused_before, + views::View* focused_now) override {} + + private: + MenuBar* menu_bar_; +}; -class MenuBar : public views::View, +class MenuBar : public views::AccessiblePaneView, public views::MenuButtonListener, - public views::FocusChangeListener { + public atom::MenuDelegate::Observer { public: static const char kViewClassName[]; - explicit MenuBar(views::View* window); + explicit MenuBar(RootView* window); ~MenuBar() override; // Replaces current menu with a new one. @@ -47,6 +64,15 @@ class MenuBar : public views::View, AtomMenuModel** menu_model, views::MenuButton** button); + // atom::MenuDelegate::Observer: + void OnBeforeExecuteCommand() override; + void OnMenuClosed() override; + + // views::AccessiblePaneView: + bool AcceleratorPressed(const ui::Accelerator& accelerator) override; + bool SetPaneFocus(views::View* initial_focus) override; + void RemovePaneFocus() override; + protected: // views::View: const char* GetClassName() const override; @@ -57,11 +83,9 @@ class MenuBar : public views::View, const ui::Event* event) override; void OnNativeThemeChanged(const ui::NativeTheme* theme) override; - // views::FocusChangeListener: - void OnDidChangeFocus(View* focused_before, View* focused_now) override; - void OnWillChangeFocus(View* focused_before, View* focused_now) override {} - private: + friend class MenuBarColorUpdater; + void RebuildChildren(); void UpdateViewColors(); @@ -72,13 +96,15 @@ class MenuBar : public views::View, SkColor disabled_color_; #endif - views::View* window_ = nullptr; + RootView* window_ = nullptr; AtomMenuModel* menu_model_ = nullptr; View* FindAccelChild(base::char16 key); bool has_focus_ = true; + std::unique_ptr color_updater_; + DISALLOW_COPY_AND_ASSIGN(MenuBar); }; diff --git a/atom/browser/ui/views/menu_delegate.cc b/atom/browser/ui/views/menu_delegate.cc index d8cb592293a76..8c24a82af5b54 100644 --- a/atom/browser/ui/views/menu_delegate.cc +++ b/atom/browser/ui/views/menu_delegate.cc @@ -14,17 +14,24 @@ namespace atom { -MenuDelegate::MenuDelegate(MenuBar* menu_bar) : menu_bar_(menu_bar), id_(-1) {} +MenuDelegate::MenuDelegate(MenuBar* menu_bar) + : menu_bar_(menu_bar), id_(-1), hold_first_switch_(false) {} MenuDelegate::~MenuDelegate() {} -void MenuDelegate::RunMenu(AtomMenuModel* model, views::MenuButton* button) { +void MenuDelegate::RunMenu(AtomMenuModel* model, + views::MenuButton* button, + ui::MenuSourceType source_type) { gfx::Point screen_loc; views::View::ConvertPointToScreen(button, &screen_loc); // Subtract 1 from the height to make the popup flush with the button border. gfx::Rect bounds(screen_loc.x(), screen_loc.y(), button->width(), button->height() - 1); + if (source_type == ui::MENU_SOURCE_KEYBOARD) { + hold_first_switch_ = true; + } + id_ = button->tag(); adapter_.reset(new MenuModelAdapter(model)); @@ -35,15 +42,18 @@ void MenuDelegate::RunMenu(AtomMenuModel* model, views::MenuButton* button) { item, views::MenuRunner::CONTEXT_MENU | views::MenuRunner::HAS_MNEMONICS)); menu_runner_->RunMenuAt(button->GetWidget()->GetTopLevelWidget(), button, - bounds, views::MENU_ANCHOR_TOPRIGHT, - ui::MENU_SOURCE_MOUSE); + bounds, views::MENU_ANCHOR_TOPRIGHT, source_type); } void MenuDelegate::ExecuteCommand(int id) { + for (Observer& obs : observers_) + obs.OnBeforeExecuteCommand(); adapter_->ExecuteCommand(id); } void MenuDelegate::ExecuteCommand(int id, int mouse_event_flags) { + for (Observer& obs : observers_) + obs.OnBeforeExecuteCommand(); adapter_->ExecuteCommand(id, mouse_event_flags); } @@ -89,6 +99,9 @@ void MenuDelegate::WillHideMenu(views::MenuItemView* menu) { } void MenuDelegate::OnMenuClosed(views::MenuItemView* menu) { + for (Observer& obs : observers_) + obs.OnMenuClosed(); + // Only switch to new menu when current menu is closed. if (button_to_open_) button_to_open_->Activate(nullptr); @@ -101,6 +114,11 @@ views::MenuItemView* MenuDelegate::GetSiblingMenu( views::MenuAnchorPosition* anchor, bool* has_mnemonics, views::MenuButton**) { + if (hold_first_switch_) { + hold_first_switch_ = false; + return nullptr; + } + // TODO(zcbenz): We should follow Chromium's logics on implementing the // sibling menu switches, this code is almost a hack. views::MenuButton* button; diff --git a/atom/browser/ui/views/menu_delegate.h b/atom/browser/ui/views/menu_delegate.h index e23262a3aa7f6..e623f763b2eca 100644 --- a/atom/browser/ui/views/menu_delegate.h +++ b/atom/browser/ui/views/menu_delegate.h @@ -8,6 +8,7 @@ #include #include "atom/browser/ui/atom_menu_model.h" +#include "base/observer_list.h" #include "ui/views/controls/menu/menu_delegate.h" namespace views { @@ -23,7 +24,19 @@ class MenuDelegate : public views::MenuDelegate { explicit MenuDelegate(MenuBar* menu_bar); ~MenuDelegate() override; - void RunMenu(AtomMenuModel* model, views::MenuButton* button); + void RunMenu(AtomMenuModel* model, + views::MenuButton* button, + ui::MenuSourceType source_type); + + class Observer { + public: + virtual void OnBeforeExecuteCommand() = 0; + virtual void OnMenuClosed() = 0; + }; + + void AddObserver(Observer* obs) { observers_.AddObserver(obs); } + + void RemoveObserver(const Observer* obs) { observers_.RemoveObserver(obs); } protected: // views::MenuDelegate: @@ -55,6 +68,9 @@ class MenuDelegate : public views::MenuDelegate { // The menu button to switch to. views::MenuButton* button_to_open_ = nullptr; + bool hold_first_switch_; + + base::ObserverList observers_; DISALLOW_COPY_AND_ASSIGN(MenuDelegate); }; diff --git a/atom/browser/ui/views/root_view.cc b/atom/browser/ui/views/root_view.cc index 716563ff0141c..db03674d2e515 100644 --- a/atom/browser/ui/views/root_view.cc +++ b/atom/browser/ui/views/root_view.cc @@ -35,7 +35,9 @@ bool IsAltModifier(const content::NativeWebKeyboardEvent& event) { } // namespace -RootView::RootView(NativeWindow* window) : window_(window) { +RootView::RootView(NativeWindow* window) + : window_(window), + last_focused_view_tracker_(std::make_unique()) { set_owned_by_client(); } @@ -89,10 +91,6 @@ void RootView::SetMenuBarVisibility(bool visible) { if (!window_->content_view() || !menu_bar_ || menu_bar_visible_ == visible) return; - // Always show the accelerator when the auto-hide menu bar shows. - if (menu_bar_autohide_) - menu_bar_->SetAcceleratorVisibility(visible); - menu_bar_visible_ = visible; if (visible) { DCHECK_EQ(child_count(), 1); @@ -121,15 +119,19 @@ void RootView::HandleKeyEvent(const content::NativeWebKeyboardEvent& event) { // Show the submenu when "Alt+Key" is pressed. if (event.GetType() == blink::WebInputEvent::kRawKeyDown && !IsAltKey(event) && IsAltModifier(event)) { - if (!menu_bar_visible_ && - (menu_bar_->HasAccelerator(event.windows_key_code))) - SetMenuBarVisibility(true); - menu_bar_->ActivateAccelerator(event.windows_key_code); - return; - } + if (menu_bar_->HasAccelerator(event.windows_key_code)) { + if (!menu_bar_visible_) { + SetMenuBarVisibility(true); + + View* focused_view = GetFocusManager()->GetFocusedView(); + last_focused_view_tracker_->SetView(focused_view); + menu_bar_->RequestFocus(); + } - if (!menu_bar_autohide_) + menu_bar_->ActivateAccelerator(event.windows_key_code); + } return; + } // Toggle the menu bar only when a single Alt is released. if (event.GetType() == blink::WebInputEvent::kRawKeyDown && IsAltKey(event)) { @@ -139,13 +141,30 @@ void RootView::HandleKeyEvent(const content::NativeWebKeyboardEvent& event) { IsAltKey(event) && menu_bar_alt_pressed_) { // When a single Alt is released right after a Alt is pressed: menu_bar_alt_pressed_ = false; - SetMenuBarVisibility(!menu_bar_visible_); + if (menu_bar_autohide_) + SetMenuBarVisibility(!menu_bar_visible_); + + View* focused_view = GetFocusManager()->GetFocusedView(); + last_focused_view_tracker_->SetView(focused_view); + menu_bar_->RequestFocus(); + // Show accelerators when menu bar is focused + menu_bar_->SetAcceleratorVisibility(true); } else { // When any other keys except single Alt have been pressed/released: menu_bar_alt_pressed_ = false; } } +void RootView::RestoreFocus() { + View* last_focused_view = last_focused_view_tracker_->view(); + if (last_focused_view) { + GetFocusManager()->SetFocusedViewWithReason( + last_focused_view, views::FocusManager::kReasonFocusRestore); + } + if (menu_bar_autohide_) + SetMenuBarVisibility(false); +} + void RootView::ResetAltState() { menu_bar_alt_pressed_ = false; } diff --git a/atom/browser/ui/views/root_view.h b/atom/browser/ui/views/root_view.h index 66c6d4e8a5571..b5b718f28fc9b 100644 --- a/atom/browser/ui/views/root_view.h +++ b/atom/browser/ui/views/root_view.h @@ -9,6 +9,7 @@ #include "atom/browser/ui/accelerator_util.h" #include "ui/views/view.h" +#include "ui/views/view_tracker.h" namespace content { struct NativeWebKeyboardEvent; @@ -34,6 +35,7 @@ class RootView : public views::View { bool IsMenuBarVisible() const; void HandleKeyEvent(const content::NativeWebKeyboardEvent& event); void ResetAltState(); + void RestoreFocus(); // views::View: void Layout() override; @@ -57,6 +59,8 @@ class RootView : public views::View { // Map from accelerator to menu item's command id. accelerator_util::AcceleratorTable accelerator_table_; + std::unique_ptr last_focused_view_tracker_; + DISALLOW_COPY_AND_ASSIGN(RootView); }; diff --git a/atom/browser/ui/views/submenu_button.cc b/atom/browser/ui/views/submenu_button.cc index 55b95abe5e8b3..b6c0c3229fe4f 100644 --- a/atom/browser/ui/views/submenu_button.cc +++ b/atom/browser/ui/views/submenu_button.cc @@ -71,15 +71,21 @@ void SubmenuButton::SetUnderlineColor(SkColor color) { underline_color_ = color; } +void SubmenuButton::GetAccessibleNodeData(ui::AXNodeData* node_data) { + node_data->SetName(accessible_name()); + node_data->role = ax::mojom::Role::kPopUpButton; +} + void SubmenuButton::PaintButtonContents(gfx::Canvas* canvas) { views::MenuButton::PaintButtonContents(canvas); if (show_underline_ && (underline_start_ != underline_end_)) { - int padding = (width() - text_width_) / 2; - int underline_height = (height() + text_height_) / 2 - 2; - canvas->DrawLine(gfx::Point(underline_start_ + padding, underline_height), - gfx::Point(underline_end_ + padding, underline_height), - underline_color_); + float padding = (width() - text_width_) / 2; + float underline_height = (height() + text_height_) / 2 - 2; + canvas->DrawSharpLine( + gfx::PointF(underline_start_ + padding, underline_height), + gfx::PointF(underline_end_ + padding, underline_height), + underline_color_); } } diff --git a/atom/browser/ui/views/submenu_button.h b/atom/browser/ui/views/submenu_button.h index cdd740ff6e886..12cab85e4658a 100644 --- a/atom/browser/ui/views/submenu_button.h +++ b/atom/browser/ui/views/submenu_button.h @@ -7,6 +7,7 @@ #include +#include "ui/accessibility/ax_node_data.h" #include "ui/views/animation/ink_drop_highlight.h" #include "ui/views/controls/button/menu_button.h" @@ -25,6 +26,8 @@ class SubmenuButton : public views::MenuButton { base::char16 accelerator() const { return accelerator_; } + void GetAccessibleNodeData(ui::AXNodeData* node_data) override; + // views::MenuButton: void PaintButtonContents(gfx::Canvas* canvas) override; diff --git a/atom/browser/ui/win/notify_icon.cc b/atom/browser/ui/win/notify_icon.cc index 9b01de8a49f6c..52081edd8a0ab 100644 --- a/atom/browser/ui/win/notify_icon.cc +++ b/atom/browser/ui/win/notify_icon.cc @@ -15,11 +15,16 @@ #include "ui/gfx/geometry/rect.h" #include "ui/gfx/image/image.h" #include "ui/views/controls/menu/menu_runner.h" +#include "ui/views/widget/widget.h" namespace atom { NotifyIcon::NotifyIcon(NotifyIconHost* host, UINT id, HWND window, UINT message) - : host_(host), icon_id_(id), window_(window), message_id_(message) { + : host_(host), + icon_id_(id), + window_(window), + message_id_(message), + weak_factory_(this) { NOTIFYICONDATA icon_data; InitIconData(&icon_data); icon_data.uFlags |= NIF_MESSAGE; @@ -142,10 +147,25 @@ void NotifyIcon::PopUpContextMenu(const gfx::Point& pos, if (pos.IsOrigin()) rect.set_origin(display::Screen::GetScreen()->GetCursorScreenPoint()); + // Create a widget for the menu, otherwise we get no keyboard events, which + // is required for accessibility. + widget_.reset(new views::Widget()); + views::Widget::InitParams params(views::Widget::InitParams::TYPE_POPUP); + params.ownership = + views::Widget::InitParams::Ownership::WIDGET_OWNS_NATIVE_WIDGET; + params.bounds = gfx::Rect(0, 0, 0, 0); + params.force_software_compositing = true; + + widget_->Init(params); + + widget_->Show(); + widget_->Activate(); menu_runner_.reset(new views::MenuRunner( menu_model != nullptr ? menu_model : menu_model_, - views::MenuRunner::CONTEXT_MENU | views::MenuRunner::HAS_MNEMONICS)); - menu_runner_->RunMenuAt(NULL, NULL, rect, views::MENU_ANCHOR_TOPLEFT, + views::MenuRunner::CONTEXT_MENU | views::MenuRunner::HAS_MNEMONICS, + base::Bind(&NotifyIcon::OnContextMenuClosed, + weak_factory_.GetWeakPtr()))); + menu_runner_->RunMenuAt(widget_.get(), NULL, rect, views::MENU_ANCHOR_TOPLEFT, ui::MENU_SOURCE_MOUSE); } @@ -172,4 +192,8 @@ void NotifyIcon::InitIconData(NOTIFYICONDATA* icon_data) { icon_data->uID = icon_id_; } +void NotifyIcon::OnContextMenuClosed() { + widget_->Close(); +} + } // namespace atom diff --git a/atom/browser/ui/win/notify_icon.h b/atom/browser/ui/win/notify_icon.h index 12c8bd67bd524..79be5078ede07 100644 --- a/atom/browser/ui/win/notify_icon.h +++ b/atom/browser/ui/win/notify_icon.h @@ -15,6 +15,7 @@ #include "atom/browser/ui/tray_icon.h" #include "base/compiler_specific.h" #include "base/macros.h" +#include "base/memory/weak_ptr.h" #include "base/win/scoped_gdi_object.h" namespace gfx { @@ -23,7 +24,8 @@ class Point; namespace views { class MenuRunner; -} +class Widget; +} // namespace views namespace atom { @@ -63,6 +65,7 @@ class NotifyIcon : public TrayIcon { private: void InitIconData(NOTIFYICONDATA* icon_data); + void OnContextMenuClosed(); // The tray that owns us. Weak. NotifyIconHost* host_; @@ -85,6 +88,12 @@ class NotifyIcon : public TrayIcon { // Context menu associated with this icon (if any). std::unique_ptr menu_runner_; + // Temporary widget for the context menu, needed for keyboard event capture. + std::unique_ptr widget_; + + // WeakPtrFactory for CloseClosure safety. + base::WeakPtrFactory weak_factory_; + DISALLOW_COPY_AND_ASSIGN(NotifyIcon); }; diff --git a/atom/common/keyboard_util.cc b/atom/common/keyboard_util.cc index 1f42ad735a01d..354cabcd8d3b8 100644 --- a/atom/common/keyboard_util.cc +++ b/atom/common/keyboard_util.cc @@ -14,8 +14,94 @@ namespace atom { namespace { -// Return key code of the char, and also determine whether the SHIFT key is -// pressed. +// Return key code represented by |str|. +ui::KeyboardCode KeyboardCodeFromKeyIdentifier(const std::string& s, + bool* shifted) { + std::string str = base::ToLowerASCII(s); + if (str == "ctrl" || str == "control") { + return ui::VKEY_CONTROL; + } else if (str == "super" || str == "cmd" || str == "command" || + str == "meta") { + return ui::VKEY_COMMAND; + } else if (str == "commandorcontrol" || str == "cmdorctrl") { +#if defined(OS_MACOSX) + return ui::VKEY_COMMAND; +#else + return ui::VKEY_CONTROL; +#endif + } else if (str == "alt" || str == "option") { + return ui::VKEY_MENU; + } else if (str == "shift") { + return ui::VKEY_SHIFT; + } else if (str == "altgr") { + return ui::VKEY_ALTGR; + } else if (str == "plus") { + *shifted = true; + return ui::VKEY_OEM_PLUS; + } else if (str == "tab") { + return ui::VKEY_TAB; + } else if (str == "space") { + return ui::VKEY_SPACE; + } else if (str == "backspace") { + return ui::VKEY_BACK; + } else if (str == "delete") { + return ui::VKEY_DELETE; + } else if (str == "insert") { + return ui::VKEY_INSERT; + } else if (str == "enter" || str == "return") { + return ui::VKEY_RETURN; + } else if (str == "up") { + return ui::VKEY_UP; + } else if (str == "down") { + return ui::VKEY_DOWN; + } else if (str == "left") { + return ui::VKEY_LEFT; + } else if (str == "right") { + return ui::VKEY_RIGHT; + } else if (str == "home") { + return ui::VKEY_HOME; + } else if (str == "end") { + return ui::VKEY_END; + } else if (str == "pageup") { + return ui::VKEY_PRIOR; + } else if (str == "pagedown") { + return ui::VKEY_NEXT; + } else if (str == "esc" || str == "escape") { + return ui::VKEY_ESCAPE; + } else if (str == "volumemute") { + return ui::VKEY_VOLUME_MUTE; + } else if (str == "volumeup") { + return ui::VKEY_VOLUME_UP; + } else if (str == "volumedown") { + return ui::VKEY_VOLUME_DOWN; + } else if (str == "medianexttrack") { + return ui::VKEY_MEDIA_NEXT_TRACK; + } else if (str == "mediaprevioustrack") { + return ui::VKEY_MEDIA_PREV_TRACK; + } else if (str == "mediastop") { + return ui::VKEY_MEDIA_STOP; + } else if (str == "mediaplaypause") { + return ui::VKEY_MEDIA_PLAY_PAUSE; + } else if (str == "printscreen") { + return ui::VKEY_SNAPSHOT; + } else if (str.size() > 1 && str[0] == 'f') { + // F1 - F24. + int n; + if (base::StringToInt(str.c_str() + 1, &n) && n > 0 && n < 25) { + return static_cast(ui::VKEY_F1 + n - 1); + } else { + LOG(WARNING) << str << "is not available on keyboard"; + return ui::VKEY_UNKNOWN; + } + } else { + if (str.size() > 2) + LOG(WARNING) << "Invalid accelerator token: " << str; + return ui::VKEY_UNKNOWN; + } +} + +} // namespace + ui::KeyboardCode KeyboardCodeFromCharCode(base::char16 c, bool* shifted) { c = base::ToLowerASCII(c); *shifted = false; @@ -198,94 +284,6 @@ ui::KeyboardCode KeyboardCodeFromCharCode(base::char16 c, bool* shifted) { } } -// Return key code represented by |str|. -ui::KeyboardCode KeyboardCodeFromKeyIdentifier(const std::string& s, - bool* shifted) { - std::string str = base::ToLowerASCII(s); - if (str == "ctrl" || str == "control") { - return ui::VKEY_CONTROL; - } else if (str == "super" || str == "cmd" || str == "command" || - str == "meta") { - return ui::VKEY_COMMAND; - } else if (str == "commandorcontrol" || str == "cmdorctrl") { -#if defined(OS_MACOSX) - return ui::VKEY_COMMAND; -#else - return ui::VKEY_CONTROL; -#endif - } else if (str == "alt" || str == "option") { - return ui::VKEY_MENU; - } else if (str == "shift") { - return ui::VKEY_SHIFT; - } else if (str == "altgr") { - return ui::VKEY_ALTGR; - } else if (str == "plus") { - *shifted = true; - return ui::VKEY_OEM_PLUS; - } else if (str == "tab") { - return ui::VKEY_TAB; - } else if (str == "space") { - return ui::VKEY_SPACE; - } else if (str == "backspace") { - return ui::VKEY_BACK; - } else if (str == "delete") { - return ui::VKEY_DELETE; - } else if (str == "insert") { - return ui::VKEY_INSERT; - } else if (str == "enter" || str == "return") { - return ui::VKEY_RETURN; - } else if (str == "up") { - return ui::VKEY_UP; - } else if (str == "down") { - return ui::VKEY_DOWN; - } else if (str == "left") { - return ui::VKEY_LEFT; - } else if (str == "right") { - return ui::VKEY_RIGHT; - } else if (str == "home") { - return ui::VKEY_HOME; - } else if (str == "end") { - return ui::VKEY_END; - } else if (str == "pageup") { - return ui::VKEY_PRIOR; - } else if (str == "pagedown") { - return ui::VKEY_NEXT; - } else if (str == "esc" || str == "escape") { - return ui::VKEY_ESCAPE; - } else if (str == "volumemute") { - return ui::VKEY_VOLUME_MUTE; - } else if (str == "volumeup") { - return ui::VKEY_VOLUME_UP; - } else if (str == "volumedown") { - return ui::VKEY_VOLUME_DOWN; - } else if (str == "medianexttrack") { - return ui::VKEY_MEDIA_NEXT_TRACK; - } else if (str == "mediaprevioustrack") { - return ui::VKEY_MEDIA_PREV_TRACK; - } else if (str == "mediastop") { - return ui::VKEY_MEDIA_STOP; - } else if (str == "mediaplaypause") { - return ui::VKEY_MEDIA_PLAY_PAUSE; - } else if (str == "printscreen") { - return ui::VKEY_SNAPSHOT; - } else if (str.size() > 1 && str[0] == 'f') { - // F1 - F24. - int n; - if (base::StringToInt(str.c_str() + 1, &n) && n > 0 && n < 25) { - return static_cast(ui::VKEY_F1 + n - 1); - } else { - LOG(WARNING) << str << "is not available on keyboard"; - return ui::VKEY_UNKNOWN; - } - } else { - if (str.size() > 2) - LOG(WARNING) << "Invalid accelerator token: " << str; - return ui::VKEY_UNKNOWN; - } -} - -} // namespace - ui::KeyboardCode KeyboardCodeFromStr(const std::string& str, bool* shifted) { if (str.size() == 1) return KeyboardCodeFromCharCode(str[0], shifted); diff --git a/atom/common/keyboard_util.h b/atom/common/keyboard_util.h index 651cf6a92024c..ad6dd9c0de6cb 100644 --- a/atom/common/keyboard_util.h +++ b/atom/common/keyboard_util.h @@ -7,10 +7,15 @@ #include +#include "base/strings/string16.h" #include "ui/events/keycodes/keyboard_codes.h" namespace atom { +// Return key code of the char, and also determine whether the SHIFT key is +// pressed. +ui::KeyboardCode KeyboardCodeFromCharCode(base::char16 c, bool* shifted); + // Return key code of the |str|, and also determine whether the SHIFT key is // pressed. ui::KeyboardCode KeyboardCodeFromStr(const std::string& str, bool* shifted); diff --git a/docs/api/menu.md b/docs/api/menu.md index 21f84c66fa65f..6517836ce5008 100644 --- a/docs/api/menu.md +++ b/docs/api/menu.md @@ -19,6 +19,12 @@ The `menu` class has the following static methods: Sets `menu` as the application menu on macOS. On Windows and Linux, the `menu` will be set as each window's top menu. +Also on Windows and Linux, you can use a `&` in the top-level item name to +indicate which letter should get a generated accelerator. For example, using +`&File` for the file menu would result in a generated `Alt-F` accelerator that +opens the associated menu. The indicated character in the button label gets an +underline. The `&` character is not displayed on the button label. + Passing `null` will remove the menu bar on Windows and Linux but has no effect on macOS.