PageLayout: Update position responsive prop API (#2199)

* Update position prop API * Update snapshots * Update docs * Create fair-tips-travel.md
primer · Jul 29, 2022 · fb385b6 · fb385b6
1 parent e0113df
commit fb385b6
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 68 deletions.
diff --git a/.changeset/fair-tips-travel.md b/.changeset/fair-tips-travel.md
@@ -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'}}
+```
diff --git a/docs/content/PageLayout.mdx b/docs/content/PageLayout.mdx
@@ -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"

diff --git a/src/PageLayout/PageLayout.tsx b/src/PageLayout/PageLayout.tsx
@@ -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'>
@@ -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'
@@ -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
@@ -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>

diff --git a/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap b/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap
@@ -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;
@@ -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"
@@ -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>