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

Color specification not backwards incompatible? #31

Open
musjj opened this issue Nov 11, 2024 · 6 comments
Open

Color specification not backwards incompatible? #31

musjj opened this issue Nov 11, 2024 · 6 comments
Assignees

Comments

@musjj
Copy link
Contributor

musjj commented Nov 11, 2024

In the past, base16 provides base08-base0E for the "base" pastel colors, allowing base24 to extend the pallette with base10-base17 to provide brighter variants of those base colors. This matches how terminal theming usually works.

But now, it's reversed:

base24/styling.md

Lines 54 to 60 in 21ac86b

| ![Colour](https://placehold.it/25/e06c75/000000?text=+) | base08 | 9 | Bright Red | Variables, XML Tags, Markup Link Text, Markup Lists, Diff Deleted |
| ![Colour](https://placehold.it/25/d19a66/000000?text=+) | base09 | ~11 | (Orange) | Integers, Boolean, Constants, XML Attributes, Markup Link Url |
| ![Colour](https://placehold.it/25/e5c07b/000000?text=+) | base0A | 11 | Bright Yellow | Classes, Markup Bold, Search Text Background |
| ![Colour](https://placehold.it/25/98c379/000000?text=+) | base0B | 10 | Bright Green | Strings, Inherited Class, Markup Code, Diff Inserted |
| ![Colour](https://placehold.it/25/56b6c2/000000?text=+) | base0C | 14 | Bright Cyan | Support, Regular Expressions, Escape Characters, Markup Quotes |
| ![Colour](https://placehold.it/25/61afef/000000?text=+) | base0D | 12 | Bright Blue | Functions, Methods, Attribute IDs, Headings |
| ![Colour](https://placehold.it/25/c678dd/000000?text=+) | base0E | 13 | Bright Purple | Keywords, Storage, Selector, Markup Italic, Diff Changed |

home/styling.md

| ![#](https://placehold.it/25/e06c75/000000?text=+) | base08 | 9  and 1 | Bright Red and Red       | Variables, XML Tags, Markup Link Text, Markup Lists, Diff Deleted |
| ![#](https://placehold.it/25/d19a66/000000?text=+) | base09 | ~3       | (Orange)                 | Integers, Boolean, Constants, XML Attributes, Markup Link Url |
| ![#](https://placehold.it/25/e5c07b/000000?text=+) | base0A | 11 and 3 | Bright Yellow and Yellow | Classes, Markup Bold, Search Text Background |
| ![#](https://placehold.it/25/98c379/000000?text=+) | base0B | 10 and 2 | Bright Green and Green   | Strings, Inherited Class, Markup Code, Diff Inserted |
| ![#](https://placehold.it/25/56b6c2/000000?text=+) | base0C | 14 and 6 | Bright Cyan and Cyan     | Support, Regular Expressions, Escape Characters, Markup Quotes |
| ![#](https://placehold.it/25/61afef/000000?text=+) | base0D | 12 and 4 | Bright Blue and Blue     | Functions, Methods, Attribute IDs, Headings |
| ![#](https://placehold.it/25/c678dd/000000?text=+) | base0E | 13 and 5 | Bright Purple and Purple | Keywords, Storage, Selector, Markup Italic, Diff Changed |

It's also doesn't match with the example color on the left.

Is this intentional? I think the previous specification makes more sense, but was there a reasoning behind the change?

@JamyGolden
Copy link
Member

Hey, these are the PRs that merged the changes that include discussions around it:

I think the first and third have the most discussion around it. I'm happy to continue discussing and change anything if it makes sense, so let me know what you think once you've read through them.

@musjj
Copy link
Contributor Author

musjj commented Nov 15, 2024

Hey, I looked at the screenshot in your PR but I don't quite see what you mean. In here, I grayscaled your screenshot:

365402815-d2888049-1a8b-4e6b-a2e1-1be8547969c7(1)

As you can see above, base12-base17 in general is lighter in luminosity. I guess there are also some semantic disagreements here. When I eyeballed over your original screenshot, base12-base17 just feels brighter to me despite being less saturated. If anything the decreased saturation makes it feel brighter to me (because they are closer to white).

Another issue is with terminal theming. Some terminals only supports 8 baseline colors, while more advanced terminals supports 8 additional brighter colors (see here). The previous specification neatly fits this paradigm: base16 can provide the first 8 baseline colors, while base24 can extend them with 8 brighter colors.

Side case: it also makes the spec a bit semantically awkward. base24 is supposed to be an extension of base16, so it feels natural for base16 to provide the baseline colors and base24 extending them with brighter variants. In the new spec, the roles feels mixed up.

@JamyGolden
Copy link
Member

I'm personally happy to agree on whatever we decide is "bright" and therefore "dim" or "muted" even though we don't use those terms (yet).

https://primer.style/primitives/storybook/?path=/story/color-datavis--highcharts-accent-colors Here the github colors "muted" colors are closer to "white" than the "emphasied" or "brighter" variants. I'm inferring brightness in this specific scenario since they don't explicitly say that, but you usually have "muted" or "dull" as apposed to something "bright".

It's important that bas16 and base24 can be used interchangeably. While it's not how we're using it in the tinted-* template repos, it was the original idea with base24. So you can just use base24 in place of base16 on a template and you get the same thing with some additional colors that can be used. This means that we need to keep base00-base0F the same across the two different scheme systems, so currently base24 is an extension of base16, which is why we made the switch (based on the definition of brightness mentioned in the doc).

In the screenshot you provided, while the closer-to-white colors are on the right, I, at least, interpretted them as dull or muted and saw base08-0F as the vibrant/brighter colors.

In summary yeah you're right that brightness here is a semantic thing and I think we just need to get a definition that works with our needs. I think people would be ok with switching the current bright (08-0E) with the standard variants (12-17), but we just need to document it well with the reasons for it being that way.

@musjj
Copy link
Contributor Author

musjj commented Nov 17, 2024

https://primer.style/primitives/storybook/?path=/story/color-datavis--highcharts-accent-colors

Thanks for the link. Here the "muted" variants have higher luminosity levels, which matches with the current base10-base17 colors. So if we are just going by objective luminosity levels, the previous "bright {color}" naming scheme works just fine IMO (as shown in the grayscale image above).

But I think the most important takeaway here is that the base12-base17 colors should be secondary colors. In other words, base16 and base24 should share the same baseline colors to make sure they are visually compatible.

This is especially true for terminals, where the baseline 8 colors are used almost everywhere. While the other 8 brighter colors are considered secondary and used rarely.

This means that in the new spec, the primary baseline colors will change when you port a base16 theme to base24.

For example, let's create a fictional base16 theme:

image

Now let's extend it for base24 according to the new spec:

image

Most of the colors are now taken over by base12-base17 because they are the new baseline colors in base24. As you can see, this completely changes the appearance of the theme. Now let's try it again with the original base24 spec:

image

As you can see here, the original baseline colors (base08-base0E) are preserved. Meanwhile the new colors (base12-base17) extended by base24 are only used to sprinkle emphasis here and there.

If we're unsatisfied with the colors introduced by the base24 themes, we should improve on that instead of changing the spec in a backwards-incompatible way.

We could also come up with better adjectives to describe base12-base17, (e.g. emphasized, accent, etc.)

@JamyGolden
Copy link
Member

In the base16 spec we define Bright colors before standard colors, so we aren't changing standard to base12-17 since bright is defined first Bright Red and Red for example.

But I do understand that it's not intuitive and would make more sense for standard to be defined first. If we do that then we need to redefine (in base24 styling spec) what "brightness" means and be generous with that meaning to mean brightness in any way, intensity, luminosity, etc. to be explicit. We also need to reorder Bright Red and Red to Red and Bright Red in the base16 spec.

Do you want to create a draft PR switching up the base24 spec and we can adjust things and merge when it makes sense?

@musjj
Copy link
Contributor Author

musjj commented Nov 18, 2024

Do you want to create a draft PR switching up the base24 spec and we can adjust things and merge when it makes sense?

I'll try to draft something up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants