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

Critical CSS #744

Merged
merged 6 commits into from
May 9, 2022
Merged

Critical CSS #744

merged 6 commits into from
May 9, 2022

Conversation

coreymcollins
Copy link
Contributor

@coreymcollins coreymcollins commented Sep 17, 2021

DESCRIPTION

This separates some critical CSS partials and compiles them into their own file, which is then added inline in the <head>.

What I did:

  • Moved menus.scss, mobile-menus.scss, and header.scss from their original locations to a src/scss/critical/ directory.
  • Added multiple entry points to our webpack.config.js file to produce multiple build files for index.css and critical.css
  • Inlined the styles in the <head>

What I'd like to get some 👀 on:

  • Make sure this is the cleanest and safest way to include this partial
  • Add to/adjust what's being compiled into critical.css as needed

SCREENSHOTS

image

image

OTHER

  • Is this issue accessible? (Section 508/WCAG 2.0AA)
  • Does this issue pass all the linting? (PHPCS, ESLint, SassLint)
  • Does this pass CBT?

STEPS TO VERIFY

  1. Checkout
  2. Run npm run build
  3. Verify the inline styles are in the <head> and the header & menus are looking good
  4. Verify the method of including these styles is safe & secure

DOCUMENTATION

Will this pull request require updating the wd_s wiki?

I can see us wanting to update around Critical CSS paths and what should be included there, so I'll start looking at the existing docs to see where this could best fit.

@coreymcollins coreymcollins changed the title Feature/critical css Critical CSS Sep 17, 2021
@coreymcollins
Copy link
Contributor Author

This PR is more of a proof of concept at the moment, I think, because there's probably a lot more to figure out as far as Critical CSS goes. This does the job for our own partials, but how do we handle Critical CSS in regards to styles set in the Tailwind config file?

We'll also need to write up documentation on when and how to add to Critical CSS. Since we don't include a hero or anything like that in the theme anymore, for instance, we'll want to educate the team on what to add to Critical CSS so we're not bloating it but also not missing anything important.

@gregrickaby
Copy link
Contributor

@coreymcollins Nicely done. I think you should demo during the FEEBEE Scrum tomorrow!!!

@coreymcollins
Copy link
Contributor Author

Merged main down to capture new updates. I'll have to merge this in manually to resolve the conflicts above once this is reviewed again and given the 👍🏻 .

@coreymcollins coreymcollins removed their assignment Nov 24, 2021
Merging main into feature branch
@coreymcollins
Copy link
Contributor Author

Giving this a bump after a fresh merge from main, if you get a chance to put 👀 back on it @gregrickaby. Thanks!

@coreymcollins coreymcollins removed the request for review from gregrickaby April 27, 2022 15:43
Copy link
Contributor

@nickyiie nickyiie left a comment

Choose a reason for hiding this comment

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

Looks good!

@coreymcollins coreymcollins merged commit 3131c07 into main May 9, 2022
@coreymcollins coreymcollins deleted the feature/critical-css branch May 9, 2022 20:52
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.

3 participants