Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Dec. 4, 2013, 7:32 p.m.) Status -- This change has been discarded. Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Oct. 21, 2013, 11:22 a.m., Kevin Ottens wrote: To get in this patch would benefit from being based on the frameworks branch and go into kdeclarative. Kevin Ottens wrote: Any chance for an update? Denis Kuplyakov wrote: Yes I will finish it, when have time. There are many pre-exams in university. Kevin Ottens wrote: Any news? we need to get in or discard all the old patches now. Kevin Ottens wrote: Last warning before getting discarded. Patches will soon not be accepted in kdelibs/frameworks in preparation of the repository split. Denis Kuplyakov wrote: Sorry, but I'm still very busy. What is deadline, and can I submit changes after repo-split? Kevin Ottens wrote: Re-submitting *after* the repository is perfectly fine. It will have to be a new review though. If you're fine with that we can just discard that one, and you can resubmit at your convenience post-split. That's good, I think you can discard this. - Denis --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review42069 --- On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Oct. 21, 2013, 11:22 a.m., Kevin Ottens wrote: To get in this patch would benefit from being based on the frameworks branch and go into kdeclarative. Kevin Ottens wrote: Any chance for an update? Denis Kuplyakov wrote: Yes I will finish it, when have time. There are many pre-exams in university. Kevin Ottens wrote: Any news? we need to get in or discard all the old patches now. Last warning before getting discarded. Patches will soon not be accepted in kdelibs/frameworks in preparation of the repository split. - Kevin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review42069 --- On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Oct. 21, 2013, 11:22 a.m., Kevin Ottens wrote: To get in this patch would benefit from being based on the frameworks branch and go into kdeclarative. Kevin Ottens wrote: Any chance for an update? Denis Kuplyakov wrote: Yes I will finish it, when have time. There are many pre-exams in university. Kevin Ottens wrote: Any news? we need to get in or discard all the old patches now. Kevin Ottens wrote: Last warning before getting discarded. Patches will soon not be accepted in kdelibs/frameworks in preparation of the repository split. Sorry, but I'm still very busy. What is deadline, and can I submit changes after repo-split? - Denis --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review42069 --- On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Oct. 21, 2013, 11:22 a.m., Kevin Ottens wrote: To get in this patch would benefit from being based on the frameworks branch and go into kdeclarative. Kevin Ottens wrote: Any chance for an update? Denis Kuplyakov wrote: Yes I will finish it, when have time. There are many pre-exams in university. Kevin Ottens wrote: Any news? we need to get in or discard all the old patches now. Kevin Ottens wrote: Last warning before getting discarded. Patches will soon not be accepted in kdelibs/frameworks in preparation of the repository split. Denis Kuplyakov wrote: Sorry, but I'm still very busy. What is deadline, and can I submit changes after repo-split? Re-submitting *after* the repository is perfectly fine. It will have to be a new review though. If you're fine with that we can just discard that one, and you can resubmit at your convenience post-split. - Kevin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review42069 --- On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Oct. 21, 2013, 11:22 a.m., Kevin Ottens wrote: To get in this patch would benefit from being based on the frameworks branch and go into kdeclarative. Kevin Ottens wrote: Any chance for an update? Denis Kuplyakov wrote: Yes I will finish it, when have time. There are many pre-exams in university. Any news? we need to get in or discard all the old patches now. - Kevin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review42069 --- On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Oct. 21, 2013, 11:22 a.m., Kevin Ottens wrote: To get in this patch would benefit from being based on the frameworks branch and go into kdeclarative. Any chance for an update? - Kevin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review42069 --- On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Oct. 21, 2013, 11:22 a.m., Kevin Ottens wrote: To get in this patch would benefit from being based on the frameworks branch and go into kdeclarative. Kevin Ottens wrote: Any chance for an update? Yes I will finish it, when have time. There are many pre-exams in university. - Denis --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review42069 --- On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review42069 --- To get in this patch would benefit from being based on the frameworks branch and go into kdeclarative. - Kevin Ottens On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Oct. 1, 2013, 2:47 p.m., Sebastian Kügler wrote: kdeui/colors/kcolorschemetoken.h, line 70 http://git.reviewboard.kde.org/r/112880/diff/6/?file=192050#file192050line70 using int here loses the type-safety. Why no use the corresponding enums? It would also make the code more readable. (Same issue for all the other methods.) Denis Kuplyakov wrote: I have tried it, but have such errors: Error: Unknown method parameter type: QPalette::ColorGroup See this: http://qt-project.org/forums/viewthread/10308/ . It seems that the int is only way to make it works correct. Sebastian Kügler wrote: Have you tried registering that enum using qmlRegisterType? Yes, I have, but 0 is always go to function. Have you read the link above, such behaviour was described there. - Denis --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review41068 --- On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Oct. 1, 2013, 2:47 p.m., Sebastian Kügler wrote: kdeui/colors/kcolorschemetoken.h, line 70 http://git.reviewboard.kde.org/r/112880/diff/6/?file=192050#file192050line70 using int here loses the type-safety. Why no use the corresponding enums? It would also make the code more readable. (Same issue for all the other methods.) Denis Kuplyakov wrote: I have tried it, but have such errors: Error: Unknown method parameter type: QPalette::ColorGroup See this: http://qt-project.org/forums/viewthread/10308/ . It seems that the int is only way to make it works correct. Sebastian Kügler wrote: Have you tried registering that enum using qmlRegisterType? Denis Kuplyakov wrote: Yes, I have, but 0 is always go to function. Have you read the link above, such behaviour was described there. Likely missing a Q_ENUMS somewhere to get that to work. You might want to roll your own enum type and map it to QPalette::* internally. - Kevin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review41068 --- On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Sept. 24, 2013, 6:55 a.m., Kevin Ottens wrote: I'm not sure we can let that in for kdelibs 4.x / kdeui... David? Any opinion? If we ignore that point for the moment, and think in terms of KF5, ATM it would go with KColorScheme in KConfigWidgets. I'm not quite sold on the Token name though. It comes from the fact that it attempts to be the QML API but provided as a C++ API, shouldn't it be in a declarative import plugin with no C++ API? That'd put less pressure on the class name. I still don't like having that in kdeui... What about kdeclarative instead? Not sure the dependencies are acceptable for it though. I'd say let's land it there first and then we'll see. - Kevin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review40635 --- On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Oct. 1, 2013, 2:47 p.m., Sebastian Kügler wrote: kdeui/colors/kcolorschemetoken.h, line 70 http://git.reviewboard.kde.org/r/112880/diff/6/?file=192050#file192050line70 using int here loses the type-safety. Why no use the corresponding enums? It would also make the code more readable. (Same issue for all the other methods.) Denis Kuplyakov wrote: I have tried it, but have such errors: Error: Unknown method parameter type: QPalette::ColorGroup See this: http://qt-project.org/forums/viewthread/10308/ . It seems that the int is only way to make it works correct. Sebastian Kügler wrote: Have you tried registering that enum using qmlRegisterType? Denis Kuplyakov wrote: Yes, I have, but 0 is always go to function. Have you read the link above, such behaviour was described there. Kevin Ottens wrote: Likely missing a Q_ENUMS somewhere to get that to work. You might want to roll your own enum type and map it to QPalette::* internally. If there are no Q_ENUMS it will not work as it is now. You can see QPalette source and it has it. - Denis --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review41068 --- On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Sept. 29, 2013, 4:24 p.m., David Faure wrote: The name token surprises me a bit. Is this a usual naming scheme for accessing C++ classes from QML? Otherwise I would think the QML code would want to just write KColorScheme. Maybe the registration could be done in a static method, rather than letting the apps do it on their own? The NEED TO FIX in the description can be removed, right? About the branch: I see that the request is correctly for 5.0 - which means less merging trouble, so it sounds good to me :) Denis Kuplyakov wrote: We can register it as KColorScheme for QML but in C++ it can't have such name as KColorScheme already exists. Token was first that came to my mind, maybe we can name it KColorSchemeQMLWrapper or smth like this? As Kevin suggested maybe it should be placed at some other place, not kdeui? I don't know all structure of kdelibs so I'm hope you will help me. Also how can I implement static registration, or what class I can investigate as example? I have removed NEED TO FIX... What is kdelibs/experimental/libkdeclarative ?? Maybe KColorSchemeToken should be put there. Does anybody know something about this? - Denis --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review41002 --- On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Oct. 1, 2013, 2:47 p.m., Sebastian Kügler wrote: kdeui/colors/kcolorschemetoken.h, line 70 http://git.reviewboard.kde.org/r/112880/diff/6/?file=192050#file192050line70 using int here loses the type-safety. Why no use the corresponding enums? It would also make the code more readable. (Same issue for all the other methods.) Denis Kuplyakov wrote: I have tried it, but have such errors: Error: Unknown method parameter type: QPalette::ColorGroup See this: http://qt-project.org/forums/viewthread/10308/ . It seems that the int is only way to make it works correct. Have you tried registering that enum using qmlRegisterType? - Sebastian --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review41068 --- On Oct. 6, 2013, 7:24 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Oct. 1, 2013, 2:47 p.m., Sebastian Kügler wrote: kdeui/colors/kcolorschemetoken.h, line 70 http://git.reviewboard.kde.org/r/112880/diff/6/?file=192050#file192050line70 using int here loses the type-safety. Why no use the corresponding enums? It would also make the code more readable. (Same issue for all the other methods.) I have tried it, but have such errors: Error: Unknown method parameter type: QPalette::ColorGroup See this: http://qt-project.org/forums/viewthread/10308/ . It seems that the int is only way to make it works correct. - Denis --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review41068 --- On Sept. 29, 2013, 4:27 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Sept. 29, 2013, 4:27 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Oct. 6, 2013, 7:24 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs (updated) - kdeui/CMakeLists.txt b439e04 includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review41068 --- I'm not a huge fan of using Q_INVOKABLE for something that actually looks more like a property. There's something to be said for keeping this object static, as otherwise it could grow a bit big, so I wouldn't regard this as a showstopper. (As we don't have really dynamic properties, we'd likely to a lot of work and keep too much things in memory.) kdeui/colors/kcolorschemetoken.h http://git.reviewboard.kde.org/r/112880/#comment30145 Maybe add example calls for this, easy to copy (and still get right). I totally see people getting it wrong. :) kdeui/colors/kcolorschemetoken.h http://git.reviewboard.kde.org/r/112880/#comment30144 using int here loses the type-safety. Why no use the corresponding enums? It would also make the code more readable. (Same issue for all the other methods.) - Sebastian Kügler On Sept. 29, 2013, 4:27 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Sept. 29, 2013, 4:27 p.m.) Review request for KDE Frameworks and kdelibs. Repository: 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 * Diffs - includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review41002 --- The name token surprises me a bit. Is this a usual naming scheme for accessing C++ classes from QML? Otherwise I would think the QML code would want to just write KColorScheme. Maybe the registration could be done in a static method, rather than letting the apps do it on their own? The NEED TO FIX in the description can be removed, right? About the branch: I see that the request is correctly for 5.0 - which means less merging trouble, so it sounds good to me :) - David Faure On Sept. 24, 2013, 4:08 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Sept. 24, 2013, 4:08 p.m.) Review request for KDE Frameworks and 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 - includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Sept. 29, 2013, 4:27 p.m.) Review request for KDE Frameworks and kdelibs. Description (updated) --- 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 * Diffs - includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Sept. 29, 2013, 4:24 p.m., David Faure wrote: The name token surprises me a bit. Is this a usual naming scheme for accessing C++ classes from QML? Otherwise I would think the QML code would want to just write KColorScheme. Maybe the registration could be done in a static method, rather than letting the apps do it on their own? The NEED TO FIX in the description can be removed, right? About the branch: I see that the request is correctly for 5.0 - which means less merging trouble, so it sounds good to me :) We can register it as KColorScheme for QML but in C++ it can't have such name as KColorScheme already exists. Token was first that came to my mind, maybe we can name it KColorSchemeQMLWrapper or smth like this? As Kevin suggested maybe it should be placed at some other place, not kdeui? I don't know all structure of kdelibs so I'm hope you will help me. Also how can I implement static registration, or what class I can investigate as example? I have removed NEED TO FIX... - Denis --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review41002 --- On Sept. 29, 2013, 4:27 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Sept. 29, 2013, 4:27 p.m.) Review request for KDE Frameworks and 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 * Diffs - includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review40635 --- I'm not sure we can let that in for kdelibs 4.x / kdeui... David? Any opinion? If we ignore that point for the moment, and think in terms of KF5, ATM it would go with KColorScheme in KConfigWidgets. I'm not quite sold on the Token name though. It comes from the fact that it attempts to be the QML API but provided as a C++ API, shouldn't it be in a declarative import plugin with no C++ API? That'd put less pressure on the class name. kdeui/colors/kcolorscheme.cpp http://git.reviewboard.kde.org/r/112880/#comment29912 We generally do that at the end of the file. kdeui/colors/kcolorschemetoken.cpp http://git.reviewboard.kde.org/r/112880/#comment29913 Use for this include. - Kevin Ottens On Sept. 23, 2013, 2:43 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Sept. 23, 2013, 2:43 p.m.) Review request for KDE Frameworks and 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 - includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Sept. 24, 2013, 4:08 p.m.) Review request for KDE Frameworks and kdelibs. Changes --- Fixed includes 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 (updated) - includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review40494 --- kdeui/colors/kcolorschemetoken.h http://git.reviewboard.kde.org/r/112880/#comment29808 trailing whitespace kdeui/colors/kcolorschemetoken.h http://git.reviewboard.kde.org/r/112880/#comment29809 needs an @since version line kdeui/colors/kcolorschemetoken.h http://git.reviewboard.kde.org/r/112880/#comment29810 while not wrong, I would put it below the main description. kdeui/colors/kcolorschemetoken.cpp http://git.reviewboard.kde.org/r/112880/#comment29811 opening block brace for function body on separate line kdeui/colors/kcolorschemetoken.cpp http://git.reviewboard.kde.org/r/112880/#comment29812 I think we prefer C++ style casts over C style casts - Kevin Krammer On Sept. 22, 2013, 4:09 p.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, 4:09 p.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/colors/kcolorschemetoken.cpp PRE-CREATION kdeui/colors/kcolorschemetoken.h PRE-CREATION kdeui/colors/kcolorscheme.cpp a6650ac kdeui/colors/kcolorscheme.h 17570fd kdeui/CMakeLists.txt b439e04 Diff: http://git.reviewboard.kde.org/r/112880/diff/ Testing --- I've tested it with KReversi's deniskup/gsoc2013/newdesign branch. Thanks, Denis Kuplyakov
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Sept. 23, 2013, 11:55 a.m.) Review request for kdelibs. Changes --- Fixed comments, C-style casts, formatting and trailing whitespace. 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 (updated) - 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review40547 --- Are you sure you've uploaded a new diff? Reviewboard shows no changes between r2 and r3. As for the Camel Case include, you have to add a forwarding header in kdelibs/include - Kevin Krammer On Sept. 23, 2013, 11:55 a.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Sept. 23, 2013, 11:55 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Sept. 23, 2013, 1:04 p.m.) Review request for kdelibs. Changes --- Same fixes as r3 with @since 5.0 and Camel Case Include 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 (updated) - includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Sept. 23, 2013, 1:04 p.m.) Review request for kdelibs. Changes --- Same fixes as r3 with @since 5.0 and Camel Case Include 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 - includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review40565 --- kdeui/colors/kcolorschemetoken.h http://git.reviewboard.kde.org/r/112880/#comment29882 just QObject, see below kdeui/colors/kcolorschemetoken.h http://git.reviewboard.kde.org/r/112880/#comment29881 This isn't a visible item, so you can just derive from QObject kdeui/colors/kcolorschemetoken.h http://git.reviewboard.kde.org/r/112880/#comment29883 just QObject *parent = 0 - Kevin Krammer On Sept. 23, 2013, 1:04 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Sept. 23, 2013, 1:04 p.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 - includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Sept. 23, 2013, 1:51 p.m.) Review request for kdelibs. Changes --- Changed QDeclarativeObject to QObject. Also deleted QT_DECLARATIVE from CMakeLists target libs. 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 (updated) - includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review40571 --- looks ok to me know. Better add the frameworks review group as well - Kevin Krammer On Sept. 23, 2013, 1:51 p.m., Denis Kuplyakov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Sept. 23, 2013, 1:51 p.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 - includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Sept. 23, 2013, 2:43 p.m.) Review request for KDE Frameworks and 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 - includes/CMakeLists.txt cdf0143 includes/KColorSchemeToken PRE-CREATION 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- 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. Changes --- I don't know why but Testing done field was empty. 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 (updated) --- I've tested it with KReversi's deniskup/gsoc2013/newdesign branch. Thanks, Denis Kuplyakov
Re: Review Request 112880: Added KColorSchemeToken class.
--- 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Sept. 22, 2013, 1:55 p.m., Kevin Krammer wrote: kdeui/colors/kcolorschemetoken.h, line 55 http://git.reviewboard.kde.org/r/112880/diff/1/?file=191110#file191110line55 a property with a setter but no NOTIFYlooks wrong to me for a QML wrapper All of the first seven properties are used only to set arguments to call apropriate KColorScheme method. It will be good to make them to be set once as KColorSchemeToken instance is created (like const in class). Is there a way to achive this? - Denis --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review40458 --- 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
Re: Review Request 112880: Added KColorSchemeToken class.
On Sept. 22, 2013, 1:55 p.m., Kevin Krammer wrote: The main question is still opened: can we make needed enums make usable within Q_INVOKABLE function? It will be much simplier in implementation for enduser. - Denis --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/#review40458 --- 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
Re: Review Request 112880: Added KColorSchemeToken class.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112880/ --- (Updated Sept. 22, 2013, 4:09 p.m.) Review request for kdelibs. Changes --- Have rewriten fully token class to use Q_INVOKABLE methods. It is better as 1) we don't need to store parameters 2) we don't need to create many KColorSchemeTokens for each color 3) it is more comfortable to use 4) we don't bother about nonsense NOTIFY, READ for parameters and nonsense NOTIFY for result. 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 (updated) - kdeui/colors/kcolorschemetoken.cpp PRE-CREATION kdeui/colors/kcolorschemetoken.h PRE-CREATION kdeui/colors/kcolorscheme.cpp a6650ac kdeui/colors/kcolorscheme.h 17570fd kdeui/CMakeLists.txt b439e04 Diff: http://git.reviewboard.kde.org/r/112880/diff/ Testing --- I've tested it with KReversi's deniskup/gsoc2013/newdesign branch. Thanks, Denis Kuplyakov