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

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to