Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix theming by no longer hardcoding color values, and switching to new default font #17348

Open
fooness opened this issue May 18, 2021 · 1 comment
Labels
A-Themes-Custom Custom theme variables or support A-Theming T-Defect Z-Labs

Comments

@fooness
Copy link
Contributor

fooness commented May 18, 2021

Hello everyone. Unfortunately, theming is not really supported throughout the application, and that’s a pity. I still think it might be relatively easy to achieve this goal by using the already introduced — and in config.json supported — color variables throughout the application instead of hardcoded color values.

As encouraged by @t3chguy yesterday in another thread …

almost any new elements that are introduced use hardcoded color values instead of existing color variables

Please report these specifically because this should not be happening.

… I would like to sum this issue up in an issue once more, after already having tried to tackle these problems last year in #13082 and matrix-org/matrix-react-sdk#4373.

Also, the custom themes are still using the Nunito font instead of Inter, which should also be adapted. See: #15095

Besides, from a typographic perspective, I’m conviced that alongside the Inter font, Roboto Mono would be a superior choice for the monospaced texts and elements instead of Inconsolata. See: #15077 as well as #9807

From a quick check, it seems there’s 377 occurences of hardcoded color values in matrix-react-sdk/res/css/ and matrix-react-sdk/res/themes/ including legacy_light and legacy_dark, and there’s 191 without the legacy_themes.

~/git/matrix-react-sdk/res $ grep -r -i "#[A-F0-9]\+" themes/ | grep -c -v ': \$'
377

~/git/matrix-react-sdk/res $ grep -r -i "#[A-F0-9]\+" themes/light* themes/dark* | grep -c -v ': \$'
191

In the very most cases, these could simply be swapped with an already existing color variabel as shown the linked issue and pull request. All colors the Element app is using could simply be defined by the already existing color variables; maybe the dev team would want to introduce some more color variables (which then definitely need to be supported by config.json) for reasons, but there’s really not that many color definitions such an interface needs.

For example, Slack manages to achieve this kind of theming by only using 9 color definitions. That’s less color definitions than Element already supports via config.json, which is currently 15 … so please, properly support custom themes.

Screen Shot 2021-05-18 at 11 33 16

@SimonBrandner SimonBrandner added the A-Themes-Custom Custom theme variables or support label May 18, 2021
@trosel
Copy link

trosel commented Jul 1, 2021

I agree that this would be a worthwhile effort. It would make it very easy to then also share themes by just pasting in a json block.

For what it’s worth, the contrast in the dark mode theme is too high. white text needs to be off-white and pure black background needs to be more muted. It hurts the eyes to look at such high contrast. Take a look at discord for an example of a more peaceful theme.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Themes-Custom Custom theme variables or support A-Theming T-Defect Z-Labs
Projects
None yet
Development

No branches or pull requests

5 participants