Fix missing digit and pixel-perfect alignment of depth action icons
ClosedPublic

Authored by trickyricky26 on Aug 12 2019, 8:51 PM.

Details

Summary

BUG: 406502

FIXED-IN: 5.62

This patch fixes the missing digit "1" in the depth8to16 icon and improves the alignment of the arrows with the pixel grid.

Test Plan

Before:

After:


Note the sharpness of the horizontal and vertical parts of the arrows.

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.
trickyricky26 created this revision.Aug 12 2019, 8:51 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptAug 12 2019, 8:51 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
trickyricky26 requested review of this revision.Aug 12 2019, 8:51 PM
trickyricky26 edited the summary of this revision. (Show Details)Aug 12 2019, 8:55 PM
trickyricky26 edited the test plan for this revision. (Show Details)
trickyricky26 added a reviewer: VDG.
trickyricky26 edited the summary of this revision. (Show Details)

I am not quite sure of the version for the FIXED-IN tag.

Also, this bug was reported in digikam running on Windows 10, are there any additional steps necessary to get this change from breeze-icons to the digikam Windows build?

ngraham edited the summary of this revision. (Show Details)Aug 12 2019, 11:10 PM
ngraham accepted this revision.Aug 12 2019, 11:26 PM
ngraham added subscribers: ndavis, ngraham.

FIXED-IN: refers to the next release (i.e. the one that this fix will make it into). The latest released version of KDE Frameworks is 5.61, so the next one is 5.62.

I imagine the Digikam Windows build scoops up all the relevant assets liek icons and includes them in the installed app. If not, then the breeze-icons framework actually gets installed on Windows somewhere. Either way, you don't need to do anything else.

LGTM! All good, @ndavis?

This revision is now accepted and ready to land.Aug 12 2019, 11:26 PM
ndavis requested changes to this revision.EditedAug 13 2019, 5:42 AM

Not having id="current-color-scheme" causes stylesheets to not work correctly. Other than that, +1.

icons-dark/actions/22/depth16to8.svg
1

style needs id="current-color-scheme"

icons-dark/actions/22/depth8to16.svg
1

style needs id="current-color-scheme"

icons/actions/22/depth16to8.svg
1

style needs id="current-color-scheme"

icons/actions/22/depth8to16.svg
1

style needs id="current-color-scheme"

This revision now requires changes to proceed.Aug 13 2019, 5:42 AM
  • Fix stylesheet ids
trickyricky26 marked 4 inline comments as done.Aug 13 2019, 10:03 AM
GB_2 added a subscriber: GB_2.Aug 13 2019, 8:03 PM

Maybe instead of hardcoding the blue and red colors you can use the stylesheet colors ButtonFocus and NegativeText: https://community.kde.org/Guidelines_and_HOWTOs/Icon_Workflow_Tips#Stylesheets

In D23116#511430, @GB_2 wrote:

Maybe instead of hardcoding the blue and red colors you can use the stylesheet colors ButtonFocus and NegativeText: https://community.kde.org/Guidelines_and_HOWTOs/Icon_Workflow_Tips#Stylesheets

I thought about that, too, and while the red used in the icon is exactly NegativeText, the blue in the current icon is darker and has a less vibrant hue than ButtonFocus, so I'm not quite comfortable with using that for the blue color. Do we have this darker blue as a CSS color that we can use in this way? I see this dark blue is also used in the channelmixer icon, which has its colors hardcoded as well. I also don't think it would make sense to only use stylesheet colors for only one of these two colors.

Btw, in the relevant HIG page (https://hig.kde.org/style/icon.html#action-and-status-icons) the color ColorScheme-ButtonFocus in the page you linked is called ColorScheme-Highlight and there are other anme differences, too. Is one of these inaccurate, or do they both work?

ndavis accepted this revision.Aug 13 2019, 11:08 PM

I thought about that, too, and while the red used in the icon is exactly NegativeText, the blue in the current icon is darker and has a less vibrant hue than ButtonFocus, so I'm not quite comfortable with using that for the blue color. Do we have this darker blue as a CSS color that we can use in this way? I see this dark blue is also used in the channelmixer icon, which has its colors hardcoded as well. I also don't think it would make sense to only use stylesheet colors for only one of these two colors.

It's a real shame that we can't specify dedicated icon colors in the colorscheme, otherwise that would be a nice blue to use. Or maybe it can be done, but it would be complicated. I don't think it's absolutely necessary to use ButtonFocus right now.

Btw, in the relevant HIG page (https://hig.kde.org/style/icon.html#action-and-status-icons) the color ColorScheme-ButtonFocus in the page you linked is called ColorScheme-Highlight and there are other anme differences, too. Is one of these inaccurate, or do they both work?

I need to update the HIG. We switched from Highlight to ButtonFocus because Highlight uses the Selection Background color. In the future, I'm going to change the Breeze/Breeze Dark colorschemes so that Focus Decoration color and Selection Background color are different. This prevents blue icon elements from blending in with the selection background.

This revision is now accepted and ready to land.Aug 13 2019, 11:08 PM
Closed by commit R266:c8ebfc2530dd: Fix missing digit and pixel-perfect alignment of depth action icons (authored by Rafael Brandmaier <rafael.brandmaier@kdemail.net>). · Explain WhyAug 14 2019, 6:48 PM
This revision was automatically updated to reflect the committed changes.