Differential D23778

Remove superfluous 32px recent documents icons
ClosedPublic
Actions

Authored by ngraham on Sep 7 2019, 8:21 PM.

Details

Reviewers
ndavis
Group Reviewers
VDG
Commits
R266:fbbc49bf219b: Remove superfluous 32px recent documents icons
Summary

The 32px versions of these icons are unnecessary (we don't generally do >22px monochrome
action icons) and have a few visual bugs. Let's remove them to reduce the icon burden
a bit.

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.
ngraham created this revision.Sep 7 2019, 8:21 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptSep 7 2019, 8:21 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
ngraham requested review of this revision.Sep 7 2019, 8:21 PM
ndavis accepted this revision.Sep 7 2019, 10:24 PM

I think we should still have 24px icons for compatibility, but since they're exactly the same as the 22px icons with wider margins, it might be better to generate 24px icons at build time.

This revision is now accepted and ready to land.Sep 7 2019, 10:24 PM
ndavis requested changes to this revision.Sep 7 2019, 11:35 PM

Keep the 24px icon for now. It's in Adwaita at 24px too, so there might be some app out there that uses 24px icons. When the repo is setup to generate 24px icons at build time, then we can remove all of the 24px icons. If you need to update it to match the 22px version, just copy the 22px icon, open it in a text editor, change the viewbox or document height/width to 24x24, then group the shapes:

<g transform="translate(1 1)">
    shapes go here
</g>
This revision now requires changes to proceed.Sep 7 2019, 11:35 PM
ngraham added a comment.Sep 8 2019, 10:03 AM
In D23778#527411, @ndavis wrote:

I think we should still have 24px icons for compatibility, but since they're exactly the same as the 22px icons with wider margins, it might be better to generate 24px icons at build time.

+1, makes total sense. We can delete all the 22px icons once the time comes that we can auto-generate them. For now, are you okay with just not touching the existing 24px version? Seems unnecessary to do work that we know we're going to throw away.

ndavis added a comment.Sep 8 2019, 12:12 PM

For now

In D23778#527622, @ngraham wrote:
In D23778#527411, @ndavis wrote:

I think we should still have 24px icons for compatibility, but since they're exactly the same as the 22px icons with wider margins, it might be better to generate 24px icons at build time.

+1, makes total sense. We can delete all the 22px icons once the time comes that we can auto-generate them. For now, are you okay with just not touching the existing 24px version? Seems unnecessary to do work that we know we're going to throw away.

I guess it's fine. It's not much work if you do what I said above, but it's probably very rare that you will see it used in the wild.

ngraham updated this revision to Diff 65708.Sep 9 2019, 10:18 PM

Restore 24px versions (they will be deleted later)

ngraham retitled this revision from Remove superfluous >22px recent documents icons to Remove superfluous 32px recent documents icons.Sep 9 2019, 10:18 PM
ngraham edited the summary of this revision. (Show Details)
ndavis accepted this revision.Sep 10 2019, 12:14 AM
This revision is now accepted and ready to land.Sep 10 2019, 12:14 AM
Closed by commit R266:fbbc49bf219b: Remove superfluous 32px recent documents icons (authored by ngraham). · Explain WhySep 10 2019, 2:12 AM
This revision was automatically updated to reflect the committed changes.

Showing Only Differences

This revision modifies 2 more files that are hidden because they were not modified between selected diffs and they have no inline comments.

Revision Contents
Changeset List

Path
No data available.

Diff 65717