Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[⏪ Reverted] Deprecate position prop for PageLayout.Pane, update stories and tests. #3389

Merged
merged 22 commits into from
Jul 18, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
b02d971
Deprecate position prop for PageLayout.Pane, update stories and tests.
radglob Jun 7, 2023
d9dff24
Create hungry-spies-remember.md
radglob Jun 7, 2023
90cf7cd
Merge branch 'main' into deprecate-pagelayout-pane-position-prop
radglob Jun 7, 2023
a1d3625
Fix typo in docs.
radglob Jun 7, 2023
044149d
Merge branch 'main' into deprecate-pagelayout-pane-position-prop
radglob Jun 8, 2023
d22cd80
Merge branch 'main' into deprecate-pagelayout-pane-position-prop
radglob Jun 8, 2023
9d11a86
Revert PRs that are breaking dotcom to get the current release out (#…
broccolinisoup Jun 16, 2023
06fa960
Version Packages (#3218)
primer-css Jun 16, 2023
19f1b6e
Merge branch 'main' of github.com:primer/react into deprecate-pagelay…
radglob Jun 16, 2023
9423e76
Merge branch 'main' into deprecate-pagelayout-pane-position-prop
radglob Jun 16, 2023
ca95066
Merge branch 'main' into deprecate-pagelayout-pane-position-prop
radglob Jun 16, 2023
a791216
Merge branch 'main' into deprecate-pagelayout-pane-position-prop
mperrotti Jun 19, 2023
dae8bfc
Merge branch 'main' into deprecate-pagelayout-pane-position-prop
radglob Jun 20, 2023
53e6121
Merge branch 'main' into deprecate-pagelayout-pane-position-prop
radglob Jun 20, 2023
04de0f9
Merge branch 'main' into deprecate-pagelayout-pane-position-prop
radglob Jun 20, 2023
32d48b5
Merge branch 'main' into deprecate-pagelayout-pane-position-prop
radglob Jun 21, 2023
73fdd6f
Merge branch 'main' of github.com:primer/react into deprecate-pagelay…
radglob Jul 10, 2023
f6a41b1
Update changeset to mention changed component.
radglob Jul 10, 2023
68e0e4e
Put changed components comment after changeset write up.
radglob Jul 10, 2023
e64cfa5
Merge branch 'main' of github.com:primer/react into deprecate-pagelay…
radglob Jul 18, 2023
f32db23
Fix some spacing issues, docs and story changes.
radglob Jul 18, 2023
b05f2a6
Update snapshots, formatting and linting.
radglob Jul 18, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/hungry-spies-remember.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

Deprecates position prop for PageLayout.Pane. Users will receive a console warning when it is used.
3 changes: 2 additions & 1 deletion generated/components.json
Original file line number Diff line number Diff line change
Expand Up @@ -2797,7 +2797,8 @@
"name": "position",
"type": "| 'start' | 'end' | { narrow?: | 'start' | 'end' regular?: | 'start' | 'end' wide?: | 'start' | 'end' }",
"defaultValue": "'end'",
"description": ""
"description": "",
"deprecated": true
},
{
"name": "positionWhenNarrow",
Expand Down
3 changes: 2 additions & 1 deletion src/PageLayout/PageLayout.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,8 @@
"name": "position",
"type": "| 'start' | 'end' | { narrow?: | 'start' | 'end' regular?: | 'start' | 'end' wide?: | 'start' | 'end' }",
"defaultValue": "'end'",
"description": ""
"description": "",
"deprecated": true
radglob marked this conversation as resolved.
Show resolved Hide resolved
},
{
"name": "positionWhenNarrow",
Expand Down
96 changes: 41 additions & 55 deletions src/PageLayout/PageLayout.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,49 +83,42 @@ export const StickyPane: Story = args => (
<PageLayout.Header padding="normal" divider="line">
<Placeholder label="Header" height={64} />
</PageLayout.Header>
<PageLayout.Content padding="normal" width="large">
<PageLayout.Pane resizable padding="normal" divider="line" sticky={args.sticky} aria-label="Side pane">
<Box sx={{display: 'grid', gap: 3}}>
{Array.from({length: args.numParagraphsInContent}).map((_, i) => {
const testId = `content${i}`
{Array.from({length: args.numParagraphsInPane}).map((_, i) => {
const testId = `paragraph${i}`
return (
<Box key={i} as="p" sx={{margin: 0}}>
<span data-testid={testId}>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nam at enim id lorem tempus egestas a non
ipsum. Maecenas imperdiet ante quam, at varius lorem molestie vel. Sed at eros consequat, varius tellus
et, auctor felis. Donec pulvinar lacinia urna nec commodo. Phasellus at imperdiet risus. Donec sit amet
massa purus. Nunc sem lectus, bibendum a sapien nec, tristique tempus felis. Ut porttitor auctor tellus
in imperdiet. Ut blandit tincidunt augue, quis fringilla nunc tincidunt sed. Vestibulum auctor euismod
nisi. Nullam tincidunt est in mi tincidunt dictum. Sed consectetur aliquet velit ut ornare.
massa purus.
</span>
</Box>
)
})}
</Box>
</PageLayout.Content>
<PageLayout.Pane
position="start"
resizable
padding="normal"
divider="line"
sticky={args.sticky}
aria-label="Side pane"
>
</PageLayout.Pane>
<PageLayout.Content padding="normal" width="large">
<Box sx={{display: 'grid', gap: 3}}>
{Array.from({length: args.numParagraphsInPane}).map((_, i) => {
const testId = `paragraph${i}`
{Array.from({length: args.numParagraphsInContent}).map((_, i) => {
const testId = `content${i}`
return (
<Box key={i} as="p" sx={{margin: 0}}>
<span data-testid={testId}>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nam at enim id lorem tempus egestas a non
ipsum. Maecenas imperdiet ante quam, at varius lorem molestie vel. Sed at eros consequat, varius tellus
et, auctor felis. Donec pulvinar lacinia urna nec commodo. Phasellus at imperdiet risus. Donec sit amet
massa purus.
massa purus. Nunc sem lectus, bibendum a sapien nec, tristique tempus felis. Ut porttitor auctor tellus
in imperdiet. Ut blandit tincidunt augue, quis fringilla nunc tincidunt sed. Vestibulum auctor euismod
nisi. Nullam tincidunt est in mi tincidunt dictum. Sed consectetur aliquet velit ut ornare.
</span>
</Box>
)
})}
</Box>
</PageLayout.Pane>
</PageLayout.Content>
<PageLayout.Footer padding="normal" divider="line">
<Placeholder label="Footer" height={64} />
</PageLayout.Footer>
Expand Down Expand Up @@ -158,32 +151,32 @@ export const NestedScrollContainer: Story = args => (
<PageLayout.Header padding="normal" divider="line">
<Placeholder label="Header" height={64} />
</PageLayout.Header>
<PageLayout.Content padding="normal" width="large">
<PageLayout.Pane padding="normal" divider="line" sticky aria-label="Side pane">
<Box sx={{display: 'grid', gap: 3}}>
{Array.from({length: args.numParagraphsInContent}).map((_, i) => (
{Array.from({length: args.numParagraphsInPane}).map((_, i) => (
<Box key={i} as="p" sx={{margin: 0}}>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nam at enim id lorem tempus egestas a non
ipsum. Maecenas imperdiet ante quam, at varius lorem molestie vel. Sed at eros consequat, varius tellus
et, auctor felis. Donec pulvinar lacinia urna nec commodo. Phasellus at imperdiet risus. Donec sit amet
massa purus. Nunc sem lectus, bibendum a sapien nec, tristique tempus felis. Ut porttitor auctor tellus
in imperdiet. Ut blandit tincidunt augue, quis fringilla nunc tincidunt sed. Vestibulum auctor euismod
nisi. Nullam tincidunt est in mi tincidunt dictum. Sed consectetur aliquet velit ut ornare.
massa purus.
</Box>
))}
</Box>
</PageLayout.Content>
<PageLayout.Pane position="start" padding="normal" divider="line" sticky aria-label="Side pane">
</PageLayout.Pane>
<PageLayout.Content padding="normal" width="large">
<Box sx={{display: 'grid', gap: 3}}>
{Array.from({length: args.numParagraphsInPane}).map((_, i) => (
{Array.from({length: args.numParagraphsInContent}).map((_, i) => (
<Box key={i} as="p" sx={{margin: 0}}>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nam at enim id lorem tempus egestas a non
ipsum. Maecenas imperdiet ante quam, at varius lorem molestie vel. Sed at eros consequat, varius tellus
et, auctor felis. Donec pulvinar lacinia urna nec commodo. Phasellus at imperdiet risus. Donec sit amet
massa purus.
massa purus. Nunc sem lectus, bibendum a sapien nec, tristique tempus felis. Ut porttitor auctor tellus
in imperdiet. Ut blandit tincidunt augue, quis fringilla nunc tincidunt sed. Vestibulum auctor euismod
nisi. Nullam tincidunt est in mi tincidunt dictum. Sed consectetur aliquet velit ut ornare.
</Box>
))}
</Box>
</PageLayout.Pane>
</PageLayout.Content>
<PageLayout.Footer padding="normal" divider="line">
<Placeholder label="Footer" height={64} />
</PageLayout.Footer>
Expand Down Expand Up @@ -228,6 +221,23 @@ export const CustomStickyHeader: Story = args => (
Custom sticky header
</Box>
<PageLayout rowGap="none" columnGap="none" padding="none" containerWidth="full">
<PageLayout.Pane padding="normal" divider="line" aria-label="Aside pane" sticky offsetHeader={args.offsetHeader}>
<Box sx={{display: 'grid', gap: 3}}>
{Array.from({length: args.numParagraphsInPane}).map((_, i) => {
const testId = `paragraph${i}`
return (
<Box key={i} as="p" sx={{margin: 0}}>
<span data-testid={testId}>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nam at enim id lorem tempus egestas a non
ipsum. Maecenas imperdiet ante quam, at varius lorem molestie vel. Sed at eros consequat, varius
tellus et, auctor felis. Donec pulvinar lacinia urna nec commodo. Phasellus at imperdiet risus. Donec
sit amet massa purus.
</span>
</Box>
)
})}
</Box>
</PageLayout.Pane>
<PageLayout.Content padding="normal" width="large">
<Box sx={{display: 'grid', gap: 3}} data-testid="scrollContainer">
{Array.from({length: args.numParagraphsInContent}).map((_, i) => {
Expand All @@ -248,30 +258,6 @@ export const CustomStickyHeader: Story = args => (
})}
</Box>
</PageLayout.Content>
<PageLayout.Pane
position="start"
padding="normal"
divider="line"
aria-label="Aside pane"
sticky
offsetHeader={args.offsetHeader}
>
<Box sx={{display: 'grid', gap: 3}}>
{Array.from({length: args.numParagraphsInPane}).map((_, i) => {
const testId = `paragraph${i}`
return (
<Box key={i} as="p" sx={{margin: 0}}>
<span data-testid={testId}>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nam at enim id lorem tempus egestas a non
ipsum. Maecenas imperdiet ante quam, at varius lorem molestie vel. Sed at eros consequat, varius
tellus et, auctor felis. Donec pulvinar lacinia urna nec commodo. Phasellus at imperdiet risus. Donec
sit amet massa purus.
</span>
</Box>
)
})}
</Box>
</PageLayout.Pane>
<PageLayout.Footer padding="normal" divider="line">
<Placeholder label="Footer" height={64} />
</PageLayout.Footer>
Expand Down Expand Up @@ -306,7 +292,7 @@ export const ResizablePane: Story = () => (
<PageLayout.Header>
<Placeholder height={64} label="Header" />
</PageLayout.Header>
<PageLayout.Pane resizable position="start">
<PageLayout.Pane resizable>
<Placeholder height={320} label="Pane" />
</PageLayout.Pane>
<PageLayout.Content>
Expand All @@ -323,7 +309,7 @@ export const ScrollContainerWithinPageLayoutPane: Story = () => (
<Box sx={{overflow: 'auto'}}>
<Placeholder label="Above inner scroll container" height={120} />
<PageLayout rowGap="none" columnGap="none" padding="none" containerWidth="full">
<PageLayout.Pane position="start" padding="normal" divider="line" sticky aria-label="Sticky pane">
<PageLayout.Pane padding="normal" divider="line" sticky aria-label="Sticky pane">
<Box sx={{overflow: 'auto'}}>
<PageLayout.Pane padding="normal">
<Placeholder label="Inner scroll container" height={800} />
Expand Down
33 changes: 0 additions & 33 deletions src/PageLayout/PageLayout.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ const meta: Meta = {
'Content.hidden.regular': false,
'Content.hidden.narrow': false,
'Content.hidden.wide': false,
'Pane.position.regular': 'end',
'Pane.position.narrow': 'end',
'Pane.position.wide': 'end',
'Pane.width': 'medium',
'Pane.sticky': false,
'Pane.resizable': false,
Expand Down Expand Up @@ -205,31 +202,6 @@ const meta: Meta = {
table: {category: 'Content props'},
},

// Pane prop controls
radglob marked this conversation as resolved.
Show resolved Hide resolved
'Pane.position.regular': {
type: {
name: 'enum',
value: ['start', 'end'],
},
control: {type: 'radio'},
table: {category: 'Pane props'},
},
'Pane.position.narrow': {
type: {
name: 'enum',
value: ['start', 'end'],
},
control: {type: 'radio'},
table: {category: 'Pane props'},
},
'Pane.position.wide': {
type: {
name: 'enum',
value: ['start', 'end'],
},
control: {type: 'radio'},
table: {category: 'Pane props'},
},
'Pane.width': {
type: {
name: 'enum',
Expand Down Expand Up @@ -382,11 +354,6 @@ const Template: Story = args => (
</PageLayout.Content>
{args['Render pane?'] ? (
<PageLayout.Pane
position={{
narrow: args['Pane.position.narrow'],
regular: args['Pane.position.regular'],
wide: args['Pane.position.wide'],
}}
width={args['Pane.width']}
minWidth={args['Pane.minWidth']}
sticky={args['Pane.sticky']}
Expand Down
4 changes: 2 additions & 2 deletions src/PageLayout/PageLayout.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ describe('PageLayout', () => {
<PageLayout.Header divider="line" dividerWhenNarrow="filled">
Header
</PageLayout.Header>
<PageLayout.Content>Content</PageLayout.Content>
<PageLayout.Pane position="start" divider="line" dividerWhenNarrow="filled">
<PageLayout.Pane divider="line" dividerWhenNarrow="filled">
Pane
</PageLayout.Pane>
<PageLayout.Content>Content</PageLayout.Content>
<PageLayout.Footer dividerWhenNarrow="line">Footer</PageLayout.Footer>
</PageLayout>
</ThemeProvider>,
Expand Down
31 changes: 24 additions & 7 deletions src/PageLayout/PageLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,20 @@ const Root: React.FC<React.PropsWithChildren<PageLayoutProps>> = ({
}}
>
{slots.header}
<Box sx={{display: 'flex', flex: '1 1 100%', flexWrap: 'wrap', maxWidth: '100%'}}>{rest}</Box>
<Box
// eslint-disable-next-line @typescript-eslint/no-explicit-any
sx={(theme: any) => ({
display: 'flex',
flex: '1 1 100%',
flexWrap: 'wrap',
maxWidth: '100%',
[`@media screen and (min-width: ${theme.breakpoints[1]})`]: {
columnGap: SPACING_MAP[columnGap],
},
})}
radglob marked this conversation as resolved.
Show resolved Hide resolved
>
{rest}
</Box>
{slots.footer}
</Box>
</Box>
Expand Down Expand Up @@ -428,7 +441,6 @@ const Content: React.FC<React.PropsWithChildren<PageLayoutContentProps>> = ({
{
display: isHidden ? 'none' : 'flex',
flexDirection: 'column',
order: REGION_ORDER.content,
// Set flex-basis to 0% to allow flex-grow to control the width of the content region.
// Without this, the content region could wrap onto a different line
// than the pane region on wide viewports if its contents are too wide.
Expand Down Expand Up @@ -530,7 +542,7 @@ const Pane = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageLayout
{
'aria-label': label,
'aria-labelledby': labelledBy,
position: responsivePosition = 'end',
position: responsivePosition = undefined,
positionWhenNarrow = 'inherit',
width = 'medium',
minWidth = 256,
Expand All @@ -548,6 +560,13 @@ const Pane = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageLayout
},
forwardRef,
) => {
if (responsivePosition !== undefined) {
// eslint-disable-next-line no-console
console.warn(
'The `position` prop will be removed on the next major version. You should order your markup as you want it to render instead.',
)
}

radglob marked this conversation as resolved.
Show resolved Hide resolved
// Combine position and positionWhenNarrow for backwards compatibility
const positionProp =
!isResponsiveValue(responsivePosition) && positionWhenNarrow !== 'inherit'
Expand Down Expand Up @@ -678,7 +697,7 @@ const Pane = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageLayout
{
// Narrow viewports
display: isHidden ? 'none' : 'flex',
order: panePositions[position],
order: position !== undefined ? panePositions[position] : undefined,
width: '100%',
marginX: 0,
...(position === 'end'
Expand All @@ -698,9 +717,7 @@ const Pane = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageLayout
maxHeight: 'var(--sticky-pane-height)',
}
: {}),
...(position === 'end'
? {flexDirection: 'row', marginLeft: SPACING_MAP[columnGap]}
: {flexDirection: 'row-reverse', marginRight: SPACING_MAP[columnGap]}),
...(position === 'end' ? {flexDirection: 'row'} : {flexDirection: 'row-reverse'}),
},
},
sx,
Expand Down
Loading