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

Switch to postcss + css variables #38

Closed
wants to merge 4 commits into from
Closed

Switch to postcss + css variables #38

wants to merge 4 commits into from

Conversation

bcomnes
Copy link
Contributor

@bcomnes bcomnes commented Jul 6, 2019

Also simplify site and build steps and some general cleanup.

I was experimenting with switching this over to post-css for a while now, and here is the result of that work. The main advantage is that style.css is based on css variables, so doing things like dark mode are a bit easier, since you can just switch out pointer vars and things like that.

wdyt @ungoldman ?

@bcomnes
Copy link
Contributor Author

bcomnes commented Jul 6, 2019

Oh, one other thing, I fixed a bug in post-css cli, so landing this will depend on that: postcss/postcss-cli#286

@bcomnes
Copy link
Contributor Author

bcomnes commented Jul 6, 2019

The other ope questions would be:

  • How to handle the website. Maybe put this version in /v2/style.css and also ship a copy of the old version at the current URL. Or should we just recommend unpkg urls only?
  • Is this a path you want to go down?
  • Should we support dark mode out the box?

Also simplify site and build steps and some general cleanup.
@bcomnes
Copy link
Contributor Author

bcomnes commented Jul 6, 2019

Also, I should still go through the docs and website to make sure all that info is up to date.

@bcomnes
Copy link
Contributor Author

bcomnes commented Jul 6, 2019

@ungoldman
Copy link
Owner

Hey @bcomnes, I'm out on vacation for a few more days so I won't be able to give this a full review until next week. However at a glance, this looks like a lot of different major breaking changes! I get the sense that in addition to switching to postcss, there are also changes in where the distribution files are kept, how source files are organized, possibly changes in design, a new dark mode that I'm not sure how to review, major bumps for almost all dependencies, major readme file changes, changes to CI setup, and probably other stuff I'm not yet aware of? In summary this is ... unreviewable? It would be a lot easier to review if this were broken up into PRs with separate concerns. I can't reasonably say this is OK to merge when I have no idea how much has changed and what this will break.

@bcomnes
Copy link
Contributor Author

bcomnes commented Jul 13, 2019

Yeah sorry, the work resulted from wanting to iterate on what was here in a somewhat breaking way. I originally was intending to create a personal fork with these changes, but I figured it would be worth offering them up as an upstream change before I settle for that.

Basically it boils down to these changes:

Switch to using css variables instead of sass variables

CSS variables make it easier to support dark mode themes and generally can assist in simplified theming when it comes to colors and such. It also makes it easier to support the serif variant. All you need it to override a var instead of use a different css file.

Support dark mode by default

Still a WIP but it would be a nice addition I think to support this by default.

Move the preprocessor to postcss

Provides a lot of the same features, with a syntax closer to theoretical future css. Personal preference, not sure where you are on this one.

Reorg the website

This work came out of the original motivation (soft personal fork to achieve the above goals) and was finished before I decided to offer up the changes to you in this repo.


How to reconcile. Are you interested in any of the above changes landing here? I'm guessing maybe the css-var support and dark mode? The post-css and website reorg were mainly the path I took to try those out and are the bigger change. It could be possible to separate these in various ways.

In terms of what would break. The release would be a breaking change 2.0. The website would be updated, and people consuming from unpkg without a major version number might break. Also old browser that don't support css vars would break.

The other alternative is to do a soft fork, maybe like @bret/style.css or something.

@ungoldman
Copy link
Owner

Sorry @bcomnes, I don't mean to be keep blocking your PR, I've just had no time at all in the last month for open source. 😭

If you want to just keep going with it feel free to do the soft fork, I'll try to pull it down and do a real review soon. 🤞

@bcomnes
Copy link
Contributor Author

bcomnes commented Aug 14, 2019

No problem at all 🤗 happy to converge in the future if it makes sense

@bcomnes
Copy link
Contributor Author

bcomnes commented Jan 16, 2020

Sorry to keep this open for ages. Moving it into its own repo.

@bcomnes bcomnes closed this Jan 16, 2020
@bcomnes bcomnes deleted the css-vars branch January 16, 2020 18:56
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.

2 participants