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

[node-build-scripts] fix(css-variables): preserve string quotes #5303

Merged
merged 1 commit into from
May 16, 2022

Conversation

adidahiya
Copy link
Contributor

@adidahiya adidahiya commented May 16, 2022

Changes proposed in this pull request:

Before this PR, strings inside Sass lists would lose their quotes during the generate-css-variables build step.

The following source code ...

// src/common/_variables.scss
$pt-font-family: -apple-system,
                 "BlinkMacSystemFont",
                 "Segoe UI",
                 "Roboto",
                 "Oxygen",
                 "Ubuntu",
                 "Cantarell",
                 "Open Sans",
                 "Helvetica Neue",
                 $icons16-family, // support inline Blueprint icons
                 sans-serif !default;

... would become ...

// lib/scss/variables.scss
$pt-font-family: -apple-system, BlinkMacSystemFont, Segoe UI, Roboto, Oxygen,
  Ubuntu, Cantarell, Open Sans, Helvetica Neue, blueprint-icons-16, sans-serif !default;

This created some problems for strings containing any characters which would confuse the Sass syntax parser, such as ../ (used in file path strings, not in this repo but in another repo which uses @blueprintjs/node-build-scripts).

To work around this problem and preserve the quotes for those strings, I've added some hacky logic to detect quoted strings in generate-css-variables, and wrap those values with quotes before writing the output to variables.scss.

After this PR, we get:

// lib/scss/variables.scss
$pt-font-family: -apple-system, "BlinkMacSystemFont", "Segoe UI", "Roboto",
  "Oxygen", "Ubuntu", "Cantarell", "Open Sans", "Helvetica Neue",
  "blueprint-icons-16", sans-serif !default;

Note that I limited the special case code added here to apply just those Sass lists which contain , comma delimiters. This was done to avoid acting on lists of values which make up a compound CSS property like box-shadow, e.g. inset 0 0 0 1px rgba($black, 0.2) -- Sass considers those to be lists as well, but the values inside those "lists" should not be treated as strings.

Reviewers should focus on:

No regressions in other parts of lib/scss/variables.scss

@adidahiya adidahiya changed the title [node-build-scripts] fix(generate-css-variables): preserve string quotes [node-build-scripts] fix(css-variables): preserve string quotes May 16, 2022
@adidahiya adidahiya merged commit a5245e1 into develop May 16, 2022
@adidahiya adidahiya deleted the ad/css-variables-quotes branch May 16, 2022 14:46
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.

None yet

2 participants