[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: RFC: An action class to ease implementation of show/hide-like
From:       Aurélien_Gâteau <agateau () kde ! org>
Date:       2010-09-20 21:02:34
Message-ID: 4C97CBEA.3090807 () kde ! org
[Download RAW message or body]

On 20/09/2010 20:48, Ingo Klöcker wrote:
> On Monday 20 September 2010, Aurélien Gâteau wrote:
>> On 19/09/2010 16:47, Ingo Klöcker wrote:
>>> On Sunday 19 September 2010, Aurélien Gâteau wrote:
>>>> On 18/09/2010 12:33, Ingo Klöcker wrote:
>>>>> On Saturday 18 September 2010, Aurélien Gâteau wrote:
>>>>>> On 18/09/2010 00:06, Ingo Klöcker wrote:
>>>>>>> On Friday 17 September 2010, Aurélien Gâteau wrote:
>>>>>> 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?
>>>>>
>>>>> Makes more sense since a dual action will probably always have
>>>>> two different texts. (Otherwise, one wouldn't use it.)
>>>>
>>>> Indeed.
>>>>
>>>>> I'd revert the two texts, but that's probably just me.
>>>>
>>>> What do you mean?
>>>
>>> I meant change their order, i.e.
>>>
>>>   KDualAction( activeText, inactiveText, parent )
>>
>> Oh I see. If we take my sidebar example, then if the API is inactive,
>> active the call looks like this:
>>
>>   new KDualAction(i18n("Show Sidebar"), i18n("Hide Sidebar"), parent)
>>
>> It feels more natural to me than:
>>
>>   new KDualAction(i18n("Hide Sidebar"), i18n("Show Sidebar"), parent)
> 
> True. I didn't realize that activeText is the text that talks about the 
> inactive state and vice-versa. I think this is rather confusing, but I 
> guess there's not really much we can do about it.
> 
> 
>>>> Would you prefer Matthew solution (two different signals?)
>>>
>>> I'm not sure whether it's worth adding two different signals. But
>>> I'd prefer Matthew's solution over a silentSetActive() method
>>> because it let's the listener decide whether he is interested in
>>> changes triggered by the user only or in all changes.
>>
>> OK. I just gave it a try with Konqueror and it works fine. Still it
>> felt a bit odd because at connect() time you take the decision of
>> whether all calls to setActive() will trigger your slot, instead of
>> taking this decision at the time you actually want to change the
>> action active property.
> 
> As long as you are the only listener you might have the choice to make 
> the decision at the time you want to change the action's active 
> property. But as soon as there is a second listener you have no way to 
> know whether this second listener must be notified about the change of 
> the action's active property. With two different signals the second 
> listener has the choice. With just one signal and a "silent" setter you 
> take the choice away from the second listener.
> 
> This is very similar to model/view in the case of multiple views for one 
> model. Only the views know which notifications they want to receive from 
> the model. Therefore none of the views must control which notifications 
> the model sends out.

To me this action can also be considered a view: it represents the state
of an element in a model. When the view which contains this action sets
itself up, it has to adjust its components according to the state of the
model. This is the situation where I see the view setting the action
active property without emitting signals.

Having said that, I see how your vision is valid as well. Since using
different signals is more common than "silentSet*" methods I am going to
propose the class with different signals for review.

Aurélien
[prev in list] [next in list] [prev in thread] [next in thread]