Skip to content

Conversation

laurkim
Copy link
Contributor

@laurkim laurkim commented Oct 19, 2022

WHY are these changes introduced?

Resolves #7340.
Refactors Modal and its child components to use our layout primitives.

WHAT is this pull request doing?

Storybook URL.
Refactors Modal, Modal.Section, Modal.Header, and Modal.Footer to use our layout primitives and remove css.

How to 🎩

Storybook URL.
I've tophatted all of our stories as well as the UI for all of our breakpoints.

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

🎩 checklist

@github-actions
Copy link
Contributor

github-actions bot commented Oct 19, 2022

size-limit report 📦

Path Size
polaris-react-cjs 209.54 KB (+0.06% 🔺)
polaris-react-esm 136.03 KB (+0.11% 🔺)
polaris-react-esnext 191.25 KB (-0.07% 🔽)
polaris-react-css 41.52 KB (-0.34% 🔽)

@laurkim laurkim force-pushed the lo/rebuild-modal branch 5 times, most recently from 3cbadd6 to 1d16db7 Compare October 24, 2022 20:07
@laurkim laurkim force-pushed the lo/rebuild-modal branch 2 times, most recently from 4fe0f93 to d7f76f0 Compare October 26, 2022 19:03
@laurkim laurkim marked this pull request as ready for review October 26, 2022 19:57
@laurkim laurkim self-assigned this Oct 26, 2022
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.

Nice!

@laurkim laurkim force-pushed the layout-components-beta branch 2 times, most recently from a160643 to 871e8fd Compare October 28, 2022 11:55
@laurkim laurkim force-pushed the layout-components-beta branch from dfb1279 to 3edf54e Compare October 28, 2022 13:11
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.

Looks like there's a max-width set that's prevent the dialog from going full width at breakpoint-sm

image

@laurkim
Copy link
Contributor Author

laurkim commented Oct 31, 2022

Looks like there's a max-width set that's prevent the dialog from going full width at breakpoint-sm

image

@sarahill that behavior exists today for Modals, it's not something that was introduced in this pr (Storybook for reference that points to main).

@sarahill sarahill self-requested a review October 31, 2022 13:57
@laurkim laurkim merged commit a27635f into layout-components-beta Oct 31, 2022
@laurkim laurkim deleted the lo/rebuild-modal branch October 31, 2022 15:46
chazdean pushed a commit that referenced this pull request Nov 22, 2022
Resolves #7340.
Refactors `Modal` and its child components to use our layout primitives.

[Storybook
URL](https://5d559397bae39100201eedc1-nrruulmwsh.chromatic.com/?path=/story/all-components-modal--default).
Refactors `Modal`, `Modal.Section`, `Modal.Header`, and `Modal.Footer`
to use our layout primitives and remove css.

[Storybook
URL](https://5d559397bae39100201eedc1-nrruulmwsh.chromatic.com/?path=/story/all-components-modal--default).
I've tophatted all of our stories as well as the UI for all of our
breakpoints.

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

- [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
chazdean pushed a commit that referenced this pull request Nov 22, 2022
Resolves #7340.
Refactors `Modal` and its child components to use our layout primitives.

[Storybook
URL](https://5d559397bae39100201eedc1-nrruulmwsh.chromatic.com/?path=/story/all-components-modal--default).
Refactors `Modal`, `Modal.Section`, `Modal.Header`, and `Modal.Footer`
to use our layout primitives and remove css.

[Storybook
URL](https://5d559397bae39100201eedc1-nrruulmwsh.chromatic.com/?path=/story/all-components-modal--default).
I've tophatted all of our stories as well as the UI for all of our
breakpoints.

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

- [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
laurkim added a commit that referenced this pull request Nov 23, 2022
Resolves #7340.
Refactors `Modal` and its child components to use our layout primitives.

[Storybook
URL](https://5d559397bae39100201eedc1-nrruulmwsh.chromatic.com/?path=/story/all-components-modal--default).
Refactors `Modal`, `Modal.Section`, `Modal.Header`, and `Modal.Footer`
to use our layout primitives and remove css.

[Storybook
URL](https://5d559397bae39100201eedc1-nrruulmwsh.chromatic.com/?path=/story/all-components-modal--default).
I've tophatted all of our stories as well as the UI for all of our
breakpoints.

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

- [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
chazdean pushed a commit that referenced this pull request Nov 23, 2022
Resolves #7340.
Refactors `Modal` and its child components to use our layout primitives.

[Storybook
URL](https://5d559397bae39100201eedc1-nrruulmwsh.chromatic.com/?path=/story/all-components-modal--default).
Refactors `Modal`, `Modal.Section`, `Modal.Header`, and `Modal.Footer`
to use our layout primitives and remove css.

[Storybook
URL](https://5d559397bae39100201eedc1-nrruulmwsh.chromatic.com/?path=/story/all-components-modal--default).
I've tophatted all of our stories as well as the UI for all of our
breakpoints.

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

- [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
chazdean pushed a commit that referenced this pull request Nov 28, 2022
Resolves #7340.
Refactors `Modal` and its child components to use our layout primitives.

[Storybook
URL](https://5d559397bae39100201eedc1-nrruulmwsh.chromatic.com/?path=/story/all-components-modal--default).
Refactors `Modal`, `Modal.Section`, `Modal.Header`, and `Modal.Footer`
to use our layout primitives and remove css.

[Storybook
URL](https://5d559397bae39100201eedc1-nrruulmwsh.chromatic.com/?path=/story/all-components-modal--default).
I've tophatted all of our stories as well as the UI for all of our
breakpoints.

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

- [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
laurkim added a commit that referenced this pull request Dec 5, 2022
Resolves #7340.
Refactors `Modal` and its child components to use our layout primitives.

[Storybook
URL](https://5d559397bae39100201eedc1-nrruulmwsh.chromatic.com/?path=/story/all-components-modal--default).
Refactors `Modal`, `Modal.Section`, `Modal.Header`, and `Modal.Footer`
to use our layout primitives and remove css.

[Storybook
URL](https://5d559397bae39100201eedc1-nrruulmwsh.chromatic.com/?path=/story/all-components-modal--default).
I've tophatted all of our stories as well as the UI for all of our
breakpoints.

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

- [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
laurkim added a commit that referenced this pull request Dec 7, 2022
Resolves #7340.
Refactors `Modal` and its child components to use our layout primitives.

[Storybook
URL](https://5d559397bae39100201eedc1-nrruulmwsh.chromatic.com/?path=/story/all-components-modal--default).
Refactors `Modal`, `Modal.Section`, `Modal.Header`, and `Modal.Footer`
to use our layout primitives and remove css.

[Storybook
URL](https://5d559397bae39100201eedc1-nrruulmwsh.chromatic.com/?path=/story/all-components-modal--default).
I've tophatted all of our stories as well as the UI for all of our
breakpoints.

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

- [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
laurkim added a commit that referenced this pull request Dec 9, 2022
Resolves #7340.
Refactors `Modal` and its child components to use our layout primitives.

[Storybook
URL](https://5d559397bae39100201eedc1-nrruulmwsh.chromatic.com/?path=/story/all-components-modal--default).
Refactors `Modal`, `Modal.Section`, `Modal.Header`, and `Modal.Footer`
to use our layout primitives and remove css.

[Storybook
URL](https://5d559397bae39100201eedc1-nrruulmwsh.chromatic.com/?path=/story/all-components-modal--default).
I've tophatted all of our stories as well as the UI for all of our
breakpoints.

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

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