Remove border from format-border-set-* icons
ClosedPublic

Authored by davidhurka on Apr 4 2020, 2:51 PM.

Details

Summary

These icons had a kind of border in ViewBackground color, which made them look a bit strange. I don’t know whether that was intended.
But with swap_colors.sed (T12855), the generated dark version didn’t match the original dark version.
So I removed the ViewBackground elements.

  • set-bottom and set-internal didn’t have this border, but they had elements outside the canvas (actually the border), which are removed now.
  • set-internal also had one element outside the canvas.
  • set-internal-vertical had the vertical edge outside the canvas. I fixed that.
  • set-none had all outer edges, so it was identical to set-all. I removed the edges, because it should probably have “none” edges.
  • set-all does not need fixing.

This patch includes the auto-generated dark versions.

Screenshots:
Before: Light and dark versions have the ViewBackground border.
16px:


In the autogenerated dark version, the border looks different.
16px:

After: No border in light version and autogenerated dark version.
16px:



22px:

Template file. Cyan is a proxy color for ColorScheme-Text with transparency 0.5.
16px:


22px:

Diff Detail

Repository
R266 Breeze Icons
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidhurka created this revision.Apr 4 2020, 2:51 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 4 2020, 2:51 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
davidhurka requested review of this revision.Apr 4 2020, 2:51 PM
davidhurka edited the summary of this revision. (Show Details)Apr 4 2020, 2:56 PM

There are 22px versions of these icons. They passed the auto-generation, because they use a different approach. They have a border, but it is not in ViewBackground, but in Text with transparency 0.1. Should I add such a border to the 16px version, or remove it in the 22px version?

ndavis accepted this revision.Apr 4 2020, 11:57 PM
This revision is now accepted and ready to land.Apr 4 2020, 11:57 PM

Remove the border. I don't think it makes any sense to use different styled borders to show a lack of borders in certain places.

davidhurka updated this revision to Diff 79386.Apr 5 2020, 10:43 AM
  • Remove border elements in 22px versions of format-border-*

I would like if you accept this new patch version, to be formally correct.

davidhurka edited the summary of this revision. (Show Details)Apr 5 2020, 10:45 AM
ndavis accepted this revision.Apr 5 2020, 11:02 AM

LGTM

This revision was automatically updated to reflect the committed changes.