Transmission-Qt tray icon added
AbandonedPublic

Authored by ndavis on Dec 8 2019, 10:26 PM.

Details

Reviewers
ngraham
vinzenzv
Group Reviewers
VDG
Summary

Replace the default Transmission-Qt icon with a breeze version

Before:

After:

BUG: 414335

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
vinzenzv created this revision.Dec 8 2019, 10:26 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptDec 8 2019, 10:26 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
vinzenzv requested review of this revision.Dec 8 2019, 10:26 PM

Very nice! You even embedded the stylesheet. However there are a number of lines and corners that would look better aligned to grid lines or points. You don't have to always to this, but in general it's better to. Also the Breeze icon style generally has pointy corners rather than rounded ones.

ndavis requested changes to this revision.Dec 8 2019, 11:37 PM
ndavis added a subscriber: ndavis.

Hey, thanks for the patch!

  • Breeze usually uses 1px lines, but does use 2px in some instances (mainly centered lines). You can use a 2px thickness at the bottom in this case to match the look of the original logo.
  • The shapes need to be aligned to 1px grid spaces. If using strokes, use miter joins (value: 4), square caps, align to the centers of pixels and then convert to paths. I won't be too picky about diagonal lines except for 45 degree angles: https://community.kde.org/Guidelines_and_HOWTOs/Icon_Workflow_Tips#Diagonal_lines
  • The margins must to be 3px on the top and bottom, but you can fudge the horizontal margins as needed
This revision now requires changes to proceed.Dec 8 2019, 11:37 PM
ndavis added a comment.EditedDec 9 2019, 12:07 AM

The actual name of the icon is transmission, so you would have to rename the file to that and add id="transmission" to the group. Since this is a desktop theme icon, you would also have to add an invisible 22x22 rectangle to the group.

However, even after doing all that myself, I can't get this icon to work. I'm not sure why. It doesn't work with transmission-qt either.

Okay, gotcha. I played around a bit but am not too satisfied yet. I tends to look like a grave...

Best I could get yet:

vinzenzv updated this revision to Diff 71107.Dec 9 2019, 12:53 AM

The actual name of the icon is transmission, so you would have to rename the file to that and add id="transmission" to the group. Since this is a desktop theme icon, you would also have to add an invisible 22x22 rectangle to the group.

However, even after doing all that myself, I can't get this icon to work. I'm not sure why. It doesn't work with transmission-qt either.

I rebuilt the whole icon taking the SVG of Konversation. Maybe I messed up something with the initial one.

ndavis added a comment.EditedDec 9 2019, 12:54 AM

The actual name of the icon is transmission, so you would have to rename the file to that and add id="transmission" to the group. Since this is a desktop theme icon, you would also have to add an invisible 22x22 rectangle to the group.

However, even after doing all that myself, I can't get this icon to work. I'm not sure why. It doesn't work with transmission-qt either.

I rebuilt the whole icon taking the SVG of Konversation. Maybe I messed up something with the initial one.

Nope, I even completely redid it and it wouldn't work with transmission or transmission-qt. I don't think this is your fault.

broulik added a subscriber: broulik.Dec 9 2019, 9:19 AM

I chceked transmission git code and the icon name it uses is actually transmission-tray-icon, so you want to have a transmission.svg file and the "id" to be transmission-tray-icon, then it should work: https://github.com/transmission/transmission/blob/master/qt/MainWindow.cc#L302

ngraham requested changes to this revision.Dec 9 2019, 1:42 PM

I liked the original design with the trapezoid shape better TBH. The square one does indeed look quite "grave-like"👻👻 and IMO isn't identifiable as Transmission anymore.

This revision now requires changes to proceed.Dec 9 2019, 1:42 PM
ndavis added a comment.Dec 9 2019, 1:46 PM

I chceked transmission git code and the icon name it uses is actually transmission-tray-icon, so you want to have a transmission.svg file and the "id" to be transmission-tray-icon, then it should work: https://github.com/transmission/transmission/blob/master/qt/MainWindow.cc#L302

Thanks!

ndavis added a comment.Dec 9 2019, 1:56 PM

Here's one way to make a nice looking trapezoid in the Breeze style. I started by making a stroke with end points in the middle of pixels and the other settings I mentioned in my first comment. Then I converted it to a path and I added a 2px high rectangle to increase the thickness of the bottom part.

Here's one way to make a nice looking trapezoid in the Breeze style. I started by making a stroke with end points in the middle of pixels and the other settings I mentioned in my first comment. Then I converted it to a path and I added a 2px high rectangle to increase the thickness of the bottom part.

So as you already built it the icon is finished, right?

ndavis added a comment.EditedDec 9 2019, 10:26 PM

Here's one way to make a nice looking trapezoid in the Breeze style. I started by making a stroke with end points in the middle of pixels and the other settings I mentioned in my first comment. Then I converted it to a path and I added a 2px high rectangle to increase the thickness of the bottom part.

So as you already built it the icon is finished, right?

Yes. Here's the file if you want it:

We should probably add a 32px version too though. Panel tooltips use 32px icons and I know @ngraham likes to set his systray icons to 32px. 22px icons scaled to 32px usually look blurry.

I ran into another roadblock though. I tried to set the group id to transmission-tray-icon as @broulik said and it still wouldn't work. I wonder if the icon is hardcoded.

ndavis added a comment.EditedDec 9 2019, 10:29 PM

wait scratch that, it does work. I had to remove the 22-22- prefix, which is what you normally need when you have multiple icon sizes.

@vinzenzv are you interested in finishing this, or should someone else take it over?

@ndavis, since you already submitted an alternative icon, maybe you could take this over. @vinzenzv would that be okay with you?

ndavis commandeered this revision.Mar 15 2020, 5:32 PM
ndavis edited reviewers, added: vinzenzv; removed: ndavis.

Does this have to live in the plasma theme? Could it live in the Breeze icon theme?

Does this have to live in the plasma theme? Could it live in the Breeze icon theme?

yes, I think I'll move this patch to breeze-icons since there is no longer a name conflict

ndavis abandoned this revision.Mar 18 2020, 1:23 AM

This icon has been added to breeze-icons