[shelter-ui] Button text uses the wrong variable

[mangkoran]

I found that Button has low contrast if user use dark theme.

Example with catppuccin-mocha. Both client are in dark mode.

Shelter (in Dorion client)

Compared to Vencord (in official Discord client). This is the same as Discord's default button style

I expect Button style should be similar to Discord's default button style in dark theme. But correct me if this is intended.

[yellowsink]

This theme overrides classes specific to the Discord button, instead of overriding the relevant css variables, which would work fine in shelter.

This is not intentional behaviour as such, but we cannot realistically support every theme due to cases like this where they just do things in different ways.

This is a natural consequence of us remaking all of Discord's UI components instead of using them verbatim, which is a very necessary and non-negotiable part of our stability model, so unfortunately I can't offer to fix this case.

In fact, this theme appears to even be inconsistent in its presentation with Discord's buttons!

Sorry.

[mangkoran]

Thank you for your reply. You are correct that it seems it's due to catppuccin theme isn't using --button-filled-brand-text variable as button text color. However, it seems shelter-ui is also not using --button-filled-brand-text as button text color. Instead shelter-ui button use --interactive-active.

https://github.com/uwu/shelter/blob/061ab9bba8c9a7540a45efcc400e64bbf6f891ba/packages/shelter-ui/src/button.tsx#L28-L30

And it seems --interactive-active is also used in other component such as text in selected friend in friend list. This is how it looks if I set --interactive-active to #000000.

Compared to the theme default

May I know which one is the correct variable to use for button text color? Is it --button-filled-brand-text or --interactive-active? I could help to make changes in catppuccin repo regarding the correct variable usage.

[yellowsink]

I do believe shelter UI is wrong about which variable to use here, it should be button-filled-xxxx-text, and I do plan to change this, but this theme would still sadly be broken

[mangkoran]

but this theme would still sadly be broken

It's okay no worries I could help to discuss this in catppuccin repo. Nice to hear that it's acknowledged at least. Maybe we could reopen the ticket to track the fix for shelter-ui? 👀

[yellowsink]

that is actually a sensible idea yeah sure

[mangkoran]

Speaking of themes, is there any reference theme implementation that we could use as base/reference for making themes? So far I found BetterDiscord variables but let me know if there's better/more preferable reference.

[yellowsink]

I'm not aware of any better resource, or any resources at all really for theming other than just seeing what others do and mimicking that.

quite honestly my method back when I did theming was just to find the thing I wanted to change and see what variable it used, then see if overriding that had the desired effect without breaking everything else.