Skip to content

Conversation

@aveline
Copy link
Contributor

@aveline aveline commented Oct 4, 2022

WHY are these changes introduced?

#7345 Refactors the AccountConnection component to use the new layout primitives including AlphaCard.
AlphaCard uses useBreakpoints() which required setting up a mock to set media width for the tests. It appears setting the media width in the tests becomes necessary for any parent components using a component that uses useBreakpoints().

WHAT is this pull request doing?

This adds a setMediaWidth test utility which can be used in tests for any components that require mocked media widths in their tests.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2022

size-limit report 📦

Path Size
polaris-react-cjs 209.01 KB (0%)
polaris-react-esm 135.51 KB (0%)
polaris-react-esnext 190.87 KB (0%)
polaris-react-css 41.52 KB (0%)

Copy link
Member

@kyledurand kyledurand left a comment

Choose a reason for hiding this comment

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

I'm all for the utility but I don't think we need to set it globally. There won't be that many components where this is a problem and it's just going to slow down the whole test suite. Easy enough to add the before and after in specific tests with the utility in place

@aveline
Copy link
Contributor Author

aveline commented Oct 4, 2022

Yes those are good points @kyledurand. The concern is that sometimes the component that triggers the need for a set media width can be nested several deep so it won't always be obvious why the tests start to fail. Basically any component that has a nested component with useBreakpoints() needs it set.

@aveline aveline changed the title Set media width globally for tests Add setMediaWidth breakpoints test utility Oct 11, 2022
@aveline
Copy link
Contributor Author

aveline commented Oct 11, 2022

After further discussion we've decided to add the setMediaWidth utility and use it manually on any tests that need it, rather than setting the media width globally.

@aveline aveline requested a review from alex-page October 11, 2022 21:43
@aveline aveline merged commit ea2a45b into main Oct 12, 2022
@aveline aveline deleted the global-test-setup-media-width branch October 12, 2022 15:17
@github-actions github-actions bot mentioned this pull request Oct 12, 2022
kyledurand pushed a commit that referenced this pull request Oct 14, 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

- [#7373](#7373)
[`56c82ee8d`](56c82ee)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Add
`getFunctionArgs` utility

### Patch Changes

- Updated dependencies
\[[`c3f427c17`](c3f427c)]:
    -   @shopify/[email protected]

## @shopify/[email protected]

### Minor Changes

- [#7364](#7364)
[`e4b2c36d8`](e4b2c36)
Thanks [@Bringer128](https://github.com/Bringer128)! - Deprecated
Collapsible preventMeasuringOnChildrenUpdate.
Fixed bug where Collapsible would get stuck in animating state when
duration is 0.
Add support for intentionally disabling the transition in Collapsible.

### Patch Changes

- [#7363](#7363)
[`8a6c323e2`](8a6c323)
Thanks [@aveline](https://github.com/aveline)! - Added `id` prop to
`Text` and `Box`


- [#7348](#7348)
[`ea2a45bbb`](ea2a45b)
Thanks [@aveline](https://github.com/aveline)! - Added `setMediaWidth`
breakpoints test utility


- [#7388](#7388)
[`5bc885765`](5bc8857)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed a re-render
bug with Page Actions

- Updated dependencies
\[[`c3f427c17`](c3f427c)]:
    -   @shopify/[email protected]

## @shopify/[email protected]

### Patch Changes

- Updated dependencies
\[[`56c82ee8d`](56c82ee)]:
    -   @shopify/[email protected]

## @shopify/[email protected]

### Patch Changes

- [#7385](#7385)
[`c3f427c17`](c3f427c)
Thanks [@laurkim](https://github.com/laurkim)! - Refactored exported
alias and scale types in `breakpoints`, `depth`, `font`, `motion`,
`shape`, `spacing`, and `zIndex`.

## @shopify/[email protected]

### Patch Changes

- Updated dependencies
\[[`c3f427c17`](c3f427c)]:
    -   @shopify/[email protected]

## [email protected]

### Minor Changes

- [#7032](#7032)
[`40ee692aa`](40ee692)
Thanks [@gwyneplaine](https://github.com/gwyneplaine)! - Added Playroom
integration to Polaris docs site.

### Patch Changes

- [#7032](#7032)
[`40ee692aa`](40ee692)
Thanks [@gwyneplaine](https://github.com/gwyneplaine)! - Improved the
design of the Sandbox feature.


- [#7400](#7400)
[`9f9fe1f99`](9f9fe1f)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed a scaling
bug caused by content overflow
Fixed a bug where examples that don't have any content wouldn't show up
- Updated dependencies
\[[`8a6c323e2`](8a6c323),
[`e4b2c36d8`](e4b2c36),
[`c3f427c17`](c3f427c),
[`ea2a45bbb`](ea2a45b),
[`5bc885765`](5bc8857)]:
    -   @shopify/[email protected]
    -   @shopify/[email protected]

## [email protected]

### Patch Changes

- Updated dependencies
\[[`8a6c323e2`](8a6c323),
[`e4b2c36d8`](e4b2c36),
[`ea2a45bbb`](ea2a45b),
[`5bc885765`](5bc8857)]:
    -   @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.

2 participants