Review Request: Add NOTIFYs to Q_PROPERTYs for plasma widgets

[Alan Alpert]

> On 2009-06-15 00:13:10, Aaron Seigo wrote:
> > let's here it for the undocumented features! :) pls commit

Committed. And I'm sure you'll be even more glad to hear that NOTIFY is documented in 4.6.


- Alan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/844/#review1320
-----------------------------------------------------------


On 2009-06-15 00:05:29, Alan Alpert wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/844/
> -----------------------------------------------------------
> 
> (Updated 2009-06-15 00:05:29)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> The current Plasma Widgets use properties, but don't set a NOTIFY function when appropriate. Setting a NOTIFY signal in the property declaration is extremely useful when working with frameworks that work primarily on properties (like QML). I see this as a bug with the properties that they have signals when they change that aren't communicated to the meta-object system.
> 
> This patch adds a smattering of NOTIFY signals to properties of Plasma widgets. For the most part the added NOTIFY signals were where I thought one would be needed and where the signal already existed, it just wasn't in the property declaration. The single exception is in the ComboBox widget, where I added the signal and connected it. That one isn't too important (although probably useful) and so I'm fine with removing it from the patch - then it just adds NOTIFY signals to properties that already have the signals, they just aren't communicated to the meta-object system.
> 
> I'm assuming that the only reason there weren't NOTIFY signals on those properties is that no-one thought of it yet, and that it is not a deliberate choice. This assumption is based off of the fact that I can't find the NOTIFY signal for Q_PROPERTY in the 4.5 property documentation even though it is in 4.5. So I would particularly appreciate this patch being reviewed by someone who uses 4.5 (or the oldest version of Qt against which KDE is expected to build against at the moment).
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/plasma/widgets/checkbox.h 982121 
>   trunk/KDE/kdelibs/plasma/widgets/combobox.h 982121 
>   trunk/KDE/kdelibs/plasma/widgets/combobox.cpp 982121 
>   trunk/KDE/kdelibs/plasma/widgets/lineedit.h 982121 
>   trunk/KDE/kdelibs/plasma/widgets/radiobutton.h 982121 
>   trunk/KDE/kdelibs/plasma/widgets/scrollbar.h 982121 
>   trunk/KDE/kdelibs/plasma/widgets/slider.h 982121 
>   trunk/KDE/kdelibs/plasma/widgets/spinbox.h 982121 
>   trunk/KDE/kdelibs/plasma/widgets/textbrowser.h 982121 
>   trunk/KDE/kdelibs/plasma/widgets/textedit.h 982121 
> 
> Diff: http://reviewboard.kde.org/r/844/diff
> 
> 
> Testing
> -------
> 
> KDE svn r982121 compiles fine with the patch, against Qt 4.6 (declarative repository). With the patch applied the new Plasma Widgets example in playground/base/plasma/kinetic-declarative works properly.
> 
> A trivial Qt 4.4 application compiles fine with a NOTIFY signal on one of it's properties, but that's the extent of my testing for Qt versions older than 4.6.
> 
> 
> Thanks,
> 
> Alan
> 
>