From a3e312e063264145dcdd84fcf655eb0b37fad5c1 Mon Sep 17 00:00:00 2001 From: Lo Kim Date: Tue, 18 Oct 2022 09:18:59 -0400 Subject: [PATCH 01/14] [Modal] Rebuild with layout primitives --- polaris-react/src/components/Modal/Modal.scss | 12 ------------ polaris-react/src/components/Modal/Modal.tsx | 18 ++++++++++-------- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/polaris-react/src/components/Modal/Modal.scss b/polaris-react/src/components/Modal/Modal.scss index 93d5ca718a3..79e5be9e9b9 100644 --- a/polaris-react/src/components/Modal/Modal.scss +++ b/polaris-react/src/components/Modal/Modal.scss @@ -2,13 +2,6 @@ $small-width: 620px; -.BodyWrapper { - display: flex; - flex-grow: 1; - overflow-x: hidden; - -webkit-overflow-scrolling: touch; -} - .Body { width: 100%; } @@ -23,8 +16,3 @@ $small-width: 620px; max-width: $small-width; } } - -.Spinner { - margin: var(--p-space-4); - text-align: center; -} diff --git a/polaris-react/src/components/Modal/Modal.tsx b/polaris-react/src/components/Modal/Modal.tsx index 0967d78e459..a4a4d9cfde3 100644 --- a/polaris-react/src/components/Modal/Modal.tsx +++ b/polaris-react/src/components/Modal/Modal.tsx @@ -7,6 +7,8 @@ import {useI18n} from '../../utilities/i18n'; import {WithinContentContext} from '../../utilities/within-content-context'; import {wrapWithComponent} from '../../utilities/components'; import {Backdrop} from '../Backdrop'; +import {Box} from '../Box'; +import {Inline} from '../Inline'; import {Scrollable} from '../Scrollable'; import {Spinner} from '../Spinner'; import {Portal} from '../Portal'; @@ -152,15 +154,17 @@ export const Modal: React.FunctionComponent & { : children; const body = loading ? ( -
- -
+ + + + + ) : ( content ); const scrollContainerMarkup = noScroll ? ( -
{body}
+ {body} ) : ( & {
{title}
-
{bodyMarkup}
+ {bodyMarkup} {footerMarkup} ); @@ -210,9 +214,7 @@ export const Modal: React.FunctionComponent & { const animated = !instant; const activatorMarkup = - activator && !isRef(activator) ? ( -
{activator}
- ) : null; + activator && !isRef(activator) ? {activator} : null; return ( From 7761699f75fc9f70be036c2d9c2c4f82ecb2a293 Mon Sep 17 00:00:00 2001 From: Lo Kim Date: Wed, 19 Oct 2022 12:02:36 -0400 Subject: [PATCH 02/14] [Modal.Footer] Rebuild with layout primitives --- .../Modal/components/Footer/Footer.scss | 15 ----------- .../Modal/components/Footer/Footer.tsx | 25 +++++++++++-------- 2 files changed, 15 insertions(+), 25 deletions(-) delete mode 100644 polaris-react/src/components/Modal/components/Footer/Footer.scss diff --git a/polaris-react/src/components/Modal/components/Footer/Footer.scss b/polaris-react/src/components/Modal/components/Footer/Footer.scss deleted file mode 100644 index c3f23fe3173..00000000000 --- a/polaris-react/src/components/Modal/components/Footer/Footer.scss +++ /dev/null @@ -1,15 +0,0 @@ -@import '../../../../styles/common'; - -.Footer { - display: flex; - align-self: flex-end; - align-items: center; - width: 100%; - min-height: var(--p-space-16); - padding: var(--p-space-4); - border-top: var(--p-border-divider); -} - -.FooterContent { - width: 100%; -} diff --git a/polaris-react/src/components/Modal/components/Footer/Footer.tsx b/polaris-react/src/components/Modal/components/Footer/Footer.tsx index 75c56c64243..5609060804f 100644 --- a/polaris-react/src/components/Modal/components/Footer/Footer.tsx +++ b/polaris-react/src/components/Modal/components/Footer/Footer.tsx @@ -3,9 +3,9 @@ import React from 'react'; import type {ComplexAction} from '../../../../types'; import {buttonsFrom} from '../../../Button'; import {ButtonGroup} from '../../../ButtonGroup'; -import {Stack} from '../../../Stack'; - -import styles from './Footer.scss'; +import {Box} from '../../../Box'; +import {Columns} from '../../../Columns'; +import {Inline} from '../../../Inline'; export interface FooterProps { /** Primary action */ @@ -34,13 +34,18 @@ export function Footer({ ) : null; return ( -
-
- - {children} + + + {children} + {actions} - -
-
+ + + ); } From 22b76ab9487b95c71c8848d38f014689c84f896c Mon Sep 17 00:00:00 2001 From: Lo Kim Date: Wed, 19 Oct 2022 13:25:15 -0400 Subject: [PATCH 03/14] [Modal.Header] Rebuild with layout primitives --- .../Modal/components/Header/Header.scss | 23 ---------- .../Modal/components/Header/Header.tsx | 43 ++++++++++++++----- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/polaris-react/src/components/Modal/components/Header/Header.scss b/polaris-react/src/components/Modal/components/Header/Header.scss index 2271de2e18a..12e1a4d5618 100644 --- a/polaris-react/src/components/Modal/components/Header/Header.scss +++ b/polaris-react/src/components/Modal/components/Header/Header.scss @@ -1,28 +1,5 @@ -@import '../../../../styles/common'; - -.Header { - display: flex; - align-items: flex-start; - flex-shrink: 0; - padding: var(--p-space-4) var(--p-space-5); - border-bottom: var(--p-border-divider); -} - .titleHidden { position: absolute; right: 0; z-index: 1; - width: 100%; - display: flex; - justify-content: flex-end; - - .Title { - display: none; - } -} - -.Title { - @include text-breakword; - flex: 1; - margin-top: var(--p-space-1); } diff --git a/polaris-react/src/components/Modal/components/Header/Header.tsx b/polaris-react/src/components/Modal/components/Header/Header.tsx index 7b8f08399e5..fcd64f11723 100644 --- a/polaris-react/src/components/Modal/components/Header/Header.tsx +++ b/polaris-react/src/components/Modal/components/Header/Header.tsx @@ -1,6 +1,9 @@ import React from 'react'; -import {DisplayText} from '../../../DisplayText'; +import {Box} from '../../../Box'; +import {Columns} from '../../../Columns'; +import {Inline} from '../../../Inline'; +import {Text} from '../../../Text'; import {CloseButton} from '../CloseButton'; import styles from './Header.scss'; @@ -13,16 +16,36 @@ export interface HeaderProps { } export function Header({id, titleHidden, children, onClose}: HeaderProps) { + const titleHiddenMarkup = ( +
+ + + +
+ ); + + if (titleHidden || !children) { + return titleHiddenMarkup; + } + return ( -
-
- - {children} - -
- -
+ + + + + {children} + + + + + + ); } From b960dc820f926d7208b383ea0d4d3676835ee5c3 Mon Sep 17 00:00:00 2001 From: Lo Kim Date: Mon, 24 Oct 2022 14:39:36 -0400 Subject: [PATCH 04/14] [Modal.Section] Rebuild with layout primitives --- .../Modal/components/Section/Section.scss | 11 ----------- .../Modal/components/Section/Section.tsx | 15 ++++++++++++--- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/polaris-react/src/components/Modal/components/Section/Section.scss b/polaris-react/src/components/Modal/components/Section/Section.scss index f2b5ae68efb..91f1c4ee2ef 100644 --- a/polaris-react/src/components/Modal/components/Section/Section.scss +++ b/polaris-react/src/components/Modal/components/Section/Section.scss @@ -1,23 +1,12 @@ -@import '../../../../styles/common'; - $close-button-width: calc(var(--p-space-12) + var(--p-space-1)); .Section { flex: 0 0 auto; - padding: var(--p-space-5); &:not(:last-of-type) { border-bottom: var(--p-border-divider); } - &.subdued { - background: var(--p-surface-subdued); - } - - &.flush { - padding: 0; - } - &.titleHidden { padding-right: $close-button-width; } diff --git a/polaris-react/src/components/Modal/components/Section/Section.tsx b/polaris-react/src/components/Modal/components/Section/Section.tsx index 8840a9d0f91..3c5168a2ba6 100644 --- a/polaris-react/src/components/Modal/components/Section/Section.tsx +++ b/polaris-react/src/components/Modal/components/Section/Section.tsx @@ -1,5 +1,6 @@ import React from 'react'; +import {Box} from '../../../Box'; import {classNames} from '../../../../utilities/css'; import styles from './Section.scss'; @@ -19,10 +20,18 @@ export function Section({ }: SectionProps) { const className = classNames( styles.Section, - flush && styles.flush, - subdued && styles.subdued, titleHidden && styles.titleHidden, ); - return
{children}
; + return ( +
+ + {children} + +
+ ); } From e3e2a71b826e1ae2754d356349601579edff83a3 Mon Sep 17 00:00:00 2001 From: Lo Kim Date: Mon, 24 Oct 2022 14:41:25 -0400 Subject: [PATCH 05/14] [Modal.Header] Use `DisplayText` due to `Text` not supporting responsive style yet --- .../components/Modal/components/Header/Header.scss | 6 ++++++ .../src/components/Modal/components/Header/Header.tsx | 11 +++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/polaris-react/src/components/Modal/components/Header/Header.scss b/polaris-react/src/components/Modal/components/Header/Header.scss index 12e1a4d5618..3eea2814916 100644 --- a/polaris-react/src/components/Modal/components/Header/Header.scss +++ b/polaris-react/src/components/Modal/components/Header/Header.scss @@ -1,3 +1,9 @@ +@import '../../../../styles/common'; + +.Title { + @include text-breakword; +} + .titleHidden { position: absolute; right: 0; diff --git a/polaris-react/src/components/Modal/components/Header/Header.tsx b/polaris-react/src/components/Modal/components/Header/Header.tsx index fcd64f11723..dcfb3980e87 100644 --- a/polaris-react/src/components/Modal/components/Header/Header.tsx +++ b/polaris-react/src/components/Modal/components/Header/Header.tsx @@ -2,8 +2,8 @@ import React from 'react'; import {Box} from '../../../Box'; import {Columns} from '../../../Columns'; +import {DisplayText} from '../../../DisplayText'; import {Inline} from '../../../Inline'; -import {Text} from '../../../Text'; import {CloseButton} from '../CloseButton'; import styles from './Header.scss'; @@ -39,9 +39,12 @@ export function Header({id, titleHidden, children, onClose}: HeaderProps) { - - {children} - + {/* Replace with once responsive styles supported */} +
+ + {children} + +
From 7e27c750f164648281bf963d49fdd48c0a758be3 Mon Sep 17 00:00:00 2001 From: Lo Kim Date: Wed, 26 Oct 2022 10:39:36 -0400 Subject: [PATCH 06/14] [Modal.Header] Fix header position for hidden titles --- .../src/components/Modal/components/Header/Header.tsx | 8 +++++--- .../src/components/Modal/components/Section/Section.scss | 4 ---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/polaris-react/src/components/Modal/components/Header/Header.tsx b/polaris-react/src/components/Modal/components/Header/Header.tsx index dcfb3980e87..b8297929bb0 100644 --- a/polaris-react/src/components/Modal/components/Header/Header.tsx +++ b/polaris-react/src/components/Modal/components/Header/Header.tsx @@ -37,8 +37,8 @@ export function Header({id, titleHidden, children, onClose}: HeaderProps) { borderBottom="divider" > - - + + {/* Replace with once responsive styles supported */}
@@ -47,7 +47,9 @@ export function Header({id, titleHidden, children, onClose}: HeaderProps) {
- + + +
); diff --git a/polaris-react/src/components/Modal/components/Section/Section.scss b/polaris-react/src/components/Modal/components/Section/Section.scss index 91f1c4ee2ef..5b6e2cc0f0c 100644 --- a/polaris-react/src/components/Modal/components/Section/Section.scss +++ b/polaris-react/src/components/Modal/components/Section/Section.scss @@ -6,8 +6,4 @@ $close-button-width: calc(var(--p-space-12) + var(--p-space-1)); &:not(:last-of-type) { border-bottom: var(--p-border-divider); } - - &.titleHidden { - padding-right: $close-button-width; - } } From 9c019971b359184da39e1a5d2feb54b56f5f3ddb Mon Sep 17 00:00:00 2001 From: Lo Kim Date: Wed, 26 Oct 2022 14:20:59 -0400 Subject: [PATCH 07/14] [Modal.Section] Fix padding when `titleHidden` is true --- .../src/components/Modal/components/Section/Section.scss | 4 ++++ .../src/components/Modal/components/Section/Section.tsx | 1 + 2 files changed, 5 insertions(+) diff --git a/polaris-react/src/components/Modal/components/Section/Section.scss b/polaris-react/src/components/Modal/components/Section/Section.scss index 5b6e2cc0f0c..212622955d8 100644 --- a/polaris-react/src/components/Modal/components/Section/Section.scss +++ b/polaris-react/src/components/Modal/components/Section/Section.scss @@ -7,3 +7,7 @@ $close-button-width: calc(var(--p-space-12) + var(--p-space-1)); border-bottom: var(--p-border-divider); } } + +.titleHidden { + padding-right: $close-button-width; +} diff --git a/polaris-react/src/components/Modal/components/Section/Section.tsx b/polaris-react/src/components/Modal/components/Section/Section.tsx index 3c5168a2ba6..0539004c36a 100644 --- a/polaris-react/src/components/Modal/components/Section/Section.tsx +++ b/polaris-react/src/components/Modal/components/Section/Section.tsx @@ -28,6 +28,7 @@ export function Section({ {children} From 734993c7d58e3c5c70d0a1912e571af04a4219ed Mon Sep 17 00:00:00 2001 From: Lo Kim Date: Wed, 26 Oct 2022 14:43:14 -0400 Subject: [PATCH 08/14] [Modal] Update tests --- polaris-react/src/components/Modal/tests/Modal.test.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/polaris-react/src/components/Modal/tests/Modal.test.tsx b/polaris-react/src/components/Modal/tests/Modal.test.tsx index 2bb477dd756..4345b79de8e 100644 --- a/polaris-react/src/components/Modal/tests/Modal.test.tsx +++ b/polaris-react/src/components/Modal/tests/Modal.test.tsx @@ -3,6 +3,7 @@ import {animationFrame} from '@shopify/jest-dom-mocks'; import {mountWithApp} from 'tests/utilities'; import {Badge} from '../../Badge'; +import {Box} from '../../Box'; import {Button} from '../../Button'; import {Scrollable} from '../../Scrollable'; import {Spinner} from '../../Spinner'; @@ -302,8 +303,8 @@ describe('', () => { , ); - expect(modal.find(Header)).toContainReactComponent('div', { - className: 'Header', + expect(modal.find(Box)).toContainReactComponent('div', { + className: 'Title', }); }); @@ -315,6 +316,7 @@ describe('', () => { expect(modal.find(Header)).toContainReactComponent('div', { className: 'titleHidden', }); + expect(modal.find(Header)).not.toContainReactComponent(Box); }); }); From 7215453ebaaa49581b900e70b652da673a0b4bee Mon Sep 17 00:00:00 2001 From: Lo Kim Date: Wed, 26 Oct 2022 15:03:07 -0400 Subject: [PATCH 09/14] [Modal.Dialog] Add `aria-label` for accessibility --- polaris-react/src/components/Modal/components/Dialog/Dialog.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/polaris-react/src/components/Modal/components/Dialog/Dialog.tsx b/polaris-react/src/components/Modal/components/Dialog/Dialog.tsx index f640db87986..3442f22256a 100644 --- a/polaris-react/src/components/Modal/components/Dialog/Dialog.tsx +++ b/polaris-react/src/components/Modal/components/Dialog/Dialog.tsx @@ -73,6 +73,7 @@ export function Dialog({
Date: Wed, 26 Oct 2022 15:15:23 -0400 Subject: [PATCH 10/14] Add changeset --- .changeset/hip-wombats-sin.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/hip-wombats-sin.md diff --git a/.changeset/hip-wombats-sin.md b/.changeset/hip-wombats-sin.md new file mode 100644 index 00000000000..8781aee4f1d --- /dev/null +++ b/.changeset/hip-wombats-sin.md @@ -0,0 +1,5 @@ +--- +'@shopify/polaris': minor +--- + +Refactored `Modal` and its children components to use layout primitives From 35e8c7f0e7b91e61dd1e721600eb3dd8b1fd6aca Mon Sep 17 00:00:00 2001 From: Lo Kim Date: Thu, 27 Oct 2022 08:42:15 -0400 Subject: [PATCH 11/14] [Modal.Header] Test `Text` instead of `DisplayText` --- polaris-react/src/components/Modal/Modal.tsx | 4 +++- .../components/Modal/components/Header/Header.tsx | 12 ++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/polaris-react/src/components/Modal/Modal.tsx b/polaris-react/src/components/Modal/Modal.tsx index a4a4d9cfde3..b50b97d873f 100644 --- a/polaris-react/src/components/Modal/Modal.tsx +++ b/polaris-react/src/components/Modal/Modal.tsx @@ -214,7 +214,9 @@ export const Modal: React.FunctionComponent & { const animated = !instant; const activatorMarkup = - activator && !isRef(activator) ? {activator} : null; + activator && !isRef(activator) ? ( + {activator} + ) : null; return ( diff --git a/polaris-react/src/components/Modal/components/Header/Header.tsx b/polaris-react/src/components/Modal/components/Header/Header.tsx index b8297929bb0..fd410173db0 100644 --- a/polaris-react/src/components/Modal/components/Header/Header.tsx +++ b/polaris-react/src/components/Modal/components/Header/Header.tsx @@ -1,10 +1,11 @@ import React from 'react'; import {Box} from '../../../Box'; +import {CloseButton} from '../CloseButton'; import {Columns} from '../../../Columns'; -import {DisplayText} from '../../../DisplayText'; +// import {DisplayText} from '../../../DisplayText'; import {Inline} from '../../../Inline'; -import {CloseButton} from '../CloseButton'; +import {Text} from '../../../Text'; import styles from './Header.scss'; @@ -40,11 +41,14 @@ export function Header({id, titleHidden, children, onClose}: HeaderProps) { {/* Replace with once responsive styles supported */} -
+ {/*
{children} -
+
*/} + + {children} +
From 25e6efc588c81576525c3e1c5080315e75294cce Mon Sep 17 00:00:00 2001 From: Lo Kim Date: Fri, 28 Oct 2022 08:07:08 -0400 Subject: [PATCH 12/14] [Modal] Fix references to updated `Box` props --- .../components/Modal/components/Footer/Footer.tsx | 2 +- .../components/Modal/components/Header/Header.tsx | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/polaris-react/src/components/Modal/components/Footer/Footer.tsx b/polaris-react/src/components/Modal/components/Footer/Footer.tsx index 5609060804f..4ee3ba50231 100644 --- a/polaris-react/src/components/Modal/components/Footer/Footer.tsx +++ b/polaris-react/src/components/Modal/components/Footer/Footer.tsx @@ -35,7 +35,7 @@ export function Footer({ return ( - + {/* Replace with once responsive styles supported */} {/*
From f92be30f432bf41b315e1a522af719592be90720 Mon Sep 17 00:00:00 2001 From: Lo Kim Date: Fri, 28 Oct 2022 09:49:04 -0400 Subject: [PATCH 13/14] [Modal.Header] Use `Text` and remove `DisplayText` and css class --- .../components/Modal/components/Header/Header.scss | 6 ------ .../components/Modal/components/Header/Header.tsx | 13 ++++--------- .../components/Modal/components/Section/Section.tsx | 2 +- 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/polaris-react/src/components/Modal/components/Header/Header.scss b/polaris-react/src/components/Modal/components/Header/Header.scss index 3eea2814916..12e1a4d5618 100644 --- a/polaris-react/src/components/Modal/components/Header/Header.scss +++ b/polaris-react/src/components/Modal/components/Header/Header.scss @@ -1,9 +1,3 @@ -@import '../../../../styles/common'; - -.Title { - @include text-breakword; -} - .titleHidden { position: absolute; right: 0; diff --git a/polaris-react/src/components/Modal/components/Header/Header.tsx b/polaris-react/src/components/Modal/components/Header/Header.tsx index 92c79db8e5a..83a758930dd 100644 --- a/polaris-react/src/components/Modal/components/Header/Header.tsx +++ b/polaris-react/src/components/Modal/components/Header/Header.tsx @@ -3,7 +3,6 @@ import React from 'react'; import {Box} from '../../../Box'; import {CloseButton} from '../CloseButton'; import {Columns} from '../../../Columns'; -// import {DisplayText} from '../../../DisplayText'; import {Inline} from '../../../Inline'; import {Text} from '../../../Text'; @@ -40,15 +39,11 @@ export function Header({id, titleHidden, children, onClose}: HeaderProps) { - {/* Replace with once responsive styles supported */} - {/*
- + + {children} - -
*/} - - {children} - +
+
diff --git a/polaris-react/src/components/Modal/components/Section/Section.tsx b/polaris-react/src/components/Modal/components/Section/Section.tsx index 0539004c36a..032159fbbf6 100644 --- a/polaris-react/src/components/Modal/components/Section/Section.tsx +++ b/polaris-react/src/components/Modal/components/Section/Section.tsx @@ -28,7 +28,7 @@ export function Section({ {children} From e62c51b00e895217171e79803557dd57f34c633c Mon Sep 17 00:00:00 2001 From: Lo Kim Date: Fri, 28 Oct 2022 09:51:19 -0400 Subject: [PATCH 14/14] [Modal.Header] Update test to check for `Text` component --- polaris-react/src/components/Modal/tests/Modal.test.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/polaris-react/src/components/Modal/tests/Modal.test.tsx b/polaris-react/src/components/Modal/tests/Modal.test.tsx index 4345b79de8e..fc5994374b0 100644 --- a/polaris-react/src/components/Modal/tests/Modal.test.tsx +++ b/polaris-react/src/components/Modal/tests/Modal.test.tsx @@ -2,13 +2,14 @@ import React, {useRef} from 'react'; import {animationFrame} from '@shopify/jest-dom-mocks'; import {mountWithApp} from 'tests/utilities'; +import {Backdrop} from '../../Backdrop'; import {Badge} from '../../Badge'; import {Box} from '../../Box'; import {Button} from '../../Button'; +import {Portal} from '../../Portal'; import {Scrollable} from '../../Scrollable'; import {Spinner} from '../../Spinner'; -import {Portal} from '../../Portal'; -import {Backdrop} from '../../Backdrop'; +import {Text} from '../../Text'; import {Footer, Dialog, Header} from '../components'; import {Modal} from '../Modal'; import {WithinContentContext} from '../../../utilities/within-content-context'; @@ -303,9 +304,7 @@ describe('', () => { , ); - expect(modal.find(Box)).toContainReactComponent('div', { - className: 'Title', - }); + expect(modal.find(Box)).toContainReactComponent(Text); }); it('only renders a close button when titleHidden is present', () => {