Review Request 113685: New KColorSchemeManager to support changing color scheme in app

[Martin Gräßlin]

> On Nov. 10, 2013, 10:54 a.m., David Faure wrote:
> > tier3/kconfigwidgets/src/kcolorscheme.h, line 596
> > <http://git.reviewboard.kde.org/r/113685/diff/1/?file=210267#file210267line596>
> >
> >     Wouldn't "Oxygen" be translated, normally?
> >     
> >     If so, isn't this bad API, giving a translated name and hoping to find it in a list?
> >     
> >     Or is this the non-translated name used as a key, and the widget displays translated names? That's how it should be, but "const QString &text" makes me think it's not how it is :)

I just checked the .colors files in kde-workspace. They are not translated at all. The respective area looks like this:

[General]
Name=Steel

I'm not sure whether this is intended or not - there's no Messages.sh for that directory.

So right now it operates on key and that's how I intended the API to be. In case we decide to translate the color scheme names, I would suggest to change the internal logic to use a KDesktopFile, get the translated name from the model and operate on the actual key for comparison.


> On Nov. 10, 2013, 10:54 a.m., David Faure wrote:
> > tier3/kconfigwidgets/src/kcolorschememanager_p.h, line 52
> > <http://git.reviewboard.kde.org/r/113685/diff/1/?file=210269#file210269line52>
> >
> >     I'm curious, why not just by value?

personal taste :-) I prefer to not create QObjects by value and am used to wrap the pointer in a scoped pointer if I don't have a QObject parent (granted one could just pass the q-pointer to it).


- Martin


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


On Nov. 6, 2013, 4:22 p.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113685/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2013, 4:22 p.m.)
> 
> 
> Review request for KDE Frameworks, Gilles Caulier and Boudewijn Rempt.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> This class is inspired by functionality offered by e.g. Krita and
> Digikam to allow the user to select a different color scheme for the
> application.
>     
> This manager simplifies this task and also ensures that the required
> property on QApplication is set, so that a QStyle can pass the scheme
> to the window manager/compositor for the windows of the application.
> 
> @boud and @cgilles: please have a look whether this approach is sufficient for your usecases in digkam and Krita.
> 
> 
> Diffs
> -----
> 
>   tier3/kconfigwidgets/src/CMakeLists.txt 36ffca8 
>   tier3/kconfigwidgets/src/kcolorscheme.h 43da913 
>   tier3/kconfigwidgets/src/kcolorscheme.cpp 62326a6 
>   tier3/kconfigwidgets/src/kcolorschememanager_p.h PRE-CREATION 
>   tier3/kconfigwidgets/src/kcolorschememanager_p.cpp PRE-CREATION 
>   tier3/kconfigwidgets/tests/CMakeLists.txt f66dc32 
>   tier3/kconfigwidgets/tests/kcolorschemedemo.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/113685/diff/
> 
> 
> Testing
> -------
> 
> see demo application
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20131112/28b7d0b2/attachment-0001.html>