diff --git a/autotests/CMakeLists.txt b/autotests/CMakeLists.txt index 886dbaff5..6825ce6c0 100644 --- a/autotests/CMakeLists.txt +++ b/autotests/CMakeLists.txt @@ -133,3 +133,8 @@ if(Discount_FOUND) LINK_LIBRARIES Qt5::Test okularcore KF5::I18n PkgConfig::Discount ) endif() + +ecm_add_test(toggleactionmenutest.cpp ../part/toggleactionmenu.cpp + TEST_NAME "toggleactionmenutest" + LINK_LIBRARIES Qt5::Test KF5::WidgetsAddons +) diff --git a/autotests/toggleactionmenutest.cpp b/autotests/toggleactionmenutest.cpp new file mode 100644 index 000000000..ece098929 --- /dev/null +++ b/autotests/toggleactionmenutest.cpp @@ -0,0 +1,88 @@ +/*************************************************************************** + * Copyright (C) 2020 David Hurka * + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + ***************************************************************************/ + +#include + +#include "../part/toggleactionmenu.h" +#include + +class ToggleActionMenuTest : public QObject +{ + Q_OBJECT + +private Q_SLOTS: + void testSetDefaultAction(); + void testDeleteToolBarButton(); +}; + +void ToggleActionMenuTest::testSetDefaultAction() +{ + QToolBar dummyToolBar; + ToggleActionMenu menu(QStringLiteral("Menu"), this); + QAction *actionA = new QAction(QStringLiteral("A"), this); + QAction *actionB = new QAction(QStringLiteral("B"), this); + + // Do not set a default action, the menu should behave as plain KActionMenu. + QCOMPARE(menu.defaultAction(), &menu); + QToolButton *menuButton = qobject_cast(menu.createWidget(&dummyToolBar)); + QVERIFY(menuButton); + QCOMPARE(menuButton->defaultAction(), &menu); + + // Should still behave as plain KActionMenu when actions are added. + menu.addAction(actionA); + QCOMPARE(menu.defaultAction(), &menu); + QCOMPARE(menuButton->defaultAction(), &menu); + + // Set an action from the menu as default action, should work. + menu.setDefaultAction(actionA); + QCOMPARE(menu.defaultAction(), actionA); + QCOMPARE(menuButton->defaultAction(), actionA); + + // Set a foreign action as default action, should reset the default action. + menu.setDefaultAction(actionB); + QCOMPARE(menu.defaultAction(), &menu); + QCOMPARE(menuButton->defaultAction(), &menu); + + // Set an action of the menu as default action, should work. + menu.setDefaultAction(actionA); + QCOMPARE(menu.defaultAction(), actionA); + QCOMPARE(menuButton->defaultAction(), actionA); + + // Remove default action from menu, should reset the default action. + menu.removeAction(actionA); + QCOMPARE(menu.defaultAction(), &menu); + QCOMPARE(menuButton->defaultAction(), &menu); +} + +void ToggleActionMenuTest::testDeleteToolBarButton() +{ + QToolBar dummyToolBar; + ToggleActionMenu menu(QStringLiteral("Menu"), this); + QAction *actionA = new QAction(QStringLiteral("A"), this); + QAction *actionB = new QAction(QStringLiteral("B"), this); + + // Setup: set a default action and create two toolbar buttons. + menu.addAction(actionA); + menu.addAction(actionB); + menu.setDefaultAction(actionA); + QToolButton *menuButtonA = qobject_cast(menu.createWidget(&dummyToolBar)); + QVERIFY(menuButtonA); + QCOMPARE(menuButtonA->defaultAction(), actionA); + QToolButton *menuButtonB = qobject_cast(menu.createWidget(&dummyToolBar)); + QVERIFY(menuButtonB); + + // Delete button B, and set a new default action. Button A shall be updated without segfaulting on the deleted button B. + delete menuButtonB; + menu.setDefaultAction(actionB); + QCOMPARE(menuButtonA->defaultAction(), actionB); +} + +QTEST_MAIN(ToggleActionMenuTest) + +#include "toggleactionmenutest.moc" diff --git a/part/toggleactionmenu.cpp b/part/toggleactionmenu.cpp index f78cd6d35..e2092743b 100644 --- a/part/toggleactionmenu.cpp +++ b/part/toggleactionmenu.cpp @@ -1,5 +1,5 @@ /*************************************************************************** - * Copyright (C) 2019 by David Hurka * + * Copyright (C) 2019-2021 by David Hurka * * * * Inspired by and replacing toolaction.h by: * * Copyright (C) 2004-2006 by Albert Astals Cid * @@ -12,8 +12,8 @@ #include "toggleactionmenu.h" +#include #include -#include ToggleActionMenu::ToggleActionMenu(QObject *parent) : ToggleActionMenu(QIcon(), QString(), parent) @@ -25,43 +25,38 @@ ToggleActionMenu::ToggleActionMenu(const QString &text, QObject *parent) { } -ToggleActionMenu::ToggleActionMenu(const QIcon &icon, const QString &text, QObject *parent, PopupMode popupMode, MenuLogic logic) +ToggleActionMenu::ToggleActionMenu(const QIcon &icon, const QString &text, QObject *parent) : KActionMenu(icon, text, parent) , m_defaultAction(nullptr) - , m_suggestedDefaultAction(nullptr) - , m_menuLogic(logic) { - connect(this, &QAction::changed, this, &ToggleActionMenu::updateButtons); - - if (popupMode == DelayedPopup) { - setDelayed(true); - } else { - setDelayed(false); - } - setStickyMenu(false); - - if (logic & ImplicitDefaultAction) { - connect(menu(), &QMenu::triggered, this, &ToggleActionMenu::setDefaultAction); - } + slotMenuChanged(); } QWidget *ToggleActionMenu::createWidget(QWidget *parent) { - QToolButton *button = qobject_cast(KActionMenu::createWidget(parent)); + QWidget *buttonWidget = KActionMenu::createWidget(parent); + QToolButton *button = qobject_cast(buttonWidget); if (!button) { // This function is used to add a button into the toolbar. // KActionMenu will plug itself as QToolButton. // So, if no QToolButton was returned, this was not called the intended way. - return button; + Q_ASSERT_X(false, + "ToggleActionMenu::createWidget()", + "Parent implementation KActionMenu::createWidget() did not return a QToolButton, but ToggleActionMenu is designed for QToolButton. Did you call createWidget() manually, with something else than a QToolBar?"); + return buttonWidget; } + // BEGIN QToolButton hack + // Setting the default action of a QToolButton + // to an action of its menu() is tricky. // Remove this menu action from the button, // so it doesn't compose a menu of this menu action and its own menu. button->removeAction(this); // The button has lost the menu now, let it use the correct menu. button->setMenu(menu()); + // END QToolButton hack - m_buttons.append(QPointer(button)); + m_buttons.append(button); // Apply other properties to the button. updateButtons(); @@ -69,46 +64,28 @@ QWidget *ToggleActionMenu::createWidget(QWidget *parent) return button; } +QAction *ToggleActionMenu::defaultAction() +{ + return m_defaultAction ? m_defaultAction : this; +} + void ToggleActionMenu::setDefaultAction(QAction *action) { - m_defaultAction = action; - updateButtons(); -} - -void ToggleActionMenu::suggestDefaultAction(QAction *action) -{ - m_suggestedDefaultAction = action; -} - -QAction *ToggleActionMenu::checkedAction(QMenu *menu) const -{ - // Look at each action a in the menu whether it is checked. - // If a is a menu, recursively call checkedAction(). - const QList actions = menu->actions(); - for (QAction *a : actions) { - if (a->isChecked()) { - return a; - } else if (a->menu()) { - QAction *b = checkedAction(a->menu()); - if (b) { - return b; - } - } + if (action && menu()->actions().contains(action)) { + m_defaultAction = action; + } else { + m_defaultAction = nullptr; } - return nullptr; + updateButtons(); } void ToggleActionMenu::updateButtons() { - for (const QPointer &button : qAsConst(m_buttons)) { + for (QToolButton *button : qAsConst(m_buttons)) { if (button) { - button->setDefaultAction(defaultAction()); + button->setDefaultAction(this->defaultAction()); - // Override some properties of the default action, - // where the property of this menu makes more sense. - button->setEnabled(isEnabled()); - - if (delayed()) { + if (delayed()) { // TODO deprecated interface. button->setPopupMode(QToolButton::DelayedPopup); } else if (stickyMenu()) { button->setPopupMode(QToolButton::InstantPopup); @@ -119,13 +96,21 @@ void ToggleActionMenu::updateButtons() } } -QAction *ToggleActionMenu::defaultAction() +bool ToggleActionMenu::eventFilter(QObject *watched, QEvent *event) { - if ((m_menuLogic & ImplicitDefaultAction) && !m_defaultAction) { - m_defaultAction = checkedAction(menu()); + // If the defaultAction() is removed from the menu, reset the default action. + if (watched == menu() && event->type() == QEvent::ActionRemoved) { + QActionEvent *actionEvent = static_cast(event); + if (actionEvent->action() == defaultAction()) { + setDefaultAction(nullptr); + } } - if (!m_defaultAction) { - m_defaultAction = m_suggestedDefaultAction; - } - return m_defaultAction; + + return KActionMenu::eventFilter(watched, event); +} + +void ToggleActionMenu::slotMenuChanged() +{ + menu()->installEventFilter(this); + // Not removing old event filter, because we would need to remember the old menu. } diff --git a/part/toggleactionmenu.h b/part/toggleactionmenu.h index cb61f5917..d6c51cb07 100644 --- a/part/toggleactionmenu.h +++ b/part/toggleactionmenu.h @@ -1,5 +1,5 @@ /*************************************************************************** - * Copyright (C) 2019 by David Hurka * + * Copyright (C) 2019-2021 by David Hurka * * * * Inspired by and replacing toolaction.h by: * * Copyright (C) 2004-2006 by Albert Astals Cid * @@ -14,54 +14,36 @@ #define TOGGLEACTIONMENU_H #include +#include #include #include /** - * @brief A KActionMenu, with allows to set the default action of its toolbar buttons. + * @brief A KActionMenu, which allows to set the default action of its toolbar buttons. * - * Usually, a KActionMenu creates toolbar buttons which reflect its own action properties - * (icon, text, tooltip, checked state,...), as it is a QAction itself. - * - * ToggleActionMenu will use its own action properties only when plugged as submenu in another menu. - * The default action of the toolbar buttons can easily be changed with the slot setDefaultAction(). - * - * Naming: The user can *Toggle* the checked state of an *Action* by directly clicking the toolbar button, - * but can also open a *Menu*. + * This behaves like a KActionMenu, with the addition of setDefaultAction(). * * @par Intention - * Setting the default action of the toolbar button can be useful for: - * * Providing the most probably needed entry of a menu directly on the menu button. - * * Showing the last used menu entry on the menu button, including its checked state. - * The advantage is that the user often does not need to open the menu, - * and that the toolbar button shows additional information - * like checked state or the user's last selection. + * Setting the default action of toolbar buttons has the advantage that the user + * can trigger a frequently used action directly without opening the menu. + * Additionally, the state of the default action is visible in the toolbar. * - * This shall replace the former ToolAction in Okular, - * while being flexible enough for other (planned) action menus. + * @par Example + * You can make the toolbar button show the last used action with only one connection. + * You may want to initialize the default action. + * \code + * if (myToggleActionMenu->defaultAction() == myToggleActionMenu) { + * myToggleActionMenu->setDefaultAction(myFirstAction); + * } + * connect(myToggleActionMenu->menu(), &QMenu::triggered, + * myToggleActionMenu, &ToggleActionMenu::setDefaultAction); + * \endcode */ class ToggleActionMenu : public KActionMenu { Q_OBJECT public: - /** - * Defines how the menu behaves. - */ - enum MenuLogic { - DefaultLogic = 0x0, - /** - * Automatically makes the triggered action the default action, even if in a submenu. - * When a toolbar button is constructed, - * the default action is set to the default action set with setDefaultAction() before, - * otherwise to the first checked action in the menu, - * otherwise to the action suggested with suggestDefaultAction(). - */ - ImplicitDefaultAction = 0x1 - }; - - enum PopupMode { DelayedPopup, MenuButtonPopup }; - explicit ToggleActionMenu(QObject *parent); ToggleActionMenu(const QString &text, QObject *parent); /** @@ -70,78 +52,61 @@ public: * @param icon The icon of this menu, when plugged into another menu. * @param text The name of this menu, when plugged into another menu. * @param parent Parent @c QOject. - * @param popupMode The popup mode of the toolbar buttons. - * You will want to use @c DelayedPopup or @c MenuButtonPopup, - * @c InstantPopup would make @c ToggleActionMenu pointless. - * @param logic To define special behaviour of @c ToggleActionMenu, - * to simplify the usage. */ - ToggleActionMenu(const QIcon &icon, const QString &text, QObject *parent, PopupMode popupMode = MenuButtonPopup, MenuLogic logic = DefaultLogic); + ToggleActionMenu(const QIcon &icon, const QString &text, QObject *parent); QWidget *createWidget(QWidget *parent) override; /** * Returns the current default action of the toolbar buttons. + * May be @c this. * - * In ImplicitDefaultAction mode, - * when the default action was not yet set with setDefaultAction(), - * it will determine it from the first checked action in the menu, - * otherwise from the action set with suggestDefaultAction(). + * This action is set by setDefaultAction(). */ QAction *defaultAction(); - /** - * Suggests a default action to be used as fallback. - * - * It will be used if the default action is not determined another way. - * This is useful for ImplicitDefaultAction mode, - * when you can not guarantee that one action in the menu - * will be checked. - * - * @note - * In DefaultLogic mode, or when you already have called setDefaultAction(), - * you have to use setDefaultAction() instead. - */ - void suggestDefaultAction(QAction *action); - -public slots: +public Q_SLOTS: /** * Sets the default action of the toolbar buttons. * - * This action will be triggered by clicking directly on the toolbar buttons. - * It will also set the text, icon, checked state, etc. of the toolbar buttons. + * Toolbar buttons are updated immediately. + * + * Calling setDefaultAction(nullptr) will reset the default action + * to this menu itself. * * @note - * The default action will not set the enabled state or popup mode of the menu buttons. - * These properties are still set by the corresponding properties of this ToggleActionMenu. + * @p action must be present in the menu as direct child action. + * The default action will be reset to this menu itself + * when @p action is removed from the menu. * - * @warning - * The action will not be added to the menu, - * it usually makes sense to addAction() it before to setDefaultAction() it. - * - * @see suggestDefaultAction() + * @note + * @p action will define all properties of the toolbar buttons. + * When you disable @p action, the toolbar button will become disabled too. + * Then the menu can no longer be accessed. */ void setDefaultAction(QAction *action); -private: - QAction *m_defaultAction; - QAction *m_suggestedDefaultAction; +protected: + /** Can store @c nullptr, which means this menu itself will be the default action. */ + QPointer m_defaultAction; QList> m_buttons; - MenuLogic m_menuLogic; /** - * Returns the first checked action in @p menu and its submenus, - * or nullptr if no action is checked. - */ - QAction *checkedAction(QMenu *menu) const; - -private slots: - /** - * Updates the toolbar buttons, using both the default action and properties of this menu itself. + * Updates the toolbar buttons by setting the current defaultAction() on them. * - * This ensures that the toolbar buttons reflect e. g. a disabled state of this menu. + * (If the current defaultAction() is invalid, `this` is used instead.) */ void updateButtons(); + + /** + * Updates the event filter, which listens to QMenu’s QActionEvent. + * + * This is connected to QAction::changed(). + * That signal is emmited when the menu changes, but that’s not documented. + */ + void slotMenuChanged(); + + bool eventFilter(QObject *watched, QEvent *event) override; }; #endif // TOGGLEACTIONMENU_H