Skip to content

fix: Refactor Box border radius properties #9108

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 27, 2023
Merged

fix: Refactor Box border radius properties #9108

merged 2 commits into from
Apr 27, 2023

Conversation

@laurkim
Copy link
Contributor

@laurkim laurkim commented Apr 27, 2023 •
edited
Loading

WHY are these changes introduced?

There was an issue with cascading CSS in production where the border-radius property was ordered after border-radius-end-start and border-radius-end-end and overriding border radius to 0. The border radius styles were not showing up as expected in the admin.


Bug in admin production
Bug in admin production

WHAT is this pull request doing?

Refactors Box styles to remove the border-radius shorthand, set the --pc-box-border-radius custom property to initial, and use the --pc-box-border-radius property as the value to initialize the widths for --pc-box-border-radius-start-start, --pc-box-border-radius-start-end, --pc-box-border-radius-end-start, and --pc-box-border-radius-end-end.

We tophatted this change in staging (c0n6) as the border width properties were cascading in the expected order in Storybook and Spin instances.


Card with 0 border bottom radius
Card with 0 border bottom radius


Card with correct border bottom radius
Card with correct border bottom radius

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

🎩 checklist

@laurkim
Copy link
Contributor Author

laurkim commented Apr 27, 2023

/snapit

@github-actions
Copy link
Contributor

github-actions bot commented Apr 27, 2023

size-limit report 📦

Path Size
polaris-react-cjs 241.3 KB (0%)
polaris-react-esm 157.08 KB (0%)
polaris-react-esnext 219.55 KB (-0.01% 🔽)
polaris-react-css 47.47 KB (-0.01% 🔽)

@github-actions
Copy link
Contributor

github-actions bot commented Apr 27, 2023

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

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

yarn add @shopify/polaris-cli@0.0.0-snapshot-release-20230427155459
yarn add @shopify/polaris-codemods@0.0.0-snapshot-release-20230427155459
yarn add @shopify/polaris-migrator@0.0.0-snapshot-release-20230427155459
yarn add @shopify/polaris@0.0.0-snapshot-release-20230427155459
yarn add @shopify/stylelint-polaris@0.0.0-snapshot-release-20230427155459

@laurkim laurkim self-assigned this Apr 27, 2023
@laurkim laurkim marked this pull request as ready for review April 27, 2023 17:09
@laurkim laurkim merged commit 6fd00eb into main Apr 27, 2023
@laurkim laurkim deleted the lo/fix-box-border-radius branch April 27, 2023 17:15
@shopify-github-actions-access shopify-github-actions-access bot mentioned this pull request Apr 27, 2023
Merged
laurkim pushed a commit that referenced this pull request Apr 27, 2023
@shopify-github-actions-access @github-actions
Version Packages (#9078)
367712c 
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/stylelint-polaris@12.0.0

### Major Changes

- [#9097](#9097)
[`a64e38948`](a64e389)
Thanks [@samrose3](https://github.com/samrose3)! - Re-enabled layout
warnings for coverage rules

## @shopify/polaris@10.49.0

### Minor Changes

- [#9051](#9051)
[`722b818b4`](722b818)
Thanks [@camielvs](https://github.com/camielvs)! - Added `integer`
option for the `type` prop of TextField

### Patch Changes

- [#9108](#9108)
[`6fd00eb7c`](6fd00eb)
Thanks [@laurkim](https://github.com/laurkim)! - Refactored `Box` border
radius properties to fix issues with cascading styles

## @shopify/polaris-cli@0.1.35

### Patch Changes

-   Updated dependencies \[]:
    -   @shopify/polaris-migrator@0.18.1

## @shopify/polaris-codemods@0.1.1

### Patch Changes

- Updated dependencies
\[[`a64e38948`](a64e389)]:
    -   @shopify/stylelint-polaris@12.0.0

## @shopify/polaris-migrator@0.18.1

### Patch Changes

- Updated dependencies
\[[`a64e38948`](a64e389)]:
    -   @shopify/stylelint-polaris@12.0.0

## polaris.shopify.com@0.52.0

### Minor Changes

- [#9087](#9087)
[`efdcb10db`](efdcb10)
Thanks [@lgriffee](https://github.com/lgriffee)! - Updated instances of
"spacing" on `/design/space` to "space".


- [#9088](#9088)
[`13e9b1e47`](13e9b1e)
Thanks [@lgriffee](https://github.com/lgriffee)! - Matched v11 migration
docs to migrations folder


- [#9099](#9099)
[`227490514`](2274905)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `ButtonGroup`
examples to include `Pressed with segmented buttons`

### Patch Changes

- [#9062](#9062)
[`32dae8638`](32dae86)
Thanks [@jesstelford](https://github.com/jesstelford)! - Merge
/patterns-legacy index page into /patterns to ease discovery (includes a
redirect so old links continue to work)

- Updated dependencies
\[[`722b818b4`](722b818),
[`6fd00eb7c`](6fd00eb)]:
    -   @shopify/polaris@10.49.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
@laurkim @juzser
fix: Refactor Box border radius properties (Shopify#9108)
afb7b87 
### WHY are these changes introduced?

There was an issue with cascading CSS in production where the
`border-radius` property was ordered after `border-radius-end-start` and
`border-radius-end-end` and overriding border radius to 0. The border
radius styles were not showing up as expected in the admin.
    <details>
      <summary>Bug in admin production</summary>
<img
src="https://user-images.githubusercontent.com/26749317/234937073-138efe8e-d7f9-40e5-ad4e-4eba9ab59136.png"
alt="Bug in admin production">
    </details>

### WHAT is this pull request doing?

Refactors Box styles to remove the `border-radius` shorthand, set the
`--pc-box-border-radius` custom property to `initial`, and use the
`--pc-box-border-radius` property as the value to initialize the widths
for `--pc-box-border-radius-start-start`,
`--pc-box-border-radius-start-end`, `--pc-box-border-radius-end-start`,
and `--pc-box-border-radius-end-end`.

We tophatted this change in staging (`c0n6`) as the border width
properties were cascading in the expected order in Storybook and Spin
instances.
    <details>
      <summary>Card with 0 border bottom radius</summary>
<img
src="https://user-images.githubusercontent.com/26749317/234937453-6c5fa294-3563-4ded-bffd-6ffa56294595.png"
alt="Card with 0 border bottom radius">
    </details>
    <details>
      <summary>Card with correct border bottom radius</summary>
<img
src="https://user-images.githubusercontent.com/26749317/234937448-9943cdb4-d28a-4bd9-988d-bcf0f6331d7c.png"
alt="Card with correct border bottom radius">
    </details>

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

### 🎩 checklist

- [ ] 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)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
@shopify-github-actions-access @github-actions @juzser
Version Packages (Shopify#9078)
809bf65 
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/stylelint-polaris@12.0.0

### Major Changes

- [Shopify#9097](Shopify#9097)
[`a64e38948`](Shopify@a64e389)
Thanks [@samrose3](https://github.com/samrose3)! - Re-enabled layout
warnings for coverage rules

## @shopify/polaris@10.49.0

### Minor Changes

- [Shopify#9051](Shopify#9051)
[`722b818b4`](Shopify@722b818)
Thanks [@camielvs](https://github.com/camielvs)! - Added `integer`
option for the `type` prop of TextField

### Patch Changes

- [Shopify#9108](Shopify#9108)
[`6fd00eb7c`](Shopify@6fd00eb)
Thanks [@laurkim](https://github.com/laurkim)! - Refactored `Box` border
radius properties to fix issues with cascading styles

## @shopify/polaris-cli@0.1.35

### Patch Changes

-   Updated dependencies \[]:
    -   @shopify/polaris-migrator@0.18.1

## @shopify/polaris-codemods@0.1.1

### Patch Changes

- Updated dependencies
\[[`a64e38948`](Shopify@a64e389)]:
    -   @shopify/stylelint-polaris@12.0.0

## @shopify/polaris-migrator@0.18.1

### Patch Changes

- Updated dependencies
\[[`a64e38948`](Shopify@a64e389)]:
    -   @shopify/stylelint-polaris@12.0.0

## polaris.shopify.com@0.52.0

### Minor Changes

- [Shopify#9087](Shopify#9087)
[`efdcb10db`](Shopify@efdcb10)
Thanks [@lgriffee](https://github.com/lgriffee)! - Updated instances of
"spacing" on `/design/space` to "space".


- [Shopify#9088](Shopify#9088)
[`13e9b1e47`](Shopify@13e9b1e)
Thanks [@lgriffee](https://github.com/lgriffee)! - Matched v11 migration
docs to migrations folder


- [Shopify#9099](Shopify#9099)
[`227490514`](Shopify@2274905)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `ButtonGroup`
examples to include `Pressed with segmented buttons`

### Patch Changes

- [Shopify#9062](Shopify#9062)
[`32dae8638`](Shopify@32dae86)
Thanks [@jesstelford](https://github.com/jesstelford)! - Merge
/patterns-legacy index page into /patterns to ease discovery (includes a
redirect so old links continue to work)

- Updated dependencies
\[[`722b818b4`](Shopify@722b818),
[`6fd00eb7c`](Shopify@6fd00eb)]:
    -   @shopify/polaris@10.49.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this pull request Apr 22, 2024
@laurkim
fix: Refactor Box border radius properties (Shopify#9108)
6c91a77 
### WHY are these changes introduced?

There was an issue with cascading CSS in production where the
`border-radius` property was ordered after `border-radius-end-start` and
`border-radius-end-end` and overriding border radius to 0. The border
radius styles were not showing up as expected in the admin.
    <details>
      <summary>Bug in admin production</summary>
<img
src="https://user-images.githubusercontent.com/26749317/234937073-138efe8e-d7f9-40e5-ad4e-4eba9ab59136.png"
alt="Bug in admin production">
    </details>

### WHAT is this pull request doing?

Refactors Box styles to remove the `border-radius` shorthand, set the
`--pc-box-border-radius` custom property to `initial`, and use the
`--pc-box-border-radius` property as the value to initialize the widths
for `--pc-box-border-radius-start-start`,
`--pc-box-border-radius-start-end`, `--pc-box-border-radius-end-start`,
and `--pc-box-border-radius-end-end`.

We tophatted this change in staging (`c0n6`) as the border width
properties were cascading in the expected order in Storybook and Spin
instances.
    <details>
      <summary>Card with 0 border bottom radius</summary>
<img
src="https://user-images.githubusercontent.com/26749317/234937453-6c5fa294-3563-4ded-bffd-6ffa56294595.png"
alt="Card with 0 border bottom radius">
    </details>
    <details>
      <summary>Card with correct border bottom radius</summary>
<img
src="https://user-images.githubusercontent.com/26749317/234937448-9943cdb4-d28a-4bd9-988d-bcf0f6331d7c.png"
alt="Card with correct border bottom radius">
    </details>

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

### 🎩 checklist

- [ ] 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)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
ascherkus pushed a commit to ascherkus/polaris that referenced this pull request Feb 19, 2025
@shopify-github-actions-access @github-actions
Version Packages (Shopify#9078)
46354ea 
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/stylelint-polaris@12.0.0

### Major Changes

- [Shopify#9097](Shopify#9097)
[`a64e38948`](Shopify@d289bb9)
Thanks [@samrose3](https://github.com/samrose3)! - Re-enabled layout
warnings for coverage rules

## @shopify/polaris@10.49.0

### Minor Changes

- [Shopify#9051](Shopify#9051)
[`722b818b4`](Shopify@a7b28bf)
Thanks [@camielvs](https://github.com/camielvs)! - Added `integer`
option for the `type` prop of TextField

### Patch Changes

- [Shopify#9108](Shopify#9108)
[`6fd00eb7c`](Shopify@6c91a77)
Thanks [@laurkim](https://github.com/laurkim)! - Refactored `Box` border
radius properties to fix issues with cascading styles

## @shopify/polaris-cli@0.1.35

### Patch Changes

-   Updated dependencies \[]:
    -   @shopify/polaris-migrator@0.18.1

## @shopify/polaris-codemods@0.1.1

### Patch Changes

- Updated dependencies
\[[`a64e38948`](Shopify@d289bb9)]:
    -   @shopify/stylelint-polaris@12.0.0

## @shopify/polaris-migrator@0.18.1

### Patch Changes

- Updated dependencies
\[[`a64e38948`](Shopify@d289bb9)]:
    -   @shopify/stylelint-polaris@12.0.0

## polaris.shopify.com@0.52.0

### Minor Changes

- [Shopify#9087](Shopify#9087)
[`efdcb10db`](Shopify@1a72f1e)
Thanks [@lgriffee](https://github.com/lgriffee)! - Updated instances of
"spacing" on `/design/space` to "space".


- [Shopify#9088](Shopify#9088)
[`13e9b1e47`](Shopify@33a15a2)
Thanks [@lgriffee](https://github.com/lgriffee)! - Matched v11 migration
docs to migrations folder


- [Shopify#9099](Shopify#9099)
[`227490514`](Shopify@d525b82)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `ButtonGroup`
examples to include `Pressed with segmented buttons`

### Patch Changes

- [Shopify#9062](Shopify#9062)
[`32dae8638`](Shopify@3c7b4fc)
Thanks [@jesstelford](https://github.com/jesstelford)! - Merge
/patterns-legacy index page into /patterns to ease discovery (includes a
redirect so old links continue to work)

- Updated dependencies
\[[`722b818b4`](Shopify@a7b28bf),
[`6fd00eb7c`](Shopify@6c91a77)]:
    -   @shopify/polaris@10.49.0

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

Reviewers

@aaronccasanova aaronccasanova aaronccasanova approved these changes

@lgriffee lgriffee Awaiting requested review from lgriffee

Assignees

@laurkim laurkim

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@laurkim @aaronccasanova