From 4e614dc43916a72d307107ad125602f99e6f37e1 Mon Sep 17 00:00:00 2001 From: aveline Date: Thu, 6 Oct 2022 11:39:09 -0700 Subject: [PATCH 1/8] Refactor `Banner` with layout components --- .../src/components/Banner/Banner.scss | 17 ----------- .../src/components/Banner/Banner.tsx | 28 ++++++++----------- 2 files changed, 12 insertions(+), 33 deletions(-) diff --git a/polaris-react/src/components/Banner/Banner.scss b/polaris-react/src/components/Banner/Banner.scss index f3054443264..aee97f410e1 100644 --- a/polaris-react/src/components/Banner/Banner.scss +++ b/polaris-react/src/components/Banner/Banner.scss @@ -137,11 +137,6 @@ // stylelint-enable selector-max-class, selector-max-combinators, selector-max-specificity } -.ContentWrapper { - margin-top: calc(-1 * var(--p-space-05)); - flex: 1 1 auto; -} - .withinContentContainer { padding: var(--p-space-4); @@ -151,10 +146,6 @@ position: absolute; } - .Ribbon { - padding-right: var(--p-space-4); - } - @include banner-variants($in-page: false); + .Banner { @@ -195,19 +186,11 @@ padding-right: calc(var(--p-space-8) + var(--p-icon-size-small)); } -.Heading { - word-break: break-word; -} - .Content { @include text-breakword; padding: var(--p-space-05) 0; } -.Ribbon { - flex: 0 0 var(--p-space-8); -} - .PrimaryAction { margin-right: var(--p-space-2); } diff --git a/polaris-react/src/components/Banner/Banner.tsx b/polaris-react/src/components/Banner/Banner.tsx index eb3dc1875d2..891497caf15 100644 --- a/polaris-react/src/components/Banner/Banner.tsx +++ b/polaris-react/src/components/Banner/Banner.tsx @@ -19,13 +19,15 @@ import {useUniqueId} from '../../utilities/unique-id'; import {useI18n} from '../../utilities/i18n'; import type {Action, DisableableAction, LoadableAction} from '../../types'; import {Button} from '../Button'; -import {Heading} from '../Heading'; import {ButtonGroup} from '../ButtonGroup'; import {UnstyledButton, unstyledButtonFrom} from '../UnstyledButton'; import {UnstyledLink} from '../UnstyledLink'; import {Spinner} from '../Spinner'; import {Icon, IconProps} from '../Icon'; import {WithinContentContext} from '../../utilities/within-content-context'; +import {Text} from '../Text'; +import {Box} from '../Box'; +import {Bleed} from '../Bleed'; import styles from './Banner.scss'; @@ -79,14 +81,12 @@ export const Banner = forwardRef(function Banner( ); let headingMarkup: React.ReactNode = null; - let headingID: string | undefined; if (title) { - headingID = `${id}Heading`; headingMarkup = ( -
- {title} -
+ + {title} + ); } @@ -133,15 +133,13 @@ export const Banner = forwardRef(function Banner( ) : null; let contentMarkup: React.ReactNode = null; - let contentID: string | undefined; if (children || actionMarkup) { - contentID = `${id}Content`; contentMarkup = ( -
+ {children} {actionMarkup} -
+ ); } @@ -168,19 +166,17 @@ export const Banner = forwardRef(function Banner( onMouseUp={handleMouseUp} onKeyUp={handleKeyUp} onBlur={handleBlur} - aria-labelledby={headingID} - aria-describedby={contentID} > {dismissButton} -
+ -
+ -
+ {headingMarkup} {contentMarkup} -
+ ); From a8e2143177e846c5f175984ffd3c15bf4882db14 Mon Sep 17 00:00:00 2001 From: aveline Date: Thu, 6 Oct 2022 12:25:45 -0700 Subject: [PATCH 2/8] Refactor `Banner` actions styles --- .../src/components/Banner/Banner.scss | 25 +++---------------- .../src/components/Banner/Banner.tsx | 13 ++++++---- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/polaris-react/src/components/Banner/Banner.scss b/polaris-react/src/components/Banner/Banner.scss index aee97f410e1..cd15d7a0cba 100644 --- a/polaris-react/src/components/Banner/Banner.scss +++ b/polaris-react/src/components/Banner/Banner.scss @@ -55,7 +55,7 @@ display: flex; // stylelint-disable selector-max-class, selector-max-combinators, selector-max-specificity - &.statusCritical .PrimaryAction .Button { + &.statusCritical .PrimaryAction.Button { border-color: var(--p-border-critical-subdued); background: var(--p-surface-critical-subdued); @@ -75,7 +75,7 @@ } } - &.statusWarning .PrimaryAction .Button { + &.statusWarning .PrimaryAction.Button { border-color: var(--p-border-warning-subdued); background: var(--p-surface-warning-subdued); @@ -95,7 +95,7 @@ } } - &.statusInfo .PrimaryAction .Button { + &.statusInfo .PrimaryAction.Button { border-color: var(--p-border-highlight-subdued); background: var(--p-surface-highlight-subdued); @@ -115,7 +115,7 @@ } } - &.statusSuccess .PrimaryAction .Button { + &.statusSuccess .PrimaryAction.Button { border-color: var(--p-border-success-subdued); background: var(--p-surface-success-subdued); @@ -151,10 +151,6 @@ + .Banner { margin-top: var(--p-space-2); } - - .Actions { - padding: var(--p-space-3) 0 var(--p-space-1) 0; - } } .withinPage { @@ -171,10 +167,6 @@ padding-right: var(--p-space-4); } - .Actions { - padding-top: var(--p-space-4); - } - .Dismiss { right: var(--p-space-4); top: var(--p-space-5); @@ -186,15 +178,6 @@ padding-right: calc(var(--p-space-8) + var(--p-icon-size-small)); } -.Content { - @include text-breakword; - padding: var(--p-space-05) 0; -} - -.PrimaryAction { - margin-right: var(--p-space-2); -} - // We need pretty high specificity to do the descendant selectors // onto the text, which needs to be the relative positioned wrapper // so that the borders/ backgrounds do not extend outside of it. diff --git a/polaris-react/src/components/Banner/Banner.tsx b/polaris-react/src/components/Banner/Banner.tsx index 891497caf15..f821ddd81ff 100644 --- a/polaris-react/src/components/Banner/Banner.tsx +++ b/polaris-react/src/components/Banner/Banner.tsx @@ -109,13 +109,13 @@ export const Banner = forwardRef(function Banner( ) : null; const primaryActionMarkup = action ? ( -
+ {action.loading ? spinnerMarkup : unstyledButtonFrom(action, { - className: styles.Button, + className: `${styles.Button} ${styles.PrimaryAction}`, })} -
+ ) : null; const secondaryActionMarkup = secondaryAction ? ( @@ -124,12 +124,15 @@ export const Banner = forwardRef(function Banner( const actionMarkup = action || secondaryAction ? ( -
+ {primaryActionMarkup} {secondaryActionMarkup} -
+ ) : null; let contentMarkup: React.ReactNode = null; From 1b385ce15133bba36898e81f8abaf91b1a0372ae Mon Sep 17 00:00:00 2001 From: aveline Date: Thu, 6 Oct 2022 12:34:28 -0700 Subject: [PATCH 3/8] Update Banner.test.tsx --- polaris-react/src/components/Banner/tests/Banner.test.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/polaris-react/src/components/Banner/tests/Banner.test.tsx b/polaris-react/src/components/Banner/tests/Banner.test.tsx index f6cf3e3042b..4de1228e953 100644 --- a/polaris-react/src/components/Banner/tests/Banner.test.tsx +++ b/polaris-react/src/components/Banner/tests/Banner.test.tsx @@ -9,7 +9,7 @@ import { import {mountWithApp} from 'tests/utilities'; import {Button} from '../../Button'; -import {Heading} from '../../Heading'; +import {Text} from '../../Text'; import {Icon} from '../../Icon'; import {Spinner} from '../../Spinner'; import {UnstyledButton} from '../../UnstyledButton'; @@ -21,12 +21,12 @@ import {Banner, BannerHandles} from '../Banner'; describe('', () => { it('renders a title', () => { const banner = mountWithApp(); - expect(banner.find(Heading)).toContainReactText('Banner title'); + expect(banner.find(Text)).toContainReactText('Banner title'); }); - it('passes a p element to Heading', () => { + it('passes an h2 element to Heading', () => { const banner = mountWithApp(); - expect(banner).toContainReactComponent(Heading, {element: 'p'}); + expect(banner).toContainReactComponent(Text, {as: 'h2'}); }); it('passes the provided icon source to Icon', () => { From a5af2a3625b9ad92c4ecdd5090805c485aaff967 Mon Sep 17 00:00:00 2001 From: aveline Date: Thu, 6 Oct 2022 14:58:57 -0700 Subject: [PATCH 4/8] Remove unused const --- polaris-react/src/components/Banner/Banner.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/polaris-react/src/components/Banner/Banner.tsx b/polaris-react/src/components/Banner/Banner.tsx index f821ddd81ff..25a512cd9a6 100644 --- a/polaris-react/src/components/Banner/Banner.tsx +++ b/polaris-react/src/components/Banner/Banner.tsx @@ -66,7 +66,6 @@ export const Banner = forwardRef(function Banner( bannerRef, ) { const withinContentContainer = useContext(WithinContentContext); - const id = useUniqueId('Banner'); const i18n = useI18n(); const {wrapperRef, handleKeyUp, handleBlur, handleMouseUp, shouldShowFocus} = useBannerFocus(bannerRef); From 08de3f61d4701088aa2e304bcc86db73c71974b6 Mon Sep 17 00:00:00 2001 From: aveline Date: Thu, 6 Oct 2022 15:00:10 -0700 Subject: [PATCH 5/8] changeset --- .changeset/healthy-apricots-rule.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/healthy-apricots-rule.md diff --git a/.changeset/healthy-apricots-rule.md b/.changeset/healthy-apricots-rule.md new file mode 100644 index 00000000000..4a9135dba7d --- /dev/null +++ b/.changeset/healthy-apricots-rule.md @@ -0,0 +1,5 @@ +--- +'@shopify/polaris': minor +--- + +Updated `Banner` component to use new layout primitives From 72cdd60f54f7f2d5af3835a07036bb2717f51664 Mon Sep 17 00:00:00 2001 From: aveline Date: Thu, 6 Oct 2022 15:24:24 -0700 Subject: [PATCH 6/8] Remove unused import --- polaris-react/src/components/Banner/Banner.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/polaris-react/src/components/Banner/Banner.tsx b/polaris-react/src/components/Banner/Banner.tsx index 25a512cd9a6..5a794b91923 100644 --- a/polaris-react/src/components/Banner/Banner.tsx +++ b/polaris-react/src/components/Banner/Banner.tsx @@ -15,7 +15,6 @@ import { import {classNames, variationName} from '../../utilities/css'; import {BannerContext} from '../../utilities/banner-context'; -import {useUniqueId} from '../../utilities/unique-id'; import {useI18n} from '../../utilities/i18n'; import type {Action, DisableableAction, LoadableAction} from '../../types'; import {Button} from '../Button'; From 72dcd5de3aa5e2121776a576b3a134242ec4c3d9 Mon Sep 17 00:00:00 2001 From: aveline Date: Thu, 6 Oct 2022 16:32:17 -0700 Subject: [PATCH 7/8] =?UTF-8?q?=F0=9F=91=95=20Remove=20needless=20disable?= =?UTF-8?q?=20for=20"selector-max-combinators"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- polaris-react/src/components/Banner/Banner.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/polaris-react/src/components/Banner/Banner.scss b/polaris-react/src/components/Banner/Banner.scss index cd15d7a0cba..427fc7f479c 100644 --- a/polaris-react/src/components/Banner/Banner.scss +++ b/polaris-react/src/components/Banner/Banner.scss @@ -54,7 +54,7 @@ position: relative; display: flex; - // stylelint-disable selector-max-class, selector-max-combinators, selector-max-specificity + // stylelint-disable selector-max-class, selector-max-specificity &.statusCritical .PrimaryAction.Button { border-color: var(--p-border-critical-subdued); background: var(--p-surface-critical-subdued); @@ -134,7 +134,7 @@ background: var(--p-surface-success-subdued); } } - // stylelint-enable selector-max-class, selector-max-combinators, selector-max-specificity + // stylelint-enable selector-max-class, selector-max-specificity } .withinContentContainer { From bbbd721eb7f44651484363ec32e4e97c7537a76c Mon Sep 17 00:00:00 2001 From: aveline Date: Wed, 12 Oct 2022 13:48:22 -0700 Subject: [PATCH 8/8] Refactor `AccountConnection` with new layout and type components (#7345) ### WHY are these changes introduced? Fixes #7341 --- .changeset/pink-apricots-help.md | 5 +++ .../AccountConnection/AccountConnection.scss | 10 ----- .../AccountConnection/AccountConnection.tsx | 40 +++++++++---------- .../tests/AccountConnection.test.tsx | 9 +++++ 4 files changed, 33 insertions(+), 31 deletions(-) create mode 100644 .changeset/pink-apricots-help.md delete mode 100644 polaris-react/src/components/AccountConnection/AccountConnection.scss diff --git a/.changeset/pink-apricots-help.md b/.changeset/pink-apricots-help.md new file mode 100644 index 00000000000..e58b098d36a --- /dev/null +++ b/.changeset/pink-apricots-help.md @@ -0,0 +1,5 @@ +--- +'@shopify/polaris': minor +--- + +Refactored `AccountConnection` to use new layout primitives diff --git a/polaris-react/src/components/AccountConnection/AccountConnection.scss b/polaris-react/src/components/AccountConnection/AccountConnection.scss deleted file mode 100644 index 2d95d597e42..00000000000 --- a/polaris-react/src/components/AccountConnection/AccountConnection.scss +++ /dev/null @@ -1,10 +0,0 @@ -.TermsOfService { - margin-top: var(--p-space-5); -} - -.Content { - // stylelint-disable-next-line selector-max-combinators - > * + * { - margin-top: var(--p-space-2); - } -} diff --git a/polaris-react/src/components/AccountConnection/AccountConnection.tsx b/polaris-react/src/components/AccountConnection/AccountConnection.tsx index 0810bb83c14..19933ddee88 100644 --- a/polaris-react/src/components/AccountConnection/AccountConnection.tsx +++ b/polaris-react/src/components/AccountConnection/AccountConnection.tsx @@ -3,12 +3,12 @@ import React from 'react'; import type {Action} from '../../types'; import {Avatar} from '../Avatar'; import {buttonFrom} from '../Button'; -import {Card} from '../Card'; -import {Stack} from '../Stack'; -import {TextStyle} from '../TextStyle'; import {SettingAction} from '../SettingAction'; - -import styles from './AccountConnection.scss'; +import {AlphaCard} from '../AlphaCard'; +import {Box} from '../Box'; +import {Inline} from '../Inline'; +import {Text} from '../Text'; +import {AlphaStack} from '../AlphaStack'; export interface AccountConnectionProps { /** Content to display as title */ @@ -54,19 +54,19 @@ export function AccountConnection({ let titleMarkup: React.ReactNode = null; if (title) { - titleMarkup =
{title}
; + titleMarkup = <>{title}; } else if (accountName) { - titleMarkup =
{accountName}
; + titleMarkup = <>{accountName}; } const detailsMarkup = details ? ( -
- {details} -
+ + {details} + ) : null; const termsOfServiceMarkup = termsOfService ? ( -
{termsOfService}
+ {termsOfService} ) : null; const actionElement = action @@ -74,19 +74,17 @@ export function AccountConnection({ : null; return ( - + - + {avatarMarkup} - -
- {titleMarkup} - {detailsMarkup} -
-
-
+ + {titleMarkup} + {detailsMarkup} + +
{termsOfServiceMarkup} -
+ ); } diff --git a/polaris-react/src/components/AccountConnection/tests/AccountConnection.test.tsx b/polaris-react/src/components/AccountConnection/tests/AccountConnection.test.tsx index 3258a5c60ad..aac5109963c 100644 --- a/polaris-react/src/components/AccountConnection/tests/AccountConnection.test.tsx +++ b/polaris-react/src/components/AccountConnection/tests/AccountConnection.test.tsx @@ -1,11 +1,20 @@ import React from 'react'; import {mountWithApp} from 'tests/utilities'; +import {matchMedia} from '@shopify/jest-dom-mocks'; import {Avatar} from '../../Avatar'; import {Button} from '../../Button'; import {AccountConnection} from '../AccountConnection'; describe('', () => { + beforeEach(() => { + matchMedia.mock(); + }); + + afterEach(() => { + matchMedia.restore(); + }); + describe('title', () => { it('shows the title when one is provided', () => { const title = 'Example app';