proposal for redesign of toolbar menu feature in kdeui

[Simon Hausmann]

On Tue, Jul 02, 2002 at 10:08:24AM -0400, Nadeem Hasan wrote:
> > I attached three patches: One holding the actual diff for kdeui and
> > two for kate/konqueror (both not using ui_standards.rc, illustrating
> > the changes necessary) .
> 
> I like it :) Though I have a few comments:
> 
> 1. The m_toolBars property in ToolBarHandler is used unintialized the first 
>    time ToolBarHandler::setupActions()  is called.

(you seem to have gotten that one :)

> 2. In ToolBarHandler::setupActions(), I don't see a need to connect to the
>    destroyed() signal of KToolBar, since ToolBarHandler::setupActions() is
>    called anyway when aboutToShow() signal is emitted by QPopupMenu. Why create
>    multiple code paths to achieve the same thing?

Right, it's not technically neccessary. In fact before I went for
the aboutToShow() approach I tried to reliably detect toolbar
additions and removals, but that turned out to be impossible (for
various reasons) or would have required more changes in the
KToolBar<>KMainWindow communication that I wasn't interested in
doing, mostly because I think it's not an often used functionality
(I doubt anyone would connect to such signals, except this special
case -> unnecessary API addition) .

I left the connections to destroyed() in though because of my
paranoia about having dangling in m_toolBars :-)

> I apologze if I am completely out of line here.

You're definitely not. It's great to see people looking at patches.


Simon