Skip to content

Commit 9884730

Browse files
graceparkgracepark
authored
Keyboard fix header medium viewports (github#40240)
1 parent abe7a9c commit 9884730

File tree

3 files changed

+58
-55
lines changed

3 files changed

+58
-55
lines changed

‎components/page-header/Header.tsx‎

Lines changed: 53 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,7 @@ export const Header = () => {
4747
const signupCTAVisible =
4848
hasAccount === false && // don't show if `null`
4949
(currentVersion === DEFAULT_VERSION || currentVersion === 'enterprise-cloud@latest')
50-
const [windowSize, setWindowSize] = useState(0)
51-
const handleWindowResize = useCallback(() => {
52-
setWindowSize(window.innerWidth)
53-
}, [])
50+
const { width } = useWidth()
5451

5552
useEffect(() => {
5653
function onScroll() {
@@ -92,11 +89,9 @@ export const Header = () => {
9289
if (bodyDiv && body) {
9390
// The full sidebar automatically shows at the xl window size so unlock
9491
// scrolling if the overlay was opened and the window size is increased to xl.
95-
body.style.overflow = isSidebarOpen && windowSize < 1280 ? 'hidden' : 'auto'
92+
body.style.overflow = isSidebarOpen && width && width < 1280 ? 'hidden' : 'auto'
9693
}
97-
window.addEventListener('resize', handleWindowResize)
98-
return () => window.removeEventListener('resize', handleWindowResize)
99-
}, [isSidebarOpen, windowSize])
94+
}, [isSidebarOpen])
10095

10196
// with client side navigation clicking sidebar overlay links doesn't dismiss
10297
// the overlay so we close it ourselves when the path changes
@@ -118,6 +113,32 @@ export const Header = () => {
118113
}
119114
}, [])
120115

116+
function useWidth() {
117+
const hasWindow = typeof window !== 'undefined'
118+
119+
function getWidth() {
120+
const width = hasWindow ? window.innerWidth : null
121+
return {
122+
width,
123+
}
124+
}
125+
126+
const [width, setWidth] = useState(getWidth())
127+
128+
useEffect(() => {
129+
if (hasWindow) {
130+
const handleResize = function () {
131+
setWidth(getWidth())
132+
}
133+
134+
window.addEventListener('resize', handleResize)
135+
return () => window.removeEventListener('resize', handleResize)
136+
}
137+
}, [hasWindow])
138+
139+
return width
140+
}
141+
121142
return (
122143
<>
123144
<div
@@ -268,51 +289,37 @@ export const Header = () => {
268289
/>
269290
</ActionMenu.Anchor>
270291
<ActionMenu.Overlay align="start">
271-
{/* Mobile Menu at XS browser width */}
272-
<ActionList
273-
sx={{
274-
'@media (min-width: 544px)': {
275-
display: 'none',
276-
},
277-
}}
278-
>
279-
<ActionList.Group data-testid="open-xs-mobile-menu">
280-
<LanguagePicker xs={true} />
292+
<ActionList>
293+
<ActionList.Group data-testid="open-mobile-menu">
294+
{width && width > 544 ? (
295+
<LanguagePicker mediumOrLower={true} />
296+
) : (
297+
<LanguagePicker xs={true} />
298+
)}
281299
<ActionList.Divider />
282-
<VersionPicker xs={true} />
283-
{signupCTAVisible && (
300+
{width && width < 545 && (
284301
<>
302+
<VersionPicker xs={true} />
285303
<ActionList.Divider />
286-
<ActionList.LinkItem
287-
href="https://github.com/signup?ref_cta=Sign+up&ref_loc=docs+header&ref_page=docs"
288-
target="_blank"
289-
rel="noopener"
290-
data-testid="xs-mobile-signup"
291-
className="d-flex color-fg-muted"
292-
>
293-
{t`sign_up_cta`}
294-
<LinkExternalIcon
295-
className="height-full float-right"
296-
aria-label="(external site)"
297-
/>
298-
</ActionList.LinkItem>
299304
</>
305+
)}
306+
{signupCTAVisible && (
307+
<ActionList.LinkItem
308+
href="https://github.com/signup?ref_cta=Sign+up&ref_loc=docs+header&ref_page=docs"
309+
target="_blank"
310+
rel="noopener"
311+
data-testid="mobile-signup"
312+
className="d-flex color-fg-muted"
313+
>
314+
{t`sign_up_cta`}
315+
<LinkExternalIcon
316+
className="height-full float-right"
317+
aria-label="(external site)"
318+
/>
319+
</ActionList.LinkItem>
300320
)}{' '}
301321
</ActionList.Group>
302322
</ActionList>
303-
<LanguagePicker mediumOrLower={true} />
304-
{signupCTAVisible && (
305-
<Link
306-
href="https://github.com/signup?ref_cta=Sign+up&ref_loc=docs+header&ref_page=docs"
307-
target="_blank"
308-
rel="noopener"
309-
data-testid="mobile-signup"
310-
className="hide-sm d-flex flex-justify-between flex-items-center color-fg-muted border-top px-3 py-3"
311-
>
312-
{t`sign_up_cta`}
313-
<LinkExternalIcon aria-label="(external site)" />
314-
</Link>
315-
)}{' '}
316323
</ActionMenu.Overlay>
317324
</ActionMenu>
318325
</div>

‎components/page-header/LanguagePicker.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useRouter } from 'next/router'
2-
import { GlobeIcon, KebabHorizontalIcon } from '@primer/octicons-react'
2+
import { GlobeIcon } from '@primer/octicons-react'
33

44
import { useLanguages } from 'components/context/LanguagesContext'
55
import { useTranslation } from 'components/hooks/useTranslation'
@@ -103,7 +103,7 @@ export const LanguagePicker = ({ xs, mediumOrLower }: Props) => {
103103
<ActionMenu>
104104
<ActionMenu.Anchor>
105105
<IconButton
106-
icon={mediumOrLower ? KebabHorizontalIcon : GlobeIcon}
106+
icon={GlobeIcon}
107107
aria-label={`Select language: current language is ${selectedLang.name}`}
108108
/>
109109
</ActionMenu.Anchor>

‎tests/rendering-fixtures/playwright-rendering.spec.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -401,17 +401,13 @@ test.describe('test nav at different viewports', () => {
401401
// version picker is in mobile menu
402402
await expect(page.getByTestId('version-picker')).not.toBeVisible()
403403
await page.getByTestId('mobile-menu').click()
404-
await expect(
405-
page.getByTestId('open-xs-mobile-menu').getByTestId('version-picker'),
406-
).toBeVisible()
404+
await expect(page.getByTestId('open-mobile-menu').getByTestId('version-picker')).toBeVisible()
407405

408406
// language picker is in mobile menu
409-
await expect(
410-
page.getByTestId('open-xs-mobile-menu').getByTestId('language-picker'),
411-
).toBeVisible()
407+
await expect(page.getByTestId('open-mobile-menu').getByTestId('language-picker')).toBeVisible()
412408

413409
// sign up button is in mobile menu
414-
await expect(page.getByTestId('xs-mobile-signup')).toBeVisible()
410+
await expect(page.getByTestId('mobile-signup')).toBeVisible()
415411

416412
// hamburger button for sidebar overlay is visible
417413
await expect(page.getByTestId('sidebar-hamburger')).toBeVisible()

0 commit comments

Comments
 (0)