[Yakuake] Use the svg icons instead of the png icons for the default theme
AcceptedPublic

Authored by matthieugras on Feb 22 2019, 10:15 PM.

Details

Reviewers
hein
Summary

BUG: 397590

Diff Detail

Repository
R369 Yakuake
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8712
Build 8730: arc lint + arc unit
matthieugras requested review of this revision.Feb 22 2019, 10:15 PM
matthieugras created this revision.
filipf added a subscriber: filipf.

Hi there, thanks for your patch. I've added our Yakuake dev as a reviewer.

In the meantime could you format the commit message as described here (https://community.kde.org/Infrastructure/Phabricator#Formatting_your_patch)? Since this is a visual change it would also be good to see a before and after screenshot.

matthieugras retitled this revision from BUG 397590 to [Yakuake] Use the svg icons instead of the png icons for the default theme.Feb 23 2019, 2:53 PM
matthieugras edited the summary of this revision. (Show Details)
hein added a comment.Feb 23 2019, 4:14 PM

Screenshot would indeed be nice, but it's a promising idea.

In D19237#418095, @hein wrote:

Screenshot would indeed be nice, but it's a promising idea.

Comparison screenshots with and without the patch:

cfeck added a subscriber: cfeck.Mar 12 2019, 10:20 AM

Screenshots were added, any comment?

hein accepted this revision.Mar 13 2019, 12:08 AM

Looks great!

There's a small thing needed here: We need to add Qt::Svg as an explicit dependency, otherwise displaying SVG files only works by accident. But I can do that seperately if you don't want to expand the patch.

Do you have dev access or do you need help landing this?

This revision is now accepted and ready to land.Mar 13 2019, 12:08 AM
hein added a comment.Mar 13 2019, 12:08 AM

Also, maybe we can just delete the .png files then and make the tarball smaller?

Removed *.png files in default skin and added Qt5::Svg dependency

In D19237#429986, @hein wrote:

Looks great!

There's a small thing needed here: We need to add Qt::Svg as an explicit dependency, otherwise displaying SVG files only works by accident. But I can do that seperately if you don't want to expand the patch.

Do you have dev access or do you need help landing this?

I do not have commit access

Does this also fix the blurry button icons in preferences? See https://bugsfiles.kde.org/attachment.cgi?id=120181

The line mentioned by Nate in https://bugs.kde.org/show_bug.cgi?id=407726#c2 is not present.

@matthieugras sorry this got lost. Once it's ready to land I can land it for you. While we're at it, would you like to try fixing blurry hidpi buttons too? See the suggestion at https://bugs.kde.org/show_bug.cgi?id=407726#c2