Skip to content

Commit

Permalink
PageLayout: Use HTML5 landmark elements (#1973)
Browse files Browse the repository at this point in the history 
* Use HTML5 landmark elements

* Remove separator role

because the dividers are only decorative

* Create brown-maps-yawn.md

* Update snapshot tests

* Update component checklist
  • Loading branch information
@colebemis
colebemis committed Mar 23, 2022
1 parent a377a51 commit adbcd3b
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 51 deletions.
5 changes: 5 additions & 0 deletions .changeset/brown-maps-yawn.md
View file
Original file line number Diff line number Diff line change
@@ -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)
2 changes: 1 addition & 1 deletion docs/content/PageLayout.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 4 additions & 2 deletions src/PageLayout/PageLayout.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand Down Expand Up @@ -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>(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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>(
Expand Down Expand Up @@ -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,
Expand Down
80 changes: 32 additions & 48 deletions src/PageLayout/__snapshots__/PageLayout.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,50 +154,46 @@ 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
class="c5"
>
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>
Expand Down Expand Up @@ -403,50 +399,46 @@ 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
class="c5"
>
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>
Expand Down Expand Up @@ -652,50 +644,46 @@ 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
class="c5"
>
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>
Expand Down Expand Up @@ -936,50 +924,46 @@ 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
class="c5"
>
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>
Expand Down

0 comments on commit adbcd3b

Please sign in to comment.