UI/Qt: Remove icons from menus

Let application actions opt out of icon creation when created for
menus. Use that path for browser menus, tab context menus, and
WebView-backed popup menus.

Keep icon creation for toolbar and bookmarks bar actions, where the
same helper still provides visible button icons. Remove the icon setup
that is now only used by menu actions, and shrink popup menu item
padding now that there is no icon column.
This commit is contained in:
Andreas Kling 2026-05-27 18:12:10 +02:00 committed by Andreas Kling
parent 0baeddd972
commit 972c19b292
6 changed files with 43 additions and 138 deletions

View file

@ -254,13 +254,11 @@ BrowserWindow::BrowserWindow(Vector<URL::URL> const& initial_urls, IsPopupWindow
update_reopen_recently_closed_action();
auto* close_current_tab_action = new QAction("&Close Current Tab", this);
close_current_tab_action->setIcon(load_icon_from_uri("resource://icons/16x16/close-tab.png"sv));
close_current_tab_action->setShortcuts(QKeySequence::keyBindings(QKeySequence::StandardKey::Close));
m_hamburger_menu->addAction(close_current_tab_action);
file_menu->addAction(close_current_tab_action);
auto* open_file_action = new QAction("&Open File...", this);
open_file_action->setIcon(load_icon_from_uri("resource://icons/16x16/filetype-folder-open.png"sv));
open_file_action->setShortcut(QKeySequence(QKeySequence::StandardKey::Open));
m_hamburger_menu->addAction(open_file_action);
file_menu->addAction(open_file_action);
@ -270,14 +268,13 @@ BrowserWindow::BrowserWindow(Vector<URL::URL> const& initial_urls, IsPopupWindow
auto* edit_menu = m_hamburger_menu->addMenu("&Edit");
menuBar()->addMenu(edit_menu);
edit_menu->addAction(create_application_action(*this, Application::the().cut_selection_action()));
edit_menu->addAction(create_application_action(*this, Application::the().copy_selection_action()));
edit_menu->addAction(create_application_action(*this, Application::the().paste_action()));
edit_menu->addAction(create_application_action(*this, Application::the().select_all_action()));
edit_menu->addAction(create_application_action(*this, Application::the().cut_selection_action(), IncludeActionIcon::No));
edit_menu->addAction(create_application_action(*this, Application::the().copy_selection_action(), IncludeActionIcon::No));
edit_menu->addAction(create_application_action(*this, Application::the().paste_action(), IncludeActionIcon::No));
edit_menu->addAction(create_application_action(*this, Application::the().select_all_action(), IncludeActionIcon::No));
edit_menu->addSeparator();
m_find_in_page_action = new QAction("&Find in Page...", this);
m_find_in_page_action->setIcon(load_icon_from_uri("resource://icons/16x16/find.png"sv));
m_find_in_page_action->setShortcuts(QKeySequence::keyBindings(QKeySequence::StandardKey::Find));
auto find_previous_shortcuts = QKeySequence::keyBindings(QKeySequence::StandardKey::FindPrevious);
@ -298,7 +295,7 @@ BrowserWindow::BrowserWindow(Vector<URL::URL> const& initial_urls, IsPopupWindow
QObject::connect(m_find_in_page_action, &QAction::triggered, this, &BrowserWindow::show_find_in_page);
edit_menu->addSeparator();
edit_menu->addAction(create_application_action(*edit_menu, Application::the().open_settings_page_action()));
edit_menu->addAction(create_application_action(*edit_menu, Application::the().open_settings_page_action(), IncludeActionIcon::No));
auto* view_menu = m_hamburger_menu->addMenu("&View");
menuBar()->addMenu(view_menu);
@ -346,7 +343,7 @@ BrowserWindow::BrowserWindow(Vector<URL::URL> const& initial_urls, IsPopupWindow
auto* help_menu = m_hamburger_menu->addMenu("&Help");
menuBar()->addMenu(help_menu);
help_menu->addAction(create_application_action(*help_menu, Application::the().open_about_page_action()));
help_menu->addAction(create_application_action(*help_menu, Application::the().open_about_page_action(), IncludeActionIcon::No));
m_hamburger_menu->addSeparator();
file_menu->addSeparator();

View file

@ -99,7 +99,6 @@ public:
QMenu& hamburger_menu() const { return *m_hamburger_menu; }
QAction& new_tab_action() const { return *m_new_tab_action; }
QAction& new_window_action() const { return *m_new_window_action; }
QAction& find_action() const { return *m_find_in_page_action; }

View file

@ -265,7 +265,7 @@ QMenu::item {{
background: transparent;
border-radius: 5px;
min-height: 20px;
padding: 5px 28px;
padding: 5px 14px;
}}
QMenu::item:selected {{

View file

@ -19,9 +19,9 @@ namespace Ladybird {
class ActionObserver final : public WebView::Action::Observer {
public:
static NonnullOwnPtr<ActionObserver> create(WebView::Action& action, QAction& qaction)
static NonnullOwnPtr<ActionObserver> create(WebView::Action& action, QAction& qaction, IncludeActionIcon include_action_icon)
{
return adopt_own(*new ActionObserver(action, qaction));
return adopt_own(*new ActionObserver(action, qaction, include_action_icon));
}
virtual void on_text_changed(WebView::Action& action) override
@ -50,6 +50,9 @@ public:
virtual void on_engaged_state_changed(WebView::Action& action) override
{
if (m_include_action_icon == IncludeActionIcon::No)
return;
if (!m_action)
return;
@ -73,8 +76,9 @@ public:
}
private:
ActionObserver(WebView::Action& action, QAction& qaction)
ActionObserver(WebView::Action& action, QAction& qaction, IncludeActionIcon include_action_icon)
: m_action(&qaction)
, m_include_action_icon(include_action_icon)
{
QObject::connect(m_action, &QAction::triggered, [weak_action = action.make_weak_ptr()](bool checked) {
if (auto action = weak_action.strong_ref()) {
@ -90,6 +94,7 @@ private:
}
QPointer<QAction> m_action;
IncludeActionIcon m_include_action_icon { IncludeActionIcon::Yes };
};
template<typename T>
@ -114,70 +119,66 @@ static QIcon icon_from_base64_png(StringView favicon_base64_png)
return pixmap.scaled(MENU_ICON_SIZE, MENU_ICON_SIZE, Qt::KeepAspectRatio, Qt::SmoothTransformation);
}
static void initialize_native_control(WebView::Action& action, QAction& qaction, QPalette const& palette)
static void initialize_native_control(WebView::Action& action, QAction& qaction, QPalette const& palette, IncludeActionIcon include_action_icon)
{
switch (action.id()) {
case WebView::ActionID::NavigateBack:
qaction.setIcon(create_chrome_icon(ChromeIcon::Back, palette));
if (include_action_icon == IncludeActionIcon::Yes)
qaction.setIcon(create_chrome_icon(ChromeIcon::Back, palette));
qaction.setShortcut(QKeySequence::StandardKey::Back);
break;
case WebView::ActionID::NavigateForward:
qaction.setIcon(create_chrome_icon(ChromeIcon::Forward, palette));
if (include_action_icon == IncludeActionIcon::Yes)
qaction.setIcon(create_chrome_icon(ChromeIcon::Forward, palette));
qaction.setShortcut(QKeySequence::StandardKey::Forward);
break;
case WebView::ActionID::Reload:
qaction.setIcon(create_chrome_icon(ChromeIcon::Reload, palette));
if (include_action_icon == IncludeActionIcon::Yes)
qaction.setIcon(create_chrome_icon(ChromeIcon::Reload, palette));
qaction.setShortcuts({ QKeySequence(Qt::CTRL | Qt::Key_R), QKeySequence(Qt::Key_F5) });
break;
case WebView::ActionID::CopySelection:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/edit-copy.png"sv));
qaction.setShortcut(QKeySequence::StandardKey::Copy);
break;
case WebView::ActionID::CutSelection:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/edit-cut.png"sv));
qaction.setShortcut(QKeySequence::StandardKey::Cut);
break;
case WebView::ActionID::Paste:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/paste.png"sv));
qaction.setShortcut(QKeySequence::StandardKey::Paste);
break;
case WebView::ActionID::SelectAll:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/select-all.png"sv));
qaction.setShortcut(QKeySequence::StandardKey::SelectAll);
break;
case WebView::ActionID::SearchSelectedText:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/find.png"sv));
break;
case WebView::ActionID::ToggleBookmark:
qaction.setIcon(create_chrome_icon(action.engaged() ? ChromeIcon::StarFilled : ChromeIcon::Star, palette));
if (include_action_icon == IncludeActionIcon::Yes)
qaction.setIcon(create_chrome_icon(action.engaged() ? ChromeIcon::StarFilled : ChromeIcon::Star, palette));
qaction.setShortcut(QKeySequence(Qt::CTRL | Qt::Key_D));
break;
case WebView::ActionID::ToggleBookmarkViaToolbar:
qaction.setIcon(create_chrome_icon(action.engaged() ? ChromeIcon::StarFilled : ChromeIcon::Star, palette));
if (include_action_icon == IncludeActionIcon::Yes)
qaction.setIcon(create_chrome_icon(action.engaged() ? ChromeIcon::StarFilled : ChromeIcon::Star, palette));
break;
case WebView::ActionID::ToggleBookmarksBar:
qaction.setShortcut(QKeySequence(Qt::CTRL | Qt::SHIFT | Qt::Key_B));
break;
case WebView::ActionID::BookmarkItem:
if (auto icon = action.base64_png_icon(); icon.has_value())
qaction.setIcon(icon_from_base64_png(*icon));
else
qaction.setIcon(create_tvg_icon_with_theme_colors("globe", palette));
if (include_action_icon == IncludeActionIcon::Yes) {
if (auto icon = action.base64_png_icon(); icon.has_value())
qaction.setIcon(icon_from_base64_png(*icon));
else
qaction.setIcon(create_tvg_icon_with_theme_colors("globe", palette));
}
break;
case WebView::ActionID::OpenProcessesPage:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/app-system-monitor.png"sv));
qaction.setShortcut(QKeySequence(Qt::CTRL | Qt::SHIFT | Qt::Key_M));
break;
case WebView::ActionID::OpenSettingsPage:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/settings.png"sv));
qaction.setShortcut(QKeySequence::StandardKey::Preferences);
break;
case WebView::ActionID::ToggleDevTools:
qaction.setIcon(load_icon_from_uri("resource://icons/browser/dom-tree.png"sv));
qaction.setShortcuts({
QKeySequence(Qt::CTRL | Qt::SHIFT | Qt::Key_I),
QKeySequence(Qt::CTRL | Qt::SHIFT | Qt::Key_C),
@ -185,62 +186,10 @@ static void initialize_native_control(WebView::Action& action, QAction& qaction,
});
break;
case WebView::ActionID::ViewSource:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/filetype-html.png"sv));
qaction.setShortcut(QKeySequence(Qt::CTRL | Qt::Key_U));
break;
case WebView::ActionID::TakeVisibleScreenshot:
case WebView::ActionID::TakeFullScreenshot:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/filetype-image.png"sv));
break;
case WebView::ActionID::OpenInNewTab:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/new-tab.png"sv));
break;
case WebView::ActionID::OpenInNewWindow:
// FIXME: should be a separate icon for new window.
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/new-tab.png"sv));
break;
case WebView::ActionID::CopyURL:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/edit-copy.png"sv));
break;
case WebView::ActionID::OpenImage:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/filetype-image.png"sv));
break;
case WebView::ActionID::SaveImage:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/download.png"sv));
break;
case WebView::ActionID::CopyImage:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/edit-copy.png"sv));
break;
case WebView::ActionID::OpenAudio:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/filetype-sound.png"sv));
break;
case WebView::ActionID::OpenVideo:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/filetype-video.png"sv));
break;
case WebView::ActionID::PlayMedia:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/play.png"sv));
break;
case WebView::ActionID::PauseMedia:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/pause.png"sv));
break;
case WebView::ActionID::MuteMedia:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/audio-volume-muted.png"sv));
break;
case WebView::ActionID::UnmuteMedia:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/audio-volume-high.png"sv));
break;
case WebView::ActionID::EnterFullscreen:
case WebView::ActionID::ExitFullscreen: // FIXME: Create a separate icon for exiting fullscreen.
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/fullscreen.png"sv));
break;
case WebView::ActionID::ZoomIn: {
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/zoom-in.png"sv));
auto zoom_in_shortcuts = QKeySequence::keyBindings(QKeySequence::StandardKey::ZoomIn);
auto secondary_zoom_in_shortcut = QKeySequence(Qt::CTRL | Qt::Key_Equal);
@ -251,48 +200,12 @@ static void initialize_native_control(WebView::Action& action, QAction& qaction,
break;
}
case WebView::ActionID::ZoomOut:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/zoom-out.png"sv));
qaction.setShortcut(QKeySequence::StandardKey::ZoomOut);
break;
case WebView::ActionID::ResetZoom:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/zoom-reset.png"sv));
qaction.setShortcut(QKeySequence(Qt::CTRL | Qt::Key_0));
break;
case WebView::ActionID::DumpSessionHistoryTree:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/history.png"sv));
break;
case WebView::ActionID::DumpDOMTree:
qaction.setIcon(load_icon_from_uri("resource://icons/browser/dom-tree.png"sv));
break;
case WebView::ActionID::DumpLayoutTree:
case WebView::ActionID::DumpPaintTree:
case WebView::ActionID::DumpDisplayList:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/layout.png"sv));
break;
case WebView::ActionID::DumpStackingContextTree:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/layers.png"sv));
break;
case WebView::ActionID::DumpStyleSheets:
case WebView::ActionID::DumpStyles:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/filetype-css.png"sv));
break;
case WebView::ActionID::DumpCSSErrors:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/error.png"sv));
break;
case WebView::ActionID::DumpCookies:
qaction.setIcon(load_icon_from_uri("resource://icons/browser/cookie.png"sv));
break;
case WebView::ActionID::DumpLocalStorage:
qaction.setIcon(load_icon_from_uri("resource://icons/browser/local-storage.png"sv));
break;
case WebView::ActionID::ShowLineBoxBorders:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/box.png"sv));
break;
case WebView::ActionID::CollectGarbage:
qaction.setIcon(load_icon_from_uri("resource://icons/16x16/trash-can.png"sv));
break;
default:
break;
}
@ -300,7 +213,7 @@ static void initialize_native_control(WebView::Action& action, QAction& qaction,
if (action.is_checkable())
qaction.setCheckable(true);
action.add_observer(ActionObserver::create(action, qaction));
action.add_observer(ActionObserver::create(action, qaction, include_action_icon));
add_properties(qaction, action);
}
@ -311,21 +224,13 @@ static void add_items_to_menu(QMenu& qmenu, QWidget& parent, WebView::Menu& menu
for (auto& menu_item : menu.items()) {
menu_item.visit(
[&](NonnullRefPtr<WebView::Action>& action) {
auto* qaction = create_application_action(parent, action);
auto* qaction = create_application_action(parent, action, IncludeActionIcon::No);
qmenu.addAction(qaction);
if (action->id() == WebView::ActionID::SpoofUserAgent || action->id() == WebView::ActionID::NavigatorCompatibilityMode) {
if (qmenu.icon().isNull())
qmenu.setIcon(load_icon_from_uri("resource://icons/16x16/spoof.png"sv));
}
},
[&](NonnullRefPtr<WebView::Menu> const& submenu) {
auto* qsubmenu = new QMenu(qstring_from_ak_string(submenu->title()), &qmenu);
add_items_to_menu(*qsubmenu, parent, submenu);
if (submenu->render_group_icon())
qsubmenu->setIcon(create_tvg_icon_with_theme_colors("folder", parent.palette()));
add_properties(*qsubmenu, *submenu);
qmenu.addMenu(qsubmenu);
},
@ -360,10 +265,10 @@ QMenu* create_context_menu(QWidget& parent, WebContentView& view, WebView::Menu&
return application_menu;
}
QAction* create_application_action(QWidget& parent, WebView::Action& action)
QAction* create_application_action(QWidget& parent, WebView::Action& action, IncludeActionIcon include_action_icon)
{
auto* qaction = new QAction(&parent);
initialize_native_control(action, *qaction, parent.palette());
initialize_native_control(action, *qaction, parent.palette(), include_action_icon);
return qaction;
}

View file

@ -16,10 +16,15 @@ namespace Ladybird {
class WebContentView;
enum class IncludeActionIcon {
No,
Yes,
};
QMenu* create_application_menu(QWidget& parent, WebView::Menu&);
void repopulate_application_menu(QMenu& menu, QWidget& parent, WebView::Menu& source);
QMenu* create_context_menu(QWidget& parent, WebContentView&, WebView::Menu&);
QAction* create_application_action(QWidget& parent, WebView::Action&);
QAction* create_application_action(QWidget& parent, WebView::Action&, IncludeActionIcon = IncludeActionIcon::Yes);
}

View file

@ -495,7 +495,7 @@ Tab::Tab(BrowserWindow* window, RefPtr<WebView::WebContentClient> parent_client,
});
m_context_menu = new QMenu("Context menu", this);
m_context_menu->addAction(create_application_action(*this, WebView::Application::the().reload_action()));
m_context_menu->addAction(create_application_action(*this, WebView::Application::the().reload_action(), IncludeActionIcon::No));
m_context_menu->addAction(duplicate_tab_action);
m_context_menu->addSeparator();
auto* move_tab_menu = m_context_menu->addMenu("Mo&ve Tab");
@ -684,7 +684,6 @@ void Tab::recreate_toolbar_icons()
m_navigate_back_action->setIcon(create_chrome_icon(ChromeIcon::Back, palette()));
m_navigate_forward_action->setIcon(create_chrome_icon(ChromeIcon::Forward, palette()));
m_reload_action->setIcon(create_chrome_icon(ChromeIcon::Reload, palette()));
m_window->new_tab_action().setIcon(create_chrome_icon(ChromeIcon::NewTab, palette()));
m_hamburger_button->setIcon(create_chrome_icon(ChromeIcon::Menu, palette()));
if (auto* action = m_location_edit->trailing_action()) {
auto icon = view().toggle_bookmark_action().engaged() ? ChromeIcon::StarFilled : ChromeIcon::Star;