Review Request: Add NOTIFYs to Q_PROPERTYs for plasma widgets

Alan Alpert alanalpert at optusnet.com.au
Mon Jun 15 09:05:30 CEST 2009


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

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



More information about the Plasma-devel mailing list