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

Add light/dark scheme-variant support to builder.md #66

Merged
merged 1 commit into from
Oct 7, 2022

Conversation

JamyGolden
Copy link
Member

@JamyGolden JamyGolden commented Aug 3, 2022

There are times when the generated colorschemes requires a dark/light variant, such as for VSCode: https://github.com/golf1052/base16-generator/blob/master/themes/base16-apathy-dark.json#L4. Until we add this support to the spec, vscode templates can't use any of our official builders.

type isn't an amazing property name, maybe variant would work better? My thought was to go for a string value instead of a boolean variant since one day we may have light, dark and medium for example 🤷

We'd have to add this field to each scheme, but I could do that after we merge this. There may be several things I'm missing here, but I just wanted to get a PR out so we could discuss.

@Misterio77
Copy link
Member

I think variant seems good!

@golf1052
Copy link
Member

golf1052 commented Aug 4, 2022

Yeah I also prefer variant.

If you want to automate adding variants to existing themes here's the code I use to automatically determine if the theme is light or dark

return (5 * c.green() + 2 * c.red() + c.blue()) <= 8 * 128;

And the original source is from here

However this doesn't work in all cases I think, as evidenced by this issue so manual review might be needed.

@belak
Copy link
Member

belak commented Aug 4, 2022

I like the idea of it being variant in the scheme files as well.

One other thing worth noting - there's no way to check if scheme-variant == dark in the mustache files. Would exporting is_{{ scheme-variant }}_variant in the builder be worthwhile to fix that? Values should default to falsey, so that should allow checking against specific variants if necessary.

Also, could you bump the version to 0.11.0-dev rather than 0.11.0? I'd like to bundle changes together if possible, especially because the scheme system changes should be close.

@JamyGolden JamyGolden force-pushed the jamy/feature/scheme-type branch from ef34fca to a112d9d Compare August 5, 2022 06:35
@JamyGolden JamyGolden changed the title Add light/dark scheme-type support to builder.md Add light/dark scheme-variant support to builder.md Aug 5, 2022
@JamyGolden JamyGolden force-pushed the jamy/feature/scheme-type branch from a112d9d to 1179eb1 Compare August 5, 2022 06:53
@JamyGolden
Copy link
Member Author

JamyGolden commented Aug 5, 2022

Yeah I think exporting is_{{scheme-variant}}_variant is a good idea. I've updated the template variable information. We don't have a dynamic property-name in there yet so I wasn't able to follow any example information for it. If something is unclear or should be changed just leave a change request.

@JamyGolden JamyGolden force-pushed the jamy/feature/scheme-type branch from 1179eb1 to 6603b65 Compare October 6, 2022 18:18
builder.md Show resolved Hide resolved
@JamyGolden JamyGolden force-pushed the jamy/feature/scheme-type branch from 6603b65 to c8bc5c6 Compare October 7, 2022 09:20
@JamyGolden JamyGolden merged commit 68a1458 into main Oct 7, 2022
@JamyGolden JamyGolden deleted the jamy/feature/scheme-type branch October 7, 2022 09:28
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

Successfully merging this pull request may close these issues.

4 participants