From 99627e3daf7e0ee68239bda22c006d7677ee6dd6 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 11 Jan 2023 13:17:38 +1000 Subject: [PATCH 1/6] Add visual ordering and pageLayout example --- src/PageHeader/PageHeader.tsx | 26 +++++++ src/PageHeader/examples.stories.tsx | 106 +++++++++++++++++++++++++++- 2 files changed, 131 insertions(+), 1 deletion(-) diff --git a/src/PageHeader/PageHeader.tsx b/src/PageHeader/PageHeader.tsx index f43aa9a8d40..d35d1aef168 100644 --- a/src/PageHeader/PageHeader.tsx +++ b/src/PageHeader/PageHeader.tsx @@ -14,6 +14,21 @@ const REGION_ORDER = { Navigation: 3, } +const CONTEXT_AREA_REGION_ORDER = { + ParentLink: 0, + ContextBar: 1, + ContextAreaActions: 2, +} + +const TITLE_AREA_REGION_ORDER = { + LeadingAction: 0, + LeadingVisual: 1, + Title: 2, + TrailingVisual: 3, + TrailingAction: 4, + Actions: 5, +} + // Types that are shared between sub components export type sharedPropTypes = { hidden?: boolean | ResponsiveValue @@ -105,6 +120,7 @@ const ParentLink = React.forwardRef( { display: 'flex', alignItems: 'center', + order: CONTEXT_AREA_REGION_ORDER.ParentLink, gap: '0.5rem', ...getBreakpointDeclarations(hidden, 'display', value => { return value ? 'none' : 'flex' @@ -136,6 +152,7 @@ const ContextBar: React.FC> = ({ sx={merge( { display: 'flex', + order: CONTEXT_AREA_REGION_ORDER.ContextBar, ...getBreakpointDeclarations(hidden, 'display', value => { return value ? 'none' : 'flex' }), @@ -161,6 +178,7 @@ const ContextAreaActions: React.FC> = ( { display: 'flex', flexDirection: 'row', + order: CONTEXT_AREA_REGION_ORDER.ContextAreaActions, alignItems: 'center', gap: '0.5rem', flexGrow: '1', @@ -238,6 +256,7 @@ const LeadingAction: React.FC> = ({ sx={merge( { display: 'flex', + order: TITLE_AREA_REGION_ORDER.LeadingAction, ...getBreakpointDeclarations(hidden, 'display', value => { return value ? 'none' : 'flex' }), @@ -259,6 +278,7 @@ const LeadingVisual: React.FC> = ({chil sx={merge( { display: 'flex', + order: TITLE_AREA_REGION_ORDER.LeadingVisual, ...getBreakpointDeclarations(hidden, 'display', value => { return value ? 'none' : 'flex' }), @@ -302,6 +322,7 @@ const Title: React.FC> = ({children, sx = {} subtitle: '400', }[titleVariant], display: 'flex', + order: TITLE_AREA_REGION_ORDER.Title, ...getBreakpointDeclarations(hidden, 'display', value => { return value ? 'none' : 'flex' }), @@ -321,6 +342,7 @@ const TrailingVisual: React.FC> = ({chi sx={merge( { display: 'flex', + order: TITLE_AREA_REGION_ORDER.TrailingVisual, ...getBreakpointDeclarations(hidden, 'display', value => { return value ? 'none' : 'flex' }), @@ -347,6 +369,7 @@ const TrailingAction: React.FC> = ({ sx={merge( { display: 'flex', + order: TITLE_AREA_REGION_ORDER.TrailingAction, ...getBreakpointDeclarations(hidden, 'display', value => { return value ? 'none' : 'flex' }), @@ -368,6 +391,7 @@ const Actions: React.FC> = ({children, sx={merge( { display: 'flex', + order: TITLE_AREA_REGION_ORDER.Actions, ...getBreakpointDeclarations(hidden, 'display', value => { return value ? 'none' : 'flex' }), @@ -393,6 +417,7 @@ const Description: React.FC> = ({childr sx={merge( { display: 'flex', + order: REGION_ORDER.Description, ...getBreakpointDeclarations(hidden, 'display', value => { return value ? 'none' : 'flex' }), @@ -415,6 +440,7 @@ const Navigation: React.FC> = ({childre sx={merge( { display: 'flex', + order: REGION_ORDER.Navigation, ...getBreakpointDeclarations(hidden, 'display', value => { return value ? 'none' : 'block' }), diff --git a/src/PageHeader/examples.stories.tsx b/src/PageHeader/examples.stories.tsx index 92eec4312e6..06d11b7a0c0 100644 --- a/src/PageHeader/examples.stories.tsx +++ b/src/PageHeader/examples.stories.tsx @@ -1,6 +1,6 @@ import React from 'react' import {Meta} from '@storybook/react' -import {Button, IconButton, Breadcrumbs, Link, Text, StateLabel, BranchName, Box} from '..' +import {Button, IconButton, Breadcrumbs, Link, Text, StateLabel, BranchName, Box, PageLayout, TabNav, Heading} from '..' import { KebabHorizontalIcon, GitBranchIcon, @@ -158,6 +158,110 @@ export const FilesPageOnNarrowViewport = () => { return } +export const WithPageLayout = () => { + return ( + + + + + Pull requests + + + + PageHeader component initial layout explorations extra long pull request title   + #1831 + + + + + {/* Pop up actions */} + + + + + + + + + + + + Open + + + + broccolinisoup + {' '} + wants to merge 3 commits into main from{' '} + broccolinisoup/switch-to-new-underlineNav + + + + + main + + page-header-initial + + + + + + + Conversation + + + Commits + + + Checks + + + Files Changes + + + + + + + + + This box has really long content. If it is too long, it will cause x overflow and should show a scrollbar. + When this overflows, it should not break to overall page layout! + + + + + + Assignees + + No one –{' '} + + assign yourself + + + + + + Labels + None yet + + + + + ) +} + FilesPageOnNarrowViewport.parameters = setViewportParamToNarrow export default meta From 3ff5069e8a8e14a55a0e57faec19ec75d76a6fbe Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 12 Jan 2023 12:09:24 +1000 Subject: [PATCH 2/6] Mock IntersectionObserver for in story render --- .../__snapshots__/PageHeader.test.tsx.snap | 12 ++++++++++++ src/utils/testing.tsx | 19 ++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/PageHeader/__snapshots__/PageHeader.test.tsx.snap b/src/PageHeader/__snapshots__/PageHeader.test.tsx.snap index 25f75b6d9b1..8d5bf1cb354 100644 --- a/src/PageHeader/__snapshots__/PageHeader.test.tsx.snap +++ b/src/PageHeader/__snapshots__/PageHeader.test.tsx.snap @@ -50,6 +50,9 @@ exports[`PageHeader renders consistently 1`] = ` display: -webkit-flex; display: -ms-flexbox; display: flex; + -webkit-order: 2; + -ms-flex-order: 2; + order: 2; -webkit-flex-direction: row; -ms-flex-direction: row; flex-direction: row; @@ -62,6 +65,9 @@ exports[`PageHeader renders consistently 1`] = ` .c4 { display: block; + -webkit-order: 3; + -ms-flex-order: 3; + order: 3; } @media screen and (max-width:calc(768px - 0.02px)) { @@ -147,6 +153,9 @@ exports[`PageHeader renders default layout 1`] = ` display: -webkit-flex; display: -ms-flexbox; display: flex; + -webkit-order: 2; + -ms-flex-order: 2; + order: 2; -webkit-flex-direction: row; -ms-flex-direction: row; flex-direction: row; @@ -159,6 +168,9 @@ exports[`PageHeader renders default layout 1`] = ` .c4 { display: block; + -webkit-order: 3; + -ms-flex-order: 3; + order: 3; } @media screen and (max-width:calc(768px - 0.02px)) { diff --git a/src/utils/testing.tsx b/src/utils/testing.tsx index a84e0053dc4..ef5433002bb 100644 --- a/src/utils/testing.tsx +++ b/src/utils/testing.tsx @@ -247,8 +247,25 @@ export function checkStoriesForAxeViolations(name: string, storyDir?: string) { if (typeof Story !== 'function') return const {storyName, name: StoryFunctionName} = Story as StoryType + + beforeEach(() => { + // IntersectionObserver isn't available in test environment + const mockIntersectionObserver = jest.fn() + mockIntersectionObserver.mockReturnValue({ + observe: () => null, + unobserve: () => null, + disconnect: () => null, + }) + window.IntersectionObserver = mockIntersectionObserver + }) + it(`story ${storyName || StoryFunctionName} should have no axe violations`, async () => { - const {container} = HTMLRender() + const {container} = HTMLRender( + + + , + ) + const results = await axe(container) expect(results).toHaveNoViolations() }) From 376d219cdc1bd19ab135514c1568bcc6c8f5854b Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 12 Jan 2023 12:27:17 +1000 Subject: [PATCH 3/6] remove unused components --- src/PageHeader/examples.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PageHeader/examples.stories.tsx b/src/PageHeader/examples.stories.tsx index 06d11b7a0c0..7b2c8dbfd6d 100644 --- a/src/PageHeader/examples.stories.tsx +++ b/src/PageHeader/examples.stories.tsx @@ -1,6 +1,6 @@ import React from 'react' import {Meta} from '@storybook/react' -import {Button, IconButton, Breadcrumbs, Link, Text, StateLabel, BranchName, Box, PageLayout, TabNav, Heading} from '..' +import {Button, IconButton, Breadcrumbs, Link, Text, StateLabel, BranchName, Box, PageLayout} from '..' import { KebabHorizontalIcon, GitBranchIcon, From bcc8e49b1a9da4088b126e42ff2b1e87b8495987 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 18 Jan 2023 14:52:04 +1000 Subject: [PATCH 4/6] add TitleArea order --- src/PageHeader/PageHeader.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/PageHeader/PageHeader.tsx b/src/PageHeader/PageHeader.tsx index d35d1aef168..b702359816e 100644 --- a/src/PageHeader/PageHeader.tsx +++ b/src/PageHeader/PageHeader.tsx @@ -229,6 +229,7 @@ const TitleArea: React.FC> = ({ { display: 'flex', gap: '0.5rem', + order: REGION_ORDER.TitleArea, ...getBreakpointDeclarations(hidden, 'display', value => { return value ? 'none' : 'flex' }), From 4aa73ed2d6a50e4f9d1e55454a36a4e09b8b9c11 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 18 Jan 2023 21:34:39 +1000 Subject: [PATCH 5/6] add changeset --- .changeset/happy-snails-learn.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/happy-snails-learn.md diff --git a/.changeset/happy-snails-learn.md b/.changeset/happy-snails-learn.md new file mode 100644 index 00000000000..80f96117fef --- /dev/null +++ b/.changeset/happy-snails-learn.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +PageHeader: Add visual ordering for layout enforcement From d49844b62b32d2550ab993035378e668dde97933 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 18 Jan 2023 21:59:46 +1000 Subject: [PATCH 6/6] update the snapshot --- src/PageHeader/__snapshots__/PageHeader.test.tsx.snap | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/PageHeader/__snapshots__/PageHeader.test.tsx.snap b/src/PageHeader/__snapshots__/PageHeader.test.tsx.snap index 8d5bf1cb354..3ed3eac0cd7 100644 --- a/src/PageHeader/__snapshots__/PageHeader.test.tsx.snap +++ b/src/PageHeader/__snapshots__/PageHeader.test.tsx.snap @@ -36,6 +36,9 @@ exports[`PageHeader renders consistently 1`] = ` display: -ms-flexbox; display: flex; gap: 0.5rem; + -webkit-order: 1; + -ms-flex-order: 1; + order: 1; -webkit-flex-direction: row; -ms-flex-direction: row; flex-direction: row; @@ -139,6 +142,9 @@ exports[`PageHeader renders default layout 1`] = ` display: -ms-flexbox; display: flex; gap: 0.5rem; + -webkit-order: 1; + -ms-flex-order: 1; + order: 1; -webkit-flex-direction: row; -ms-flex-direction: row; flex-direction: row;