A KSelectAction dedicated to QTextCodec selection ?

[Michel Hermier]

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: 3437 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20061021/20f75178/attachment.obj>