From 35e557c6570f1e2a6e892b78136822af41a1b126 Mon Sep 17 00:00:00 2001 From: thankyouverycool <66646555+thankyouverycool@users.noreply.github.com> Date: Sun, 4 Sep 2022 19:48:37 -0400 Subject: [PATCH] Browser+LibGUI+WindowServer: Open Button menus uniformly Instead of letting buttons determine the relative position of their menus, a workaround only used by Statusbar segments, open them all uniformly for a nice, consistent UI. Passing a rect to popup() now routes to open_button_menu(), an analog to open_menubar_menu(), which adjusts the menu's popup position in the same way. Fixes button menus obscuring the buttons which spawn them and jutting out at odd corners depending on screen position. --- .../Browser/BookmarksBarWidget.cpp | 1 - Userland/Libraries/LibGUI/Button.cpp | 17 +-------------- Userland/Libraries/LibGUI/Button.h | 4 ---- Userland/Libraries/LibGUI/Menu.cpp | 4 ++-- Userland/Libraries/LibGUI/Menu.h | 2 +- Userland/Libraries/LibGUI/Statusbar.cpp | 1 - Userland/Libraries/LibGUI/Toolbar.cpp | 3 +-- .../WindowServer/ConnectionFromClient.cpp | 7 +++++-- .../WindowServer/ConnectionFromClient.h | 2 +- Userland/Services/WindowServer/Menu.cpp | 21 +++++++++++++++++++ Userland/Services/WindowServer/Menu.h | 1 + .../Services/WindowServer/WindowServer.ipc | 2 +- 12 files changed, 34 insertions(+), 31 deletions(-) diff --git a/Userland/Applications/Browser/BookmarksBarWidget.cpp b/Userland/Applications/Browser/BookmarksBarWidget.cpp index 35d78c9b819..c4796071336 100644 --- a/Userland/Applications/Browser/BookmarksBarWidget.cpp +++ b/Userland/Applications/Browser/BookmarksBarWidget.cpp @@ -112,7 +112,6 @@ BookmarksBarWidget::BookmarksBarWidget(String const& bookmarks_file, bool enable m_additional = GUI::Button::construct(); m_additional->set_tooltip("Show hidden bookmarks"); m_additional->set_menu(m_additional_menu); - m_additional->set_menu_position(GUI::Button::MenuPosition::BottomLeft); auto bitmap_or_error = Gfx::Bitmap::try_load_from_file("/res/icons/16x16/overflow-menu.png"sv); if (!bitmap_or_error.is_error()) m_additional->set_icon(bitmap_or_error.release_value()); diff --git a/Userland/Libraries/LibGUI/Button.cpp b/Userland/Libraries/LibGUI/Button.cpp index 735d40e86f6..20a242569fb 100644 --- a/Userland/Libraries/LibGUI/Button.cpp +++ b/Userland/Libraries/LibGUI/Button.cpp @@ -215,22 +215,7 @@ void Button::set_menu(RefPtr menu) void Button::mousedown_event(MouseEvent& event) { if (m_menu) { - switch (m_menu_position) { - case TopLeft: - m_menu->popup(screen_relative_rect().top_left()); - break; - case TopRight: - m_menu->popup(screen_relative_rect().top_right()); - break; - case BottomLeft: - m_menu->popup(screen_relative_rect().bottom_left()); - break; - case BottomRight: - m_menu->popup(screen_relative_rect().bottom_right()); - break; - default: - VERIFY_NOT_REACHED(); - } + m_menu->popup(screen_relative_rect().bottom_left(), {}, rect()); update(); return; } diff --git a/Userland/Libraries/LibGUI/Button.h b/Userland/Libraries/LibGUI/Button.h index ca8f79ff1f4..efdf1b12d64 100644 --- a/Userland/Libraries/LibGUI/Button.h +++ b/Userland/Libraries/LibGUI/Button.h @@ -66,9 +66,6 @@ public: void set_mimic_pressed(bool mimic_pressed); bool is_mimic_pressed() const { return m_mimic_pressed; }; - MenuPosition menu_position() const { return m_menu_position; } - void set_menu_position(MenuPosition position) { m_menu_position = position; } - virtual Optional calculated_min_size() const override; protected: @@ -88,7 +85,6 @@ private: int m_icon_spacing { 4 }; bool m_another_button_has_focus { false }; bool m_mimic_pressed { false }; - MenuPosition m_menu_position { MenuPosition::TopLeft }; }; class DialogButton final : public Button { diff --git a/Userland/Libraries/LibGUI/Menu.cpp b/Userland/Libraries/LibGUI/Menu.cpp index 7363688f57f..11461cd476c 100644 --- a/Userland/Libraries/LibGUI/Menu.cpp +++ b/Userland/Libraries/LibGUI/Menu.cpp @@ -116,10 +116,10 @@ void Menu::realize_if_needed(RefPtr const& default_action) realize_menu(default_action); } -void Menu::popup(Gfx::IntPoint const& screen_position, RefPtr const& default_action) +void Menu::popup(Gfx::IntPoint const& screen_position, RefPtr const& default_action, Gfx::IntRect const& button_rect) { realize_if_needed(default_action); - ConnectionToWindowServer::the().async_popup_menu(m_menu_id, screen_position); + ConnectionToWindowServer::the().async_popup_menu(m_menu_id, screen_position, button_rect); } void Menu::dismiss() diff --git a/Userland/Libraries/LibGUI/Menu.h b/Userland/Libraries/LibGUI/Menu.h index 79c7000e355..84defebcab0 100644 --- a/Userland/Libraries/LibGUI/Menu.h +++ b/Userland/Libraries/LibGUI/Menu.h @@ -41,7 +41,7 @@ public: Menu& add_submenu(String name); void remove_all_actions(); - void popup(Gfx::IntPoint const& screen_position, RefPtr const& default_action = nullptr); + void popup(Gfx::IntPoint const& screen_position, RefPtr const& default_action = nullptr, Gfx::IntRect const& button_rect = {}); void dismiss(); void visibility_did_change(Badge, bool visible); diff --git a/Userland/Libraries/LibGUI/Statusbar.cpp b/Userland/Libraries/LibGUI/Statusbar.cpp index 0ae54eb3f8a..3618cad0d66 100644 --- a/Userland/Libraries/LibGUI/Statusbar.cpp +++ b/Userland/Libraries/LibGUI/Statusbar.cpp @@ -146,7 +146,6 @@ Statusbar::Segment::Segment() set_focus_policy(GUI::FocusPolicy::NoFocus); set_button_style(Gfx::ButtonStyle::Tray); set_text_alignment(Gfx::TextAlignment::CenterLeft); - set_menu_position(GUI::Button::MenuPosition::TopRight); } void Statusbar::Segment::paint_event(PaintEvent& event) diff --git a/Userland/Libraries/LibGUI/Toolbar.cpp b/Userland/Libraries/LibGUI/Toolbar.cpp index bed81721561..5380af356af 100644 --- a/Userland/Libraries/LibGUI/Toolbar.cpp +++ b/Userland/Libraries/LibGUI/Toolbar.cpp @@ -159,7 +159,7 @@ Optional Toolbar::calculated_min_size() const ErrorOr Toolbar::create_overflow_objects() { m_overflow_action = Action::create("Overflow Menu", { Mod_Ctrl | Mod_Shift, Key_O }, TRY(Gfx::Bitmap::try_load_from_file("/res/icons/16x16/overflow-menu.png"sv)), [&](auto&) { - m_overflow_menu->popup(m_overflow_button->screen_relative_rect().bottom_left()); + m_overflow_menu->popup(m_overflow_button->screen_relative_rect().bottom_left(), {}, m_overflow_button->rect()); }); m_overflow_action->set_status_tip("Show hidden toolbar actions"); m_overflow_action->set_enabled(false); @@ -168,7 +168,6 @@ ErrorOr Toolbar::create_overflow_objects() m_overflow_button = TRY(try_add_action(*m_overflow_action)); m_overflow_button->set_visible(false); - m_overflow_button->set_menu_position(Button::MenuPosition::BottomLeft); return {}; } diff --git a/Userland/Services/WindowServer/ConnectionFromClient.cpp b/Userland/Services/WindowServer/ConnectionFromClient.cpp index 3c50949aff2..3b2706909fb 100644 --- a/Userland/Services/WindowServer/ConnectionFromClient.cpp +++ b/Userland/Services/WindowServer/ConnectionFromClient.cpp @@ -145,7 +145,7 @@ void ConnectionFromClient::add_menu_item(i32 menu_id, i32 identifier, i32 submen menu.add_item(move(menu_item)); } -void ConnectionFromClient::popup_menu(i32 menu_id, Gfx::IntPoint const& screen_position) +void ConnectionFromClient::popup_menu(i32 menu_id, Gfx::IntPoint const& screen_position, Gfx::IntRect const& button_rect) { auto position = screen_position; auto it = m_menus.find(menu_id); @@ -154,7 +154,10 @@ void ConnectionFromClient::popup_menu(i32 menu_id, Gfx::IntPoint const& screen_p return; } auto& menu = *(*it).value; - menu.popup(position); + if (!button_rect.is_null()) + menu.open_button_menu(position, button_rect); + else + menu.popup(position); } void ConnectionFromClient::dismiss_menu(i32 menu_id) diff --git a/Userland/Services/WindowServer/ConnectionFromClient.h b/Userland/Services/WindowServer/ConnectionFromClient.h index 9ac48ea202d..085f23f975b 100644 --- a/Userland/Services/WindowServer/ConnectionFromClient.h +++ b/Userland/Services/WindowServer/ConnectionFromClient.h @@ -138,7 +138,7 @@ private: virtual void show_screen_numbers(bool) override; virtual void set_window_cursor(i32, i32) override; virtual void set_window_custom_cursor(i32, Gfx::ShareableBitmap const&) override; - virtual void popup_menu(i32, Gfx::IntPoint const&) override; + virtual void popup_menu(i32, Gfx::IntPoint const&, Gfx::IntRect const&) override; virtual void dismiss_menu(i32) override; virtual void set_window_icon_bitmap(i32, Gfx::ShareableBitmap const&) override; virtual Messages::WindowServer::StartDragResponse start_drag(String const&, HashMap const&, Gfx::ShareableBitmap const&) override; diff --git a/Userland/Services/WindowServer/Menu.cpp b/Userland/Services/WindowServer/Menu.cpp index 3254f05bda6..bb5582d333c 100644 --- a/Userland/Services/WindowServer/Menu.cpp +++ b/Userland/Services/WindowServer/Menu.cpp @@ -597,6 +597,27 @@ void Menu::redraw_if_theme_changed() redraw(); } +void Menu::open_button_menu(Gfx::IntPoint const& position, Gfx::IntRect const& button_rect) +{ + if (is_empty()) + return; + + auto& screen = Screen::closest_to_location(position); + auto& window = ensure_menu_window(position); + Gfx::IntPoint adjusted_pos = position; + + if (window.rect().right() > screen.width()) + adjusted_pos = adjusted_pos.translated(-(window.rect().right() - screen.width()) - 1, 0); + + if (window.rect().bottom() > screen.height()) + adjusted_pos = adjusted_pos.translated(0, -window.rect().height() - button_rect.height() + 1); + + window.set_rect(adjusted_pos.x(), adjusted_pos.y(), window.rect().width(), window.rect().height()); + window.move_to(adjusted_pos); + MenuManager::the().open_menu(*this, true); + WindowManager::the().did_popup_a_menu({}); +} + void Menu::popup(Gfx::IntPoint const& position) { do_popup(position, true); diff --git a/Userland/Services/WindowServer/Menu.h b/Userland/Services/WindowServer/Menu.h index 17cd6080629..f5ecbf3e05f 100644 --- a/Userland/Services/WindowServer/Menu.h +++ b/Userland/Services/WindowServer/Menu.h @@ -115,6 +115,7 @@ public: void popup(Gfx::IntPoint const&); void do_popup(Gfx::IntPoint const&, bool make_input, bool as_submenu = false); + void open_button_menu(Gfx::IntPoint const& position, Gfx::IntRect const& button_rect); bool is_menu_ancestor_of(Menu const&) const; diff --git a/Userland/Services/WindowServer/WindowServer.ipc b/Userland/Services/WindowServer/WindowServer.ipc index 20542a518da..b4aa09c4313 100644 --- a/Userland/Services/WindowServer/WindowServer.ipc +++ b/Userland/Services/WindowServer/WindowServer.ipc @@ -100,7 +100,7 @@ endpoint WindowServer set_fullscreen(i32 window_id, bool fullscreen) =| set_frameless(i32 window_id, bool frameless) =| set_forced_shadow(i32 window_id, bool shadow) =| - popup_menu(i32 menu_id, Gfx::IntPoint screen_position) =| + popup_menu(i32 menu_id, Gfx::IntPoint screen_position, Gfx::IntRect button_rect) =| dismiss_menu(i32 menu_id) =| set_wallpaper(Gfx::ShareableBitmap wallpaper_bitmap) => (bool success)