Skip to content

Conversation

@aveline
Copy link
Contributor

@aveline aveline commented Oct 4, 2022

WHY are these changes introduced?

Fixes #7341

@aveline aveline changed the base branch from main to layout-components-beta October 4, 2022 19:25
@aveline aveline self-assigned this Oct 4, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2022

size-limit report 📦

Path Size
polaris-react-cjs 208.99 KB (-0.01% 🔽)
polaris-react-esm 135.52 KB (+0.02% 🔺)
polaris-react-esnext 190.87 KB (-0.01% 🔽)
polaris-react-css 41.53 KB (+0.04% 🔺)


const termsOfServiceMarkup = termsOfService ? (
<div className={styles.TermsOfService}>{termsOfService}</div>
<Box paddingTop="5">{termsOfService}</Box>
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we be using tokens, rather than hardcoded numbers?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, wait. I see. This is a number from the set made available in the Box component props. 🤦

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it maps to our spacing tokens. But perhaps it's not very clear? I wonder if there's anything we can do to make that easier?

aveline added a commit that referenced this pull request Oct 12, 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.

Co-authored-by: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com>
@aveline aveline force-pushed the layout-account-connection branch from 3468945 to db25106 Compare October 12, 2022 18:23
@aveline aveline force-pushed the layout-account-connection branch from db25106 to 69e2dfc Compare October 12, 2022 18:29
@aveline aveline marked this pull request as ready for review October 12, 2022 18:46
@aveline aveline requested a review from laurkim October 12, 2022 18:46
Copy link
Contributor

@laurkim laurkim left a comment

Choose a reason for hiding this comment

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

Nice!! Changes on Storybook lgtm 🚀

@aveline aveline merged commit bbbd721 into layout-components-beta Oct 12, 2022
@aveline aveline deleted the layout-account-connection branch October 12, 2022 20:48
laurkim pushed a commit that referenced this pull request Oct 14, 2022
### WHY are these changes introduced?

Fixes #7341  <!-- link to issue if one exists -->
laurkim pushed a commit that referenced this pull request Oct 27, 2022
### WHY are these changes introduced?

Fixes #7341  <!-- link to issue if one exists -->
laurkim pushed a commit that referenced this pull request Oct 28, 2022
### WHY are these changes introduced?

Fixes #7341  <!-- link to issue if one exists -->
laurkim pushed a commit that referenced this pull request Oct 28, 2022
### WHY are these changes introduced?

Fixes #7341  <!-- link to issue if one exists -->
chazdean pushed a commit that referenced this pull request Nov 22, 2022
laurkim pushed a commit that referenced this pull request Nov 23, 2022
chazdean pushed a commit that referenced this pull request Nov 28, 2022
laurkim pushed a commit that referenced this pull request Dec 5, 2022
laurkim pushed a commit that referenced this pull request Dec 7, 2022
laurkim pushed a commit that referenced this pull request Dec 9, 2022
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.

[Layout foundations] Rebuild AccountConnection with new layout primitives

4 participants