Make 32px document-save icon colorful
Needs ReviewPublic

Authored by ngraham on Mon, Sep 30, 7:03 PM.

Details

Reviewers
ndavis
Group Reviewers
VDG
Maniphest Tasks
T10165: Large category icons should all be colorful
Summary

We're trying to move away from using the monochrome style for >22px action icons. This
patch replaces the 32px monochrome document-save icon with a colorful version.

BUG: 412404
FIXED-IN: 5.63

Test Plan

This icon used in various apps' settings windows now looks good:
Spectacle:


KWrite:

Diff Detail

Repository
R266 Breeze Icons
Branch
colorful-32px-save-icon (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 17319
Build 17337: arc lint + arc unit
ngraham created this revision.Mon, Sep 30, 7:03 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptMon, Sep 30, 7:03 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
ngraham requested review of this revision.Mon, Sep 30, 7:03 PM
ndavis requested changes to this revision.EditedMon, Sep 30, 8:18 PM
ndavis added a subscriber: ndavis.
  • I think this doesn't look enough like the monochrome versions and it looks too much like the KFloppy icon.
    • Make the label area more similar to the monochrome icons.
    • The top is kind of hard to see against #eff0f1, so maybe choose a darker set of grays. These are based on the current Breeze colors. You might already know, but avoid going too close to #fcfcfc, #eff0f1, #31363b and #232629 unless the colors you use are surrounded by colors that don't blend on at least 2 opposite sides.
  • Margins need to be 2px, not 4px. 4px was for monochrome 32px.
  • The corners of the hole in the metal slide don't quite reach the nearest grid snapping points.
This revision now requires changes to proceed.Mon, Sep 30, 8:18 PM
ngraham updated this revision to Diff 67263.Thu, Oct 3, 2:19 PM

Update according to review comments

ngraham edited the test plan for this revision. (Show Details)Thu, Oct 3, 2:20 PM
ngraham edited the test plan for this revision. (Show Details)Fri, Oct 4, 4:14 AM
ngraham updated this revision to Diff 67298.Fri, Oct 4, 4:14 AM

Use a nicer color for the label

ndavis added a comment.Fri, Oct 4, 7:28 AM

How about this?


GB_2 added a subscriber: GB_2.Fri, Oct 4, 7:30 AM

How about this?


I love it!

How about this?


Should the metal tab on the top cast a shadow this long? It looks strange to me.

The tab is normally perfectly flush with the non-indented region of the cover (picture).

Heh that's a ton better than mine. :)

@ndavis You wanna just submit a patch with that and I'll abandon this?

ndavis added a comment.Fri, Oct 4, 3:58 PM

@ndavis You wanna just submit a patch with that and I'll abandon this?

Alright

ndavis added a comment.Fri, Oct 4, 3:59 PM

How about this?


Should the metal tab on the top cast a shadow this long? It looks strange to me.

The tab is normally perfectly flush with the non-indented region of the cover (picture).

Yeah, that was a feature of the original icon that I didn't remove. I wasn't so sure about it either.

GB_2 added a comment.Sat, Oct 12, 6:41 PM

Yeah, that was a feature of the original icon that I didn't remove. I wasn't so sure about it either.

If you don't like it then remove it :P