Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PageLayout.Content should not use main landmark by default. #3154

Merged
merged 41 commits into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
67b0aa1
PageLayout.Content should not use main landmark by default.
radglob Apr 11, 2023
6f24481
Create eleven-humans-sneeze.md
radglob Apr 11, 2023
350db20
Revert unrelated snapshots.
radglob Apr 11, 2023
e88e8e2
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 12, 2023
e341fd6
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 13, 2023
4fcb556
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 13, 2023
410e2f5
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 13, 2023
49ae6da
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 14, 2023
5c0fc74
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 17, 2023
bdbfe23
Use explicit main tag in PageLayout test instead of passing as prop.
radglob Apr 17, 2023
dc125cb
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 17, 2023
9bcab34
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 17, 2023
2375b7a
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 17, 2023
dd4fec7
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 18, 2023
7ee5c5e
Update generated/components.json
radglob Apr 18, 2023
92cde4c
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 18, 2023
d982926
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 18, 2023
227ca09
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 18, 2023
d576e8c
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 18, 2023
87d9eb3
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 19, 2023
bb3f847
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 20, 2023
8e46eb0
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 20, 2023
765357a
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 21, 2023
2b5fd6e
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 21, 2023
6b99802
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 24, 2023
d7f0d05
PageLayout.Content should not have as prop.
radglob Apr 24, 2023
b46b95b
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 24, 2023
4049117
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 24, 2023
1b44603
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 24, 2023
fd3f34c
Update generated/components.json
radglob Apr 24, 2023
36dc124
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 25, 2023
791f595
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 25, 2023
b9cb513
Merge branch 'main' into remove-main-from-pagelayout
radglob Apr 28, 2023
455ecf0
Merge branch 'main' into remove-main-from-pagelayout
radglob May 1, 2023
6af4fc0
Merge branch 'main' into remove-main-from-pagelayout
radglob May 2, 2023
d7e4fbf
Merge branch 'main' into remove-main-from-pagelayout
radglob May 2, 2023
0fe571e
Update .changeset/eleven-humans-sneeze.md
radglob May 3, 2023
1614457
Don't add role to PageLayout.Content.
radglob May 8, 2023
791e307
Merge branch 'main' into remove-main-from-pagelayout
radglob May 8, 2023
f8c24df
Merge branch 'main' into remove-main-from-pagelayout
radglob May 8, 2023
4eb259c
Merge branch 'main' into remove-main-from-pagelayout
radglob May 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/eleven-humans-sneeze.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

PageLayout.Content no longer renders as `main` by default. Instead, developers may add a `main` landmark within `Pagelayout.Content` themselves.
6 changes: 4 additions & 2 deletions src/PageLayout/PageLayout.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,9 @@ describe('PageLayout', () => {
<ThemeProvider>
<PageLayout>
<PageLayout.Header aria-label="header">Header</PageLayout.Header>
<PageLayout.Content aria-label="content">Content</PageLayout.Content>
<PageLayout.Content>
<main aria-label="content">Content</main>
</PageLayout.Content>
<PageLayout.Pane>Pane</PageLayout.Pane>
<PageLayout.Footer aria-label="footer">Footer</PageLayout.Footer>
</PageLayout>
Expand All @@ -142,7 +144,7 @@ describe('PageLayout', () => {
<PageLayout.Header aria-labelledby="header-label">
<span id="header-label">header</span>
</PageLayout.Header>
<PageLayout.Content aria-labelledby="content-label">
<PageLayout.Content aria-labelledby="content-label" role="main">
<span id="content-label">content</span>
</PageLayout.Content>
<PageLayout.Pane>Pane</PageLayout.Pane>
Expand Down
4 changes: 3 additions & 1 deletion src/PageLayout/PageLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ export type PageLayoutContentProps = {
width?: keyof typeof contentWidths
padding?: keyof typeof SPACING_MAP
hidden?: boolean | ResponsiveValue<boolean>
role?: string
radglob marked this conversation as resolved.
Show resolved Hide resolved
} & SxProp

// TODO: Account for pane width when centering content
Expand All @@ -413,6 +414,7 @@ const Content: React.FC<React.PropsWithChildren<PageLayoutContentProps>> = ({
width = 'full',
padding = 'none',
hidden = false,
role,
children,
sx = {},
}) => {
Expand All @@ -421,9 +423,9 @@ const Content: React.FC<React.PropsWithChildren<PageLayoutContentProps>> = ({

return (
<Box
as="main"
aria-label={label}
aria-labelledby={labelledBy}
role={role}
sx={merge<BetterSystemStyleObject>(
{
display: isHidden ? 'none' : 'flex',
Expand Down
16 changes: 8 additions & 8 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 @@ -507,7 +507,7 @@ exports[`PageLayout renders default layout 1`] = `
<div
class=""
/>
</main>
</div>
<div
class="c8"
>
Expand Down Expand Up @@ -787,7 +787,7 @@ exports[`PageLayout renders pane in different position when narrow 1`] = `
<div
class="c5"
>
<main
<div
class="c6"
>
<div
Expand All @@ -801,7 +801,7 @@ exports[`PageLayout renders pane in different position when narrow 1`] = `
<div
class=""
/>
</main>
</div>
<div
class="c8"
>
Expand Down Expand Up @@ -1081,7 +1081,7 @@ exports[`PageLayout renders with dividers 1`] = `
<div
class="c5"
>
<main
<div
class="c6"
>
<div
Expand All @@ -1095,7 +1095,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