Add icons with added background to system.svgz
Needs RevisionPublic

Authored by pstefan on Oct 7 2018, 10:02 AM.

Details

Summary

Added duplicates of system.svgz' icons' with added background.
The background color is hardcoded atm. Update: I set the background color to 0.9 #232627 and edited the color scheme of the files. This way a future update of the lockscreen can overwrite the background color now, while the file now displays the correct, dark background.

Picture of how the icons look in various situations:

FEATURE: 398741
FEATURE: 398742
FIXED-IN: 5.52

Diff Detail

Repository
R242 Plasma Framework (Library)
Branch
arcpatch-D15999
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 5601
Build 5619: arc lint + arc unit
pstefan created this revision.Oct 7 2018, 10:02 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptOct 7 2018, 10:02 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
pstefan requested review of this revision.Oct 7 2018, 10:02 AM
pstefan edited the summary of this revision. (Show Details)Oct 7 2018, 10:07 AM
pstefan added a reviewer: VDG.
pstefan edited the summary of this revision. (Show Details)
pstefan updated this revision to Diff 43051.Oct 7 2018, 3:45 PM

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

filipf added a subscriber: filipf.EditedOct 7 2018, 8:53 PM

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.

RE:

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.

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?

This comment was removed by pstefan.
ngraham added a subscriber: broulik.EditedOct 9 2018, 3:18 PM

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 nice appearance (IMHO):


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 darker (maybe not quite as dark as above), and then make the color able to change so a Breeze Light version is possible with a light background and dark text. Then we shouldn't need a shadow or blur to ensure adequate contrast.

Thoughts?

filipf added a comment.EditedOct 9 2018, 9:36 PM

So what I suggested and tried to sketch in D16031#339968 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:

EDIT: But it is a bit better if we blur behind the circles (disregard the blur elsewhere)

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
ngraham requested changes to this revision.Oct 19 2018, 6:59 PM
This revision now requires changes to proceed.Oct 19 2018, 6:59 PM

Any update on this, @pstefan?

pstefan updated this revision to Diff 44862.Nov 4 2018, 6:27 PM
  • 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
pstefan edited the summary of this revision. (Show Details)Nov 4 2018, 6:35 PM

Hmm, I just deployed with the latest change and suddenly the icons are weird:

pstefan updated this revision to Diff 44889.Nov 5 2018, 10:26 AM
  • Added missing class attributes
pstefan updated this revision to Diff 44890.Nov 5 2018, 10:28 AM
  • Fixed z order

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.

pstefan updated this revision to Diff 45137.Nov 8 2018, 9:45 PM
  • Hardcoded background color

Ahh, much better! Reviewing...

ngraham added a subscriber: ndavis.

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:

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:

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?

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

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.

ndavis requested changes to this revision.EditedNov 30 2018, 12:28 AM

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.

This revision now requires changes to proceed.Nov 30 2018, 12:28 AM

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.

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

filipf added a comment.EditedNov 30 2018, 2:05 AM

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.

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:

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.

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.

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.

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:

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

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.

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:

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.

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!

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.

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.

Obviously it's a bit better when there's strokes:

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.

pstefan updated this revision to Diff 46583.Nov 30 2018, 6:55 PM
  • 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

With colorschemes applied, these will be your colors:

            | Ring    | Background | Symbol  |
            | ------- | ---------- | ------- |
     Breeze | #fcfcfc | #eff0f1    | #232627 |
Breeze Dark | #232629 | #31363b    | #eff0f1 |

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.

ndavis added a comment.Dec 1 2018, 5:40 AM

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?

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.

ndavis added a comment.Dec 1 2018, 6:38 AM

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:

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:

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.

pstefan updated this revision to Diff 46626.Dec 1 2018, 4:15 PM
  • Fix stylesheet in go.svgz
ndavis added a comment.Dec 1 2018, 8:25 PM

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

pstefan updated this revision to Diff 46655.Dec 1 2018, 9:11 PM
  • Fix transparency of home icon in go.svgz
ndavis accepted this revision as: ndavis.Dec 1 2018, 9:24 PM

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

ngraham accepted this revision.Dec 8 2018, 3:34 PM

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

This revision is now accepted and ready to land.Dec 8 2018, 3:34 PM
ngraham requested changes to this revision.Jan 16 2019, 11:26 PM
ngraham added a subscriber: rooty.

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!

This revision now requires changes to proceed.Jan 16 2019, 11:26 PM

They only work for me when using a dark theme:

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

+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? 😛