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

Polaris v12 border migration #10596

Merged
merged 4 commits into from
Sep 21, 2023
Merged

Polaris v12 border migration #10596

merged 4 commits into from
Sep 21, 2023

Conversation

sam-b-rose
Copy link
Member

@sam-b-rose sam-b-rose commented Sep 20, 2023

WHY are these changes introduced?

Part of #10519

WHAT is this pull request doing?

  • Migrates the CSS custom properties for border token groups
  • Migrates the component props for border

This the pre-v12 migration step to create a safe CSS custom property mapping to the new border tokens.

RegExp token search
(?:--p-border-radius-1|--p-border-radius-1_5-experimental|--p-border-radius-2|--p-border-radius-3|--p-border-radius-4|--p-border-radius-5|--p-border-radius-6|--p-border-width-1|--p-border-width-1-experimental|--p-border-width-2|--p-border-width-2-experimental|--p-border-width-4)(?!\d)

🎩 checklist

Copy link
Contributor

@sophschneider sophschneider left a comment

Choose a reason for hiding this comment

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

💫

@sam-b-rose sam-b-rose merged commit 7db26ac into main Sep 21, 2023
17 checks passed
@sam-b-rose sam-b-rose deleted the polaris-v12-border-migration branch September 21, 2023 15:16
@@ -165,7 +165,7 @@
}

// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($border-width: var(--p-border-width-2));
@include focus-ring($border-width: var(--p-border-width-0165));
Copy link
Member

@lgriffee lgriffee Sep 21, 2023

Choose a reason for hiding this comment

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

Suggested change
@include focus-ring($border-width: var(--p-border-width-0165));
@include focus-ring($border-width: var(--p-border-width-050));

--p-border-width-2 should be replaced with --p-border-width-050 here.

@@ -60,7 +60,7 @@
width: 100%;
height: 100%;
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($border-width: var(--p-border-width-2));
@include focus-ring($border-width: var(--p-border-width-0165));
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
@include focus-ring($border-width: var(--p-border-width-0165));
@include focus-ring($border-width: var(--p-border-width-050));

--p-border-width-2 should be replaced with --p-border-width-050 here.

@@ -142,13 +142,12 @@
width: 100%;
height: 100%;
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($border-width: var(--p-border-width-2));
@include focus-ring($border-width: var(--p-border-width-0165));
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
@include focus-ring($border-width: var(--p-border-width-0165));
@include focus-ring($border-width: var(--p-border-width-050));

--p-border-width-2 should be replaced with --p-border-width-050 here.

@sam-b-rose
Copy link
Member Author

Addressed incorrect manual migrations here #10608

sam-b-rose added a commit that referenced this pull request Sep 21, 2023
sam-b-rose added a commit that referenced this pull request Sep 21, 2023
sam-b-rose added a commit that referenced this pull request Sep 21, 2023
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this pull request Apr 22, 2024
### WHY are these changes introduced?

Part of Shopify#10519

### WHAT is this pull request doing?

- Migrates the CSS custom properties for `border` token groups
- Migrates the component props for `border`

This the pre-v12 migration step to create a safe CSS custom property
mapping to the new border tokens.

<details>
<summary>RegExp token search</summary>

<pre><code>(?:--p-border-radius-1|--p-border-radius-1_5-experimental|--p-border-radius-2|--p-border-radius-3|--p-border-radius-4|--p-border-radius-5|--p-border-radius-6|--p-border-width-1|--p-border-width-1-experimental|--p-border-width-2|--p-border-width-2-experimental|--p-border-width-4)(?!\d)</code></pre>
</details>

### 🎩 checklist

- [x] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [x] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [x] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [x] Updated the component's `README.md` with documentation changes
- [x] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this pull request Apr 22, 2024
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