Review Request: Custom combo boxes for Plasma::ComboBox

Marco Martin notmart at gmail.com
Thu Jul 23 18:04:54 CEST 2009


On Thursday 23 July 2009, Michal Dutkiewicz wrote:
> > On 2009-07-23 04:12:32, Aaron Seigo wrote:
> > > if this is needed (which it may be in this case) then it should not be
> > > done using a constructor imo but rather a setNativeWidget method. that
> > > a combobox is created and then discarded in the process is a non-issue
> > > (it's not a hot path and the execution time of that code is quite
> > > negligable).
>
> Right, I was thinking also about this solution.
> Maybe we should move all code that sets style and connects signals and
> slots to that method and call it with new widget from constructor (to avoid
> code duplication)?
+1
so the constructor will just call this
>
> I'll prepare new patch later.
>
>
> - Michal
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1094/#review1722
> -----------------------------------------------------------
>
> On 2009-07-21 15:55:33, Michal Dutkiewicz wrote:
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > http://reviewboard.kde.org/r/1094/
> > -----------------------------------------------------------
> >
> > (Updated 2009-07-21 15:55:33)
> >
> >
> > Review request for Plasma and Marco Martin.
> >
> >
> > Summary
> > -------
> >
> > This patch adds possibility to use custom combo box widgets (subclassed
> > from KComboBox) instead of plain KComboBox. It's intended for 4.3 (better
> > later than never..., didn't know that there is still no possibility to do
> > something like this and no possibility to create custom Plasma themed
> > widget). It adds new constructor to set widget without creating new like
> > in first. It will be used by Run Command applet (KHistoryComboBox,
> > waiting for theming possibility for very long time) and probably by
> > others in the future. If it will be accepted I'll probably prepare
> > similar patches to some other widgets (most common) for consistency and
> > bigger flexibility that comes without costs (as far as I know).
> >
> >
> > Diffs
> > -----
> >
> >   /trunk/KDE/kdelibs/plasma/widgets/combobox.h 1000582
> >   /trunk/KDE/kdelibs/plasma/widgets/combobox.cpp 1000582
> >
> > Diff: http://reviewboard.kde.org/r/1094/diff
> >
> >
> > Testing
> > -------
> >
> > Tested by my friend, compiles and works.
> >
> >
> > Screenshots
> > -----------
> >
> > Plasma themed KHistoryComboBox in Run Command applet
> >   http://reviewboard.kde.org/r/1094/s/151/
> >
> >
> > Thanks,
> >
> > Michal


-- 
Marco Martin


More information about the Plasma-devel mailing list