This looks redundant?

Why not (continue to?) pull this from the environment?

True, fixed. Thanks!

We're not pulling from the environment on nightly and beta now. Basically because otherwise we need to somehow mint some paddings based on the radius for stuff to be readable, which doesn't always work out great, see the screenshot on bug 1828413 etc.

Before seeing this bug I was actually going to file a bug on using --panel-border-radius (with calc()) in more places, e.g. for the menu item radius and probably some padding somewhere. Could you please elaborate on how that wouldn't work well in principle? It's not obvious to me from looking at bug 1828413.

Ah, I guess I forgot to attach a hovered screenshot, etc.

So I think the gtk3 menu border radius (which is what this environment variable exposes) might not quite be what we want to use.

There are a few common themes (like Breeze) where border-radius is minimal, and we're diverging enough from the native theme rendering (kind of intentionally) where just honoring that doesn't make a lot of sense.

Plus, gtk3 menuitems have no radius, but gtk4 menuitems (and now ours) do. Our look matches gtk4 more closely, but we can't read the gtk4 styles. gtk3 themes might assume a different kind of look.

Anyways, I'm not opposed to keep the menu-radius environment variable if you want, but our current behavior is --panel-border-radius: 8px;, and this patch is not intended to introduce behavior changes.

Ah, I guess I forgot to attach a hovered screenshot, etc.

Do you still have it? :) I find it hard to talk about this in the abstract without knowing what problems you ran into...

There are a few common themes (like Breeze) where border-radius is minimal, and we're diverging enough from the native theme rendering (kind of intentionally) where just honoring that doesn't make a lot of sense.

Same here, I understand it won't be perfect but I don't quite understand why matching the native theme more closely wouldn't be better from the user's perspective.

Plus, gtk3 menuitems have no radius, but gtk4 menuitems (and now ours) do. Our look matches gtk4 more closely, but we can't read the gtk4 styles. gtk3 themes might assume a different kind of look.

Same here, yes, we'd force a dynamic radius on menuitems, but how's that worse than forcing a fixed one? Is the number we get via gtk3 for popups totally off base compared to gtk4?

Idk, you seem to have looked into this quite a bit, and I'm okay with declaring defeat if the alternatives aren't practical. I'm curious to hear what @stransky thinks.

Do you still have it? :) I find it hard to talk about this in the abstract without knowing what problems you ran into...

Sure, I can take it on release, see

Same here, I understand it won't be perfect but I don't quite understand why matching the native theme more closely wouldn't be better from the user's perspective.

I guess my point is that our menus aren't really native and, in general, I believe the menu border radius is not a super-valuable customization to have, given the workarounds it'd require (see below). In particular I think making Breeze look good is a requirement, because it's the default KDE theme.

Same here, yes, we'd force a dynamic radius on menuitems, but how's that worse than forcing a fixed one? Is the number we get via gtk3 for popups totally off base compared to gtk4?

Yeah, that's the crux of the issue I guess. For the obvious example I know, the gtk3 Breeze theme (which is the default KDE theme) has a 2px menu radius, while gtk4 apps have a radius that's closer to our current menus. I guess we could special-case as needed for some well known themes.

(In general, it seems to me that the Linux theming stuff is moving more towards less in-depth theming, more based in the color-scheme, which is pretty much what we do with our popups, see https://github.com/flatpak/xdg-desktop-portal/pull/815 / https://stopthemingmy.app/, etc.)

Anyways, again, happy to keep the env var if you insist and you want to deal with that, but without uses for now to avoid changing behavior in this patch.

Do you still have it? :) I find it hard to talk about this in the abstract without knowing what problems you ran into...

Sure, I can take it on release, see

.

I can't see the image, you still need to "attach" it I think after having uploaded it. Phabricator UI is insane here.

I guess the idea would be that that the radius of the items scale with the radius of the popup, so that should look ok ish, but maybe too much padding?

Yeah, I was thinking the padding could also -- to some extent -- take the radius into account, i.e. grow with it.

Same here, I understand it won't be perfect but I don't quite understand why matching the native theme more closely wouldn't be better from the user's perspective.

I guess my point is that our menus aren't really native and, in general, I believe the menu border radius is not a super-valuable customization to have, given the workarounds it'd require (see below). In particular I think making Breeze look good is a requirement, because it's the default KDE theme.

Same here, yes, we'd force a dynamic radius on menuitems, but how's that worse than forcing a fixed one? Is the number we get via gtk3 for popups totally off base compared to gtk4?

Yeah, that's the crux of the issue I guess. For the obvious example I know, the gtk3 Breeze theme (which is the default KDE theme) has a 2px menu radius, while gtk4 apps have a radius that's closer to our current menus. I guess we could special-case as needed for some well known themes.

(In general, it seems to me that the Linux theming stuff is moving more towards less in-depth theming, more based in the color-scheme, which is pretty much what we do with our popups, see https://github.com/flatpak/xdg-desktop-portal/pull/815 / https://stopthemingmy.app/, etc.)

Okay, this is now strictly a widget decision. I'll defer to @stransky.

I can't see the image, you still need to "attach" it I think after having uploaded it. Phabricator UI is insane here.

Sigh, fixed.

Okay, this is now strictly a widget decision. I'll defer to @stransky.

Alright, so modulo that does it look good to you? We can always re-introduce the menu radius variable if needed in the future.