Make lock on plasmavault icon visible with breeze-dark
ClosedPublic

Authored by ndavis on Sep 24 2018, 4:55 AM.

Details

Summary

Set lock color on Breeze version to #eff0f1 (light2dark script compatibility) and the lock on the Breeze Dark version to #31363b.

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.
ndavis created this revision.Sep 24 2018, 4:55 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptSep 24 2018, 4:55 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
ndavis requested review of this revision.Sep 24 2018, 4:55 AM

Can you provide steps to reproduce the problem so I can test it? In my naive testing, the plasmavault icon looks like a lock and shows up fine on both Breeze Light and Breeze Dark. I'm also confused by the images you posted (which should be in the Test Plan{ section, BTW that depict different icons. Can you help a brotha out?

Can you provide steps to reproduce the problem so I can test it? In my naive testing, the plasmavault icon looks like a lock and shows up fine on both Breeze Light and Breeze Dark.

Here's one way to reproduce the bug that this change fixes:

  1. Switch to the Breeze Dark icon theme
  2. Use the plasmavault icon for a favorite place in Dolphin. In this case, it's the ~/Vaults folder.

Here's a close-up of the problem.

I'm also confused by the images you posted (which should be in the Test Plan{ section, BTW that depict different icons. Can you help a brotha out?

The pictures are just showing how the icon looks after the change. I suppose I should have shown the old version for comparison. I don't actually know what the test plan section is for and I don't know where I would find that information.

ngraham accepted this revision.Sep 25 2018, 4:49 PM

Ah thanks. For some reason Cuttlefish was not displaying the icon properly. :/

Can confirm the problem and that this fixes it! Will land the patch shortly.

This revision is now accepted and ready to land.Sep 25 2018, 4:49 PM
This revision was automatically updated to reflect the committed changes.

FYI, this earned a place in next week's Usability & Productivity report. :)

FYI, this earned a place in next week's Usability & Productivity report. :)

Thanks, I really like that series. It gives a real feeling of forward momentum for KDE.

Thanks, I really like that series. It gives a real feeling of forward momentum for KDE.

Yep, that's one of the big reasons why I do it. :)