RFC: An action class to ease implementation of show/hide-like actions

[Aurélien Gâteau]

Hi,

After putting some work on toggle buttons in the HIG [1], I started to
look around at applications which were not complying with this new
addition to the guidelines. I realized lots of applications misuse
KToggleAction and KToggleAction::setCheckedState() to create actions
like "Show/Hide Sidebar": they create toggle actions which change their
labels, so they look like this:

[ ] Show Sidebar
[x] Hide Sidebar

This is even worse when used in a toolbar because when the sidebar is
visible you end up with a button which says "Hide Sidebar" and stays down.

One of the solutions to this is to keep the same label, regardless of
the visibility of the UI element. This is the easiest solution, but it
does not feel as intuitive in my opinion as changing the label,
especially in menus.

I have been working on a class which would make it easy to create such
actions, without making them checked actions. I posted a first patch on
reviewboard but it was not really ready so I discarded it. I have
something here which is starting to be usable as an almost drop-in
replacement for KToggleAction. But before going to reviewboard, I would
like to discuss the API of the class here.

The class looks like this:

---
class KDEUI_EXPORT KDualAction : public KAction
{
    Q_OBJECT
public:
    KDualAction(const QString &offActionText, QObject *parent);
    explicit KDualAction(QObject *parent);

    void setGuiItem(bool, const KGuiItem &item);
    KGuiItem guiItem(bool) const;

    using QAction::text;
    using QAction::icon;
    using QAction::toolTip;

    void setText(bool, const QString &text);
    QString text(bool) const;

    void setIcon(bool, const QIcon &icon);
    QIcon icon(bool) const;

    void setToolTip(bool, const QString &tip);
    QString toolTip(bool) const;

    bool isOn() const;
    void setOn(bool);
    void silentSetOn(bool);

    void setAutoToggle(bool);
    bool autoToggle() const;

Q_SIGNALS:
    void statusChanged(bool);

private:
    Q_PRIVATE_SLOT(d, void slotTriggered())
    KDualActionPrivate *const d;
    friend class KDualActionPrivate;
};
---

Note that while this class is handy to implement show/hide actions it is
not limited to this use case. It could be used for a reload/stop action
in a browser or a play/pause action in a media player for example.

Here are the questions I'd like to hear from you about.

1. Does the class name make sense? I am not fully satisfied with it, but
couldn't find anything else.

2. I wanted the class to have a boolean state and decided to call it
"on". I am not so sure it is a good idea now because I realized QAction
already has an "on" property in qt3support. It was renamed to "checked"
in Qt4, but it still appears in the DBus API when we expose our actions.

3. The bool parameter in the methods to get/set the icon/text,tooltip is
there to make it possible to define those for "off" and "on" values. I
dislike however having to use the "using" keyword. Would it be better to
use different names?

4. Some of the actions in KStandardAction could benefit from this class,
but because of BC issues we would need to introduce new methods and
deprecate the current ones. For example KStandardAction::showMenubar()
returns a KToggleAction. I played with this and created
KStandardAction::showHideMenubar(), which returns a KDualAction. Other
candidates are showStatusbar() and fullscreen(), we could also add
showHideToolbar(). Do you think we should go this way?

Aurélien

PS: I was tempted at some point to make the class uses two KActions
instead of KGuiItems, but it turned out to be overkill: KDualAction
would have to track the state of the two actions and it would also be
easy to misuse the class by using checkable actions or widget actions.

PPS: A nice side-effect of "standardizing" this behavior in a specific
class is that the shortcut dialog could take advantage of it: instead of
showing the action text, it could do something like this:

KDualAction* dualAction = qobject_cast<KDualAction*>(action);
if (dualAction) {
  text = i18n("%s | %s")
    .arg(dualAction->text(false), dualAction->text(true));
} else {
  text = action->text();
}

Which would show "Play | Pause" in the dialog instead of either "Play"
or "Pause" depending on the action state.

[1]: http://techbase.kde.org/Projects/Usability/HIG/Toggle_Buttons