Skip to content

Commit

Permalink
a11y: increase breakpoint width where we hide/show sidebar (#39465)
Browse files Browse the repository at this point in the history
Co-authored-by: Kevin Heis <[email protected]>
  • Loading branch information
rsese and heiskr authored Jul 26, 2023
1 parent 5b2c714 commit 913703d
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 12 deletions.
2 changes: 1 addition & 1 deletion components/article/ArticleInlineLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export const ArticleInlineLayout = ({
return (
<Box className={cx(styles.containerBox, className)}>
{breadcrumbs && (
<Box gridArea="breadcrumbs" className={cx('d-none d-xl-block mt-3 mr-auto width-full')}>
<Box gridArea="breadcrumbs" className={cx('d-none d-xxl-block mt-3 mr-auto width-full')}>
{breadcrumbs}
</Box>
)}
Expand Down
2 changes: 1 addition & 1 deletion components/article/ArticlePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export const ArticlePage = () => {
</ArticleInlineLayout>
) : (
<div className="container-xl px-3 px-md-6 my-4">
<div className={cx('d-none d-xl-block mt-3 mr-auto width-full')}>
<div className={cx('d-none d-xxl-block mt-3 mr-auto width-full')}>
<Breadcrumbs />
</div>

Expand Down
2 changes: 1 addition & 1 deletion components/page-header/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ export const Header = () => {
</div>
</div>
{!isHomepageVersion && !isSearchResultsPage && (
<div className="d-flex flex-items-center d-xl-none mt-2">
<div className="d-flex flex-items-center d-xxl-none mt-2">
<div className={cx(styles.sidebarOverlayCloseButtonContainer, 'mr-2')}>
<IconButton
data-testid="sidebar-hamburger"
Expand Down
6 changes: 3 additions & 3 deletions components/sidebar/SidebarNav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ export const SidebarNav = ({ variant = 'full' }: Props) => {

return (
<div
className={cx(variant === 'full' ? 'position-sticky d-none border-right d-xl-block' : '')}
className={cx(variant === 'full' ? 'position-sticky d-none border-right d-xxl-block' : '')}
style={{ width: 326, height: 'calc(100vh - 65px)', top: '65px' }}
>
{variant === 'full' && currentProduct && (
<nav
className={cx('d-none px-4 pb-3 border-bottom d-xl-block')}
className={cx('d-none px-4 pb-3 border-bottom d-xxl-block')}
aria-labelledby="sidebar-header"
>
<AllProductsLink />
Expand All @@ -47,7 +47,7 @@ export const SidebarNav = ({ variant = 'full' }: Props) => {
)}
<div
className={cx(
variant === 'overlay' ? 'd-xl-none' : 'border-right d-none d-xl-block',
variant === 'overlay' ? 'd-xxl-none' : 'border-right d-none d-xxl-block',
'bg-primary overflow-y-auto flex-shrink-0',
)}
style={{ width: 326, height: 'calc(100vh - 175px)', paddingBottom: sidebarPaddingBottom }}
Expand Down
18 changes: 16 additions & 2 deletions playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,15 @@ export default defineConfig({
projects: [
{
name: 'chromium',
use: { ...devices['Desktop Chrome'] },
use: {
...devices['Desktop Chrome'],
// need this wider width because of our slightly wider than normal xl
// breakpoint that helps prevent overlapping main content with the minitoc
viewport: {
width: 1400,
height: 720,
},
},
},

{
Expand Down Expand Up @@ -79,7 +87,13 @@ export default defineConfig({
// },
{
name: 'Google Chrome',
use: { channel: 'chrome' },
use: {
channel: 'chrome',
viewport: {
width: 1400,
height: 720,
},
},
},
],

Expand Down
18 changes: 18 additions & 0 deletions stylesheets/utilities.scss
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,21 @@
white-space: normal;
}
}

/* Responsive display utilities
------------------------------------------------------------------------------*/

// between Primer's xl and xxl widths, used to help prevent overlapping main
// content and the minitoc sidebar

.d-xxl-block {
@media (min-width: 1400px) {
display: block !important;
}
}

.d-xxl-none {
@media (min-width: 1400px) {
display: none !important;
}
}
8 changes: 4 additions & 4 deletions tests/rendering-fixtures/playwright-rendering.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,15 +256,15 @@ test('hovercards', async ({ page }) => {
})

test.describe('test nav at different viewports', () => {
test('x-large viewports - 1280+', async ({ page }) => {
test('xx-large viewports - 1400+', async ({ page }) => {
page.setViewportSize({
width: 1300,
width: 1400,
height: 700,
})
await page.goto('/get-started/foo/bar')

// in article breadcrumbs at xl viewport should remove last breadcrumb so
// for this page we should only have 'Get Started / Foo'
// in article breadcrumbs at our custom xl viewport should remove last
// breadcrumb so for this page we should only have 'Get Started / Foo'
expect(await page.getByTestId('breadcrumbs-in-article').getByRole('link').all()).toHaveLength(2)
await expect(page.getByTestId('breadcrumbs-in-article').getByText('Foo')).toBeVisible()
await expect(page.getByTestId('breadcrumbs-in-article').getByText('Bar')).not.toBeVisible()
Expand Down

0 comments on commit 913703d

Please sign in to comment.