Skip to content

Commit 4afece0

Browse files
chazdeanlaurkim
chazdean
and
laurkim
authored
[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 --> ### WHY are these changes introduced? Fixes [#7714](#7714) <!-- Context about the problem that’s being addressed. --> ### WHAT is this pull request doing? - 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 #### Issues with current PR: <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 --> ### 🎩 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 Co-authored-by: Lo Kim <lo.kim@shopify.com>
1 parent d2ff651 commit 4afece0

File tree

6 files changed

+172
-112
lines changed

6 files changed

+172
-112
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/AlphaStack/AlphaStack.scss‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,6 @@
2626
@media #{$p-breakpoints-xl-up} {
2727
gap: var(--pc-stack-gap-xl);
2828
}
29-
30-
> * {
31-
max-width: 100%;
32-
}
3329
}
3430

3531
.listReset {

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

Lines changed: 5 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -2,62 +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-
@include page-layout;
9-
}
10-
11-
.fullWidth {
12-
max-width: none;
13-
}
14-
15-
.narrowWidth {
16-
max-width: $layout-width-primary-max;
17-
}
18-
19-
.Content {
20-
@include page-content-layout;
21-
}
22-
23-
.Header {
24-
@include page-header-layout;
25-
padding-bottom: var(--p-space-2);
26-
}
27-
28-
.BreadcrumbAction {
29-
padding-top: var(--p-space-4);
30-
padding-bottom: var(--p-space-4);
31-
margin-top: calc(-1 * var(--p-space-1));
32-
margin-bottom: calc(-1 * var(--p-space-1));
33-
}
34-
35-
.TitleAndPrimaryAction {
36-
display: flex;
37-
align-items: flex-start;
38-
}
39-
40-
.TitleWrapper {
41-
flex: 1 1 0%;
6+
:root {
7+
--pc-skeleton-page-max-width: 998px;
8+
--pc-skeleton-page-max-width-narrow: 662px;
429
}
4310

4411
.Title {
4512
font-weight: var(--p-font-weight-semibold);
46-
font-size: 24px;
47-
line-height: 28px;
48-
49-
@media #{$p-breakpoints-md-up} {
50-
font-size: 20px;
51-
}
13+
font-size: var(--p-font-size-300);
14+
line-height: var(--p-font-line-height-4);
5215
}
5316

5417
.SkeletonTitle {
55-
display: flex;
56-
background-color: var(--p-surface-neutral);
57-
border-radius: var(--p-border-radius-base);
58-
max-width: $skeleton-display-text-max-width;
59-
height: 28px;
60-
6118
@media screen and (-ms-high-contrast: active) {
6219
background-color: grayText;
6320
}
@@ -71,32 +28,3 @@ $skeleton-display-text-max-width: 120px;
7128
min-width: $primary-action-button-width;
7229
}
7330
}
74-
75-
.Actions {
76-
margin-top: var(--p-space-2);
77-
display: flex;
78-
flex-direction: row-reverse;
79-
justify-content: flex-end;
80-
align-items: center;
81-
}
82-
83-
.Action {
84-
display: flex;
85-
flex-direction: column;
86-
justify-content: center;
87-
min-height: control-slim-height();
88-
padding-right: var(--p-space-6);
89-
margin-top: calc(-1 * var(--p-space-1));
90-
margin-bottom: calc(-1 * var(--p-space-1));
91-
padding-top: var(--p-space-4);
92-
93-
&:first-child {
94-
padding-right: 0;
95-
}
96-
97-
@media #{$p-breakpoints-lg-down} {
98-
&:not(:last-child) {
99-
display: none;
100-
}
101-
}
102-
}

‎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)