D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham added a comment.


  In D15999#394818 , @filipf wrote:
  
  > +1 in regards to at least getting the new switch user icon into master, 
it's great
  
  
  @pstefan, any chance we can cudgel you into doing this? 

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: rooty, ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, 
bruns

D15999: Add icons with added background to system.svgz

[Filip Fila]

filipf added a comment.


  They only work for me when using a dark theme:
  
  F6552425: image.png 
  
  F6552428: image.png 
  
  We could just do QML circles. Pro is they assure legibility with many 
non-Breeze icons, con is they might be unnecessary for circular icons. It 
however remains to be seen if there are even people using different icons.
  
  +1 in regards to at least getting the new switch user icon into master, it's 
great

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: rooty, ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, 
bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a subscriber: rooty.
ngraham added a comment.
This revision now requires changes to proceed.


  I still haven't managed to get these icons to actually work, per recent 
discussions. The background is always light instead of dark. Also the patch 
this was in support of (D16031 ) was 
abandoned. @filipf and/or @rooty, did you guys want to use these icons for your 
5.16 login screen work? Or were we going with QML circles behind the icons?
  
  Regardless, @pstefan, I'd really love the new switch user icon in another 
patch so we could at least get that in!

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: rooty, ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, 
bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

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


  Can you also bump the version numbers in the `metadata.desktop` files?

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  arcpatch-D15999

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Noah Davis]

ndavis accepted this revision as: ndavis.
ndavis added a comment.


  Looks like you've addressed all of my concerns, so I'll accept this. @ngraham 
?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan updated this revision to Diff 46655.
pstefan added a comment.


  - Fix transparency of home icon in go.svgz

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15999?vs=46626=46655

BRANCH
  arcpatch-D15999

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

AFFECTED FILES
  src/desktoptheme/breeze/icons/go.svgz
  src/desktoptheme/breeze/icons/system.svgz

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Noah Davis]

ndavis added a comment.


  Is the home button in `go.svgz` meant to be partially transparent?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan updated this revision to Diff 46626.
pstefan added a comment.


  - Fix stylesheet in go.svgz

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15999?vs=46583=46626

BRANCH
  arcpatch-D15999

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

AFFECTED FILES
  src/desktoptheme/breeze/icons/go.svgz
  src/desktoptheme/breeze/icons/system.svgz

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan added a comment.


  In D15999#369135 , @ndavis wrote:
  
  > In D15999#369134 , @pstefan 
wrote:
  >
  > > In D15999#369132 , @ndavis 
wrote:
  > >
  > > > In D15999#369131 , @pstefan 
wrote:
  > > >
  > > > > In D15999#369066 , @ndavis 
wrote:
  > > > >
  > > > > > With colorschemes applied, these will be your colors:
  > > > > >
  > > > > >   | Ring| Background | Symbol  |
  > > > > >   | --- | -- | --- |
  > > > > >Breeze | #fcfcfc | #eff0f1| #232627 |
  > > > > >   Breeze Dark | #232629 | #31363b| #eff0f1 |
  > > > > >
  > > > >
  > > > >
  > > > > Why would the ring and the background have different colors? They 
both have the same class attribute.
  > > >
  > > >
  > > > I must have had a slightly older version of the patch because I swear I 
saw the ring using `ViewBackground` and the background using `Background`. I 
downloaded the patch again and everything looks fine there. `go.svgz` still has 
`#31363b` and `#232629` as the default colors for the background and ring. Is 
this intentional?
  > >
  > >
  > > They do not have a default color for me.
  >
  >
  > Here's what I'm seeing: F6449126: Screenshot_20181201_013830.png 

  
  
  Ahh, now I get what you mean. Yes that was intentional. But only insofar as 
to check if I had set the class attributes correctly. I will change it when I 
get home.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Noah Davis]

ndavis added a comment.


  In D15999#369134 , @pstefan wrote:
  
  > In D15999#369132 , @ndavis wrote:
  >
  > > In D15999#369131 , @pstefan 
wrote:
  > >
  > > > In D15999#369066 , @ndavis 
wrote:
  > > >
  > > > > With colorschemes applied, these will be your colors:
  > > > >
  > > > >   | Ring| Background | Symbol  |
  > > > >   | --- | -- | --- |
  > > > >Breeze | #fcfcfc | #eff0f1| #232627 |
  > > > >   Breeze Dark | #232629 | #31363b| #eff0f1 |
  > > > >
  > > >
  > > >
  > > > Why would the ring and the background have different colors? They both 
have the same class attribute.
  > >
  > >
  > > I must have had a slightly older version of the patch because I swear I 
saw the ring using `ViewBackground` and the background using `Background`. I 
downloaded the patch again and everything looks fine there. `go.svgz` still has 
`#31363b` and `#232629` as the default colors for the background and ring. Is 
this intentional?
  >
  >
  > They do not have a default color for me.
  
  
  Here's what I'm seeing: F6449126: Screenshot_20181201_013830.png 


REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan added a comment.


  In D15999#369132 , @ndavis wrote:
  
  > In D15999#369131 , @pstefan 
wrote:
  >
  > > In D15999#369066 , @ndavis 
wrote:
  > >
  > > > With colorschemes applied, these will be your colors:
  > > >
  > > >   | Ring| Background | Symbol  |
  > > >   | --- | -- | --- |
  > > >Breeze | #fcfcfc | #eff0f1| #232627 |
  > > >   Breeze Dark | #232629 | #31363b| #eff0f1 |
  > > >
  > >
  > >
  > > Why would the ring and the background have different colors? They both 
have the same class attribute.
  >
  >
  > I must have had a slightly older version of the patch because I swear I saw 
the ring using `ViewBackground` and the background using `Background`. I 
downloaded the patch again and everything looks fine there. `go.svgz` still has 
`#31363b` and `#232629` as the default colors for the background and ring. Is 
this intentional?
  
  
  They do not have a default color for me.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Noah Davis]

ndavis added a comment.


  In D15999#369131 , @pstefan wrote:
  
  > In D15999#369066 , @ndavis wrote:
  >
  > > With colorschemes applied, these will be your colors:
  > >
  > >   | Ring| Background | Symbol  |
  > >   | --- | -- | --- |
  > >Breeze | #fcfcfc | #eff0f1| #232627 |
  > >   Breeze Dark | #232629 | #31363b| #eff0f1 |
  > >
  >
  >
  > Why would the ring and the background have different colors? They both have 
the same class attribute.
  
  
  I must have had a slightly older version of the patch because I swear I saw 
the ring using `ViewBackground` and the background using `Background`. I 
downloaded the patch again and everything looks fine there. `go.svgz` still has 
`#31363b` and `#232629` as the default colors for the background and ring. Is 
this intentional?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan added a comment.


  In D15999#369066 , @ndavis wrote:
  
  > With colorschemes applied, these will be your colors:
  >
  >   | Ring| Background | Symbol  |
  >   | --- | -- | --- |
  >Breeze | #fcfcfc | #eff0f1| #232627 |
  >   Breeze Dark | #232629 | #31363b| #eff0f1 |
  >
  
  
  Why would the ring and the background have different colors? They both have 
the same class attribute.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Noah Davis]

ndavis added a comment.


  With colorschemes applied, these will be your colors:
  
| Ring| Background | Symbol  |
| --- | -- | --- |
 Breeze | #fcfcfc | #eff0f1| #232627 |
Breeze Dark | #232629 | #31363b| #eff0f1 |

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan updated this revision to Diff 46583.
pstefan added a comment.


  - Remove hardcoded background color; Switch background to 
"ColorScheme-ViewBackground"
  - Reintroduce the outline border on top of the background. Opacity set to 0.3
  - Removed superfluous entries in the style properties

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15999?vs=45137=46583

BRANCH
  master

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

AFFECTED FILES
  src/desktoptheme/breeze/icons/go.svgz
  src/desktoptheme/breeze/icons/system.svgz

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Filip Fila]

filipf added a comment.


  In D15999#368500 , @ngraham wrote:
  
  > In D15999#368499 , @filipf wrote:
  >
  > > But there is a tangible problem here - imagine what the logout screen 
when using dark themes would look like if the circles were dark[;] They would 
be weird looking and would be superfluous.
  >
  >
  > That's precisely why I wanted a subtle outline around the edge of the 
background circle, which earlier versions had. With that, a dark circle on a 
dark wallpaper didn't become muddy and fade into the wallpaper; it still looked 
crisp and handsome.
  
  
  
  
  In D15999#368500 , @ngraham wrote:
  
  > In D15999#368499 , @filipf wrote:
  >
  > > But there is a tangible problem here - imagine what the logout screen 
when using dark themes would look like if the circles were dark[;] They would 
be weird looking and would be superfluous.
  >
  >
  > That's precisely why I wanted a subtle outline around the edge of the 
background circle, which earlier versions had. With that, a dark circle on a 
dark wallpaper didn't become muddy and fade into the wallpaper; it still looked 
crisp and handsome.
  
  
  I've made a **very** rough sketch (apologies for the roughness) to test 
things out. It's not that bad.
  
  F6448064: logoutnostroke.png 
  
  Obviously it's a bit better when there's strokes:
  
  F6448068: logoutwstroke.png 
  
  Though I have to admit I usually find strokes and outlines to be visually 
detrimental; in this case they also sort of overwhelm me because my brain is 
saying "there's too many lines". I also don't like how they're not of the same 
color as the icon itself, whereas if you make them white they will poke your 
eyes out.
  
  But there is a more important thing white circles would have going for them - 
how obvious it is which logout option is selected (which was also one of the 
reasons behind adding circles). A dark circle having its opacity lowered on a 
dark background is just less effective than a white circle on a dark 
background. So tl;dr = I still think white circles with dark themes are a 
better choice in the case of the logout screen.
  
  When it comes to SDDM, a lot of our problems stem from how uncustomizable it 
is... but what Phil said, if it could somehow detect the darkness of the 
wallpaper and adjust icons (maybe even other elements?) appropriately, that 
would be fantastic.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham added a comment.


  Urgh, never mind, it was my fault all along. There was some cache somewhere; 
bumping the version number of the plasma theme fixed the issue entirely. I'm 
sorry for wasting your time. :(
  
  I'm happy with this now, though now that it's all working, there are two 
nice-to-have changes that I'd like:
  
  - Restore the subtle outlines around the background circles; those helped 
with contrast against dark backgrounds, and they looked just plain good IMHO
  - Address @ndavis's suggestion and un-hard-code the colors
  
  Thanks so much for your work here, Phil!

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan added a comment.


  In D15999#368499 , @filipf wrote:
  
  > In D15999#368498 , @ngraham 
wrote:
  >
  > > In D15999#368479 , @ndavis 
wrote:
  > >
  > > > It appears that the background colors are hardcoded. If they are meant 
to be dark with the Breeze Light Plasma theme and light with Breeze Dark, then 
they should use the `ColorScheme-Text` class and the inner symbols should use 
the `ColorScheme-Background` class.F6447550: Screenshot_20181129_192418.png 

  > >
  > >
  > > The opposite: the background is supposed to be dark with breeze dark, and 
light with breeze light.
  >
  >
  > In general, with dark themes you want white monochrome icons and the circle 
is now the dominant icon element, meaning it should get painted white.
  >
  > But there is a tangible problem here - imagine what the logout screen when 
using dark themes would look like if the circles were dark:
  >
  > F6447627: image.png 
  >
  > They would be weird looking and would be superfluous.
  >
  > In the case of the SDDM theme I agree it's odd that the circles are colored 
differently than the input box, but the logout screen is more important.
  
  
  The problem here is, that it's not using a dark color-scheme. The login 
screen uses a mix of breeze light/dark items. The
  
  In D15999#368499 , @filipf wrote:
  
  > In D15999#368498 , @ngraham 
wrote:
  >
  > > In D15999#368479 , @ndavis 
wrote:
  > >
  > > > It appears that the background colors are hardcoded. If they are meant 
to be dark with the Breeze Light Plasma theme and light with Breeze Dark, then 
they should use the `ColorScheme-Text` class and the inner symbols should use 
the `ColorScheme-Background` class.F6447550: Screenshot_20181129_192418.png 

  > >
  > >
  > > The opposite: the background is supposed to be dark with breeze dark, and 
light with breeze light.
  >
  >
  > In general, with dark themes you want white monochrome icons and the circle 
is now the dominant icon element, meaning it should get painted white.
  >
  > But there is a tangible problem here - imagine what the logout screen when 
using dark themes would look like if the circles were dark:
  >
  > F6447627: image.png 
  >
  > They would be weird looking and would be superfluous.
  >
  > In the case of the SDDM theme I agree it's odd that the circles are colored 
differently than the input box, but the logout screen is more important.
  
  
  The problem is that it's not a /dark/ theme. Right now we expect the user to 
manually change the theme if they do not like the color of the icons. If we'd 
follow your suggestions the problem would remain, that the white circles would 
look out of place on light backgrounds. We have no mechanism to adjust the 
theme based on the wallpaper's lightness automagically right now.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham added a comment.


  In D15999#368499 , @filipf wrote:
  
  > But there is a tangible problem here - imagine what the logout screen when 
using dark themes would look like if the circles were dark[;] They would be 
weird looking and would be superfluous.
  
  
  That's precisely why I wanted a subtle outline around the edge of the 
background circle, which earlier versions had. With that, a dark circle on a 
dark wallpaper didn't become muddy and fade into the wallpaper; it still looked 
crisp and handsome.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Filip Fila]

filipf added a comment.


  In D15999#368498 , @ngraham wrote:
  
  > In D15999#368479 , @ndavis wrote:
  >
  > > It appears that the background colors are hardcoded. If they are meant to 
be dark with the Breeze Light Plasma theme and light with Breeze Dark, then 
they should use the `ColorScheme-Text` class and the inner symbols should use 
the `ColorScheme-Background` class.F6447550: Screenshot_20181129_192418.png 

  >
  >
  > The opposite: the background is supposed to be dark with breeze dark, and 
light with breeze light.
  
  
  In general, with dark themes you want white monochrome icons and the circle 
is now the dominant icon element, meaning it should get painted white.
  
  But there is a tangible problem here - imagine what the logout screen would 
look like if the circles were dark:
  
  F6447627: image.png 
  
  They would be weird looking and would be superfluous.
  
  In the case of the SDDM theme I agree it's odd that the circles are colored 
differently than the input box, but the logout screen is more important.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham added a comment.


  In D15999#368479 , @ndavis wrote:
  
  > It appears that the background colors are hardcoded. If they are meant to 
be dark with the Breeze Light Plasma theme and light with Breeze Dark, then 
they should use the `ColorScheme-Text` class and the inner symbols should use 
the `ColorScheme-Background` class.F6447550: Screenshot_20181129_192418.png 

  
  
  The opposite: the background is supposed to be dark with breeze dark, and 
light with breeze light.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Noah Davis]

ndavis requested changes to this revision.
ndavis added a comment.
This revision now requires changes to proceed.


  It appears that the background colors are hardcoded. If they are meant to be 
dark with the Breeze Light Plasma theme and light with Breeze Dark, then they 
should use the `ColorScheme-Text` class and the inner symbols should use the 
`ColorScheme-Background` class.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham added a comment.


  The existing icons in this style are already in the Plasma Breeze theme 
though; that was our reasoning for adding them here. In the 
medium-to-long-term, I strongly support deprecating the concept of the Plasma 
Breeze icon theme and centralizing everything in the general Breeze icon theme, 
but I think that's out of scope for this patch.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Noah Davis]

ndavis added a comment.


  Could these icons be moved into the breeze-icons repo? That would make it 
simpler to edit and review these in the future.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham added a reviewer: ndavis.
ngraham added a subscriber: ndavis.
ngraham added a comment.


  Sorry this took a while. I had to stop work on D16031 
 until other changes got accepted and 
merged in, which happened today. Back to these icons...
  
  I hate to keep torturing you here, but even with the latest revision, the 
reboot icon's symbol color is still wrong, and the background circle colors 
seems to be inverted from what they should be:
  
  F6447534: Normal colors.jpeg 
  
  The login screen sets `colorGroup: PlasmaCore.Theme.ComplementaryColorGroup` 
which has the effect of making everything use dark theme colors. If I remove 
that line, here's how it looks:
  
  F6447539: Inverted colors.jpeg 
  
  So now somehow the background circles are changing their colors, but the 
foreground symbols are not!
  
  Do these icons happen to have embedded CSS stylesheets in them that could be 
invoking the automatic color changing  feature and making this happen? Maybe 
@ndavis has an idea?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg, ndavis
Cc: ndavis, broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham added a comment.


  Ahh, much better! Reviewing...

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan updated this revision to Diff 45137.
pstefan added a comment.


  - Hardcoded background color

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15999?vs=44890=45137

BRANCH
  master

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

AFFECTED FILES
  src/desktoptheme/breeze/icons/go.svgz
  src/desktoptheme/breeze/icons/system.svgz

To: pstefan, ngraham, #vdg
Cc: broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham added a comment.


  I'm afraid that didn't change anything for me. If the experiment to make the 
background color adapt to the theme isn't working, maybe we should revert that 
change for now and just keep it hardcoded for now.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan updated this revision to Diff 44890.
pstefan added a comment.


  - Fixed z order

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15999?vs=44889=44890

BRANCH
  master

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

AFFECTED FILES
  src/desktoptheme/breeze/icons/go.svgz
  src/desktoptheme/breeze/icons/system.svgz

To: pstefan, ngraham, #vdg
Cc: broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan updated this revision to Diff 44889.
pstefan added a comment.


  - Added missing class attributes

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15999?vs=44862=44889

BRANCH
  master

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

AFFECTED FILES
  src/desktoptheme/breeze/icons/go.svgz
  src/desktoptheme/breeze/icons/system.svgz

To: pstefan, ngraham, #vdg
Cc: broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham added a comment.


  Hmm, I just deployed with the latest change and suddenly the icons are weird:
  
  F6391195: Weird icons.jpeg 

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan updated this revision to Diff 44862.
pstefan added a comment.


  - Sized down icons somewhat
  - Added "*-translucent" option of the icons in go.svgz (go-home, go-previous, 
etc). 32px only for now
  - Made background darker
  - Removed outer circle
  - Merge branch 'master' of git://anongit.kde.org/plasma-framework

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15999?vs=43051=44862

BRANCH
  master

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

AFFECTED FILES
  src/desktoptheme/breeze/icons/go.svgz
  src/desktoptheme/breeze/icons/system.svgz

To: pstefan, ngraham, #vdg
Cc: broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham added a comment.


  Any update on this, @pstefan?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham added a comment.


  So to recap recent discussions, here are the requested changes:
  
  - (Required) Create a version of the `go-previous` icon with this same style, 
and name it `go-previous-translucent`
  - (Required) Make the background circle more opaque and/or darker
  - (Optional, your call) experiment with making the symbols a bit smaller 
inside the background circle, per @filipf's suggestion

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Filip Fila]

filipf added a comment.


  So what I suggested and tried to sketch in D16031 
 was to scale the icon down a bit inside 
the circles just so it's proportionally more satisfactory to the eye. I also 
removed the faint outer border to not oversaturate the new icons with elements. 
I may be wrong with my reasoning here of course, it's just my 2 cents.
  
  @ngraham I agree with you about the fuzzy edges from the shadows. Don't have 
too strong of an opinion about the translucency, although yeah maybe not as 
opaque as in the latest screenshots.
  
  As for leaving the icon element transparent instead of the opposite color of 
the circle, scrap that suggestion because I've tested this with a theme I'm 
modifying for personal use and I don't believe this is a good idea because the 
wallpaper behind interferes with legibility:
  
  F6315927: image.png 

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham added a subscriber: broulik.
ngraham added a comment.


  This came up in D16031#339966 :
  
  @broulik suggested putting a drop shadow behind the icon, and I gave it a try 
with your new icons. The result was radically improved contrast and a very nice 
appearance (IMHO):
  
  F6315212: Drop shadows behind round icons, default background.png 

  F6315214: Drop shadows behind round icons, challenging background.png 

  
  However the fuzzy edges from the shadow aren't great. It made me wonder if we 
should simply reduce the translucency of the background circle so it's very 
dark, and then make the its color able to change so a Breeze Light version is 
possible with a light background and dark test. Then we shouldn't need a shadow 
or blur to ensure adequate contrast.
  
  Thoughts?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: broulik, filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan added a comment.


  RE:
  
  In D15999#338724 , @filipf wrote:
  
  > Would the background be grey and the icon itself black in all color 
schemes? (apropos how Inkscape shows it:)
  >
  > F6310672: image.png 
  >
  > Could we make it adaptable to the color scheme? I don't know if the code 
applies, but they managed to do it here: 
https://github.com/PapirusDevelopmentTeam/arc-kde/blob/master/plasma/desktoptheme/Arc-Color/icons/system.svg
  >
  > They do however just leave the icon itself transparent, but perhaps that 
wouldn't work well with Breeze icons.
  
  
  The background stays the same color, no matter the color scheme used. I am 
told that it's not currently possible for the login-screen to change the color 
of the background independently of the foreground. The foreground color is the 
same color as a theme's text color. In the case of the lockscreen, that's white.
  
  > Could we make it adaptable to the color scheme? I don't know if the code 
applies, but they managed to do it here: 
https://github.com/PapirusDevelopmentTeam/arc-kde/blob/master/plasma/desktoptheme/Arc-Color/icons/system.svg
  > 
  > They do however just leave the icon itself transparent, but perhaps that 
wouldn't work well with Breeze icons.
  
  I have never thought about that tbh. It's something to investigate, can you 
repost that thought on T9658 ?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan added a comment.


  RE:
  
  In D15999#338724 , @filipf wrote:
  
  > Would the background be grey and the icon itself black in all color 
schemes? (apropos how Inkscape shows it:)
  >
  > F6310672: image.png 
  >
  > Could we make it adaptable to the color scheme? I don't know if the code 
applies, but they managed to do it here: 
https://github.com/PapirusDevelopmentTeam/arc-kde/blob/master/plasma/desktoptheme/Arc-Color/icons/system.svg
  >
  > They do however just leave the icon itself transparent, but perhaps that 
wouldn't work well with Breeze icons.
  
  
  The background stays the same color, no matter the color scheme used. I am 
told that it's not currently possible for the login-screen to change the color 
of the background independently of the foreground. The foreground color is the 
same color as a theme's text color. In the case of the lockscreen, that's white.
  
  > Could we make it adaptable to the color scheme? I don't know if the code 
applies, but they managed to do it here: 
https://github.com/PapirusDevelopmentTeam/arc-kde/blob/master/plasma/desktoptheme/Arc-Color/icons/system.svg
  > 
  > They do however just leave the icon itself transparent, but perhaps that 
wouldn't work well with Breeze icons.
  
  I have never thought about that tbh. It's something to investigate, can you 
repost that thought on T9658 ?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham added a task: T9658: Rethink blur-by-default for Breeze SDDM login 
screen theme for Plasma 5.15.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham added a dependent revision: D16031: [SDDM theme] remove blur and 
increase UI contrast so it's not required.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham removed a dependent revision: D16031: [SDDM theme] remove blur and 
increase UI contrast so it's not required.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Nathaniel Graham]

ngraham added a dependent revision: D16031: [SDDM theme] remove blur and 
increase UI contrast so it's not required.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Filip Fila]

filipf added a comment.


  Would the background be grey and the icon itself black in all color schemes? 
(apropos how Inkscape shows it:)
  
  Could we make it adaptable to the color scheme? I don't know if the code 
applies, but they managed to do it here: 
https://github.com/PapirusDevelopmentTeam/arc-kde/blob/master/plasma/desktoptheme/Arc-Color/icons/system.svg
  
  They do however just leave the icon itself transparent, but perhaps that 
wouldn't work well with Breeze icons.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: filipf, kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan updated this revision to Diff 43051.
pstefan added a comment.


  Fix z-layering issue for the "-translucent" icons.

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15999?vs=43009=43051

BRANCH
  master

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

AFFECTED FILES
  src/desktoptheme/breeze/icons/system.svgz

To: pstefan, ngraham, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan edited the summary of this revision.
pstefan added a reviewer: VDG.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pstefan, ngraham, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15999: Add icons with added background to system.svgz

[Phil Stefan]

pstefan created this revision.
pstefan added a reviewer: ngraham.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
pstefan requested review of this revision.

REVISION SUMMARY
  Added duplicates of system.svgz' icons' with added background.
  The background color is hardcoded atm.
  
  For reference see T9658  and B398741 and 
B398742

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

AFFECTED FILES
  src/desktoptheme/breeze/icons/system.svgz

To: pstefan, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns