From f8a9d342ca915c4a95423637d504055c42d3ca73 Mon Sep 17 00:00:00 2001 From: Marc Thomas Date: Thu, 20 Jan 2022 17:54:12 +0000 Subject: [PATCH 1/2] chore: increase margin of divider in page --- UNRELEASED.md | 1 + src/components/Page/Page.scss | 4 +- src/components/Page/Page.tsx | 2 +- src/components/Page/README.md | 40 ++++++++++++++----- .../Page/components/Header/Header.scss | 3 ++ .../Header/components/Title/Title.scss | 6 ++- .../Header/components/Title/Title.tsx | 8 +++- src/components/Page/tests/Page.test.tsx | 4 +- src/styles/shared/_page.scss | 6 +-- 9 files changed, 54 insertions(+), 20 deletions(-) diff --git a/UNRELEASED.md b/UNRELEASED.md index 80dd65ff3c7..f215764b053 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -12,6 +12,7 @@ Use [the changelog guidelines](/documentation/Versioning%20and%20changelog.md) t - Added new `duplicateRootItem` prop to a Navigation Section to support new mobile Navigation IA ([#4902](https://github.com/Shopify/polaris-react/pull/4902)) - Updated mobile behaviour of Navigation to only show one sub-section at a time ([#4902](https://github.com/Shopify/polaris-react/pull/4902)) - Remove the info icon and external link guidance from FooterHelp ([#4982](https://github.com/Shopify/polaris-react/pull/4982)) +- Normalise spacing around the `Header` within the `Page` ([#4911](https://github.com/Shopify/polaris-react/pull/4911)) ### Bug fixes diff --git a/src/components/Page/Page.scss b/src/components/Page/Page.scss index 4abdf7f5be2..b28b59b9c52 100644 --- a/src/components/Page/Page.scss +++ b/src/components/Page/Page.scss @@ -32,11 +32,11 @@ body { max-width: layout-width(primary, max); } -.Content { +.ContentSpaced { @include page-content-layout; } .divider { - border-top: border(); + border-top: border(divider); padding-top: spacing(); } diff --git a/src/components/Page/Page.tsx b/src/components/Page/Page.tsx index f7bfab90e7d..842c6dace12 100644 --- a/src/components/Page/Page.tsx +++ b/src/components/Page/Page.tsx @@ -37,7 +37,7 @@ export function Page({ (rest.breadcrumbs != null && rest.breadcrumbs.length > 0); const contentClassName = classNames( - styles.Content, + !hasHeaderContent && styles.ContentSpaced, divider && hasHeaderContent && styles.divider, ); diff --git a/src/components/Page/README.md b/src/components/Page/README.md index 37d40eefd7f..aaf98edb029 100644 --- a/src/components/Page/README.md +++ b/src/components/Page/README.md @@ -175,7 +175,9 @@ Use for detail pages, which should have pagination and breadcrumbs, and also oft }} additionalNavigation={} > -

Page content

+ +

Credit card information

+ ``` @@ -223,7 +225,9 @@ Use to create a custom primary action. } > -

Page content

+ +

Credit card information

+ ``` @@ -286,7 +290,9 @@ Use when the page title benefits from secondary content. subtitle="Statement period: May 3, 2019 to June 2, 2019" secondaryActions={[{content: 'Download', icon: ArrowDownMinor}]} > -

Page content

+ +

Credit card information

+ ``` @@ -332,7 +338,9 @@ Use when an image will help merchants identify the purpose of the page. hasNext: true, }} > -

Page content

+ +

Credit card information

+ ``` @@ -355,7 +363,9 @@ Use when a secondary action links to another website. Actions marked external op }, ]} > -

Page Content

+ +

Credit card information

+ ``` @@ -371,7 +381,9 @@ Use when the page doesn’t represent a list of objects or a detail view for an title="General" primaryAction={{content: 'Save'}} > -

Page content

+ +

Credit card information

+ ``` @@ -391,7 +403,9 @@ Use for layouts that benefit from more screen width, such as wide tables or list hasNext: true, }} > -

Wide page content

+ +

Credit card information

+ ``` @@ -439,7 +453,9 @@ Use action groups for sets of actions that relate to one another, particularly w }, ]} > -

Page content

+ +

Credit card information

+ ``` @@ -461,7 +477,9 @@ Title metadata appears immediately after the page’s title. Use it to communica hasNext: true, }} > -

Page content

+ +

Credit card information

+ ``` @@ -477,7 +495,9 @@ Use when the page needs visual separation between the page header and the conten title="General" divider > -

Page content

+ +

Credit card information

+ ``` diff --git a/src/components/Page/components/Header/Header.scss b/src/components/Page/components/Header/Header.scss index 76ac4488909..860c3a9892f 100644 --- a/src/components/Page/components/Header/Header.scss +++ b/src/components/Page/components/Header/Header.scss @@ -246,6 +246,9 @@ $action-menu-rollup-computed-width: rem(40px); // stylelint-disable-next-line selector-max-class .RightAlign { margin-bottom: spacing(extra-tight); + @include breakpoint-after(layout-width(page-with-nav)) { + margin-bottom: 0; + } } // stylelint-disable-next-line selector-max-class .Row { diff --git a/src/components/Page/components/Header/components/Title/Title.scss b/src/components/Page/components/Header/components/Title/Title.scss index c2162cdfe4d..1821b5c2212 100644 --- a/src/components/Page/components/Header/components/Title/Title.scss +++ b/src/components/Page/components/Header/components/Title/Title.scss @@ -11,8 +11,12 @@ } } +.TitleWithSubtitle { + margin-top: spacing(extra-tight); +} + .SubTitle { - margin-top: spacing(tight); + margin-top: spacing(extra-tight); color: var(--p-text-subdued); &.SubtitleCompact { diff --git a/src/components/Page/components/Header/components/Title/Title.tsx b/src/components/Page/components/Header/components/Title/Title.tsx index 8771d129900..67f0e71a7d7 100644 --- a/src/components/Page/components/Header/components/Title/Title.tsx +++ b/src/components/Page/components/Header/components/Title/Title.tsx @@ -33,7 +33,13 @@ export function Title({ console.warn('The thumbnail prop from Page has been deprecated'); } - const titleMarkup = title ?

{title}

: null; + const titleMarkup = title ? ( +

+ {title} +

+ ) : null; const titleMetadataMarkup = titleMetadata ? (
{titleMetadata}
diff --git a/src/components/Page/tests/Page.test.tsx b/src/components/Page/tests/Page.test.tsx index 1472d524bd8..17e9b7259f7 100644 --- a/src/components/Page/tests/Page.test.tsx +++ b/src/components/Page/tests/Page.test.tsx @@ -271,10 +271,10 @@ describe('', () => { it('does not render border when divider is true and no header props exist', () => { const wrapper = mountWithApp(); expect(wrapper).not.toContainReactComponent('div', { - className: 'Content divider', + className: 'Content ContentSpaced divider', }); expect(wrapper).toContainReactComponent('div', { - className: 'Content', + className: 'Content ContentSpaced', }); }); diff --git a/src/styles/shared/_page.scss b/src/styles/shared/_page.scss index 50aedbb3b01..589cad3ab86 100644 --- a/src/styles/shared/_page.scss +++ b/src/styles/shared/_page.scss @@ -26,7 +26,7 @@ $actions-vertical-spacing: spacing(tight); margin: spacing(tight) 0; @include page-content-when-not-partially-condensed { - margin-top: spacing(tight); + margin-top: spacing(loose); } } @@ -46,7 +46,7 @@ $actions-vertical-spacing: spacing(tight); } @mixin page-header-layout { - padding: spacing() spacing() 0; + padding: spacing(); @include page-content-when-not-fully-condensed { padding-left: 0; @@ -54,7 +54,7 @@ $actions-vertical-spacing: spacing(tight); } @include page-content-when-not-partially-condensed { - padding-top: spacing(); + padding: spacing(loose) 0; } } From ed037c2f8ba6a16dd99d19d0ac5b597d7144ae25 Mon Sep 17 00:00:00 2001 From: Marc Thomas Date: Tue, 25 Jan 2022 16:36:08 +0000 Subject: [PATCH 2/2] chore: address comments from review --- src/components/Page/Page.scss | 2 +- src/components/Page/Page.tsx | 2 +- src/components/Page/tests/Page.test.tsx | 11 ++++------- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/components/Page/Page.scss b/src/components/Page/Page.scss index b28b59b9c52..922345e9b13 100644 --- a/src/components/Page/Page.scss +++ b/src/components/Page/Page.scss @@ -32,7 +32,7 @@ body { max-width: layout-width(primary, max); } -.ContentSpaced { +.Content { @include page-content-layout; } diff --git a/src/components/Page/Page.tsx b/src/components/Page/Page.tsx index 842c6dace12..cfda646ca70 100644 --- a/src/components/Page/Page.tsx +++ b/src/components/Page/Page.tsx @@ -37,7 +37,7 @@ export function Page({ (rest.breadcrumbs != null && rest.breadcrumbs.length > 0); const contentClassName = classNames( - !hasHeaderContent && styles.ContentSpaced, + !hasHeaderContent && styles.Content, divider && hasHeaderContent && styles.divider, ); diff --git a/src/components/Page/tests/Page.test.tsx b/src/components/Page/tests/Page.test.tsx index 17e9b7259f7..299cd2acec0 100644 --- a/src/components/Page/tests/Page.test.tsx +++ b/src/components/Page/tests/Page.test.tsx @@ -264,27 +264,24 @@ describe('', () => { it('renders border when divider is true and header props exist', () => { const wrapper = mountWithApp(); expect(wrapper).toContainReactComponent('div', { - className: 'Content divider', + className: 'divider', }); }); it('does not render border when divider is true and no header props exist', () => { const wrapper = mountWithApp(); expect(wrapper).not.toContainReactComponent('div', { - className: 'Content ContentSpaced divider', + className: 'Content divider', }); expect(wrapper).toContainReactComponent('div', { - className: 'Content ContentSpaced', + className: 'Content', }); }); it('does not render border when divider is false', () => { const wrapper = mountWithApp(); expect(wrapper).not.toContainReactComponent('div', { - className: 'Content divider', - }); - expect(wrapper).toContainReactComponent('div', { - className: 'Content', + className: 'divider', }); }); });