Skip to content

Commit

Permalink
Convert SASS-style inline comments to CSS-style multiline comments. (#…
Browse files Browse the repository at this point in the history
…11733)

Following on from [the conversion of SASS ->
CSS](#11677), this PR converts
the SASS style inline comments to CSS style multiline comments.

Since this was the only reason we were using `postcss-scss` parser, it
too has been removed.

---

I used this script to achieve the conversion:

```sh
#!/bin/bash

# Write a temporary postcss config for postcss to work
cat <<EOF > postcss.config.js
const spaceAtEndRegex = /\s$/;
const postCssInlineToMultilineComment = {
  postcssPlugin: 'postcss-inline-to-multiline-comment',
  Comment(node) {
    if (node.raws.inline && (!node.raws.right || !spaceAtEndRegex.test(node.raws.right))) {
      if (!node.raws.right) {
        node.raws.right = '';
      }
      node.raws.right += ' ';
    }
  },
}
module.exports = {
  // Need to understand SCSS inline comment syntax
  parser: 'postcss-scss',
  plugins: [
    postCssInlineToMultilineComment,
  ],
};
EOF

npx postcss-cli --yes postcss-cli -r --no-map --config="`pwd`" polaris-react/**/*.css

rm -f postcss.config.js  # Clean up temporary files
```

Then I confirmed it was successful with this script:

```sh
if [ -z "$1" ]; then echo "A filename is required:\n> $0 ./no-inline.css"; exit 1; fi;

# Write a temporary postcss config for cssnano to work
cat <<EOF > postcss.config.js
module.exports = {
  plugins: [
    require('cssnano')({
      preset: 'lite',
    }),
  ],
};
EOF

yarn workspace @shopify/polaris add cssnano-preset-lite
yarn
yarn build --filter="@shopify/polaris"
cp polaris-react/build/esm/styles.css "$1" # Move out of ignored directories
yarn prettier -w "$1" # Pre-format to normalize whitespace
npx postcss-cli --yes postcss-cli -r --no-map --config="`pwd`" "$1" # cssnano removes more whitespace prettier doesn't
yarn prettier -w "$1" # Format again to get back into un-minified version
rm postcss.config.js # Clean up temporary files
yarn workspace @shopify/polaris remove cssnano-preset-lite
```

Run like this:

```sh
git checkout multiline-comments
./go.sh ./no-inline.css
git checkout main
./go.sh ./inline.css
git checkout -
git diff --histogram --no-index inline.css no-inline.css
# Should be an empty diff
```
  • Loading branch information
jesstelford committed Mar 14, 2024
1 parent 2f0cbca commit 9c24a46
Show file tree
Hide file tree
Showing 102 changed files with 1,256 additions and 1,232 deletions.
5 changes: 5 additions & 0 deletions .changeset/brown-penguins-walk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': patch
---

Convert SASS-style inline comments to CSS-style multiline comments.
3 changes: 1 addition & 2 deletions polaris-react/config/rollup/plugin-styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ const path = require('path');

const {createFilter} = require('@rollup/pluginutils');
const postcss = require('postcss');
const postcssScss = require('postcss-scss');
const cssModules = require('postcss-modules');

module.exports.styles = function styles({
Expand Down Expand Up @@ -145,7 +144,7 @@ module.exports.styles = function styles({
}

const postCssOutput = await styleProcessor
.process(source, {parser: postcssScss, from: id})
.process(source, {from: id})
.then((result) => ({
css: result.css,
tokens: result.messages.find(({plugin, type}) => {
Expand Down
1 change: 0 additions & 1 deletion polaris-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@
"postcss-modules": "^4.2.2",
"postcss-nesting": "^12.0.2",
"postcss-pxtorem": "^6.0.0",
"postcss-scss": "^4.0.9",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"rimraf": "^3.0.0",
Expand Down
2 changes: 1 addition & 1 deletion polaris-react/playground/DetailsPage.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
display: flex;
align-items: center;
padding: 0 var(--p-space-300);
// stylelint-disable-next-line -- polaris custom global property
/* stylelint-disable-next-line -- polaris custom global property */
height: var(--pg-top-bar-height);
}

Expand Down
1 change: 0 additions & 1 deletion polaris-react/postcss.config.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import postcssPlugins from './config/postcss-plugins.js';

const config = {
parser: 'postcss-scss',
plugins: postcssPlugins,
};

Expand Down
38 changes: 19 additions & 19 deletions polaris-react/src/components/ActionList/ActionList.module.css
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
.Item {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
/* stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY */
@mixin unstyled-button;
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
/* stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY */
@mixin focus-ring wide;
// stylelint-disable -- Polaris component custom properties
/* stylelint-disable -- Polaris component custom properties */
--pc-action-list-item-min-height: var(--p-space-800);
--pc-action-list-indented-item-margin: calc(
var(--p-space-500) + var(--p-space-050)
Expand All @@ -17,11 +17,11 @@
--pc-action-list-item-vertical-padding: calc(
(var(--pc-action-list-item-min-height) - var(--p-font-line-height-500)) / 2
);
// stylelint-enable
/* stylelint-enable */
display: flex;
align-items: center;
width: 100%;
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
/* stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY */
min-height: var(--pc-action-list-item-min-height);
text-align: left;
text-decoration: none;
Expand Down Expand Up @@ -50,7 +50,7 @@
}

&:focus-visible:not(:active) {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
/* stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY */
@mixin no-focus-ring;
background-color: var(--p-color-bg-surface);
outline: var(--p-border-width-050) solid var(--p-color-border-focus);
Expand All @@ -69,7 +69,7 @@
}

&::before {
// stylelint-disable-next-line -- alignment for left tab style https://github.com/Shopify/polaris/pull/3619
/* stylelint-disable-next-line -- alignment for left tab style https://github.com/Shopify/polaris/pull/3619 */
@mixin list-selected-indicator;
display: none;
}
Expand All @@ -86,7 +86,7 @@
background-color: var(--p-color-bg-surface-critical-hover);
}

// stylelint-disable-next-line selector-max-class -- generated by polaris-migrator DO NOT COPY
/* stylelint-disable-next-line selector-max-class -- generated by polaris-migrator DO NOT COPY */
&:active,
&.active {
background-color: var(--p-color-bg-surface-critical-active);
Expand All @@ -102,20 +102,20 @@
background-color: unset;
}

// stylelint-disable-next-line selector-max-class, selector-max-combinators, selector-max-specificity -- generated by polaris-migrator DO NOT COPY
/* stylelint-disable-next-line selector-max-class, selector-max-combinators, selector-max-specificity -- generated by polaris-migrator DO NOT COPY */
.Prefix svg,
.Suffix svg {
fill: var(--p-color-icon-disabled);
}
}

&.indented {
// stylelint-disable -- reset custom properties
/* stylelint-disable -- reset custom properties */
--pc-action-list-image-size: 24px;
position: relative;
margin-left: var(--pc-action-list-indented-item-margin);
max-width: var(--pc-action-list-indented-item-width);
// stylelint-enable
/* stylelint-enable */

&::before {
content: '';
Expand All @@ -129,9 +129,9 @@
}

&.menu {
// stylelint-disable -- reset custom properties
/* stylelint-disable -- reset custom properties */
--pc-action-list-image-size: 24px;
// stylelint-enable
/* stylelint-enable */
}
}

Expand All @@ -140,18 +140,18 @@
flex: 0 0 auto;
justify-content: center;
align-items: center;
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
/* stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY */
height: var(--pc-action-list-image-size);
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
/* stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY */
width: var(--pc-action-list-image-size);
border-radius: var(--p-border-radius-100);

// We need the negative margin to ensure that the image does not set
// the minimum height of the action item.
// stylelint-disable -- generated by polaris-migrator DO NOT COPY
/* We need the negative margin to ensure that the image does not set
the minimum height of the action item. */
/* stylelint-disable -- generated by polaris-migrator DO NOT COPY */
margin: calc(-0.5 * var(--pc-action-list-image-size)) 0
calc(-0.5 * var(--pc-action-list-image-size)) 0;
// stylelint-enable
/* stylelint-enable */
background-size: cover;
background-position: center center;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
justify-content: flex-end;

@media print {
// stylelint-disable-next-line declaration-no-important -- Enforce print styles
/* stylelint-disable-next-line declaration-no-important -- Enforce print styles */
display: none !important;
}
}
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
.RollupActivator {
text-align: right;
// stylelint-disable-next-line selector-no-qualifying-type -- override `.iconOnly` negative margins
/* stylelint-disable-next-line selector-no-qualifying-type -- override `.iconOnly` negative margins */
button[type='button'] {
background: var(--p-color-bg-fill-tertiary);
border-radius: var(--p-border-radius-200);
border: none;
box-shadow: none;
margin: 0;

// stylelint-disable-next-line selector-max-specificity -- apply active styles
/* stylelint-disable-next-line selector-max-specificity -- apply active styles */
&:active {
background: var(--p-color-bg-fill-tertiary-active);
}

// stylelint-disable-next-line selector-max-specificity -- apply focus styles
/* stylelint-disable-next-line selector-max-specificity -- apply focus styles */
&:focus:not(:active) {
/* stylelint-disable-next-line polaris/border/polaris/at-rule-disallowed-list -- button overrides */
@mixin no-focus-ring;
Expand All @@ -22,7 +22,7 @@
background: var(--p-color-bg-fill-tertiary-active);
}

// stylelint-disable-next-line selector-max-specificity -- apply hover styles
/* stylelint-disable-next-line selector-max-specificity -- apply hover styles */
&:hover {
background: var(--p-color-bg-fill-tertiary-hover);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
.SecondaryAction {
// stylelint-disable-next-line -- Polaris component custom properties
/* stylelint-disable-next-line -- Polaris component custom properties */
--pc-secondary-action-button-spacing: var(--p-space-300);
// stylelint-disable declaration-no-important -- generated by polaris-migrator DO NOT COPY
/* stylelint-disable declaration-no-important -- generated by polaris-migrator DO NOT COPY */
a,
button {
// stylelint-disable -- generated by polaris-migrator DO NOT COPY
/* stylelint-disable -- generated by polaris-migrator DO NOT COPY */
@mixin focus-ring base, 1px;
--pc-button-padding-block: var(--p-space-100);
--pc-button-padding-inline: var(--p-space-300);
// stylelint-enable
/* stylelint-enable */
background: var(--p-color-bg-fill-tertiary) !important;
box-shadow: none !important;
border: none;
Expand All @@ -19,28 +19,28 @@
background-color: var(--p-color-bg-fill-tertiary-hover) !important;
}

// stylelint-disable-next-line selector-no-qualifying-type -- add expanded styling
/* stylelint-disable-next-line selector-no-qualifying-type -- add expanded styling */
&:active,
&[aria-expanded='true'] {
background-color: var(--p-color-bg-fill-tertiary-active) !important;
box-shadow: var(--p-shadow-inset-200) !important;
}

&:focus-visible {
// stylelint-disable-next-line polaris/border/polaris/at-rule-disallowed-list -- focus styles
/* stylelint-disable-next-line polaris/border/polaris/at-rule-disallowed-list -- focus styles */
@mixin no-focus-ring;
outline: var(--p-border-width-050) solid var(--p-color-border-focus);
outline-offset: var(--p-space-050);
}

// stylelint-disable-next-line selector-no-qualifying-type -- apply disabled styles
/* stylelint-disable-next-line selector-no-qualifying-type -- apply disabled styles */
&[aria-disabled='true'] {
background-color: var(--p-color-bg-fill-disabled) !important;
}

@media (--p-breakpoints-md-up) {
border: none !important;
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
/* stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY */
@mixin focus-ring base, 0;
}
}
Expand All @@ -50,17 +50,17 @@
button {
color: var(--p-color-text-critical) !important;

// stylelint-disable-next-line selector-max-combinators, selector-max-type -- override svg fill
/* stylelint-disable-next-line selector-max-combinators, selector-max-type -- override svg fill */
svg {
fill: var(--p-color-text-critical);
}

// stylelint-disable-next-line selector-max-specificity -- apply hover styles
/* stylelint-disable-next-line selector-max-specificity -- apply hover styles */
&:is(:hover, :focus) {
background-color: var(--p-color-bg-fill-tertiary-hover) !important;
}

// stylelint-disable-next-line selector-max-specificity -- apply focus/active styles
/* stylelint-disable-next-line selector-max-specificity -- apply focus/active styles */
&:active {
background-color: var(--p-color-bg-fill-tertiary-active) !important;
}
Expand Down
33 changes: 17 additions & 16 deletions polaris-react/src/components/AppProvider/global.css
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
@charset "UTF-8";
// Looking for media queries? They're imported in the postcss config in
// polaris-react/config/postcss-plugins.js

/* Looking for media queries? They're imported in the postcss config in
polaris-react/config/postcss-plugins.js */
@import '../../../../node_modules/@shopify/polaris-tokens/dist/css/styles.css';

:root {
// stylelint-disable -- Polaris component custom properties
/* stylelint-disable -- Polaris component custom properties */
--polaris-version-number: '{{POLARIS_VERSION}}';

--pg-navigation-width: 240px;
// @TODO simplify media queries so this isn't needed
/* @TODO simplify media queries so this isn't needed */
--pg-dangerous-magic-space-4: 16px;
--pg-dangerous-magic-space-5: 20px;
--pg-dangerous-magic-space-8: 32px;
Expand Down Expand Up @@ -47,7 +48,7 @@
--pg-control-vertical-padding: calc(
(36px - var(--p-font-line-height-600) - var(--p-space-050)) / 2
);
// eslint-enable
/* eslint-enable */
}

html,
Expand All @@ -71,14 +72,14 @@ html {
font-size: 100%;
-webkit-font-smoothing: antialiased;

// This needs to come after -webkit-font-smoothing
/* This needs to come after -webkit-font-smoothing */
-moz-osx-font-smoothing: grayscale;

text-size-adjust: 100%;
text-rendering: optimizeLegibility;
// Safari scrollbar styles until it adopts scrollbar-color, scrollbar-width
/* Safari scrollbar styles until it adopts scrollbar-color, scrollbar-width */
&::-webkit-scrollbar {
// Matches scrollbar-width: thin
/* Matches scrollbar-width: thin */
width: 11px;
background-color: var(--p-color-bg);
}
Expand Down Expand Up @@ -121,17 +122,17 @@ body {
margin: 0;
padding: 0;

// hardcoding background color because app provider does not have access to
// the --p-color-bg custom property. Will revisit best way to address in
// follow-up. PR convo for reference
// https://github.com/Shopify/polaris-react/pull/4636/files#r748646268
/* hardcoding background color because app provider does not have access to */
/* the --p-color-bg custom property. Will revisit best way to address in */
/* follow-up. PR convo for reference */
/* https://github.com/Shopify/polaris-react/pull/4636/files#r748646268 */
background-color: rgba(241, 242, 244, 1);
// Set all nested scroll container scrolltrack backgrounds to transparent
/* Set all nested scroll container scrolltrack backgrounds to transparent */
scrollbar-color: var(--p-color-scrollbar-thumb-bg-hover) transparent;

@media print {
// AppProvider sets styles on the body. These needs to
// be overridden using !important.
/* AppProvider sets styles on the body. These needs to */
/* be overridden using !important. */
background-color: transparent !important;
}
}
Expand Down Expand Up @@ -162,6 +163,6 @@ button::-moz-focus-inner,
}

html[class~='Polaris-Safari-16-Font-Optical-Sizing-Patch'] {
// Inter's "opsz" axis ranges from 14 to 32. This patch optimizes for smaller and less legible text by setting "opsz" 14 for all font sizes.
/* Inter's "opsz" axis ranges from 14 to 32. This patch optimizes for smaller and less legible text by setting "opsz" 14 for all font sizes. */
font-variation-settings: 'opsz' 14;
}
Loading

0 comments on commit 9c24a46

Please sign in to comment.