Review Request 112880: Added KColorSchemeToken class.

Kevin Krammer krammer at kde.org
Sun Sep 22 14:55:18 BST 2013


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



kdeui/colors/kcolorschemetoken.h
<http://git.reviewboard.kde.org/r/112880/#comment29799>

    a property with a setter but no NOTIFYlooks wrong to me for a QML wrapper



kdeui/colors/kcolorschemetoken.h
<http://git.reviewboard.kde.org/r/112880/#comment29800>

    not "get" for property getters in Qt or KDE, just the propert name.
    see e.g. QLabel's "text" property



kdeui/colors/kcolorschemetoken.h
<http://git.reviewboard.kde.org/r/112880/#comment29802>

    very uncommmon signal name for a QML wrapper.
    the resulting QML signal handler would be "onOnBackgroundChange".
    Change signals are usually the property name followed by "Changed"



kdeui/colors/kcolorschemetoken.h
<http://git.reviewboard.kde.org/r/112880/#comment29803>

    Public class without d-pointer?



kdeui/colors/kcolorschemetoken.cpp
<http://git.reviewboard.kde.org/r/112880/#comment29804>

    is it guaranteed that the background changes when a color set is set?
    Couldn't the argument be the same as the current value or couldn't the background brush be the same?


- Kevin Krammer


On Sept. 22, 2013, 10:17 a.m., Denis Kuplyakov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112880/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2013, 10:17 a.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> -------
> 
> It is wrapper to access KColorScheme's methods from QML code.
> Also added Q_GADGET to KColorScheme to enable Q_ENUMS using, to make them accessible from QML code.
> 
> As it will be accepted, QML-clone of KgPopupItem will be posted for review to libkdegames, as it uses it to access KDE's color theme.
> 
> More info:
> * search for "KDE theme colors API for QML" thread at kdelibs and kdegames mailinglists *
> 
> NEED TO FIX:
> I can't include it like #include <KColorSchemeToken> at KReversi's code, only "kcolorschemetoken.h". Maybe I've missed something?
> 
> 
> Diffs
> -----
> 
>   kdeui/CMakeLists.txt b439e04 
>   kdeui/colors/kcolorscheme.h 17570fd 
>   kdeui/colors/kcolorscheme.cpp a6650ac 
>   kdeui/colors/kcolorschemetoken.h PRE-CREATION 
>   kdeui/colors/kcolorschemetoken.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/112880/diff/
> 
> 
> Testing
> -------
> 
> I've tested it with KReversi's deniskup/gsoc2013/newdesign branch.
> 
> 
> Thanks,
> 
> Denis Kuplyakov
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130922/93dabed8/attachment.htm>


More information about the kde-core-devel mailing list