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

[Filip Fila]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:3245c6bc9b77: [Plasma Style KCM] Add a color-wheel sign 
denoting the theme supports the… (authored by filipf).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25785?vs=71542=71633

REVISION DETAIL
  https://phabricator.kde.org/D25785

AFFECTED FILES
  kcms/desktoptheme/kcm.cpp
  kcms/desktoptheme/kcm.h
  kcms/desktoptheme/package/contents/ui/ThemePreview.qml
  kcms/desktoptheme/package/contents/ui/main.qml

To: filipf, #plasma, #vdg, ngraham
Cc: ngraham, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Shipit!

REPOSITORY
  R119 Plasma Desktop

BRANCH
  little-color-sign (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D25785

To: filipf, #plasma, #vdg, ngraham
Cc: ngraham, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[Filip Fila]

filipf updated this revision to Diff 71542.
filipf added a comment.


  - Use grid delegate subtitle to state "Follows color scheme"
  - Consequently remove the icon tooltip that said the same

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25785?vs=71018=71542

BRANCH
  little-color-sign (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D25785

AFFECTED FILES
  kcms/desktoptheme/kcm.cpp
  kcms/desktoptheme/kcm.h
  kcms/desktoptheme/package/contents/ui/ThemePreview.qml
  kcms/desktoptheme/package/contents/ui/main.qml

To: filipf, #plasma, #vdg
Cc: ngraham, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[Nathaniel Graham]

ngraham added a comment.


  Never mind, I decided to submit it: D25999 


REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D25785

To: filipf, #plasma, #vdg
Cc: ngraham, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[Nathaniel Graham]

ngraham added a comment.


  How about this?
  
  F7818015: Screenshot_20191214_151602.png 

  
  Here's a KDeclarative patch to add an optional subtitle that you can use if 
you think this looks good is is a good starting point: 
  P508 KDeclarative patch in support of D25785 

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

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D25785

To: filipf, #plasma, #vdg
Cc: ngraham, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[Filip Fila]

filipf added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D25785

To: filipf, #plasma, #vdg
Cc: ngraham, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[Nathaniel Graham]

ngraham added a comment.


  In D25785#573291 , @broulik wrote:
  
  > 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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D25785

To: filipf, #plasma, #vdg
Cc: ngraham, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[Kai Uwe Broulik]

broulik added a comment.


  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]

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D25785

To: filipf, #plasma, #vdg
Cc: ngraham, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[Nathaniel Graham]

ngraham added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D25785

To: filipf, #plasma, #vdg
Cc: ngraham, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

[Filip Fila]

filipf created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
filipf requested review of this revision.

REVISION 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

REPOSITORY
  R119 Plasma Desktop

BRANCH
  little-color-sign (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D25785

AFFECTED FILES
  kcms/desktoptheme/kcm.cpp
  kcms/desktoptheme/kcm.h
  kcms/desktoptheme/package/contents/ui/ThemePreview.qml

To: filipf
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart