Use Control.palette
ClosedPublic

Authored by apol on May 29 2018, 1:03 AM.

Details

Reviewers
mart
Group Reviewers
Kirigami
Commits
R858:b176593771c2: Use Control.palette
Summary

Every Control instance has a palette property. Make use of it if possible.
It allows developers to override certain controls' colors from the code.

Test Plan

Ran Discover with it on Breeze and Breeze Dark. Made a button Blue

Diff Detail

Repository
R858 Qt Quick Controls 2: Desktop Style
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
apol created this revision.May 29 2018, 1:03 AM
Restricted Application added a project: Plasma. · View Herald TranscriptMay 29 2018, 1:03 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
apol requested review of this revision.May 29 2018, 1:03 AM
mart accepted this revision.May 29 2018, 10:30 AM
This revision is now accepted and ready to land.May 29 2018, 10:30 AM
mart requested changes to this revision.May 29 2018, 10:36 AM

to not break all custom kirigami color set, i think by default all controls in the desktop style should be binded to Kirigami.Theme.palette
or the colorset, complementary areas gets broken

This revision now requires changes to proceed.May 29 2018, 10:36 AM
mart added inline comments.May 29 2018, 10:37 AM
plugin/kquickstyleitem.cpp
716

it should check if that proeprty is valid and if managed to get an intelligible qpalette out of it

apol added inline comments.May 29 2018, 10:45 AM
plugin/kquickstyleitem.cpp
716

In which case will it be a wrong palette? We are reading a Control property, it's not really optional.

apol added a comment.May 29 2018, 10:48 AM
In D13183#270156, @mart wrote:

to not break all custom kirigami color set, i think by default all controls in the desktop style should be binded to Kirigami.Theme.palette
or the colorset, complementary areas gets broken

Do you want patches like this on every component?

diff --git a/org.kde.desktop/Button.qml b/org.kde.desktop/Button.qml
index 08a8f0e..2efbf9a 100644
--- a/org.kde.desktop/Button.qml
+++ b/org.kde.desktop/Button.qml
@@ -34,6 +34,8 @@ T.Button {
 
     hoverEnabled: true //Qt.styleHints.useHoverEffects TODO: how to make this work in 5.7?
 
+    palette: Kirigami.Theme.palette
+
     contentItem: Item {}
     Kirigami.MnemonicData.enabled: controlRoot.enabled && controlRoot.visible
     Kirigami.MnemonicData.controlType: Kirigami.MnemonicData.ActionElement
mart added inline comments.May 29 2018, 12:55 PM
plugin/kquickstyleitem.cpp
716

m_control comes from any qquickitem, so it's not a given that will be a qqc2 control instance

apol updated this revision to Diff 35117.May 29 2018, 1:40 PM

Only get the palette if there's such a property

mart added a comment.May 29 2018, 1:42 PM
In D13183#270171, @apol wrote:
In D13183#270156, @mart wrote:

to not break all custom kirigami color set, i think by default all controls in the desktop style should be binded to Kirigami.Theme.palette
or the colorset, complementary areas gets broken

Do you want patches like this on every component?

yeah, i think it would be nice

mart accepted this revision.May 29 2018, 1:42 PM
This revision is now accepted and ready to land.May 29 2018, 1:42 PM
apol updated this revision to Diff 35154.May 30 2018, 12:34 AM

Make sure that the right palette is defined at all times

mart accepted this revision.May 30 2018, 8:47 AM

awesome! thanks :)

This revision was automatically updated to reflect the committed changes.