Skip to content

Conversation

@laurkim
Copy link
Contributor

@laurkim laurkim commented Nov 14, 2022

WHY are these changes introduced?

v10.11.0 introduced a visual spacing regression for a specific edge case of ActionList where there are section but no titles. We didn't have a storybook example for that so I've added in an example in a temporary commit as this is not a pattern we necessarily encourage. It also introduced a visual spacing regression for Modal.Header on the xs-sm breakpoints that resulted in the header not being centered.

WHAT is this pull request doing?

  • Refactors styling for ActionList
  • Refactors vertical spacing of Modal.Header
    ActionList — Section no title before ActionList — Section no title before
    ActionList — Section no title after ActionList — Section no title after
    Modal.Header — xs breakpoint before Modal.Header — xs breakpoint before
    Modal.Header — xs breakpoint after Modal.Header — xs breakpoint after

How to 🎩

Storybook URL

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

🎩 checklist

@github-actions
Copy link
Contributor

github-actions bot commented Nov 14, 2022

size-limit report 📦

Path Size
polaris-react-cjs 210.15 KB (-0.01% 🔽)
polaris-react-esm 136.23 KB (0%)
polaris-react-esnext 191.45 KB (-0.01% 🔽)
polaris-react-css 41.4 KB (-0.04% 🔽)

@laurkim laurkim marked this pull request as ready for review November 14, 2022 16:22
@laurkim laurkim self-assigned this Nov 14, 2022
@laurkim
Copy link
Contributor Author

laurkim commented Nov 14, 2022

/snapit

@github-actions
Copy link
Contributor

🫰✨ 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/[email protected]
yarn add @shopify/[email protected]
yarn add @shopify/[email protected]

@laurkim laurkim marked this pull request as draft November 14, 2022 16:35
@laurkim laurkim force-pushed the lo/fix-v10.11.0-regressions branch from 9ed0d49 to d9f3bbb Compare November 14, 2022 16:56
@laurkim
Copy link
Contributor Author

laurkim commented Nov 14, 2022

/snapit

@github-actions
Copy link
Contributor

🫰✨ 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/[email protected]
yarn add @shopify/[email protected]
yarn add @shopify/[email protected]

@laurkim laurkim force-pushed the lo/fix-v10.11.0-regressions branch from d9f3bbb to e0795ec Compare November 14, 2022 17:11
@laurkim laurkim force-pushed the lo/fix-v10.11.0-regressions branch from e0795ec to b1f6f4e Compare November 14, 2022 17:25
@laurkim
Copy link
Contributor Author

laurkim commented Nov 14, 2022

/snapit

@laurkim laurkim marked this pull request as ready for review November 14, 2022 17:48
@github-actions
Copy link
Contributor

🫰✨ 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/[email protected]
yarn add @shopify/[email protected]
yarn add @shopify/[email protected]

@laurkim
Copy link
Contributor Author

laurkim commented Nov 14, 2022

Tophatted the ActionList visual regression with the dev who found it on the notebooks team and new snapshot confirmed fix
https://admin.web.bump-polaris.ben-milton.us.spin.dev/store/shop1/apps/notebooks-dev/1

Copy link
Contributor

@aveline aveline left a comment

Choose a reason for hiding this comment

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

I think it's worth leaving the story for Sections with no Titles. It will be good to have to catch any visual regressions when this component gets refactored with the layout primitives.

Copy link
Contributor

@sarahill sarahill left a comment

Choose a reason for hiding this comment

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

🎉

@laurkim laurkim merged commit 6b915e0 into main Nov 15, 2022
@laurkim laurkim deleted the lo/fix-v10.11.0-regressions branch November 15, 2022 20:51
@github-actions github-actions bot mentioned this pull request Nov 15, 2022
laurkim pushed a commit that referenced this pull request Nov 15, 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

- [#7648](#7648)
[`c08780522`](c087805)
Thanks [@samrose3](https://github.com/samrose3)! - Added `animation`
properties to tokenize motion migration

### Patch Changes

- [#7690](#7690)
[`2ce4d38a2`](2ce4d38)
Thanks [@samrose3](https://github.com/samrose3)! - Ignore replacing type
imports and warn when encountering unknown props for Text component
migration


- [#7679](#7679)
[`566dc20c2`](566dc20)
Thanks [@isaacbowen](https://github.com/isaacbowen)! - Add the (missing)
case for migrating DisplayText to Text when size is not specified

## @shopify/[email protected]

### Minor Changes

- [#7684](#7684)
[`2e5f8348b`](2e5f834)
Thanks [@ananyaneogi](https://github.com/ananyaneogi)! - Update the
`CalloutCard` `title` prop to accept a ReactNode type


- [#7588](#7588)
[`c1c8f73b0`](c1c8f73)
Thanks [@rcaplanshopify](https://github.com/rcaplanshopify)! - Added
optional `captureOverscroll` prop to `Popover`


- [#6986](#6986)
[`270887fcf`](270887f)
Thanks [@danielle-dsouza](https://github.com/danielle-dsouza)! -
Clicking on the modal backdrop triggers the pressed state of the modal's
close button

### Patch Changes

- [#7693](#7693)
[`bdf6fd31d`](bdf6fd3)
Thanks [@philschoefer](https://github.com/philschoefer)! - Fixed a bug
where `DataTable` summary row would not properly inherit type defined in
`columnContentTypes` prop


- [#7578](#7578)
[`217f2f8ed`](217f2f8)
Thanks [@haskelash-shopify](https://github.com/haskelash-shopify)! -
Hide bulk actions popup when all items are deselected.


- [#7710](#7710)
[`6b915e01e`](6b915e0)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed visual spacing
regressions on `ActionList` and `Modal.Header`

## @shopify/[email protected]

### Patch Changes

- Updated dependencies
\[[`2ce4d38a2`](2ce4d38),
[`c08780522`](c087805),
[`566dc20c2`](566dc20)]:
    -   @shopify/[email protected]

## [email protected]

### Minor Changes

- [#7721](#7721)
[`f490f8e7c`](f490f8e)
Thanks [@jjgali](https://github.com/jjgali)! - Updated and realigned
values in Unit of measurement abbreviations table


- [#7661](#7661)
[`e1a3c62d2`](e1a3c62)
Thanks [@jjgali](https://github.com/jjgali)! - Added keywords to Naming
page.


- [#7712](#7712)
[`88e1d875f`](88e1d87)
Thanks [@jjgali](https://github.com/jjgali)! - Fixed abbrevation for
pounds from lbs to lb.

### Patch Changes

- [#7711](#7711)
[`3984f26b6`](3984f26)
Thanks [@nayeobkim](https://github.com/nayeobkim)! - Updated the related
components copy in `AlphaStack`


- [#7569](#7569)
[`ffe1e9230`](ffe1e92)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `Tiles`
component guidance and examples


- [#7698](#7698)
[`f0e75bd0c`](f0e75bd)
Thanks [@nayeobkim](https://github.com/nayeobkim)! - Updated the Related
components links in Columns


- [#7713](#7713)
[`b649af84f`](b649af8)
Thanks [@nayeobkim](https://github.com/nayeobkim)! - Updated the related
components links in `Tiles`


- [#7594](#7594)
[`faf3dc646`](faf3dc6)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `Box`
component guidance and examples


- [#7659](#7659)
[`d28259746`](d282597)
Thanks [@kyledurand](https://github.com/kyledurand)! - Added global
styles from deps instead of linking unpkg

- Updated dependencies
\[[`bdf6fd31d`](bdf6fd3),
[`217f2f8ed`](217f2f8),
[`2e5f8348b`](2e5f834),
[`c1c8f73b0`](c1c8f73),
[`270887fcf`](270887f),
[`6b915e01e`](6b915e0)]:
    -   @shopify/[email protected]

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
<!--
  ☝️How to write a good PR title:
- Prefix it with [ComponentName] (if applicable), for example: [Button]
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

### WHY are these changes introduced?

v10.11.0 introduced a visual spacing regression for a specific edge case
of `ActionList` where there are section but no titles. We didn't have a
storybook example for that so I've added in an example in a **temporary
commit** as this is not a pattern we necessarily encourage. It also
introduced a visual spacing regression for `Modal.Header` on the xs-sm
breakpoints that resulted in the header not being centered.

### WHAT is this pull request doing?

* Refactors styling for ActionList
* Refactors vertical spacing of Modal.Header
    <details>
      <summary>ActionList — Section no title before</summary>
<img
src="https://user-images.githubusercontent.com/26749317/201711657-9ab79577-2d6c-48a9-ad8e-0ad83b9311ba.png"
alt="ActionList — Section no title before">
    </details>
        <details>
      <summary>ActionList — Section no title after</summary>
<img
src="https://user-images.githubusercontent.com/26749317/201719792-3df23996-7f6f-4e7e-84c5-6e1f3ad91c95.png"
alt="ActionList — Section no title after">
    </details>
    <details>
      <summary>Modal.Header — xs breakpoint before</summary>
<img
src="https://user-images.githubusercontent.com/26749317/201711661-3242963a-08db-493a-a20f-2de759a13d69.png"
alt="Modal.Header — xs breakpoint before">
    </details>
        <details>
      <summary>Modal.Header — xs breakpoint after</summary>
<img
src="https://user-images.githubusercontent.com/26749317/201711660-450ac6fd-23ea-4daa-ae77-018f0495f9c5.png"
alt="Modal.Header — xs breakpoint after">
    </details>

<!-- ℹ️ Delete the following for small / trivial changes -->

### How to 🎩

[Storybook
URL](https://5d559397bae39100201eedc1-lnhdfnrwrt.chromatic.com/?path=/story/all-components-actionlist--with-sections-no-titles)

🖥 [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

- [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)
- [ ] 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
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

- [Shopify#7648](Shopify#7648)
[`c08780522`](Shopify@c087805)
Thanks [@samrose3](https://github.com/samrose3)! - Added `animation`
properties to tokenize motion migration

### Patch Changes

- [Shopify#7690](Shopify#7690)
[`2ce4d38a2`](Shopify@2ce4d38)
Thanks [@samrose3](https://github.com/samrose3)! - Ignore replacing type
imports and warn when encountering unknown props for Text component
migration


- [Shopify#7679](Shopify#7679)
[`566dc20c2`](Shopify@566dc20)
Thanks [@isaacbowen](https://github.com/isaacbowen)! - Add the (missing)
case for migrating DisplayText to Text when size is not specified

## @shopify/[email protected]

### Minor Changes

- [Shopify#7684](Shopify#7684)
[`2e5f8348b`](Shopify@2e5f834)
Thanks [@ananyaneogi](https://github.com/ananyaneogi)! - Update the
`CalloutCard` `title` prop to accept a ReactNode type


- [Shopify#7588](Shopify#7588)
[`c1c8f73b0`](Shopify@c1c8f73)
Thanks [@rcaplanshopify](https://github.com/rcaplanshopify)! - Added
optional `captureOverscroll` prop to `Popover`


- [Shopify#6986](Shopify#6986)
[`270887fcf`](Shopify@270887f)
Thanks [@danielle-dsouza](https://github.com/danielle-dsouza)! -
Clicking on the modal backdrop triggers the pressed state of the modal's
close button

### Patch Changes

- [Shopify#7693](Shopify#7693)
[`bdf6fd31d`](Shopify@bdf6fd3)
Thanks [@philschoefer](https://github.com/philschoefer)! - Fixed a bug
where `DataTable` summary row would not properly inherit type defined in
`columnContentTypes` prop


- [Shopify#7578](Shopify#7578)
[`217f2f8ed`](Shopify@217f2f8)
Thanks [@haskelash-shopify](https://github.com/haskelash-shopify)! -
Hide bulk actions popup when all items are deselected.


- [Shopify#7710](Shopify#7710)
[`6b915e01e`](Shopify@6b915e0)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed visual spacing
regressions on `ActionList` and `Modal.Header`

## @shopify/[email protected]

### Patch Changes

- Updated dependencies
\[[`2ce4d38a2`](Shopify@2ce4d38),
[`c08780522`](Shopify@c087805),
[`566dc20c2`](Shopify@566dc20)]:
    -   @shopify/[email protected]

## [email protected]

### Minor Changes

- [Shopify#7721](Shopify#7721)
[`f490f8e7c`](Shopify@f490f8e)
Thanks [@jjgali](https://github.com/jjgali)! - Updated and realigned
values in Unit of measurement abbreviations table


- [Shopify#7661](Shopify#7661)
[`e1a3c62d2`](Shopify@e1a3c62)
Thanks [@jjgali](https://github.com/jjgali)! - Added keywords to Naming
page.


- [Shopify#7712](Shopify#7712)
[`88e1d875f`](Shopify@88e1d87)
Thanks [@jjgali](https://github.com/jjgali)! - Fixed abbrevation for
pounds from lbs to lb.

### Patch Changes

- [Shopify#7711](Shopify#7711)
[`3984f26b6`](Shopify@3984f26)
Thanks [@nayeobkim](https://github.com/nayeobkim)! - Updated the related
components copy in `AlphaStack`


- [Shopify#7569](Shopify#7569)
[`ffe1e9230`](Shopify@ffe1e92)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `Tiles`
component guidance and examples


- [Shopify#7698](Shopify#7698)
[`f0e75bd0c`](Shopify@f0e75bd)
Thanks [@nayeobkim](https://github.com/nayeobkim)! - Updated the Related
components links in Columns


- [Shopify#7713](Shopify#7713)
[`b649af84f`](Shopify@b649af8)
Thanks [@nayeobkim](https://github.com/nayeobkim)! - Updated the related
components links in `Tiles`


- [Shopify#7594](Shopify#7594)
[`faf3dc646`](Shopify@faf3dc6)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `Box`
component guidance and examples


- [Shopify#7659](Shopify#7659)
[`d28259746`](Shopify@d282597)
Thanks [@kyledurand](https://github.com/kyledurand)! - Added global
styles from deps instead of linking unpkg

- Updated dependencies
\[[`bdf6fd31d`](Shopify@bdf6fd3),
[`217f2f8ed`](Shopify@217f2f8),
[`2e5f8348b`](Shopify@2e5f834),
[`c1c8f73b0`](Shopify@c1c8f73),
[`270887fcf`](Shopify@270887f),
[`6b915e01e`](Shopify@6b915e0)]:
    -   @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