jnsh / arc-theme

A flat theme with transparent elements (actively maintained fork)
GNU General Public License v3.0
902 stars 77 forks source link

gtk3: Thunar fix 4.16 spinner background #106

Closed SibrenVasse closed 3 years ago

SibrenVasse commented 3 years ago

In Thunar 4.16 a spinner was added to the menu bar. Using arc-theme-gtk-git this results in the wrong background. This commit seem to resolve the issue without causing trouble. I'm not really happy with the selectors, but I could not think of anything else to use.

Before

arc image arc-dark image arc-darker image arc-lighter image

After

arc image arc-dark image arc-darker image arc-lighter image

jnsh commented 3 years ago

Thank you for the PR.

I did notice this myself while testing the new thunar, but since this seems to be considered an upstream issue, I wanted to wait for a bit. If it looks like there won't be an immediate upstream solution in the next few weeks or so, I'm happy to add a workaround.

Considering this PR, I'm not perfectly happy about the selectors either, since .thunar grid > box could easily match some other widgets. Even if this worked fine with the 4.16 release, it could cause issues with older versions, or future releases.

The safest solution I could think of would be using > grid > menubar + box {}, although a mere menubar + box {} would probably suffice.

Additionally, since thunar requires GTK 3.22 or later, the workaround should be added to the 3.22 theme as well.

Also, could you mention in the commit message that this works around an upstream issue https://gitlab.xfce.org/xfce/thunar/-/issues/448, and also add a URL to this pull request for future reference.

EDIT: Please also add Fixes: https://github.com/jnsh/arc-theme/issues/107 to the commit message :)

SibrenVasse commented 3 years ago

Ah I did not see the upstream issue. Because a upstream fix (rollback) has been queued for 4.16.1 and 4.17.0, I think that doing a temporary fix in the theme is a bit unnecessary. What do you think?

https://gitlab.xfce.org/xfce/thunar/-/commit/efba7baa296afdaedfb143af156225667eba1ff5

jnsh commented 3 years ago

Yeah, I don't think the temporary fix is worth it in this situation. Until the upstream fix lands, users can work around this with CSS injection as instructed here: https://github.com/jnsh/arc-theme/issues/107#issuecomment-751399237

Closing. Thank you for looking at fixing this regardless :)