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

[Aurélien Gâteau]

On 18/09/2010 00:06, Ingo Klöcker wrote:
> On Friday 17 September 2010, Aurélien Gâteau wrote:
>> On 10/09/2010 23:20, Aurélien Gâteau wrote:
>> class KDEUI_EXPORT KDualAction : public KAction
>> {
>>     Q_OBJECT
>> public:
>>     KDualAction(const QString &offActionText, QObject *parent);
> 
> I don't understand why one can set one of the texts via the c'tor, but 
> not the other one. Am I missing something or did you forget something? 
> :-)

I created this constructor to make it as easy as possible to replace a
KToggleAction with a KDualAction. Since KToggleAction has a constructor
which takes the offActionText, I created one as well. Maybe it should be
changed to KDualAction(offText, onText, parent). What do you think of this?

>>     bool isActive() const;
>>     void setActive(bool);
>>     void silentSetActive(bool);
> 
> I suppose this is supposed to set the active state without emitting the 
> corresponding signal, i.e. it is equivalent to
>   dualAction->blockSignals( true );
>   dualAction->setActive( true/false );
>   dualAction->blockSignals( false );

It is roughly equivalent: it does not have to block signals, it just
does not emit them.

> IMHO something like a silentSetActive() (or using blockSignals()) is 
> evil because the caller can never know whether there are other listeners 
> which rely on the signal to be emitted. Therefore, I strongly suggest 
> removing this method from the class's interface.

I think it is useful to have this method for situations where the state
of the element represented by the action has changed and you need to
update the action to reflect the new state.

A concrete example is the play/pause action of a media player: While
playing the action text is "Pause". When the played song stops, the
action text must be set to "Play".

Having it as a single method is interesting because:
- It avoids the use of blockSignals(), which can be tricky, the example
you give is not completely correct, it should be something like this:

    bool blocked = dualAction->blockSignals( true );
    dualAction->setActive( true/false );
    dualAction->blockSignals( blocked );

- The method can be turned into a slot and directly connected to the
object it represents.

Still I agree it is mostly there for developer convenience, so if you
feel strongly against it, it can be removed.

Aurélien