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

Generate "system" CSS #147

Closed
wants to merge 16 commits into from
Closed

Generate "system" CSS #147

wants to merge 16 commits into from

Conversation

shawnbot
Copy link
Contributor

@shawnbot shawnbot commented Jul 23, 2018

This PR adds a build step that generates static CSS for each of our "system" props in #145. You can see the CSS that it generates in docs/system.css.

Selectors

  • .bg-{color} for all values in colors.bg.*
  • .border-{color} for all values in colors.border.*
  • .color-{color}-{index} for all colors.*.* (except colors.bg and colors.border)
  • .color-{color} shorthands for colors.*[5]
  • .f-{index} for all fontSizes values (responsive)
  • .border-width-{index} for all values in borderWidths (missing)
  • .font-weight-{name} for all fontWeight values (missing)
  • .lh-{name} for all values in lineHeights (missing)
  • .max-width-* for all maxWidths values?
  • .round-{index} for all radii values (responsive?)
  • .w-{ratio} for column grid widths? (missing)

For now I'm also including responsive whitespace (margin and padding) utilities as a test. We won't need them in the initial release because primer-utilities already provides them.

I went with PostCSS because the AST API is super simple and easy to use, and this allows us to chain additional plugins for (later) adding custom property support, minifying the resulting CSS, etc.

@shawnbot shawnbot requested review from broccolini and a team July 23, 2018 21:22
@shawnbot shawnbot mentioned this pull request Jul 25, 2018
8 tasks
Copy link

@emplums emplums left a comment

Choose a reason for hiding this comment

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

This is really really cool work! Amazing job! I don't see anything that pops out so far 👍

Curious why we run this every time we start the server vs just running it when we build in Travis? I'm also interested in what the reasoning is for rebuilding the css often vs just generating the css once? Will new css rules need to be added fairly often as our API changes or will that be a fairly infrequent situation?

@@ -22,6 +22,7 @@ module.exports = ({
<link rel='stylesheet' href='https://unpkg.com/primer-product/build/build.css'/>
<link rel='stylesheet' href='https://unpkg.com/primer-tooltips/build/build.css'/>
<link rel='stylesheet' href='https://unpkg.com/primer-utilities/build/build.css'/>
<link rel='stylesheet' href='${publicPath}system.css' />
Copy link

Choose a reason for hiding this comment

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

Did you end up getting this to work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, not yet. I'm going to try inlining it via our Styles component.

@shawnbot
Copy link
Contributor Author

shawnbot commented Jul 25, 2018

@emplums Having it built all the time just ensures that the CSS is up-to-date if you've changed anything before starting the server. The signal that we've forgotten to run it is if there's a diff in docs/system.css, and we could even have CI fail if that's the case. If we're going to inline the contents of that file in the Styles component to make it work in x0, then we need to be sure that it's current.

@shawnbot shawnbot changed the title [WIP] Generate "system" CSS Generate "system" CSS Jul 25, 2018
@shawnbot
Copy link
Contributor Author

Okay, so I followed up with a bunch of changes to how and where this CSS is generated in #155. @emplums, do you think we should merge this and then that PR, or just merge #155 (which includes this one)?

@shawnbot
Copy link
Contributor Author

Closing in favor of merging via #155.

@shawnbot shawnbot closed this Jul 25, 2018
@shawnbot shawnbot deleted the system-css branch July 25, 2018 23:03
@shawnbot shawnbot mentioned this pull request Jul 26, 2018
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants