[Plasma Style KCM] Add a color-wheel sign denoting the theme supports the system color scheme
ClosedPublic

Authored by filipf on Dec 6 2019, 3:13 PM.

Details

Summary

The discoverability of the system color scheme awareness feature of some Plasma themes right now is very poor.

In addition to that the previews for Breeze and Breeze Light look identical when using the default color scheme.

To fix this we add a little color wheel icon in the top right corner of the theme preview that only shows up if the theme supports this.

Credits go to @broulik for most of the code, I just positioned the icon.

BUG: 364953

Test Plan

Depends on: D25999

Doesn't get messed up with scaling either.

Diff Detail

Repository
R119 Plasma Desktop
Branch
little-color-sign (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19853
Build 19871: arc lint + arc unit
filipf created this revision.Dec 6 2019, 3:13 PM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 6 2019, 3:13 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
filipf requested review of this revision.Dec 6 2019, 3:13 PM
filipf edited the test plan for this revision. (Show Details)Dec 6 2019, 3:13 PM
filipf added reviewers: Plasma, VDG.
ngraham added a subscriber: ngraham.Dec 6 2019, 3:53 PM

Very nice! I wonder if the symbol's meaning might be too obscure though (icons with no labels generally have poor usability; see https://www.nngroup.com/articles/icon-usability/). It's good to have a tooltip, but this won't help for the touch use case, or for the desktop.laptop users who don't think to hover the icon. Maybe a label in the KCM's footer explaining what that symbol means might help? There's room to the left of the buttons, so it wouldn't take up any additional vertical space.

That double tooltip looks a bit unpolished but not sure what to do about it. Perhaps instead sow that as tooltip for the icon theme? so it shows "Breeze (This theme follows your system color scheme)"`?
Alternatively, maybe we need a second line of label text?
so it is
Breeze
[lighter gray text] Follows system colors [/lighter gray text]

Alternatively, maybe we need a second line of label text?
so it is
Breeze
[lighter gray text] Follows system colors [/lighter gray text]

Ooh, I like that.

filipf added a comment.Dec 6 2019, 7:02 PM

Yeah, that might not be a bad idea. Our GridDelegates do not support more than 1 line of text right now if I'm not mistaken though.

ngraham added a comment.EditedDec 14 2019, 2:18 PM

How about this?

Here's a patch to KDeclarative that adds add an optional subtitle that you can use as a starting point if you think this looks good:

1diff --git a/src/qmlcontrols/kcmcontrols/qml/GridDelegate.qml b/src/qmlcontrols/kcmcontrols/qml/GridDelegate.qml
2index 668a20f..9a5eced 100644
3--- a/src/qmlcontrols/kcmcontrols/qml/GridDelegate.qml
4+++ b/src/qmlcontrols/kcmcontrols/qml/GridDelegate.qml
5@@ -39,6 +39,12 @@ T2.ItemDelegate {
6 */
7 property string toolTip
8
9+ /**
10+ * subtitle: string
11+ * optional string for the text to show below the main label
12+ */
13+ property string subtitle
14+
15 /**
16 * thumbnail: Item
17 * the item actually implementing the thumbnail: the visualization is up to the implementation
18@@ -72,7 +78,7 @@ T2.ItemDelegate {
19 }
20 width: Kirigami.Settings.isMobile ? delegate.width - Kirigami.Units.gridUnit : Math.min(delegate.GridView.view.implicitCellWidth, delegate.width - Kirigami.Units.gridUnit)
21 height: Kirigami.Settings.isMobile ? Math.round((delegate.width - Kirigami.Units.gridUnit) / 1.6)
22- : Math.min(delegate.GridView.view.implicitCellHeight - Kirigami.Units.gridUnit * 2,
23+ : Math.min(delegate.GridView.view.implicitCellHeight - Kirigami.Units.gridUnit * 3,
24 delegate.height - Kirigami.Units.gridUnit)
25 radius: Kirigami.Units.smallSpacing
26 Kirigami.Theme.inherit: false
27@@ -176,12 +182,26 @@ T2.ItemDelegate {
28 left: thumbnail.left
29 right: thumbnail.right
30 top: thumbnail.bottom
31- topMargin: Kirigami.Units.smallSpacing
32+ topMargin: caption.visible ? Kirigami.Units.smallSpacing : Kirigami.Units.largeSpacing
33 }
34 text: delegate.text
35 horizontalAlignment: Text.AlignHCenter
36 elide: Text.ElideRight
37 }
38+ Controls.Label {
39+ id: caption
40+ visible: delegate.subtitle && delegate.subtitle.length > 0
41+ opacity: 0.6
42+ anchors {
43+ left: thumbnail.left
44+ right: thumbnail.right
45+ top: label.bottom
46+ }
47+ text: delegate.subtitle
48+ font.pointSize: theme.smallestFont.pointSize
49+ horizontalAlignment: Text.AlignHCenter
50+ elide: Text.ElideRight
51+ }
52 Controls.ToolTip.delay: 1000
53 Controls.ToolTip.timeout: 5000
54 Controls.ToolTip.visible: hovered && delegate.toolTip.length > 0
55diff --git a/src/qmlcontrols/kcmcontrols/qml/private/GridViewInternal.qml b/src/qmlcontrols/kcmcontrols/qml/private/GridViewInternal.qml
56index 8c65082..b55dea3 100644
57--- a/src/qmlcontrols/kcmcontrols/qml/private/GridViewInternal.qml
58+++ b/src/qmlcontrols/kcmcontrols/qml/private/GridViewInternal.qml
59@@ -25,7 +25,7 @@ import org.kde.kirigami 2.3 as Kirigami
60 GridView {
61 id: view
62 property int implicitCellWidth: Kirigami.Units.gridUnit * 10
63- property int implicitCellHeight: Math.round(implicitCellWidth / 1.6) + Kirigami.Units.gridUnit*2
64+ property int implicitCellHeight: Math.round(implicitCellWidth / 1.6) + Kirigami.Units.gridUnit*3
65
66 onCurrentIndexChanged: positionViewAtIndex(currentIndex, GridView.Contain);
67

And then you'd just add subtitle: model.followsSystemColors ? i18n("Follows color scheme") : undef to the delegate in this patch.

Never mind, I decided to submit it: D25999

filipf updated this revision to Diff 71542.Dec 14 2019, 6:16 PM
  • Use grid delegate subtitle to state "Follows color scheme"
  • Consequently remove the icon tooltip that said the same
filipf edited the test plan for this revision. (Show Details)Dec 14 2019, 6:17 PM
ngraham accepted this revision.Dec 15 2019, 1:35 PM

Shipit!

This revision is now accepted and ready to land.Dec 15 2019, 1:35 PM
This revision was automatically updated to reflect the committed changes.