A KSelectAction dedicated to QTextCodec selection ?

[Michel Hermier]

Hi,

Just a little change to make addAction return KAction instead of
QAction (and use KIcon as argument instead of QIcon, needed by the
KAction interface).
This simplify some internal code.
I compiled the whole KDE, and it seems that no additional changes are
needed (in the packages I build and usually don't fails for me).

Cheers,
   Michel

2006/10/21, Michel Hermier <michel.hermier at gmail.com>:
> 2006/10/21, Hamish Rodda <rodda at kde.org>:
> > On Saturday 21 October 2006 18:25, Michel Hermier wrote:
> > > 2006/10/21, Hamish Rodda <rodda at kde.org>:
> > > > On Saturday 21 October 2006 05:36, Michel Hermier wrote:
> > > > > Hi,
> > > > >
> > > > > 2006/10/20, David Faure <faure at kde.org>:
> > > > > > Hello!
> > > > > >
> > > > > > On Friday, October 20, 2006 01:19:05 PM Michel Hermier wrote:
> > > > > > > I noticed while preparing the changes locally, that I need one of
> > > > > > > my local change for KSelectAction (well only a return change, but
> > > > > > > the patch adds a little more).
> > > > > > > So I delay the commit for now.
> > > > > > >
> > > > > > > Can someone maintaining KSelectAction review the patch.
> > > > > > > What changed with this patch:
> > > > > > > - bool setCurrentAction(QAction *, DeselectionMode mode) should be
> > > > > > > more safe, checking that the action really belongs to the action
> > > > > > > group before activating the action. Also added an extra parameter
> > > > > > > with default value to mimic the old behaviour. This extra parameter
> > > > > > > allow to not deselect the previous action in case of falure to
> > > > > > > select the action.
> > > > > >
> > > > > > Why does this need to be configurable? Deselection the previous
> > > > > > action and not selecting any new one instead looks like a bug, not a
> > > > > > feature.
> > > >
> > > > No, unselecting the current action is needed in the case where we want to
> > > > clear the current selection.  For example, in KRecentFilesAction, when a
> > > > file is closed, its action is deselected so it is no longer checked.
> > >
> > > I meant, "there migth be some usage to if I can't set to the current
> > > action, let the selection as before", especially when trying to set
> > > the current action using a text research.
> >
> > So you really want a findAction() function, which you can follow up with
> > setCurrentAction() if you find something...?
> >
> > > > > > Did you make it configurable "just in case", or is there a real use
> > > > > > case for this?
> > > > >
> > > > > I think it's needed to be configurable if we allow to have multiple
> > > > > selection enabled.
> > > > > Because in this case you have to look at all the actions to search to
> > > > > know wich ones are enabled, since the API don't offer to get all the
> > > > > selected items (directly).
> > > >
> > > > KSelectAction is not designed to allow multiple selection.
> > >
> > > Are there any anoying reason, we can't make it multiple selection aware ?
> >
> > I can think of a few:
> > 1) Menus (which is the main method which KSelectActions use) are poorly
> > designed to accomodate multiple selection (you have to open the menu multiple
> > times to make the selection, and then you have selection clearing problems)
> >
> > 2) The combo box widget wouldn't work with multiple selection
> >
> > 3) It would significantly pollute the api (and it's not that clean to start)
> >
> > I really think you're better off creating a "Configure codecs..." action and
> > putting it at the end of the select action, which can bring up a dialog with
> > a proper configuration widget.  If you really want to have multiple
> > selection, perhaps creating a KMultiSelectAction class (based off KAction)
> > would be the best course of action.
>
> The problem is that we can't do this rigth now because of the code in
> addAction(QAction *)
> which transform any given action in a checkable action. There is not
> much problem to make all the other addAction method creating checkable
> action, but I wonder if it's the proper way ?
>
> But there is another problem exposed with QCodecAction and
> QFontAction. When there are too much elements it consume all the
> graphical space. The configure dialog solution is trying to solve this
> but I don't know if it's a correct solution, and if we can make
> something enougth generic for KSelectionAction.
>
> Here is the new patch, I removed the DeselectMode code and added the
> extra checks I spoke erliear.
>
> Cheers,
>     Michel
>
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kselectaction.diff
Type: application/octet-stream
Size: 6868 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20061022/1aa22262/attachment.obj>