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

List:       kde-core-devel
Subject:    Re: A KSelectAction dedicated to QTextCodec selection ?
From:       Hamish Rodda <rodda () kde ! org>
Date:       2006-10-21 4:38:39
Message-ID: 200610211438.46288.rodda () kde ! org
[Download RAW message or body]


On Saturday 21 October 2006 05:36, Michel Hermier wrote:
> Hi,
>
> 2006/10/20, David Faure <faure@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.

> > 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.

> And yes there is a real need to allow multiple selection, since
> usually these groups are quite large (see the font and QTextCodec). So
> we need an option to configure the removal some of them, so we need to
> select multiple items.

Either have a separate way to configure the action (eg. configuration dialog), 
or create a new class which does what you want.

As to the earlier patch, I think the part which checks that the new current 
action is one of the selectable actions is worthwhile.

Cheers,
Hamish.

[Attachment #3 (application/pgp-signature)]

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