Skip to content

Reduce palette size of themes #19353

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

Open
clarfonthey opened this issue Apr 9, 2022 · 15 comments
Open

Reduce palette size of themes #19353

clarfonthey opened this issue Apr 9, 2022 · 15 comments
Labels
topic/theme type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@clarfonthey
Copy link
Contributor

Feature Description

Right now, the _base.less file defines all of the colours for the base theme (which is overridden for the dark theme), and the current palette size is… 121 colours, with 105 unique colours; some are duplicated but not referencing each other.

Honestly, I'd say it's a big stretch that this should be larger than 20, but the exact number isn't nearly as important. We definitely have too many.

Auditing things like colour contrast is basically not possible with this many combinations, and if someone wants to make simple changes (e.g. changing just the accent colour), this isn't really feasible, since there are so many different shades of each colour.

I think it's extremely worthwhile seeing what can be trimmed from the palette by either using a similar shade or opacity to blend with the background (in cases like the heatmap).

Screenshots

No response

@clarfonthey clarfonthey added type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first. labels Apr 9, 2022
@silverwind
Copy link
Member

Generally, I find needing at least 10 shades per color to have enough fine-grained control. The alpha shades we can probably eliminate.

If you take a look at GitHub's CSS, I think they have now around 500 colors, so I assume it's pretty normal to end up with many colors as projects grow.

Also, we must consider not needlessly renaming/adding CSS variables as it will break 3rd party themes. Removal should generally be fine.

@clarfonthey
Copy link
Contributor Author

I agree with needlessly eliminating variables but disagree completely with the required number of shades. At a minimum, since the stylesheet is using LESS, it should be computing required shades if necessary rather than hard-coding them, since it makes it difficult to discern what would need to be changed in case someone wanted to modify the colours.

@silverwind
Copy link
Member

since the stylesheet is using LESS, it should be computing required shades if necessary rather than hard-coding them

Computing colors in LESS is something I certainly want to avoid because those variables can not be interacted with at runtime (we already do that as part of the monaco themeing) and it makes 3rd party theming a lot more complex as they'd need to depend on our LESS sources. Imho, variables inside the styling must strictly stay custom properties.

CSS custom properties integrate much better than preprocessor variables. It's a shame that standard CSS still does not offer a way to modify colors at runtime, but there have been some recent proposals for just that in W3C, so maybe we'll get something in 5-10 years.

@clarfonthey
Copy link
Contributor Author

I think you're misunderstanding what I'm saying-- take the existing CSS variable definitions and replace the actual colours with versions computed using LESS functions. If you reference the compiled CSS, you'll still have static colours, but if you reference the LESS, you'll know where the colours came from.

@silverwind
Copy link
Member

We've migrated from Less to CSS now. Once color-mix is well supported in browsers, it can be used to reduce the number of colors, but I take it will be a year or two until that point.

@clarfonthey
Copy link
Contributor Author

This still doesn't respond at all to the argument that the stylesheet could due with substantially fewer shades than it has.

If we're talking about super-granular case like the heatmap in user profiles, this could easily be accomplished today with opacity instead of explicitly setting the background to a mixed colour. In all other cases, I still strongly disagree that a large number of shades are needed for standard UI elements; even a user with very good vision would be hard-pressed to identify ten different shades of a colour on all the standard UI elements, and much more than four is really pushing it.

I mentioned using LESS as a starting point for this issue, not the ending point.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented May 3, 2023

I agree that there are too many colors. Is there a feasible plan about how to clean them up?

@wxiaoguang wxiaoguang reopened this May 3, 2023
@wxiaoguang
Copy link
Contributor

And actually, I have slight objection for such UI ( #22864) , I am not a fan of making UI too colorful .....

It's better to have some widely-accepted UI design patterns.

image

@clarfonthey
Copy link
Contributor Author

clarfonthey commented May 3, 2023

Another big reason against those kinds of changes is that the contrast tends to be rather poor. For example, the contrast between the red and green in the bar next to "README.md" there is effectively zero if you're red-green colourblind, and the contrast between the yellow text and the white background is also very poor.

This is also the main reason why I disagree with so many shades of colours. For example, the background colour of the header for the file element is #f7f7f7, and the colour of the main background is #ffffff. This is such a low contrast that it's effectively invisible even for someone with a good monitor and good vision. This colour should be reduced to a darker shade or replaced with the original colour.

Like I said, I struggle to believe any argument that claims that 10 shades of a colour is a good baseline for stylesheets. Can you reasonably come up with a layout where all 10 of those shades are both used, and looks at least subjectively better than a version with fewer shades? Not only does a large number of shades make it very difficult for people who want to adopt the theme for themselves, but it also just leads to a sloppy design language where people don't really know what shades are appropriate for various UI elements.

@wxiaoguang
Copy link
Contributor

Luckily, the green-red gauge only provides some minor information, it only makes the UI looks slightly beautiful.

The real problem for me here is the orange color, I am the poor man who doesn't have a good monitor / good vision .... it's difficult for me to read how many files get changed at first glance .... 🤔

@wxiaoguang
Copy link
Contributor

@clarfonthey Now there are new requirements for colors (focus/active), do you have more ideas about it? #24507

@silverwind
Copy link
Member

silverwind commented May 4, 2023

These are indiscussable I'd say. Fomantic UI has 4 different color shades for each button color, if we don't overwrite all, inconsistent colors will show up on buttons like it currently does for :focus and :active states.

@clarfonthey
Copy link
Contributor Author

The discussion is about whether different states should be different colours, not whether we should set the colours in the CSS. I think we're both with you on making sure they actually follow the theme correctly, but the question is what the theme should be in the first place.

@silverwind
Copy link
Member

Interesting tidbit, number of color variables comparing GitHub and Gitea. GitHub has major spaghetti going on in their CSS variables, we are not nearly at that point, thankfully.

GitHub

Object.entries(getComputedStyle(document.documentElement)).filter(e => e[1].startsWith("--") && e[1].includes("color")).length
489

Gitea

> Object.entries(getComputedStyle(document.documentElement)).filter(e => e[1].startsWith("--") && e[1].includes("color")).length
224

@silverwind
Copy link
Member

silverwind commented Mar 28, 2024

Will be fixed with #30160 which will give us total color freedom.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/theme type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

No branches or pull requests

4 participants