Skip to content

PageLayout: Update position responsive prop API #2199

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

Merged
merged 6 commits into from
Jul 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 19 additions & 0 deletions .changeset/fair-tips-travel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
"@primer/react": minor
---

* Updated the `position` prop in `PageLayout.Pane` to use the new responsive prop API introduced in https://github.com/primer/react/pull/2174.
* Deprecated the `positionWhenNarrow` prop in favor of the new responsive prop API

**Before**

```
position="start"
positionWhenNarrow="end"
```

**After**

```
position={{regular: 'start', narrow: 'end'}}
```
15 changes: 14 additions & 1 deletion docs/content/PageLayout.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -244,15 +244,28 @@ See [storybook](https://primer.style/react/storybook?path=/story/layout-pagelayo
<PropsTableRow
name="position"
type={`| 'start'
| 'end'`}
| 'end'
| {
narrow?:
| 'start'
| 'end'
regular?:
| 'start'
| 'end'
wide?:
| 'start'
| 'end'
}`}
defaultValue="'start'"
/>
<PropsTableRow
name="positionWhenNarrow"
deprecated
type={`| 'inherit'
| 'start'
| 'end'`}
defaultValue="'inherit'"
description="Use the position prop with a responsive value instead."
/>
<PropsTableRow
name="width"
Expand Down
45 changes: 31 additions & 14 deletions src/PageLayout/PageLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,21 @@ Content.displayName = 'PageLayout.Content'
// PageLayout.Pane

export type PageLayoutPaneProps = {
position?: keyof typeof panePositions
position?: keyof typeof panePositions | ResponsiveValue<keyof typeof panePositions>
/**
* @deprecated Use the `position` prop with a responsive value instead.
*
* Before:
* ```
* position="start"
* positionWhenNarrow="end"
* ```
*
* After:
* ```
* position={{regular: 'start', narrow: 'end'}}
* ```
*/
positionWhenNarrow?: 'inherit' | keyof typeof panePositions
width?: keyof typeof paneWidths
divider?: 'none' | 'line' | ResponsiveValue<'none' | 'line', 'none' | 'line' | 'filled'>
Expand Down Expand Up @@ -322,6 +336,14 @@ const Pane: React.FC<React.PropsWithChildren<PageLayoutPaneProps>> = ({
children,
sx = {}
}) => {
// Combine position and positionWhenNarrow for backwards compatibility
const positionProp =
!isResponsiveValue(position) && positionWhenNarrow !== 'inherit'
? {regular: position, narrow: positionWhenNarrow}
: position

const responsivePosition = useResponsiveValue(positionProp, 'end')

// Combine divider and dividerWhenNarrow for backwards compatibility
const dividerProp =
!isResponsiveValue(divider) && dividerWhenNarrow !== 'inherit'
Expand All @@ -331,26 +353,25 @@ const Pane: React.FC<React.PropsWithChildren<PageLayoutPaneProps>> = ({
const dividerVariant = useResponsiveValue(dividerProp, 'none')
const isHidden = useResponsiveValue(hidden, false)
const {rowGap, columnGap} = React.useContext(PageLayoutContext)
const computedPositionWhenNarrow = positionWhenNarrow === 'inherit' ? position : positionWhenNarrow
return (
<Box
as="aside"
// eslint-disable-next-line @typescript-eslint/no-explicit-any
sx={(theme: any) =>
merge<BetterSystemStyleObject>(
{
order: panePositions[computedPositionWhenNarrow],
order: panePositions[responsivePosition],
display: isHidden ? 'none' : 'flex',
flexDirection: computedPositionWhenNarrow === 'end' ? 'column' : 'column-reverse',
flexDirection: responsivePosition === 'end' ? 'column' : 'column-reverse',
width: '100%',
marginX: 0,
[computedPositionWhenNarrow === 'end' ? 'marginTop' : 'marginBottom']: SPACING_MAP[rowGap],
[responsivePosition === 'end' ? 'marginTop' : 'marginBottom']: SPACING_MAP[rowGap],
[`@media screen and (min-width: ${theme.breakpoints[1]})`]: {
width: 'auto',
[position === 'end' ? 'marginLeft' : 'marginRight']: SPACING_MAP[columnGap],
[responsivePosition === 'end' ? 'marginLeft' : 'marginRight']: SPACING_MAP[columnGap],
marginY: `0 !important`,
flexDirection: position === 'end' ? 'row' : 'row-reverse',
order: panePositions[position]
flexDirection: responsivePosition === 'end' ? 'row' : 'row-reverse',
order: panePositions[responsivePosition]
}
},
sx
Expand All @@ -360,15 +381,11 @@ const Pane: React.FC<React.PropsWithChildren<PageLayoutPaneProps>> = ({
{/* Show a horizontal divider when viewport is narrow. Otherwise, show a vertical divider. */}
<HorizontalDivider
variant={{narrow: dividerVariant, regular: 'none'}}
sx={{
[computedPositionWhenNarrow === 'end' ? 'marginBottom' : 'marginTop']: SPACING_MAP[rowGap]
}}
sx={{[responsivePosition === 'end' ? 'marginBottom' : 'marginTop']: SPACING_MAP[rowGap]}}
/>
<VerticalDivider
variant={{narrow: 'none', regular: dividerVariant}}
sx={{
[position === 'end' ? 'marginRight' : 'marginLeft']: SPACING_MAP[columnGap]
}}
sx={{[responsivePosition === 'end' ? 'marginRight' : 'marginLeft']: SPACING_MAP[columnGap]}}
/>

<Box sx={{width: paneWidths[width]}}>{children}</Box>
Expand Down
106 changes: 53 additions & 53 deletions src/PageLayout/__snapshots__/PageLayout.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -487,48 +487,48 @@ exports[`PageLayout renders pane in different position when narrow 1`] = `
margin-right: auto;
}

.c10 {
.c6 {
-webkit-order: 3;
-ms-flex-order: 3;
order: 3;
display: -webkit-box;
display: -webkit-flex;
display: -ms-flexbox;
display: flex;
-webkit-flex-direction: column;
-ms-flex-direction: column;
flex-direction: column;
width: 100%;
margin-left: 0;
margin-right: 0;
margin-top: 16px;
}

.c7 {
margin-left: -16px;
margin-right: -16px;
display: none;
margin-bottom: 16px;
}

.c7 {
.c8 {
height: 100%;
display: none;
margin-right: 16px;
}

.c8 {
.c9 {
width: 100%;
}

.c9 {
.c10 {
-webkit-order: 4;
-ms-flex-order: 4;
order: 4;
width: 100%;
margin-top: 16px;
}

.c6 {
-webkit-order: 1;
-ms-flex-order: 1;
order: 1;
display: -webkit-box;
display: -webkit-flex;
display: -ms-flexbox;
display: flex;
-webkit-flex-direction: column-reverse;
-ms-flex-direction: column-reverse;
flex-direction: column-reverse;
width: 100%;
margin-left: 0;
margin-right: 0;
margin-bottom: 16px;
}

@media screen and (min-width:1012px) {
.c0 {
padding: 24px;
Expand Down Expand Up @@ -557,65 +557,65 @@ exports[`PageLayout renders pane in different position when narrow 1`] = `
}

@media screen and (min-width:768px) {
.c10 {
.c6 {
width: auto;
margin-left: 16px;
margin-top: 0 !important;
margin-bottom: 0 !important;
-webkit-flex-direction: row;
-ms-flex-direction: row;
flex-direction: row;
-webkit-order: 3;
-ms-flex-order: 3;
order: 3;
}
}

@media screen and (min-width:1012px) {
.c6 {
margin-top: 24px;
}
}

@media screen and (min-width:768px) {
.c7 {
margin-left: 0 !important;
margin-right: 0 !important;
}
}

@media screen and (min-width:1012px) {
.c10 {
.c7 {
margin-left: -24px;
margin-right: -24px;
margin-bottom: 24px;
}
}

@media screen and (min-width:1012px) {
.c7 {
.c8 {
margin-right: 24px;
}
}

@media screen and (min-width:768px) {
.c8 {
.c9 {
width: 256px;
}
}

@media screen and (min-width:1012px) {
.c8 {
.c9 {
width: 296px;
}
}

@media screen and (min-width:1012px) {
.c9 {
.c10 {
margin-top: 24px;
}
}

@media screen and (min-width:768px) {
.c6 {
width: auto;
margin-left: 16px;
margin-top: 0 !important;
margin-bottom: 0 !important;
-webkit-flex-direction: row;
-ms-flex-direction: row;
flex-direction: row;
-webkit-order: 3;
-ms-flex-order: 3;
order: 3;
}
}

@media screen and (min-width:1012px) {
.c6 {
margin-bottom: 24px;
}
}

<div>
<div
class="c0"
Expand Down Expand Up @@ -643,23 +643,23 @@ exports[`PageLayout renders pane in different position when narrow 1`] = `
<aside
class="c6"
>
<div
class="c3"
/>
<div
class="c7"
/>
<div
class="c8"
/>
<div
class="c9"
>
Pane
</div>
</aside>
<footer
class="c9"
class="c10"
>
<div
class="c10"
class="c7"
/>
Footer
</footer>
Expand Down