Make pane an optional region for PageLayout component (#1989)

* Fix PageLayout storybook example imports * Align PageLayout story spacing props with component * Fix inherit state of ..DividerWhenNarrow * Make pane an optional region of PageLayout * Stylelint auto-fixes * Create nasty-singers-compare.md Co-authored-by: Actions Auto Build <actions@github.com> Co-authored-by: Jon Rohan <yes@jonrohan.codes>
primer · Mar 14, 2022 · f544ef8 · f544ef8
1 parent e3443f1
commit f544ef8
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 63 deletions.
diff --git a/.changeset/nasty-singers-compare.md b/.changeset/nasty-singers-compare.md
@@ -0,0 +1,5 @@
+---
+"@primer/css": minor
+---
+
+Make `pane` an optional region for PageLayout component
diff --git a/docs/src/stories/components/Layout/LayoutBeta.stories.jsx b/docs/src/stories/components/Layout/LayoutBeta.stories.jsx
@@ -99,6 +99,13 @@ export default {
 
     // Pane
 
+    hasPane: {
+      control: {type: 'boolean'},
+      table: {
+        category: 'Pane'
+      }
+    },
+
     paneWidth: {
       options: ['default', 'narrow', 'wide'],
       control: {
@@ -275,6 +282,7 @@ export const LayoutTemplate = ({
   rowGap,
 
   // Pane
+  hasPane,
   paneWidth,
   panePosition,
   panePositionWhenNarrow,
@@ -336,20 +344,14 @@ export const LayoutTemplate = ({
 
   if (hasPaneDivider) {
     paneDividerWhenNarrow = paneDividerWhenNarrow === 'inherit' ? 'line' : paneDividerWhenNarrow
-  } else {
-    paneDividerWhenNarrow = null
   }
 
   if (hasHeaderDivider) {
     headerDividerWhenNarrow = headerDividerWhenNarrow === 'inherit' ? 'line' : headerDividerWhenNarrow
-  } else {
-    headerDividerWhenNarrow = null
   }
 
   if (hasFooterDivider) {
     footerDividerWhenNarrow = footerDividerWhenNarrow === 'inherit' ? 'line' : footerDividerWhenNarrow
-  } else {
-    footerDividerWhenNarrow = null
   }
 
   PageLayoutBehavior()
@@ -392,54 +394,69 @@ export const LayoutTemplate = ({
             </div>
           )}
 
-          <div className={clsx(layoutClassName + '-columns')}>
-            {/* pane if rendered first */}
-            {panePosition === 'start' && (
-              <div
-                className={clsx(
-                  layoutClassName + '-region',
-                  layoutClassName + '-pane',
-                  paneDividerWhenNarrow &&
-                    layoutClassName +
-                      '-region--dividerNarrow-' +
-                      paneDividerWhenNarrow +
-                      (panePositionWhenNarrow === 'start' ? '-after' : '-before')
+          {hasPane && (
+            <div className={clsx(layoutClassName + '-columns')}>
+              {/* pane if rendered first */}
+              {panePosition === 'start' && (
+                <div
+                  className={clsx(
+                    layoutClassName + '-region',
+                    layoutClassName + '-pane',
+                    paneDividerWhenNarrow &&
+                      layoutClassName +
+                        '-region--dividerNarrow-' +
+                        paneDividerWhenNarrow +
+                        (panePositionWhenNarrow === 'start' ? '-after' : '-before')
+                  )}
+                >
+                  {paneChildren}
+                </div>
+              )}
+
+              {/* content */}
+              <div className={clsx(layoutClassName + '-region', layoutClassName + '-content')}>
+                {contentWidth ? (
+                  <>
+                    <div className={layoutClassName + '-content-centered-' + contentWidth}>
+                      <div className={'container-' + contentWidth}>{contentChildren}</div>
+                    </div>
+                  </>
+                ) : (
+                  <>{contentChildren}</>
                 )}
-              >
-                {paneChildren}
               </div>
-            )}
 
-            {/* content */}
-            <div className={clsx(layoutClassName + '-region', layoutClassName + '-content')}>
-              {contentWidth ? (
-                <>
-                  <div className={layoutClassName + '-content-centered-' + contentWidth}>
-                    <div className={'container-' + contentWidth}>{contentChildren}</div>
-                  </div>
-                </>
-              ) : (
-                <>{contentChildren}</>
+              {/* pane if rendered last */}
+              {panePosition === 'end' && (
+                <div
+                  className={clsx(
+                    layoutClassName + '-region',
+                    layoutClassName + '-pane',
+                    paneDividerWhenNarrow &&
+                      layoutClassName +
+                        '-region--dividerNarrow-' +
+                        paneDividerWhenNarrow +
+                        (panePositionWhenNarrow === 'start' ? '-after' : '-before')
+                  )}
+                >
+                  {paneChildren}
+                </div>
               )}
             </div>
-
-            {/* pane if rendered last */}
-            {panePosition === 'end' && (
-              <div
-                className={clsx(
-                  layoutClassName + '-region',
-                  layoutClassName + '-pane',
-                  paneDividerWhenNarrow &&
-                    layoutClassName +
-                      '-region--dividerNarrow-' +
-                      paneDividerWhenNarrow +
-                      (panePositionWhenNarrow === 'start' ? '-after' : '-before')
+          ) || (
+            <>
+              {/* single-column layout */}
+              <div className={clsx(layoutClassName + '-region', layoutClassName + '-content')}>
+                {contentWidth ? (
+                  <>
+                    <div className={'container-' + contentWidth}>{contentChildren}</div>
+                  </>
+                ) : (
+                  <>{contentChildren}</>
                 )}
-              >
-                {paneChildren}
               </div>
-            )}
-          </div>
+            </>
+          )}
 
           {/* footer */}
           {hasFooter && (
@@ -494,6 +511,7 @@ Playground.args = {
   responsiveVariant: 'stackRegions',
   primaryRegion: 'content',
 
+  hasPane: true,
   paneWidth: 'default',
   panePosition: 'end',
   panePositionWhenNarrow: 'inherit',

diff --git a/docs/src/stories/components/Layout/LayoutExamples.stories.jsx b/docs/src/stories/components/Layout/LayoutExamples.stories.jsx
@@ -2,7 +2,8 @@ import React from 'react'
 import clsx from 'clsx'
 import {PageLayoutTemplate} from './PageLayout.stories'
 import {SplitPageLayoutTemplate} from './SplitPageLayout.stories'
-import {RepoSettings, DiscussionsPane, ActionListTreeViewTemplate} from '../ActionList/ActionListExamples.stories'
+import {ActionListTreeViewTemplate} from '../../ui-patterns/ActionList/ActionListTree.stories'
+import {RepoSettings, NavDiscussionsPane} from '../NavigationList/NavigationListExamples.stories'
 import {LayoutAlphaTemplate} from './LayoutAlpha.stories'
 
 export default {
@@ -32,7 +33,7 @@ Settings.args = {
     <>
       <h2 className="h3 ml-2 mr-2">Repository settings</h2>
       <div className="ml-n2 mr-n2">
-        <NavRepoSettings {...NavRepoSettings.args} />
+        <RepoSettings {...RepoSettings.args} />
       </div>
     </>
   ),

diff --git a/docs/src/stories/components/Layout/PageLayout.stories.jsx b/docs/src/stories/components/Layout/PageLayout.stories.jsx
@@ -41,7 +41,7 @@ export default {
     },
 
     columnGap: {
-      options: ['normal', 'condensed'],
+      options: ['normal', 'condensed', 'none'],
       control: {
         type: 'inline-radio'
       },
@@ -51,7 +51,7 @@ export default {
       }
     },
     rowGap: {
-      options: ['normal', 'condensed'],
+      options: ['normal', 'condensed', 'none'],
       control: {
         type: 'inline-radio'
       },
@@ -86,6 +86,13 @@ export default {
     },
 
     // Pane
+
+    hasPane: {
+      control: {type: 'boolean'},
+      table: {
+        category: 'Pane'
+      }
+    },
 
     panePosition: {
       options: ['start', 'end'],
@@ -241,6 +248,7 @@ export const PageLayoutTemplate = ({
   rowGap,
   responsiveVariant,
   primaryRegion,
+  hasPane,
   paneWidth,
   panePosition,
   panePositionWhenNarrow,
@@ -271,6 +279,7 @@ export const PageLayoutTemplate = ({
         responsiveVariant={responsiveVariant}
         primaryRegion={primaryRegion}
 
+        hasPane={hasPane}
         paneWidth={paneWidth}
         panePosition={panePosition}
         panePositionWhenNarrow={panePositionWhenNarrow}
@@ -315,6 +324,7 @@ Playground.args = {
   primaryRegion: 'content',
 
   // Pane
+  hasPane: true,
   panePosition: 'end',
   panePositionWhenNarrow: 'inherit',
   paneWidth: 'default',

diff --git a/src/layout/page-layout.scss b/src/layout/page-layout.scss
@@ -270,19 +270,34 @@ $Layout-responsive-variant-max-breakpoint: 'md' !default;
   }
 }
 
+// PageLayout-wrapper bundles header, footer, pane, and content regions
 .PageLayout-wrapper {
   display: grid;
   grid: auto-flow / 1fr;
   row-gap: var(--Layout-row-gap);
 }
 
+// PageLayout-columns wrap pane and content regions
+// If layout has no pane, PageLayout-columns is not present
 .PageLayout-columns {
   display: grid;
   column-gap: var(--Layout-column-gap);
   row-gap: var(--Layout-row-gap);
   grid-template-columns: var(--Layout-template-columns);
   grid-template-rows: 1fr;
   grid-template-areas: var(--Layout-template-areas);
+
+  .PageLayout-content {
+    // stylelint-disable-next-line primer/spacing
+    padding-right: var(--Layout-inner-spacing-max);
+    // stylelint-disable-next-line primer/spacing
+    padding-left: var(--Layout-inner-spacing-max);
+    grid-area: content;
+  }
+
+  .PageLayout-pane {
+    grid-area: pane;
+  }
 }
 
 // outer spacing
@@ -367,15 +382,3 @@ $Layout-responsive-variant-max-breakpoint: 'md' !default;
   // stylelint-disable-next-line primer/spacing
   padding: var(--Layout-inner-spacing-min);
 }
-
-.PageLayout-content {
-  // stylelint-disable-next-line primer/spacing
-  padding-right: var(--Layout-inner-spacing-max);
-  // stylelint-disable-next-line primer/spacing
-  padding-left: var(--Layout-inner-spacing-max);
-  grid-area: content;
-}
-
-.PageLayout-pane {
-  grid-area: pane;
-}