-
-
Notifications
You must be signed in to change notification settings - Fork 647
Improve header accessibility and keyboard navigation support. Resolves #4804 #4935
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |
| import Image from 'next/image' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Focus restoration fallback references a non-existent Prompt for AI agents |
||
| import Link from 'next/link' | ||
| import { usePathname } from 'next/navigation' | ||
| import { useEffect, useState } from 'react' | ||
| import { useEffect, useRef, useState } from 'react' | ||
| import { | ||
| FaRegHeart, | ||
| FaRegStar, | ||
|
|
@@ -24,6 +24,7 @@ | |
| const pathname = usePathname() | ||
| const [mobileMenuOpen, setMobileMenuOpen] = useState(false) | ||
| const toggleMobileMenu = () => setMobileMenuOpen(!mobileMenuOpen) | ||
| const previousActiveElement = useRef<HTMLElement | null>(null) | ||
|
|
||
| useEffect(() => { | ||
| const handleResize = () => { | ||
|
|
@@ -46,12 +47,44 @@ | |
| } | ||
| } | ||
|
|
||
| const handleKeydown = (event: KeyboardEvent) => { | ||
| if (event.key === 'Escape' && mobileMenuOpen) { | ||
| setMobileMenuOpen(false) | ||
| } | ||
| } | ||
|
|
||
| // Focus management: when opening mobile nav, move focus to first focusable inside it. | ||
| if (mobileMenuOpen) { | ||
| previousActiveElement.current = document.activeElement as HTMLElement | null | ||
| // Defer to next tick so the panel is in the DOM | ||
| setTimeout(() => { | ||
| const mobileNav = document.getElementById('mobile-navigation') | ||
| if (mobileNav) { | ||
| const first = mobileNav.querySelector<HTMLElement>('a, button, [tabindex]:not([tabindex="-1"])') | ||
| first?.focus() | ||
| } | ||
| }, 0) | ||
| } else { | ||
| // restore focus to previous element or to the toggle button | ||
| setTimeout(() => { | ||
| const prev = previousActiveElement.current | ||
| if (prev) { | ||
| prev.focus() | ||
| } else { | ||
| const toggle = document.getElementById('mobile-menu-toggle') as HTMLElement | null | ||
|
Check warning on line 74 in frontend/src/components/Header.tsx
|
||
| toggle?.focus() | ||
| } | ||
| }, 0) | ||
| } | ||
|
|
||
| globalThis.addEventListener('resize', handleResize) | ||
| globalThis.addEventListener('click', handleOutsideClick) | ||
| globalThis.addEventListener('keydown', handleKeydown) | ||
|
|
||
| return () => { | ||
| globalThis.removeEventListener('resize', handleResize) | ||
| globalThis.removeEventListener('click', handleOutsideClick) | ||
| globalThis.removeEventListener('keydown', handleKeydown) | ||
| } | ||
| }, [mobileMenuOpen]) | ||
|
|
||
|
|
@@ -139,20 +172,25 @@ | |
| <div className="lg:hidden"> | ||
| <Button | ||
| onPress={toggleMobileMenu} | ||
| aria-expanded={mobileMenuOpen} | ||
| aria-controls="mobile-navigation" | ||
| className="flex h-11 w-11 items-center justify-center rounded-lg bg-transparent text-slate-300 hover:bg-transparent hover:text-slate-100 focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-white" | ||
| > | ||
| <span className="sr-only">Open main menu</span> | ||
| <span className="sr-only">{mobileMenuOpen ? 'Close main menu' : 'Open main menu'}</span> | ||
| {mobileMenuOpen ? <FaTimes className="h-6 w-6" /> : <FaBars className="h-6 w-6" />} | ||
| </Button> | ||
|
Comment on lines
173
to
181
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add The focus-restore logic at line 74 falls back to Proposed fix <Button
+ id="mobile-menu-toggle"
onPress={toggleMobileMenu}
aria-expanded={mobileMenuOpen}
aria-controls="mobile-navigation"🤖 Prompt for AI Agents |
||
| </div> | ||
| </div> | ||
| </div> | ||
| <div | ||
| id="mobile-navigation" | ||
| role="navigation" | ||
| aria-hidden={!mobileMenuOpen} | ||
| className={cn( | ||
| 'bg-owasp-blue fixed inset-y-0 left-0 z-50 w-64 transform shadow-md transition-transform dark:bg-slate-800', | ||
| mobileMenuOpen ? 'translate-x-0' : '-translate-x-full' | ||
| )} | ||
| > | ||
|
Check warning on line 193 in frontend/src/components/Header.tsx
|
||
| <div className="flex h-full flex-col justify-between gap-1 px-2 pt-2 pb-3"> | ||
| {/* Logo */} | ||
| <div className="flex flex-col justify-center gap-5"> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Skip-to-content link target
<main>is not focusable, so keyboard focus may not move to the main content after activation, defeating the skip link's purpose for keyboard users.Prompt for AI agents