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

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

Diff Detail

Repository
R369 Yakuake
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
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

cfeck added a comment.Fri, Jun 14, 3:33 PM

I do not have commit access

Maybe you should get one, then you don't have to rely on others to commit.

This revision was automatically updated to reflect the committed changes.

Sorry this took so long @matthieugras! Since you've gotten several patches accepted now, please feel free to apply: https://techbase.kde.org/Contribute/Get_a_Contributor_Account