Skip to content

Conversation

@chazdean
Copy link
Contributor

@chazdean chazdean commented Jan 11, 2023

WHY are these changes introduced?

Fixes #8014

WHAT is this pull request doing?

  • Centers SkeletonTitle to align with PrimaryActionSkeleton on right
  • Reduces height of SkeletonTitle from 28px to 24px to closer match Page title
  • Removes paddingBlockStart from SkeletonPage Body allowing it shift up vertically matching Page
Before
Screenshot 2023-01-11 at 3 48 06 PM
After
Screenshot 2023-01-11 at 3 47 40 PM

How to 🎩

Spin Link

🎩 checklist

@chazdean chazdean changed the base branch from main to layout-rebuild-batch-2 January 11, 2023 20:23
@chazdean chazdean marked this pull request as ready for review January 11, 2023 21:23
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.

Code looks good but there's no chromatic diff. This needs a changeset entry anyway, can you push one up and see if that kicks off choromatic?

@github-actions
Copy link
Contributor

github-actions bot commented Jan 12, 2023

size-limit report 📦

Path Size
polaris-react-cjs 212.35 KB (-0.01% 🔽)
polaris-react-esm 137.35 KB (-0.01% 🔽)
polaris-react-esnext 191.53 KB (-0.01% 🔽)
polaris-react-css 40.97 KB (0%)

@chazdean
Copy link
Contributor Author

/snapit

@github-actions
Copy link
Contributor

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

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

yarn add @shopify/plugin-polaris@0.0.0-snapshot-release-20230112154554
yarn add @shopify/polaris@0.0.0-snapshot-release-20230112154554

@chazdean chazdean force-pushed the layout-rebuild-batch-2 branch from 3c4a2a3 to a5e1013 Compare January 12, 2023 20:36
@chazdean chazdean self-assigned this Jan 13, 2023
@chazdean chazdean force-pushed the layout-rebuild-batch-2 branch from a5e1013 to 40b4d87 Compare January 13, 2023 19:08
@chazdean chazdean force-pushed the fix-skeleton-page-layout branch from 05b687e to 478a89f Compare January 13, 2023 19:35
@chazdean chazdean force-pushed the fix-skeleton-page-layout branch from 478a89f to 9cec9f3 Compare January 13, 2023 19:47
@chazdean chazdean requested a review from kyledurand January 13, 2023 19:52
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.

👏

@chazdean chazdean merged commit d1981bc into layout-rebuild-batch-2 Jan 13, 2023
@chazdean chazdean deleted the fix-skeleton-page-layout branch January 13, 2023 20:01
laurkim pushed a commit that referenced this pull request Jan 26, 2023
…eletonPage to closer match Page (#8030)

<!--
  ☝️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?

Fixes #8014 <!-- link to issue if one exists -->

<!--
  Context about the problem that’s being addressed.
-->

### WHAT is this pull request doing?

- Centers `SkeletonTitle` to align with `PrimaryActionSkeleton` on right
- Reduces height of `SkeletonTitle` from `28px` to `24px` to closer
match `Page` title
- Removes `paddingBlockStart` from `SkeletonPage` Body allowing it shift
up vertically matching `Page`

Before|
:----:|
<img width="1007" alt="Screenshot 2023-01-11 at 3 48 06 PM"
src="https://user-images.githubusercontent.com/59836805/211915956-c9ee9f44-e1c5-4e65-a91f-44678caf4f31.png">|
**After**|
<img width="1024" alt="Screenshot 2023-01-11 at 3 47 40 PM"
src="https://user-images.githubusercontent.com/59836805/211916089-707a3af2-fb1f-4046-90bc-cb362bed2162.png">|

<!--
  Summary of the changes committed.

Before / after screenshots are appreciated for UI changes. Make sure to
include alt text that describes the screenshot.

If you include an animated gif showing your change, wrapping it in a
details tag is recommended. Gifs usually autoplay, which can cause
accessibility issues for people reviewing your PR:

    <details>
      <summary>Summary of your gif(s)</summary>
      <img src="..." alt="Description of what the gif shows">
    </details>
-->

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

### How to 🎩

[Spin
Link](https://admin.web.web-6ubu.chaz-dean.us.spin.dev/store/shop1/)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] 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
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.

4 participants