Differential D6324

Make sure we won't modify the aspect ratio of the icons
ClosedPublic
Actions

Authored by apol on Jun 21 2017, 7:29 PM.

Details

Reviewers
leinir
mart
Group Reviewers
Kirigami
Commits
R169:77c2f3301df1: Make sure we won't modify the aspect ratio of the icons
Summary

BUG: 381157

Diff Detail

Repository
R169 Kirigami
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
apol created this revision.Jun 21 2017, 7:29 PM
Restricted Application added a project: Kirigami. · View Herald TranscriptJun 21 2017, 7:29 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
mart accepted this revision.Jun 22 2017, 8:44 AM
This revision is now accepted and ready to land.Jun 22 2017, 8:44 AM
leinir accepted this revision.Jun 22 2017, 10:33 AM

Looks good for the initial case of getting rid of the obviously broken looking aspect ratio at the very least, though i think exposing the property might still be good... But, at the very least, it matches the HIG so that's cool :) (i don't personally like the whole scale-and-crop thing, but i'm also not the designer and i know entirely sensible reasons exist for this choice)

apol closed this revision.Jun 22 2017, 10:36 AM
davidedmundson added a subscriber: davidedmundson.Jun 22 2017, 12:53 PM

Cropping seems an odd choice.

In Plasma/KIconLoader we do PreserveAspectFit / KeepAspectRatio.

apol added a comment.Jun 22 2017, 1:10 PM
In D6324#118669, @davidedmundson wrote:

Cropping seems an odd choice.

In Plasma/KIconLoader we do PreserveAspectFit / KeepAspectRatio.

My thinking was that this could potentially mess the icons' margins.

Revision Contents
Changeset List

PathPackages
Msrc/controls/Icon.qml (1 line)
Msrc/desktopicon.cpp (2 lines)

Diff 15708

View Options

src/controls/Icon.qml

Loading...
View Options

src/desktopicon.cpp

Loading...