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

[core] fix CSS syntax in box-shadow variables #5989

Merged
merged 8 commits into from
Mar 2, 2023

Conversation

adidahiya
Copy link
Contributor

@adidahiya adidahiya commented Mar 2, 2023

Fixes #5987

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

  • Refactor generate-css-variables implementation into testable functions
  • Add jest + ts-jest testing infra to node-build-scripts package
  • Refactor generate-css-variables implementation to use postcss-simple-vars instead of get-sass-vars
    • The latter proved difficult to test via Jest (it wouldn't import successfully either as CommonJS or ESM). get-sass-vars also has little usage and probably is a little too complex / magical for us.
    • This migration required implementing a bit of the math logic which get-sass-vars was doing for us, though. I've added some special case handling for function calls in variable initializers of the form rgba($black, 0.1) where the first argument is a hex color value -- we now replace those variables with literal r, g, and b values like rgba(17, 20, 24, 0.1).
  • Add regression tests for Regression in box-shadow Sass variable syntax #5987

Reviewers should focus on:

Regression tests cover the functionality that we care about.

I compared the Sass variables output against this last known working version: https://unpkg.com/browse/@blueprintjs/core@4.16.3/lib/scss/variables.scss

Screenshot

N/A

@adidahiya
Copy link
Contributor Author

This commit contains the meaningful changes in this PR: ab10086

@adidahiya
Copy link
Contributor Author

remove dependency on @blueprintjs/colors

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor Author

Revert "bump CI cache key"

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor Author

fix getRootDir()

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya adidahiya merged commit 5eb478c into develop Mar 2, 2023
@adidahiya adidahiya deleted the ad/fix-box-shadow-variables branch March 2, 2023 02:52
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.

Regression in box-shadow Sass variable syntax
1 participant