Simple but powerful KCM utilizing LibColorCorrect from Plasma Workspace to provide configuration of Night Color in KWin.
Video demonstration: https://www.youtube.com/watch?v=s5ltZFhEIVE
Details
Details
- Reviewers
davidedmundson - Group Reviewers
Plasma - Commits
- R119:bd9207466a55: Add KCM for controlling KWin's Night Color module
Diff Detail
Diff Detail
- Repository
- R119 Plasma Desktop
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Comment Actions
looks mostly good.
Make sure you test with --reverse, I think your activator checkbox will be broken
| kcms/nightcolor/kcm_nightcolor.desktop | ||
|---|---|---|
| 17 ↗ | (On Diff #14747) | FYI, adding translations here is a waste of time Scripty will ignore this when it extracts the text; then they'll be replaced when it puts the translations back |
| kcms/nightcolor/package/contents/ui/LocationsAutoView.qml | ||
| 21 ↗ | (On Diff #14747) | you shouldn't be using Plasma Components in KCMs. Use QQC directly. Plasma Components follow the plasma theme which can lead to a white text on white situation. |
| kcms/nightcolor/package/contents/ui/main.qml | ||
| 31 ↗ | (On Diff #14747) | We have an actual implicitWidth from the layout itself, we don't need to guess |
| 341–342 ↗ | (On Diff #14747) | just fill the parent. It's already set to main.width*0.8 |
Comment Actions
small comment of the layout: align the labels (sunset begins etc) to the right instead of the left
Comment Actions
Diff update:
- Fixed inline comments
- Fixed @mart's comment
- Labels aligned to the right
- Slight renaming of labels ("Sunrise/set ends" -> "and ends")
Make sure you test with --reverse, I think your activator checkbox will be broken
You're right, but I'm just using the text attribute of QQC's CheckBox, so I'm not sure if I should write a workaround here or just wait for upstream to fix it. The visual fallout is minimal and the checkbox still usable:
| kcms/nightcolor/kcm_nightcolor.desktop | ||
|---|---|---|
| 17 ↗ | (On Diff #14747) | Ok, will omit it next time. Thanks! |
Comment Actions
Make sure you test with --reverse, I think your activator checkbox will be broken
You're right, but I'm just using the text attribute of QQC's CheckBox, so I'm not sure if I should write a workaround here or just wait for upstream to fix it. The visual fallout is minimal and the checkbox still usable:
Heh, didn't expect that.
What I meant was that X would still be 0 rather than the item being right aligned.
With RTL anchors and layouts get flipped. Not specifying anything (so an implicit x==0) it doens't get flipped.
Though given the bug you showed maybe it's all a bit moot.
| kcms/nightcolor/CMakeLists.txt | ||
|---|---|---|
| 2 | The kcm loads a translation catalog, but where is the Messages.sh extracting the strings into the catalog "kcm_nightcolor"? | |
| kcms/nightcolor/CMakeLists.txt | ||
|---|---|---|
| 2 | Ah, that's what these mysterious Messages.sh files everywhere are for. ;) Is the Messages.sh right this way? #! /usr/bin/env bash $XGETTEXT `find . -name "*.qml"` -o $podir/kcm_nightcolor.pot | |
| kcms/nightcolor/CMakeLists.txt | ||
|---|---|---|
| 2 | You need to add "*.cpp", see my comment in kcm.cpp | |
| kcms/nightcolor/kcm.cpp | ||
| 32 | i18n () calls missing, see https://websvn.kde.org/trunk/l10n-kf5/sr/messages/kde-workspace/kcm_touchpad.po?revision=1485125&view=markup line 75 | |
| 34 | Using the button Help does not make sense without a docbook and whithout an entry X-DocPath in the desktop file | |
| kcms/nightcolor/package/contents/ui/LocationsAutoView.qml | ||
| 28 | in CMakelists.txt you set the domain already to "kcm_nightcolor", therefore the i18nd call should be replaced by i18n() calls | |
Comment Actions
Is there a "daytime" setting or is it hardcoded to be 100% blue? Redshift Control for example:
Comment Actions
Burkhard's remarks:
- Messages.sh
- Translations in cpp file
- i18n instead of i18nd in qml
- Deactivate Help button
Comment Actions
Correcting color values in general is part of the later to introduce Color Correction / Color Balance configuration, which is then done per screen. The Night Color value will then be only ever more a factor multiplied to it (the slider scale then probably has to change from an absolute temperature one to a relative one).
| CMakeLists.txt | ||
|---|---|---|
| 60 ↗ | (On Diff #14747) | Why is it required? Can't this KCM be skipped if the lib isn't available? Also, this is missing a set_package_properties call so that one can find out what the lib is about and where to get it from, when hitting the raw cmake error about LibColorCorrect not being available. Could not find a package configuration file provided by "LibColorCorrect" [...] |
| kcms/nightcolor/kcm_nightcolor.desktop | ||
| 19 ↗ | (On Diff #14747) | Typo: temperatur -> temperature. |
| CMakeLists.txt | ||
|---|---|---|
| 60 ↗ | (On Diff #14747) |
This is just another library of Plasma. It's quite normal that we hard depend on other libraries of the same Plasma release. |
Comment Actions
Ah, the un-namespaced name made me think it was an external dependency.
OK, sorry for the noise, I must have updated plasma-desktop without updating plasma-workspace, it works now after a run of kdesrc-build.