From adbcd3bf37334b867bd159c012d69aa9722ebbf3 Mon Sep 17 00:00:00 2001
From: Cole Bemis <colebemis@github.com>
Date: Wed, 23 Mar 2022 10:27:44 -0700
Subject: [PATCH] PageLayout: Use HTML5 landmark elements (#1973)

* Use HTML5 landmark elements

* Remove separator role

because the dividers are only decorative

* Create brown-maps-yawn.md

* Update snapshot tests

* Update component checklist
---
 .changeset/brown-maps-yawn.md                 |  5 ++
 docs/content/PageLayout.mdx                   |  2 +-
 src/PageLayout/PageLayout.tsx                 |  6 +-
 .../__snapshots__/PageLayout.test.tsx.snap    | 80 ++++++++-----------
 4 files changed, 42 insertions(+), 51 deletions(-)
 create mode 100644 .changeset/brown-maps-yawn.md

diff --git a/.changeset/brown-maps-yawn.md b/.changeset/brown-maps-yawn.md
new file mode 100644
index 00000000000..8c2a9de17b7
--- /dev/null
+++ b/.changeset/brown-maps-yawn.md
@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+The `PageLayout` component now renders [HTML5 landmark elements](https://web.dev/use-landmarks/) (`header`, `aside`, `footer`) to improve the navigation experience for people using assistive technologies (like screen readers)
diff --git a/docs/content/PageLayout.mdx b/docs/content/PageLayout.mdx
index 5396abdb8ef..f1b65c9f184 100644
--- a/docs/content/PageLayout.mdx
+++ b/docs/content/PageLayout.mdx
@@ -258,7 +258,7 @@ See [storybook](https://primer.style/react/storybook?path=/story/layout-pagelayo
     usageExamplesDocumented: true,
     hasStorybookStories: true,
     designReviewed: false,
-    a11yReviewed: false,
+    a11yReviewed: true,
     stableApi: false,
     addressedApiFeedback: false,
     hasDesignGuidelines: false,
diff --git a/src/PageLayout/PageLayout.tsx b/src/PageLayout/PageLayout.tsx
index 9c57d1c81d1..6a70e4dcfdd 100644
--- a/src/PageLayout/PageLayout.tsx
+++ b/src/PageLayout/PageLayout.tsx
@@ -114,7 +114,6 @@ const HorizontalDivider: React.FC<DividerProps> = ({variant = 'none', variantWhe
   const {padding} = React.useContext(PageLayoutContext)
   return (
     <Box
-      role="separator"
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
       sx={(theme: any) =>
         merge<BetterSystemStyleObject>(
@@ -156,7 +155,6 @@ const verticalDividerVariants = {
 const VerticalDivider: React.FC<DividerProps> = ({variant = 'none', variantWhenNarrow = 'inherit', sx = {}}) => {
   return (
     <Box
-      role="separator"
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
       sx={(theme: any) =>
         merge<BetterSystemStyleObject>(
@@ -191,6 +189,7 @@ const Header: React.FC<PageLayoutHeaderProps> = ({
   const {rowGap} = React.useContext(PageLayoutContext)
   return (
     <Box
+      as="header"
       sx={merge<BetterSystemStyleObject>(
         {
           order: REGION_ORDER.header,
@@ -230,6 +229,7 @@ const contentWidths = {
 const Content: React.FC<PageLayoutContentProps> = ({width = 'full', children, sx = {}}) => {
   return (
     <Box
+      as="main"
       sx={merge<BetterSystemStyleObject>(
         {
           order: REGION_ORDER.content,
@@ -286,6 +286,7 @@ const Pane: React.FC<PageLayoutPaneProps> = ({
   const computedDividerWhenNarrow = dividerWhenNarrow === 'inherit' ? divider : dividerWhenNarrow
   return (
     <Box
+      as="aside"
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
       sx={(theme: any) =>
         merge<BetterSystemStyleObject>(
@@ -344,6 +345,7 @@ const Footer: React.FC<PageLayoutFooterProps> = ({
   const {rowGap} = React.useContext(PageLayoutContext)
   return (
     <Box
+      as="footer"
       sx={merge<BetterSystemStyleObject>(
         {
           order: REGION_ORDER.footer,
diff --git a/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap b/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap
index 0dda64fc669..109fa24a96d 100644
--- a/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap
+++ b/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap
@@ -154,16 +154,15 @@ exports[`PageLayout renders condensed layout 1`] = `
     <div
       class="c1"
     >
-      <div
+      <header
         class="c2"
       >
         Header
         <div
           class="c3"
-          role="separator"
         />
-      </div>
-      <div
+      </header>
+      <main
         class="c4"
       >
         <div
@@ -171,33 +170,30 @@ exports[`PageLayout renders condensed layout 1`] = `
         >
           Content
         </div>
-      </div>
-      <div
+      </main>
+      <aside
         class="c6"
       >
         <div
           class="c7"
-          role="separator"
         />
         <div
           class="c8"
-          role="separator"
         />
         <div
           class="c9"
         >
           Pane
         </div>
-      </div>
-      <div
+      </aside>
+      <footer
         class="c10"
       >
         <div
           class="c7"
-          role="separator"
         />
         Footer
-      </div>
+      </footer>
     </div>
   </div>
 </div>
@@ -403,16 +399,15 @@ exports[`PageLayout renders default layout 1`] = `
     <div
       class="c1"
     >
-      <div
+      <header
         class="c2"
       >
         Header
         <div
           class="c3"
-          role="separator"
         />
-      </div>
-      <div
+      </header>
+      <main
         class="c4"
       >
         <div
@@ -420,33 +415,30 @@ exports[`PageLayout renders default layout 1`] = `
         >
           Content
         </div>
-      </div>
-      <div
+      </main>
+      <aside
         class="c6"
       >
         <div
           class="c7"
-          role="separator"
         />
         <div
           class="c8"
-          role="separator"
         />
         <div
           class="c9"
         >
           Pane
         </div>
-      </div>
-      <div
+      </aside>
+      <footer
         class="c10"
       >
         <div
           class="c7"
-          role="separator"
         />
         Footer
-      </div>
+      </footer>
     </div>
   </div>
 </div>
@@ -652,16 +644,15 @@ exports[`PageLayout renders pane in different position when narrow 1`] = `
     <div
       class="c1"
     >
-      <div
+      <header
         class="c2"
       >
         Header
         <div
           class="c3"
-          role="separator"
         />
-      </div>
-      <div
+      </header>
+      <main
         class="c4"
       >
         <div
@@ -669,33 +660,30 @@ exports[`PageLayout renders pane in different position when narrow 1`] = `
         >
           Content
         </div>
-      </div>
-      <div
+      </main>
+      <aside
         class="c6"
       >
         <div
           class="c3"
-          role="separator"
         />
         <div
           class="c7"
-          role="separator"
         />
         <div
           class="c8"
         >
           Pane
         </div>
-      </div>
-      <div
+      </aside>
+      <footer
         class="c9"
       >
         <div
           class="c10"
-          role="separator"
         />
         Footer
-      </div>
+      </footer>
     </div>
   </div>
 </div>
@@ -936,16 +924,15 @@ exports[`PageLayout renders with dividers 1`] = `
     <div
       class="c1"
     >
-      <div
+      <header
         class="c2"
       >
         Header
         <div
           class="c3"
-          role="separator"
         />
-      </div>
-      <div
+      </header>
+      <main
         class="c4"
       >
         <div
@@ -953,33 +940,30 @@ exports[`PageLayout renders with dividers 1`] = `
         >
           Content
         </div>
-      </div>
-      <div
+      </main>
+      <aside
         class="c6"
       >
         <div
           class="c7"
-          role="separator"
         />
         <div
           class="c8"
-          role="separator"
         />
         <div
           class="c9"
         >
           Pane
         </div>
-      </div>
-      <div
+      </aside>
+      <footer
         class="c10"
       >
         <div
           class="c11"
-          role="separator"
         />
         Footer
-      </div>
+      </footer>
     </div>
   </div>
 </div>