Skip to content

Commit

Permalink
Don't render main landmark in PageLayout.Content, leave it up to deve…
Browse files Browse the repository at this point in the history
…lopers to specify. (#3675)
  • Loading branch information
radglob authored Aug 29, 2023
1 parent 438f67e commit ade10e6
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 15 deletions.
7 changes: 7 additions & 0 deletions .changeset/angry-beds-itch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@primer/react': patch
---

PageLayout.Content no longer renders as `main` by default. Instead, developers may add a `main` landmark within `Pagelayout.Content` themselves.

<!-- Changed components: PageLayout -->
8 changes: 5 additions & 3 deletions src/PageLayout/PageLayout.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ describe('PageLayout', () => {
<ThemeProvider>
<PageLayout>
<PageLayout.Header>Header</PageLayout.Header>
<PageLayout.Content>Content</PageLayout.Content>
<PageLayout.Content>
<main aria-label="Content">Content</main>
</PageLayout.Content>
<PageLayout.Pane>Pane</PageLayout.Pane>
<PageLayout.Footer>Footer</PageLayout.Footer>
</PageLayout>
Expand Down Expand Up @@ -131,7 +133,7 @@ describe('PageLayout', () => {
)

expect(screen.getByRole('banner')).toHaveAccessibleName('Header')
expect(screen.getByRole('main')).toHaveAccessibleName('Content')
expect(screen.getByLabelText('Content')).toHaveAccessibleName('Content')
expect(screen.getByRole('contentinfo')).toHaveAccessibleName('Footer')
})

Expand All @@ -154,7 +156,7 @@ describe('PageLayout', () => {
)

expect(screen.getByRole('banner')).toHaveAccessibleName('header')
expect(screen.getByRole('main')).toHaveAccessibleName('content')
expect(screen.getByLabelText('content')).toBeInTheDocument()
expect(screen.getByRole('contentinfo')).toHaveAccessibleName('footer')
})

Expand Down
1 change: 0 additions & 1 deletion src/PageLayout/PageLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,6 @@ const Content: React.FC<React.PropsWithChildren<PageLayoutContentProps>> = ({

return (
<Box
as="main"
aria-label={label}
aria-labelledby={labelledBy}
sx={merge<BetterSystemStyleObject>(
Expand Down
22 changes: 13 additions & 9 deletions src/PageLayout/__snapshots__/PageLayout.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ exports[`PageLayout renders condensed layout 1`] = `
<div
class="c5"
>
<main
<div
class="c6"
>
<div
Expand All @@ -213,7 +213,7 @@ exports[`PageLayout renders condensed layout 1`] = `
<div
class=""
/>
</main>
</div>
<div
class="c8"
>
Expand Down Expand Up @@ -493,7 +493,7 @@ exports[`PageLayout renders default layout 1`] = `
<div
class="c5"
>
<main
<div
class="c6"
>
<div
Expand All @@ -502,12 +502,16 @@ exports[`PageLayout renders default layout 1`] = `
<div
class="c7"
>
Content
<main
aria-label="Content"
>
Content
</main>
</div>
<div
class=""
/>
</main>
</div>
<div
class="c8"
>
Expand Down Expand Up @@ -787,7 +791,7 @@ exports[`PageLayout renders pane in different position when narrow 1`] = `
<div
class="c5"
>
<main
<div
class="c6"
>
<div
Expand All @@ -801,7 +805,7 @@ exports[`PageLayout renders pane in different position when narrow 1`] = `
<div
class=""
/>
</main>
</div>
<div
class="c8"
>
Expand Down Expand Up @@ -1081,7 +1085,7 @@ exports[`PageLayout renders with dividers 1`] = `
<div
class="c5"
>
<main
<div
class="c6"
>
<div
Expand All @@ -1095,7 +1099,7 @@ exports[`PageLayout renders with dividers 1`] = `
<div
class=""
/>
</main>
</div>
<div
class="c8"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ exports[`SplitPageLayout renders default layout 1`] = `
<div
class="c5"
>
<main
<div
class="c6"
>
<div
Expand All @@ -248,7 +248,7 @@ exports[`SplitPageLayout renders default layout 1`] = `
<div
class=""
/>
</main>
</div>
<div
class="c8"
>
Expand Down

0 comments on commit ade10e6

Please sign in to comment.