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

Use @primer/primitive v8 color CSS vars with fallbacks #2083

Merged
merged 20 commits into from
Jun 21, 2023

Conversation

langermank
Copy link
Contributor

@langermank langermank commented Jun 19, 2023

Authors: Please fill out this form carefully and completely.

Reviewers: By approving this Pull Request you are approving the code change, as well as its deployment and mitigation plans.
Please read this description carefully. If you feel there is anything unclear or missing, please ask for updates.

What are you trying to accomplish?

Using Stylelint, this PR replaces all color CSS vars with the new version from Primitives that we're testing behind a feature flag in github/github. PostCSS automatically adds fallback values at build time, and stylelint will ensure we only use new color tokens going forward with linting.

Screenshots

Integration

No, the feature flag is already in place.

List the issues that this change affects.

https://github.com/github/primer/issues/1410

Risk Assessment

  • Low risk the change is small, highly observable, and easily rolled back.
  • Medium risk changes that are isolated, reduced in scope or could impact few users. The change will not impact library availability.
  • High risk changes are those that could impact customers and SLOs, low or no test coverage, low observability, or slow to rollback.

What approach did you choose and why?

npm run lint:stylelint:fix and some manual find/replace

Anything you want to highlight for special attention from reviewers?

Please check that the build CSS files all have fallback values (see the build in the checks section)

Accessibility

  • No new axe scan violation - This change does not introduce any new axe scan violations.

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Added/updated previews (Lookbook)
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

@changeset-bot
Copy link

changeset-bot bot commented Jun 19, 2023

🦋 Changeset detected

Latest commit: 7d467d6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/view-components Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@langermank langermank temporarily deployed to preview June 19, 2023 23:59 — with GitHub Actions Inactive
@github-actions github-actions bot added the css Pull requests that update CSS code label Jun 19, 2023
@langermank langermank temporarily deployed to github-pages June 20, 2023 00:04 — with GitHub Actions Inactive
@langermank langermank changed the title Primitive v8 colors Use @primer/primitive v8 color CSS vars with fallbacks Jun 20, 2023
@langermank langermank marked this pull request as ready for review June 20, 2023 21:24
@langermank langermank requested a review from a team as a code owner June 20, 2023 21:24
@langermank langermank requested review from a team, rezrah and camertron June 20, 2023 21:24
@langermank langermank temporarily deployed to github-pages June 20, 2023 21:27 — with GitHub Actions Inactive
@langermank langermank temporarily deployed to preview June 20, 2023 21:34 — with GitHub Actions Inactive
@langermank langermank temporarily deployed to github-pages June 20, 2023 21:40 — with GitHub Actions Inactive
@langermank langermank temporarily deployed to preview June 21, 2023 17:50 — with GitHub Actions Inactive
@langermank langermank temporarily deployed to github-pages June 21, 2023 17:55 — with GitHub Actions Inactive
@langermank langermank temporarily deployed to preview June 21, 2023 18:09 — with GitHub Actions Inactive
@langermank langermank temporarily deployed to github-pages June 21, 2023 18:14 — with GitHub Actions Inactive
@langermank langermank temporarily deployed to preview June 21, 2023 18:24 — with GitHub Actions Inactive
missing_vars = matches.map { |match| match[0] }
assert_equal 0, matches.length, "CSS variables are missing fallbacks: #{missing_vars.join(', ')}"
end
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's what the output of this test look likes if it fails

test failure

I think since my regex is fuzzy, I can't get the full variable string. If anyone has feedback on how I can make this test failure more clear let me know!

Copy link
Contributor

Choose a reason for hiding this comment

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

I added some logic in to point at the line and column so it's easier to find the failure.

@langermank langermank temporarily deployed to github-pages June 21, 2023 18:28 — with GitHub Actions Inactive
@langermank langermank merged commit e38ae1e into main Jun 21, 2023
@langermank langermank deleted the primitive-v8-colors branch June 21, 2023 23:29
This was referenced Jun 21, 2023
@camertron camertron mentioned this pull request Jun 28, 2023
3 tasks
@primer-css primer-css mentioned this pull request Jun 28, 2023
@langermank langermank restored the primitive-v8-colors branch June 28, 2023 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css Pull requests that update CSS code patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants