kactioncollection and associated widget

[Hamish Rodda]

On Wed, 21 Nov 2007 03:06:35 pm Thomas Zander wrote:
> On Wednesday 21 November 2007 01:37:58 Hamish Rodda wrote:
> > On Wed, 21 Nov 2007 02:15:32 am Thomas Zander wrote:
> > > On Tuesday 20 November 2007 14:41:45 Hamish Rodda wrote:
> > > > On Tue, 20 Nov 2007 10:57:07 pm Thomas Zander wrote:
> > > > > The whole point of the actionCollection behaving like a dump
> > > > > collection still holds, what I and I'm sure others meant with
> > > > > that is the point that adding the action should not alter the
> > > > > context automatically. The adding of the action to the parent
> > > > > widget still makes a lot of sense to me, especially since that
> > > > > doesn't change the action and thus doesn't break the concept of
> > > > > "just a collection".
> > > >
> > > > Well, I tried to fix the automatic addition when it was broken /
> > > > partially removed, and I was told that it was too "magic" and
> > > > violated the concept of a collection because it changed the actions
> > > > (changed their shortcut context to WidgetShortcut)
> > >
> > > You are mixing together two separate actions; one is the adding of
> > > the action to the widget that owns the actionCollection and the
> > > second is the automatically altering of the context.
> > >
> > > People objected to the second point, why do you see them as being the
> > > same thing? (
> >
> > Only because I originally wrote these methods to replace KAccel, which
> > provided widget-level keyboard shortcut binding.  However, I'm open to
> > just auto-add to widgets. (I'd like that, I preferred not to remove the
> > feature before, it's just that it was broken, and consensus was against
> > fixing it).
> >
> > BTW, it appears that we were discussing the same problem, it's just
> > that you implied that all kde apps already use associateWidget, and
> > that my changes to it had broken functionality with a hidden menubar,
> > when in fact this was never ported from kde3.
>
> Right,
> I've been using this class for years and I have not been paying too much
> attention to what happened in the kde4 time.
> In the kde3 time the list of widgets it was associated with also was
> there :)  I distinctly recall working with dfaure fixing a crash where
> the actioncollection had to listen to qwidget destructed signals so the
> widget can be removed from the list of associated widgets.
> I.e. that *was* in the kde3 one ;)

Ok :)

> > <snip>
> >
> > > Summarizing;
> > > * make a widget 'own' the actioncollection
> >
> > I don't see why that would be needed.
>
> Hmm, I just checked and kxmlguiclient is not a widget, so my idea to pass
> the main widget in the constructor of kactioncollection indeed can't
> work.
> Ok, never mind this part.
>
> > > * make all actions added to the actioncollection also be added to the
> > > widget.
> >
> > Yes, we can do this, as you suggested above.  We can add back
> > addAssociatedWidget/removeAssociatedWidget/clearAssociatedWidgets, and
> > change the semantics to not setting the shortcut context, apps have to
> > do that themselves.
> >
> > However, to really fix the bugs you are referring to, we still have to
> > use it on the main action collection.  This is the real issue for this
> > bug.
>
> Yeah, I was hoping that we could use the parent argument of the
> constructor as the associated widget. But thats unpractical in our
> codebase.
> I see your patch correctly adds the associated widgets for the xmlgui
> stuff, though. Well, assuming the KXMLGUIClient::beginXMLPlug is called
> automatically...

It is, by the xmlgui builder.

> > I've attached a proposed patch which implements this.
>
> You rock :)
>
> One comment on the patch; I suggest to not return a qlist reference.

Any particular reason why? In case we want to change the underlying data 
structure - but that's unlikey, no?

Cheers,
Hamish.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20071122/b55993a3/attachment.sig>