Skip to content

Commit 7e5644c

Browse files
chazdeanlaurkim
chazdean
and
laurkim
committed
[SkeletonPage] Rebuild with layout primitives (#7797)
<!-- ☝️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 --> Fixes [#7714](#7714) <!-- Context about the problem that’s being addressed. --> - Refactors `SkeletonPage` and its child components to use our layout primitives. - Removes `max-width: 100%` property from AlphaStack - Adds storybook examples for `SkeletonPage` with `narrowWidth` and with `fullWidth` props <details> <summary>Unable to center `SkeletonPage` when `narrowWidth` prop is used - SOLVED ✅</summary> <table> <tr> <td> Before </td> <td> After </td> </tr> <tr> <td> <img width="400" alt="Screenshot 2022-11-24 at 9 13 21 AM" src="https://user-images.githubusercontent.com/59836805/203805854-4748a46a-8208-476e-9c16-d0071e4ff66b.png"> </td> <td> <img width="400" alt="Screenshot 2022-11-24 at 9 10 25 AM" src="https://user-images.githubusercontent.com/59836805/203806188-bd5cde58-0787-46f1-a2b3-cbaf2a228d52.png"> </td> </tr> <tr> <td> <img width="400" alt="Screenshot 2022-11-24 at 9 13 42 AM" src="https://user-images.githubusercontent.com/59836805/203806392-466df6a0-29ee-4d06-bc40-523b60b97853.png"> </td> <td> <img width="400" alt="Screenshot 2022-11-24 at 9 11 17 AM" src="https://user-images.githubusercontent.com/59836805/203806492-6793f6a3-591a-4cb4-9f13-46da44f64dfe.png"> </td> </tr> </table> </details> <details> <summary>Our current `Text` component has the opposite breakpoint behaviour compared to how `.Title` is used in `SkeletonPage` - SOLVED ✅</summary> <table> <tr> <td> Before </td> <td> After </td> </tr> <tr> <td> <img width="400" alt="before text" src="https://user-images.githubusercontent.com/59836805/203808391-3f3bd9d2-421e-4e09-a843-e579b936d39f.gif"> </td> <td> <img width="400" alt="before text" src="https://user-images.githubusercontent.com/59836805/203808879-55e311ef-708b-44c9-a28e-4582c77d0992.gif"> </td> </tr> <tr> <td> ```java .Title { font-weight: var(--p-font-weight-semibold); font-size: 24px; line-height: 28px; @media #{$p-breakpoints-md-up} { font-size: 20px; } } ``` </td> <td> ```java .headingXl { font-size: var(--p-font-size-300); line-height: var(--p-font-line-height-3); @media #{$p-breakpoints-md-up} { font-size: var(--p-font-size-400); line-height: var(--p-font-line-height-4); } } ``` </td> </tr> </table> </details> ~~`role` prop is missing from `Box`, creating a separate issue to solve this [here](#7799 SOLVED ✅ <!-- 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 --> - [ ] 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 Co-authored-by: Lo Kim <[email protected]>
1 parent ffa8bc2 commit 7e5644c

File tree

5 files changed

+172
-127
lines changed

5 files changed

+172
-127
lines changed

‎.changeset/strong-bottles-swim.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@shopify/polaris': minor
3+
---
4+
5+
Refactored `SkeletonPage` to use primitive Layout components
6+
Removed `max-width` on children in `AlphaStack`
7+
Added `narrowWidth` and `fullWidth` examples to `AlphaStack` stories

‎polaris-react/src/components/SkeletonPage/SkeletonPage.scss‎

Lines changed: 5 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -2,74 +2,19 @@
22

33
$primary-action-button-height: 36px;
44
$primary-action-button-width: 100px;
5-
$skeleton-display-text-max-width: 120px;
65

7-
.Page {
8-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
9-
@include page-layout;
10-
}
11-
12-
.fullWidth {
13-
max-width: none;
14-
}
15-
16-
.narrowWidth {
17-
max-width: $layout-width-primary-max;
18-
}
19-
20-
.Content {
21-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
22-
@include page-content-layout;
23-
}
24-
25-
.Header {
26-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
27-
@include page-header-layout;
28-
padding-bottom: var(--p-space-2);
29-
}
30-
31-
.BreadcrumbAction {
32-
padding-top: var(--p-space-4);
33-
padding-bottom: var(--p-space-4);
34-
margin-top: calc(-1 * var(--p-space-1));
35-
margin-bottom: calc(-1 * var(--p-space-1));
36-
}
37-
38-
.TitleAndPrimaryAction {
39-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
40-
display: flex;
41-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
42-
align-items: flex-start;
43-
}
44-
45-
.TitleWrapper {
46-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
47-
flex: 1 1 0%;
6+
:root {
7+
--pc-skeleton-page-max-width: 998px;
8+
--pc-skeleton-page-max-width-narrow: 662px;
489
}
4910

5011
.Title {
5112
font-weight: var(--p-font-weight-semibold);
52-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
53-
font-size: 24px;
54-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
55-
line-height: 28px;
56-
57-
@media #{$p-breakpoints-md-up} {
58-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
59-
font-size: 20px;
60-
}
13+
font-size: var(--p-font-size-300);
14+
line-height: var(--p-font-line-height-4);
6115
}
6216

6317
.SkeletonTitle {
64-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
65-
display: flex;
66-
background-color: var(--p-surface-neutral);
67-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
68-
border-radius: var(--p-border-radius-base);
69-
max-width: $skeleton-display-text-max-width;
70-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
71-
height: 28px;
72-
7318
@media screen and (-ms-high-contrast: active) {
7419
background-color: grayText;
7520
}
@@ -84,39 +29,3 @@ $skeleton-display-text-max-width: 120px;
8429
min-width: $primary-action-button-width;
8530
}
8631
}
87-
88-
.Actions {
89-
margin-top: var(--p-space-2);
90-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
91-
display: flex;
92-
flex-direction: row-reverse;
93-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
94-
justify-content: flex-end;
95-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
96-
align-items: center;
97-
}
98-
99-
.Action {
100-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
101-
display: flex;
102-
flex-direction: column;
103-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
104-
justify-content: center;
105-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
106-
min-height: control-slim-height();
107-
padding-right: var(--p-space-6);
108-
margin-top: calc(-1 * var(--p-space-1));
109-
margin-bottom: calc(-1 * var(--p-space-1));
110-
padding-top: var(--p-space-4);
111-
112-
&:first-child {
113-
padding-right: 0;
114-
}
115-
116-
@media #{$p-breakpoints-lg-down} {
117-
&:not(:last-child) {
118-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
119-
display: none;
120-
}
121-
}
122-
}

‎polaris-react/src/components/SkeletonPage/SkeletonPage.stories.tsx‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,103 @@ export function WithStaticContent() {
100100
</SkeletonPage>
101101
);
102102
}
103+
104+
export function WithNarrowWidth() {
105+
return (
106+
<SkeletonPage primaryAction narrowWidth>
107+
<Layout>
108+
<Layout.Section>
109+
<Card sectioned>
110+
<SkeletonBodyText />
111+
</Card>
112+
<Card sectioned>
113+
<TextContainer>
114+
<SkeletonDisplayText size="small" />
115+
<SkeletonBodyText />
116+
</TextContainer>
117+
</Card>
118+
<Card sectioned>
119+
<TextContainer>
120+
<SkeletonDisplayText size="small" />
121+
<SkeletonBodyText />
122+
</TextContainer>
123+
</Card>
124+
</Layout.Section>
125+
<Layout.Section secondary>
126+
<Card>
127+
<Card.Section>
128+
<TextContainer>
129+
<SkeletonDisplayText size="small" />
130+
<SkeletonBodyText lines={2} />
131+
</TextContainer>
132+
</Card.Section>
133+
<Card.Section>
134+
<SkeletonBodyText lines={1} />
135+
</Card.Section>
136+
</Card>
137+
<Card subdued>
138+
<Card.Section>
139+
<TextContainer>
140+
<SkeletonDisplayText size="small" />
141+
<SkeletonBodyText lines={2} />
142+
</TextContainer>
143+
</Card.Section>
144+
<Card.Section>
145+
<SkeletonBodyText lines={2} />
146+
</Card.Section>
147+
</Card>
148+
</Layout.Section>
149+
</Layout>
150+
</SkeletonPage>
151+
);
152+
}
153+
154+
export function WithFullWidth() {
155+
return (
156+
<SkeletonPage primaryAction fullWidth>
157+
<Layout>
158+
<Layout.Section>
159+
<Card sectioned>
160+
<SkeletonBodyText />
161+
</Card>
162+
<Card sectioned>
163+
<TextContainer>
164+
<SkeletonDisplayText size="small" />
165+
<SkeletonBodyText />
166+
</TextContainer>
167+
</Card>
168+
<Card sectioned>
169+
<TextContainer>
170+
<SkeletonDisplayText size="small" />
171+
<SkeletonBodyText />
172+
</TextContainer>
173+
</Card>
174+
</Layout.Section>
175+
<Layout.Section secondary>
176+
<Card>
177+
<Card.Section>
178+
<TextContainer>
179+
<SkeletonDisplayText size="small" />
180+
<SkeletonBodyText lines={2} />
181+
</TextContainer>
182+
</Card.Section>
183+
<Card.Section>
184+
<SkeletonBodyText lines={1} />
185+
</Card.Section>
186+
</Card>
187+
<Card subdued>
188+
<Card.Section>
189+
<TextContainer>
190+
<SkeletonDisplayText size="small" />
191+
<SkeletonBodyText lines={2} />
192+
</TextContainer>
193+
</Card.Section>
194+
<Card.Section>
195+
<SkeletonBodyText lines={2} />
196+
</Card.Section>
197+
</Card>
198+
</Layout.Section>
199+
</Layout>
200+
</SkeletonPage>
201+
);
202+
}

‎polaris-react/src/components/SkeletonPage/SkeletonPage.tsx‎

Lines changed: 52 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import React from 'react';
22

3-
import {classNames} from '../../utilities/css';
43
import {useI18n} from '../../utilities/i18n';
54
import {SkeletonDisplayText} from '../SkeletonDisplayText';
65
import {SkeletonBodyText} from '../SkeletonBodyText';
6+
import {Box} from '../Box';
7+
import {Inline} from '../Inline';
8+
import {AlphaStack} from '../AlphaStack';
79

810
import styles from './SkeletonPage.scss';
911

@@ -31,46 +33,71 @@ export function SkeletonPage({
3133
breadcrumbs,
3234
}: SkeletonPageProps) {
3335
const i18n = useI18n();
34-
const className = classNames(
35-
styles.Page,
36-
fullWidth && styles.fullWidth,
37-
narrowWidth && styles.narrowWidth,
38-
);
3936

4037
const titleContent = title ? (
4138
<h1 className={styles.Title}>{title}</h1>
4239
) : (
43-
<div className={styles.SkeletonTitle} />
40+
<div className={styles.SkeletonTitle}>
41+
<Box
42+
background="surface-neutral"
43+
minWidth="120px"
44+
minHeight="28px"
45+
borderRadius="base"
46+
/>
47+
</div>
4448
);
4549

46-
const titleMarkup = <div className={styles.TitleWrapper}>{titleContent}</div>;
47-
4850
const primaryActionMarkup = primaryAction ? (
4951
<div className={styles.PrimaryAction}>
5052
<SkeletonDisplayText size="large" />
5153
</div>
5254
) : null;
5355

5456
const breadcrumbMarkup = breadcrumbs ? (
55-
<div className={styles.BreadcrumbAction} style={{width: 60}}>
57+
<Box maxWidth="60px" paddingBlockStart="4" paddingBlockEnd="4">
5658
<SkeletonBodyText lines={1} />
57-
</div>
59+
</Box>
5860
) : null;
5961

6062
return (
61-
<div
62-
className={className}
63-
role="status"
64-
aria-label={i18n.translate('Polaris.SkeletonPage.loadingLabel')}
65-
>
66-
<div className={styles.Header}>
67-
{breadcrumbMarkup}
68-
<div className={styles.TitleAndPrimaryAction}>
69-
{titleMarkup}
70-
{primaryActionMarkup}
71-
</div>
72-
</div>
73-
<div className={styles.Content}>{children}</div>
74-
</div>
63+
<AlphaStack align="center" fullWidth>
64+
<Box
65+
padding="0"
66+
paddingInlineStart={{sm: '6'}}
67+
paddingInlineEnd={{sm: '6'}}
68+
maxWidth="var(--pc-skeleton-page-max-width)"
69+
aria-label={i18n.translate('Polaris.SkeletonPage.loadingLabel')}
70+
role="status"
71+
{...(narrowWidth && {
72+
maxWidth: 'var(--pc-skeleton-page-max-width-narrow)',
73+
})}
74+
{...(fullWidth && {
75+
maxWidth: 'none',
76+
})}
77+
>
78+
<AlphaStack gap="0" fullWidth>
79+
<Box
80+
padding={{xs: '4', md: '5'}}
81+
paddingBlockEnd={{xs: '2', md: '5'}}
82+
paddingInlineStart={{sm: '0'}}
83+
paddingInlineEnd={{sm: '0'}}
84+
>
85+
{breadcrumbMarkup}
86+
<Inline align="space-between" blockAlign="start">
87+
{titleContent}
88+
{primaryActionMarkup}
89+
</Inline>
90+
</Box>
91+
<Box
92+
paddingBlockStart={{xs: '2', md: '5'}}
93+
paddingBlockEnd="2"
94+
paddingInlineStart="0"
95+
paddingInlineEnd="0"
96+
>
97+
{children}
98+
</Box>
99+
</AlphaStack>
100+
</Box>
101+
</AlphaStack>
75102
);
76103
}

‎polaris-react/src/components/SkeletonPage/tests/SkeletonPage.test.tsx‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ import React from 'react';
22
import {mountWithApp} from 'tests/utilities';
33

44
import {Card} from '../../Card';
5-
import {Text} from '../../Text';
65
import {Layout} from '../../Layout';
76
import {SkeletonBodyText} from '../../SkeletonBodyText';
87
import {SkeletonDisplayText} from '../../SkeletonDisplayText';
98
import {SkeletonPage} from '../SkeletonPage';
9+
import {Box} from '../../Box';
1010

1111
describe('<SkeletonPage />', () => {
1212
it('renders its children', () => {
@@ -38,7 +38,9 @@ describe('<SkeletonPage />', () => {
3838
const skeletonPage = mountWithApp(<SkeletonPage title="Products" />);
3939

4040
expect(skeletonPage).toContainReactComponent('h1', {className: 'Title'});
41-
expect(skeletonPage).not.toContainReactComponent(Text);
41+
expect(skeletonPage).not.toContainReactComponent(Box, {
42+
background: 'surface-neutral',
43+
});
4244
});
4345

4446
it('renders SkeletonTitle when a title not defined', () => {
@@ -47,8 +49,8 @@ describe('<SkeletonPage />', () => {
4749
expect(skeletonPage).not.toContainReactComponent('h1', {
4850
className: 'Title',
4951
});
50-
expect(skeletonPage).toContainReactComponent('div', {
51-
className: 'SkeletonTitle',
52+
expect(skeletonPage).toContainReactComponent(Box, {
53+
background: 'surface-neutral',
5254
});
5355
});
5456

@@ -58,8 +60,8 @@ describe('<SkeletonPage />', () => {
5860
expect(skeletonPage).not.toContainReactComponent('h1', {
5961
className: 'Title',
6062
});
61-
expect(skeletonPage).toContainReactComponent('div', {
62-
className: 'SkeletonTitle',
63+
expect(skeletonPage).toContainReactComponent(Box, {
64+
background: 'surface-neutral',
6365
});
6466
});
6567
});

0 commit comments

Comments
 (0)