Skip to content

Conversation

@sam-b-rose
Copy link
Member

WHY are these changes introduced?

Helps migrate away the legacy Sass color() function with the appropriate CSS custom property color token.

Related #7215

WHAT is this pull request doing?

Adds the replace-sass-color migration:

- color: color('ink');
- background: color('white');
+ color: var(--p-text);
+ background: var(--p-surface);

@sam-b-rose sam-b-rose force-pushed the color-migrations branch 3 times, most recently from 1471a5b to e9163be Compare October 10, 2022 14:20
@sam-b-rose
Copy link
Member Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @samrose3! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]
yarn add @shopify/[email protected]

@sam-b-rose
Copy link
Member Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @samrose3! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]
yarn add @shopify/[email protected]

@sam-b-rose
Copy link
Member Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @samrose3! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]
yarn add @shopify/[email protected]
yarn add @shopify/[email protected]

@sam-b-rose
Copy link
Member Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @samrose3! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]
yarn add @shopify/[email protected]
yarn add @shopify/[email protected]

Comment on lines 6 to 12
export interface ColorHue {
[hue: string]: ColorValue;
}

export interface ColorValue {
[value: string]: string;
}
Copy link
Member

Choose a reason for hiding this comment

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

Note: These aren't used anymore

Comment on lines 30 to 37
const propertyMap: {[key: string]: keyof typeof maps} = {
color: 'colorMap',
background: 'backgroundColorMap',
'background-color': 'backgroundColorMap',
border: 'borderColorMap',
'border-color': 'borderColorMap',
fill: 'fillColorMap',
};
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const propertyMap: {[key: string]: keyof typeof maps} = {
color: 'colorMap',
background: 'backgroundColorMap',
'background-color': 'backgroundColorMap',
border: 'borderColorMap',
'border-color': 'borderColorMap',
fill: 'fillColorMap',
};
const propertyMaps = {
color: colorMap,
background: backgroundColorMap,
'background-color': backgroundColorMap,
border: borderColorMap,
'border-color': borderColorMap,
fill: fillColorMap,
};

question: Is it possible to add the maps here and remove the intermediary propertyMapKey access?

Comment on lines 50 to 52
const polarisCustomPropertyRegEx = new RegExp(
String.raw`--p-(${tokenColorsKeys.join('|')})`,
);
Copy link
Member

Choose a reason for hiding this comment

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

Light suggestion to use the createVar util from Polaris tokens:

Object.keys(colorsTokenGroup).map(createVar).join('|')

e.g.
image

Copy link
Contributor

@lgriffee lgriffee left a comment

Choose a reason for hiding this comment

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

💯🎉✨

color: colorMap,
background: backgroundColorMap,
'background-color': backgroundColorMap,
border: borderColorMap,
Copy link
Contributor

Choose a reason for hiding this comment

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

After the border() migration work merges in it might be worth also adding the following properties to this script as well:

  • border-top
  • border-right
  • border-bottom
  • border-left

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes, great idea! 👍

@sam-b-rose sam-b-rose mentioned this pull request Oct 20, 2022
19 tasks
@sam-b-rose sam-b-rose merged commit 8194e17 into main Oct 21, 2022
@sam-b-rose sam-b-rose deleted the color-migrations branch October 21, 2022 20:15
@github-actions github-actions bot mentioned this pull request Oct 21, 2022
laurkim pushed a commit that referenced this pull request Oct 26, 2022
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/[email protected]

### Minor Changes

- [#7342](#7342)
[`dc2990acf`](dc2990a)
Thanks [@lgriffee](https://github.com/lgriffee)! - Rename
`replace-sass-length` migration to `replace-spacing-lengths`
    Add `gap` properties to `replace-sass-space` migration


- [#7310](#7310)
[`8fb215836`](8fb2158)
Thanks [@gwyneplaine](https://github.com/gwyneplaine)! - Add sass
z-index migration


- [#7335](#7335)
[`699b70713`](699b707)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Add
migration to `replace-typography-declarations`


- [#7375](#7375)
[`8194e174a`](8194e17)
Thanks [@samrose3](https://github.com/samrose3)! - Add Sass color
function migration


- [#7387](#7387)
[`a0fa96ed4`](a0fa96e)
Thanks [@lgriffee](https://github.com/lgriffee)! - Added
`replace-border-declarations` migration

### Patch Changes

- [#7487](#7487)
[`60191f353`](60191f3)
Thanks [@lgriffee](https://github.com/lgriffee)! - Update numeric
operator detection to check all node types


- [#7412](#7412)
[`6d82e8965`](6d82e89)
Thanks [@samrose3](https://github.com/samrose3)! - Enhance Sass spacing
migration to catch Sass interpolations


- [#7423](#7423)
[`0591db26a`](0591db2)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Update
`replace-typography-declarations` comment strategy


- [#7459](#7459)
[`933a9d71b`](933a9d7)
Thanks [@lgriffee](https://github.com/lgriffee)! - Add missing values to
borderRadius length map

## @shopify/[email protected]

### Minor Changes

- [#7429](#7429)
[`e87330f5b`](e87330f)
Thanks [@aveline](https://github.com/aveline)! - Update `AlphaStack` to
be polymorphic, add list reset styles and allow spacing to change based
on breakpoint


- [#7450](#7450)
[`74237ebc5`](74237eb)
Thanks [@laurkim](https://github.com/laurkim)! - Added support for
`minHeight` and `minWidth` on `Box`


- [#7432](#7432)
[`85c022033`](85c0220)
Thanks [@laurkim](https://github.com/laurkim)! - Added support for
`overflowX`, `overflowY`, and `width` props to `Box`


- [#7491](#7491)
[`a9eff60e1`](a9eff60)
Thanks [@laurkim](https://github.com/laurkim)! - Added support for
`section` element types to `Box`


- [#7389](#7389)
[`4f184c658`](4f184c6)
Thanks [@aveline](https://github.com/aveline)! - Added `breakWord` prop
to `Text`


- [#7430](#7430)
[`87981ac01`](87981ac)
Thanks [@aveline](https://github.com/aveline)! - Added `legend` as
supported element for `Text` component


- [#7438](#7438)
[`4b4411930`](4b44119)
Thanks [@aveline](https://github.com/aveline)! - Simplify `AlphaCard` by
removing `hasBorderRadius` prop and `shadow` prop

### Patch Changes

- [#7405](#7405)
[`ea2615881`](ea26158)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed how optional `id`
prop rendered in `Text`


- [#7434](#7434)
[`62288755a`](6228875)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed reference to
`width` custom property in `Box`


- [#7396](#7396)
[`3be27bae8`](3be27ba)
Thanks [@laurkim](https://github.com/laurkim)! - Updated alias and scale
types in `Box` with type tests to check they exist in our token groups.
    Updated `get-props` script to parse utility types with unions.


- [#7435](#7435)
[`4097d539b`](4097d53)
Thanks [@kyledurand](https://github.com/kyledurand)! - Cleaned up styles
on Box


- [#7410](#7410)
[`13546e344`](13546e3)
Thanks [@kyledurand](https://github.com/kyledurand)! - Apply default
value to Columns


- [#7483](#7483)
[`7a78e07bf`](7a78e07)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed missing
`children` prop in `Columns` prop type


- [#7492](#7492)
[`612e02786`](612e027)
Thanks [@henryyi](https://github.com/henryyi)! - Only apply scroll-lock
with scrollbar when body is scrollable


- [#7481](#7481)
[`4f5f6cf50`](4f5f6cf)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed horizontal
scroll bug in Scrollable


- [#7471](#7471)
[`2f16b2310`](2f16b23)
Thanks [@kyledurand](https://github.com/kyledurand)! - Added viewport
addon to storybook and matched values to our breakpoints


- [#7433](#7433)
[`90487fd70`](90487fd)
Thanks [@kellymiller-shop](https://github.com/kellymiller-shop)! - fixed
heading offset on IndexTable


- [#7453](#7453)
[`3191d13c0`](3191d13)
Thanks [@AndrewMusgrave](https://github.com/AndrewMusgrave)! - Fixed
IndexTable not rendering bulk actions on resize


- [#7395](#7395)
[`89fea23f7`](89fea23)
Thanks [@AndrewMusgrave](https://github.com/AndrewMusgrave)! - Fixed
scrolling performance for IndexTable on safari


- [#7442](#7442)
[`77057d5b6`](77057d5)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fix SkeletonPage
actions layout

## @shopify/[email protected]

### Patch Changes

- Updated dependencies
\[[`dc2990acf`](dc2990a),
[`8fb215836`](8fb2158),
[`60191f353`](60191f3),
[`6d82e8965`](6d82e89),
[`0591db26a`](0591db2),
[`699b70713`](699b707),
[`8194e174a`](8194e17),
[`933a9d71b`](933a9d7),
[`a0fa96ed4`](a0fa96e)]:
    -   @shopify/[email protected]

## [email protected]

### Minor Changes

- [#7396](#7396)
[`3be27bae8`](3be27ba)
Thanks [@laurkim](https://github.com/laurkim)! - Updated alias and scale
types in `Box` with type tests to check they exist in our token groups.
    Updated `get-props` script to parse utility types with unions.


- [#7393](#7393)
[`cd4577a3f`](cd4577a)
Thanks [@martenbjork](https://github.com/martenbjork)! - Update markdown
file structure to not require folders for each markdown page. Now you
can simply create a markdown.md file and it will be rendered as a page.


- [#7418](#7418)
[`c5cfe9026`](c5cfe90)
Thanks [@kyledurand](https://github.com/kyledurand)! - Support beta
status components


- [#7449](#7449)
[`31437dfb2`](31437df)
Thanks [@jjgali](https://github.com/jjgali)! - Removed duplicate image
on error messages page.


- [#7491](#7491)
[`a9eff60e1`](a9eff60)
Thanks [@laurkim](https://github.com/laurkim)! - Added support for
`section` element types to `Box`


- [#7441](#7441)
[`0d446cb46`](0d446cb)
Thanks [@jjgali](https://github.com/jjgali)! - Remove section on
compositions vs patterns.


- [#7420](#7420)
[`d462e2e5d`](d462e2e)
Thanks [@jjgali](https://github.com/jjgali)! - Clarify rules for
formatting dates and times.

### Patch Changes

- [#7398](#7398)
[`c21ea83d4`](c21ea83)
Thanks [@tracytees](https://github.com/tracytees)! - - Updated the
"Getting started" and "Polaris 101" pages
- Removed /design, /develop, /shared-resources,
/patterns-components-and-tokens


- [#7426](#7426)
[`442d51ddf`](442d51d)
Thanks [@gwyneplaine](https://github.com/gwyneplaine)! - Fix nav bug


- [#7452](#7452)
[`8a05e840f`](8a05e84)
Thanks [@gui-santos](https://github.com/gui-santos)! - fix visual bug in
token page's navigation where text was breaking into two lines


- [#7469](#7469)
[`a8d80cfd5`](a8d80cf)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `AlphaStack`
component docs


- [#7483](#7483)
[`7a78e07bf`](7a78e07)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed missing
`children` prop in `Columns` prop type


- [#7436](#7436)
[`95a8ac0d2`](95a8ac0)
Thanks [@laurkim](https://github.com/laurkim)! - Fix references to `Box`
margin props in alpha layout examples


- [#7444](#7444)
[`e0d6b00ba`](e0d6b00)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fix tile image
and increase example widths on alpha components


- [#7427](#7427)
[`25e70a8d9`](25e70a8)
Thanks [@gwyneplaine](https://github.com/gwyneplaine)! - Declare sandbox
nav in a more idiomatic fashion


- [#7482](#7482)
[`0ec6961aa`](0ec6961)
Thanks [@alex-page](https://github.com/alex-page)! - Bump GitHub action
versions and add p-map for gen-og-images CI failure

- Updated dependencies
\[[`ea2615881`](ea26158),
[`62288755a`](6228875),
[`3be27bae8`](3be27ba),
[`4097d539b`](4097d53),
[`e87330f5b`](e87330f),
[`74237ebc5`](74237eb),
[`85c022033`](85c0220),
[`13546e344`](13546e3),
[`7a78e07bf`](7a78e07),
[`a9eff60e1`](a9eff60),
[`4f184c658`](4f184c6),
[`612e02786`](612e027),
[`4f5f6cf50`](4f5f6cf),
[`87981ac01`](87981ac),
[`2f16b2310`](2f16b23),
[`90487fd70`](90487fd),
[`3191d13c0`](3191d13),
[`89fea23f7`](89fea23),
[`77057d5b6`](77057d5),
[`4b4411930`](4b44119)]:
    -   @shopify/[email protected]

## [email protected]

### Patch Changes

- Updated dependencies
\[[`ea2615881`](ea26158),
[`62288755a`](6228875),
[`3be27bae8`](3be27ba),
[`4097d539b`](4097d53),
[`e87330f5b`](e87330f),
[`74237ebc5`](74237eb),
[`85c022033`](85c0220),
[`13546e344`](13546e3),
[`7a78e07bf`](7a78e07),
[`a9eff60e1`](a9eff60),
[`4f184c658`](4f184c6),
[`612e02786`](612e027),
[`4f5f6cf50`](4f5f6cf),
[`87981ac01`](87981ac),
[`2f16b2310`](2f16b23),
[`90487fd70`](90487fd),
[`3191d13c0`](3191d13),
[`89fea23f7`](89fea23),
[`77057d5b6`](77057d5),
[`4b4411930`](4b44119)]:
    -   @shopify/[email protected]

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

3 participants