search

shimmerproject / Greybird

Desktop Suite for Xfce
https://shimmerproject.org
Other
422 stars 78 forks source link

Stop forcing icon style for sidebars #337

Closed newhoa closed 1 year ago

newhoa commented 1 year ago

The sidebar property -gtk-icon-style: regular was forcing regular icons even if apps requested symbolic icons and the icon theme provided them.

Removing it now allows Thunar to use a symbolic eject icon. It also allows Nemo and Rhythmbox to use symbolic icons. It improves the look and readability when using the dark theme.

For reference, this line was previously added in 0cd4db5

andreldm commented 1 year ago

Hi @newhoa is there a bug reported for this PR? Here with Papipus-Dark I don't notice any difference with or without this fix in Thunar's eject icon. Screenshots?

newhoa commented 1 year ago

Hi @newhoa is there a bug reported for this PR? Here with Papipus-Dark I don't notice any difference with or without this fix in Thunar's eject icon. Screenshots?

Hey @andreldm, thanks for checking on this!

Papirus-Dark won't show any huge difference in Thunar because they use the same icon for regular as they do symbolic. The only way you'll see it with Papirus-Dark + Greybird-Dark in Thunar is if you have the window unfocused, below another window (you will see the font color change, but not the eject icon color. If it were using the symbolic icon it would dim the same as the text). But any icon theme that uses different regular and symbolic icons will be easier to see with and show a much bigger difference.

papirus-dark

elementary-xfce uses a different icon for regular and for symbolic.

exfce-dark

So these next two are an example in Thunar using Greybird-dark gtk theme + elementary-xfce icon theme. (Also, thunar requires a hard restart to see the gtk-icon-style removal changes proposed here)

Current: thunar-eject-current

Proposed: thunar-eject-prop

It's a lot more clear in Rhythmbox. Here is what Rhytmbox looks like in Adwaita and Numix using Papirus-Dark:

current-adwaita

current-numix

Greybird with Papirus-Dark icons. Current (it forces use of Papirus-Dark's regular icons):

current-greybird

It should look like Adwaita and Numix and use Papirus-Dark's symbolic icons, but Greybird's gtk-icon-style forces it not to. Here is Greybird with Papirus-Dark icons with the proposed change:

prop-greybird

Anyway, thanks again for checking this out! And hope the explanation makes sense!

Edit: And oh, no, there is no bug report. Just something I noticed when working on icon themes. I thought for a long time it was something with the icon theme but figured out it was the Gtk theme so just went ahead and submitted. If I should make bug reports along with patch submissions let me know and I can do that. Thanks!

andreldm commented 1 year ago

Thanks for the thorough explanation, I didn't notice any regression, thanks for the fix.