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

List:       kde-core-devel
Subject:    Re: [PATCH] KToggleToolbarAction, KWidgetAction
From:       John Firebaugh <jfirebaugh () kde ! org>
Date:       2002-04-17 0:44:55
[Download RAW message or body]

On Tuesday 16 April 2002 2:43, Tobias Koenig wrote:
> Ok, this is the new patch. I called the class KToggleBarAction
> (better suggestions?) and it takes a QWidget pointer as first argument.
> So you have to use
> 	new KToggleBarAction(toolBar(), "Show Toolbar", actionCollection(),
> "show_toolbar"); new KToggleBarAction(menuBar(), "Show Menubar",
> actionCollection(), "show_menubar"); new KToggleBarAction(statusBar(),
> "Show Statusbar", actionCollection(), "show_statusbar"); to create the
> actions.
>
> Is it an acceptable solution?

I'm not crazy about this patch. First, it relies on the fact that connect() is 
a macro that does not type-check its arguments. It will work if m_barWidget 
has a visibilityChanged() signal but not otherwise. Second, what is the point 
of requiring a QWidget* parameter in the constructor, but then changing the 
widget to the widget named m_barWidget->name() if it's a toolbar? Also, 
please use the same indentation style as the rest of the file.

My preference would be a pure-virtual KToggleBarAction and subclasses 
KToggleMenuBarAction, KToggleStatusBarAction, and KToggleToolBar action that 
take a KMenuBar*, KStatusBar*, and const char* respectively as constructor 
arguments.

-John
[prev in list] [next in list] [prev in thread] [next in thread]