kactioncollection and associated widget

[Thomas Zander]

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 ;)

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

> I've attached a proposed patch which implements this.  

You rock :)

One comment on the patch; I suggest to not return a qlist reference. 
-- 
Thomas Zander
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20071121/c4d1dfa9/attachment.sig>