[Development] QAction in Qt 6 - next step proposal

Jaroslaw Kobus Jaroslaw.Kobus at qt.io
Wed Nov 27 09:53:28 CET 2019

Dear Qt friends, hackers and freaks,

recently I've noticed a refactoring changes regarding QAction and QActionGroup,
happening inside "dev" branch of Qt
(https://codereview.qt-project.org/c/qt/qtbase/+/265909). I didn't have a chance
to review it before it got merged, I just missed it. So I took a liberty to look
what we are going to have in Qt 6 and reviewed it "post factum".

I need to admit, that this is a very good change! The main goal, which is
moving QAction out of Widget lib and placing it in some more general lib
in order to use it not only in Widget world, is a really good choice! Good work
Friedemann and all reviewers!

However, when we say A, I'm very tempted to say also B (or even C), so why
not to take one or two more steps with it?

The current state in short ("dev" branch of Qt)

The QAction (and QActionGroup) has been split into two classes:

- QGuiAction (and QGuiActionGroup) is the base, lives outside of Widget lib,
  contains most of the API of old QAction (QActionGroup) except the API
  referring to Widget lib, like methods containing pointers to QMenu, QWidget.

- QAction (and QActionGroup) is derived from QGuiAction (QGuiActionGroup),
  enriching the base QGuiAction (QAGuictionGroup) a little bit, lives in old
  place inside Widget lib

At a quick glance:

class QAction : public QGuiAction
    QMenu *menu() const;
    void setMenu(QMenu *menu);
    bool showStatusText(QWidget *widget = nullptr);
    QWidget *parentWidget() const;
    QList<QWidget *> associatedWidgets() const;
    QList<QGraphicsWidget *> associatedGraphicsWidgets() const;

And nothing really more than the above.

So, QAction is now a small decoration, enriching the QGuiAction with
methods referring to QWidget world.

The rationale for further steps

While I think the very first step, very good step,
has already been done, I believe we may
do something more to make people more happy.

With the current state users might start wondering:

- There are (will be) libs which instatiate QGuiActions and QActions. How then
  I use them together in one app? E.g. I've received pointer to QGuiAction let's
  say from a lib which implements QML bindings (so I receive an action from QML
  world). Can I put this action into the QMenu directly?
  Probably I need to duplicate the QGuiAction by creating the respective QAction,
  keeping them in sync on some map and put the QAction to the QMenu.

- I'm writing my lib (not linked against QtWidgets), and I want to expose
  actions through my API. Should I use QGuiAction in the API? Should I
  instantiate the QGuiAction in my lib? But then the other lib, which is
  dependent on QtWidgets won't be able to use actions returned by my API.

- I'm probably going to introduce quite a lot downcasts in my code nowadays,
  like q-object_cast<QAction *>(q_gui_action_pointer)
  since parts of Qt API are going to use QGuiAction, while other parts are
  going to use QAction.

That's why I believe, that having just one class (QAction) instead of two
(QGuiAction, QAction) would solve all the above concerns.

The next step: keep one class, enrich not by inheritance

Being a fan of a principle "prefer aggregation over inheritance", I would like
to propose the following changes to the current state:

1. Rename QGuiAction (QGuiActionGroup) back to QAction (QActionGroup)
   without any other changes to its current API.
2. Remove current QAction (QActionGroup) subclasses
   (which currently live in QtWidgets lib) and don't introduce any new class for it.
   We than need to figure out what to do with the API which currently is there,
   and how to enrich existing QAction, not by inheritance.
3. For the existing QAction's API I propose to move the respective functions
   to the other classes involved (e.g. as static methods), like:

QMenu *menu() const;
-> static QMenu *QMenu::menuForAction(QAction *action);

void setMenu(QMenu *menu);
-> static void QMenu::setMenuForAction(QAction *action, QMenu *menu);

bool showStatusText(QWidget *widget = nullptr);
-> static bool QWidget::showStatusText(QAction *action, QWidget *widget = nullptr);

QWidget *parentWidget() const;
-> static QWidget *QWidget::parentWidget(QAction *action);

QList<QWidget *> associatedWidgets() const;
-> static QList<QWidget *> QWidget::associatedWidgets(QAction *action);

QList<QGraphicsWidget *> associatedGraphicsWidgets() const;
-> static QList<QGraphicsWidget *> QGraphicsWidget::associatedGraphicsWidgets(QAction *action);

4. For the QActionGroup it looks like all functions may just disappear,
as they are kind of repetition of functions from QGuiActionGroup. Regarding
triggered() and hovered() signals: we may move them back to the base class, as
currently we have similar signals in the action base class.

Further steps

1. I would like to ask all of you what do you think about the above proposal?
   It would be really cool if you could state whether you like the proposed
   direction or not.

2. In case we agree if we want further changes, I invite you for brainstorming
   on proposed names for functions moved out of current QAction, their location
   (contained in right class or the other class would be better) and their
   "staticness". We may also discuss other possibilities on how to handle
    the remaining API of current QAction / QActionGroup.

3. In case we develop together the API we want, we could start discussing
   the implementation details, like where to put the enriched internals. Qt
   is very comprehensive on that, so for sure we will have couple of choices

I would very much like to try this API-driven (not implementation-driven)
approach when doing API changes. Like in old Jasmin's time: we need to first
develop the API we would like to have and use, and later start figuring out
implementation challenges. So, discussing point 2 or 3, without having
agreement on 1 wouldn't be much useful I guess.

Big thanks and regards


More information about the Development mailing list