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 theme support #39

Closed
wants to merge 4 commits into from
Closed

Conversation

tpenguinltg
Copy link

Allow configuring the entire set of Windows Classic colours through CSS variables. The variables are named after the corresponding keys in the Windows theme file converted to kebab case. Many of them are not currently used but may prove to be useful later on. The colours are presented in rgb() format for easy conversion and verification with theme files. I don't have a copy of windows handy to check so I had to guess the mapping for some of the elements, most notably the dotted outlines.

The old variables are still in place for backwards compatibility and have been mapped to the new variables accordingly. Note, however, that there is a breaking change because of there is a naming conflict with --button-face, which maps to --button-light. I see three solutions: 1) leave it as-is and accept the breaking change, 2) go all the way and remove the old variables, or 3) use the original PascalCase names to avoid the conflict. I don't have a preference either way.

For this change to be meaningful, the build process has been modified to preserve CSS variables while providing the computed value as a fallback. This allows a downstream user to easily add a new stylesheet that overrides the custom properties in the root without modifying the original CSS.

Known bug: SVG assets still have hard-coded colours and are not affected by the variables.

Closes #33


To test this patch, you can load one of these stylesheets (attached as .txt because of GitHub limitations) or one of your own and into the demo. The DeviantArt links to the original themes are also included for reference.

@jdan
Copy link
Owner

jdan commented Apr 25, 2020

Oooooooh I like this idea very much!

We've since changed the build process to minify these with cssnano, so variables will be wiped out. One potential path forward could be the following:

  • Build both a 98.css which keeps variables around (but maybe continues to inline SVGs and copy font assets?) and a 98.min.css which will be what 98.css currently is
  • Change the package.json's main to dist/98.min.css, but still allow access to dist/98.css via import "98.css/dist/98.css"

Then folks could import dist/98.css and use the variables in subsequent documents. wdyt?

Copy link
Owner

@jdan jdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See last comment

@tpenguinltg
Copy link
Author

I wouldn't be opposed to having two builds: one with variables intact and one without, although I wouldn't call the latter .min because it's a fundamentally different bundle. I'd probably call it 98-full.css and 98.css, or 98.css and 98-stripped.css, and you can minify them both in the strict sense if you wanted to (i.e. just removing whitespace and other non-destructive optimizations; these bundles would get the .min.css extension). One could argue that removing the variables should be part of the downstream build process if they want it, but I'm not opposed to having a build optimized for the default theme.

As for what to put for the main field, I'm not familiar with what non-JS packages are supposed to put for it, so I can't comment much on it. But I think what you're proposing is fine if we expect the majority of library users not to change the colour scheme.

By the way, I will be doing a rebase when I update this branch if that's alright with you. It'll be a lot cleaner to update that way.

@vercel
Copy link

vercel bot commented May 18, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/jdan/98css/qulxywfus
✅ Preview: https://98css-git-fork-tpenguinltg-theme-support.jdan.now.sh

@tpenguinltg
Copy link
Author

@jdan I finally got around to updating this. There was a lot to update with all the new features, but I think this is all up to date now. For the build process, I implemented my 98.css/98-full.css suggestion above (I couldn't figure out how to set the name for the filename template, so there's probably a better way to switch between the two assets).

Support the full range of configurable colors in a Windows Classic
theme. The colors are configured using CSS variables named after the
corresponding Windows theme file property converted from PascalCase to
kebab-case.

Colors are given in rgb format to make it easier to convert and
recognize values from a Windows theme file.

The old variables are kept for backwards compatibility and are mapped
accordingly. However, the old `--button-face`, which maps to
`--button-light`, has not been mapped because of a name conflict with
the new `--button-face`; this is a breaking change.
Remove the radio width from the calculation for the radio dot top value.
I'm not sure why this works, but it does.

Also remove redundant precalc variables. Nested calc calls are reduced
to a single one automatically.
The computed asset, 98.css, precomputes the values that uses CSS
variables. The non-computed asset, 98-full.css, preserves the variables
and uses the computed values as fallbacks for browsers without support
for custom properties.
@jdan
Copy link
Owner

jdan commented Jun 19, 2020

Looks like this closed because I deleted the master branch in favor of main, sorry! Feel free to re-open and I'll take another look.

@tpenguinltg tpenguinltg mentioned this pull request Jun 20, 2020
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.

Theme support
2 participants