This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5237/ |
On September 3rd, 2010, 1:30 a.m., Christoph Feck wrote:
Some questions from my side: * A tool button would need to make sure that the widest of the text labels is used as the size hint, so that the tool bar does not need to re-layout on state changes. Does your code allow that? * Does it make sense to hard-code the limit to two states? What about maybe "KMultiAction", which also allows a third or fourth action if the programmer has a use case? Or allow something which uses QState? KStateAction? * Do we need the "First" and "Second" enums? I would just use an "int" for the state. The programmer probably wants his own descriptive names, such as enum State { Stopped = 0, Playing = 1 }; and I think numbers are just more readable here. Other than that, an action which has "more" state than just "On/Off" would make a useful addition to kdeui.
> A tool button would need to make sure that the widest of the text labels is used as the size hint, so that the tool bar does not need to re-layout on state changes. Does your code allow that? No. As far as I know there is no way for a QAction to influence the way it is rendered by widgets. Note that using KToggleAction + KToggleAction::setCheckedState() suffers from the same problem. > Does it make sense to hard-code the limit to two states? What about maybe "KMultiAction", which also allows a third or fourth action if the programmer has a use case? Or allow something which uses QState? KStateAction? I am a bit reluctant to go this way because I don't think it would be good for usability: having to click repeatedly on a button to reach the wanted state sounds painful, better use a combobox or a set of toggle buttons in this case. I had a discussion with a developer of Rekonq (hi Lionel!) and he made me realize KDualAction should use real QAction instead of KGuiItem, reasons behind this was the ability to have separate shortcuts for each action, and ensuring that KDualAction can notice when one of the actions is triggered so that it updates itself (example: user press F5 in Rekonq, this triggers the "reload" action, the "reload/stop" KDualAction should change to show the "stop" action). Therefore I am going to rework the class this way. This should remove the need for enums. I may discard this request and reopen a new one if it takes too much time.
- Aurélien
On September 2nd, 2010, 10:17 p.m., Aurélien Gâteau wrote:
Review request for kdelibs.
By Aurélien Gâteau.
Updated 2010-09-02 22:17:33 Description
Testing
Diffs
|