Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 18 additions & 0 deletions frontend/__tests__/a11y/components/Header.a11y.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,25 @@
import React from 'react'
import { render, screen } from '@testing-library/react'

Check warning on line 2 in frontend/__tests__/a11y/components/Header.a11y.test.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'@testing-library/react' imported multiple times.

See more on https://sonarcloud.io/project/issues?id=OWASP_Nest&issues=AZ7OBIN9xrtxLwevCip3&open=AZ7OBIN9xrtxLwevCip3&pullRequest=4935
import { axe } from 'jest-axe'

Check warning on line 3 in frontend/__tests__/a11y/components/Header.a11y.test.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'jest-axe' imported multiple times.

See more on https://sonarcloud.io/project/issues?id=OWASP_Nest&issues=AZ7OBIN9xrtxLwevCip4&open=AZ7OBIN9xrtxLwevCip4&pullRequest=4935
import 'jest-axe/extend-expect'
import '@testing-library/jest-dom'
import Header from 'components/Header'

Check warning on line 6 in frontend/__tests__/a11y/components/Header.a11y.test.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'components/Header' imported multiple times.

See more on https://sonarcloud.io/project/issues?id=OWASP_Nest&issues=AZ7OBIN9xrtxLwevCip5&open=AZ7OBIN9xrtxLwevCip5&pullRequest=4935

describe('Header accessibility', () => {
test('has no basic a11y violations and exposes mobile toggle attributes', async () => {
const { container } = render(<Header isGitHubAuthEnabled={false} />)
const results = await axe(container)
expect(results).toHaveNoViolations()

const toggle = screen.getByRole('button', { name: /open main menu|close main menu/i })
expect(toggle).toHaveAttribute('aria-controls', 'mobile-navigation')
expect(toggle).toHaveAttribute('aria-expanded', 'false')
})
})
import { render } from '@testing-library/react'

Check warning on line 19 in frontend/__tests__/a11y/components/Header.a11y.test.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'@testing-library/react' imported multiple times.

See more on https://sonarcloud.io/project/issues?id=OWASP_Nest&issues=AZ7OBIN9xrtxLwevCip6&open=AZ7OBIN9xrtxLwevCip6&pullRequest=4935
import { axe } from 'jest-axe'

Check warning on line 20 in frontend/__tests__/a11y/components/Header.a11y.test.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'jest-axe' imported multiple times.

See more on https://sonarcloud.io/project/issues?id=OWASP_Nest&issues=AZ7OBIN9xrtxLwevCip7&open=AZ7OBIN9xrtxLwevCip7&pullRequest=4935
import { useTheme } from 'next-themes'
import Header from 'components/Header'

Check warning on line 22 in frontend/__tests__/a11y/components/Header.a11y.test.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'components/Header' imported multiple times.

See more on https://sonarcloud.io/project/issues?id=OWASP_Nest&issues=AZ7OBIN9xrtxLwevCip8&open=AZ7OBIN9xrtxLwevCip8&pullRequest=4935

jest.mock('hooks/useDjangoSession', () => ({
useDjangoSession: jest.fn(() => ({
Expand Down
24 changes: 24 additions & 0 deletions frontend/src/app/globals.css
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,30 @@ select:disabled,
color: black;
}

/* Skip link for keyboard users */
.skip-link {
position: absolute;
left: -999px;
top: auto;
width: 1px;
height: 1px;
overflow: hidden;
z-index: 9999;
}

.skip-link:focus,
.skip-link:active {
left: 1rem;
top: 1rem;
width: auto;
height: auto;
padding: 0.5rem 0.75rem;
background-color: #111827; /* gray-900 */
color: #ffffff;
border-radius: 0.375rem;
box-shadow: 0 2px 8px rgba(0, 0, 0, 0.2);
}

.dark .popup-content {
color: white;
}
Expand Down
5 changes: 4 additions & 1 deletion frontend/src/app/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,16 @@ export default function RootLayout({
className={`${geistSans.variable} ${geistMono.variable} antialiased`}
style={{ minHeight: '100vh' }}
>
<a href="#main" className="skip-link">
Skip to main content
</a>
<Providers>
<BreadcrumbRoot>
<div className="flex min-h-screen flex-col">
<AutoScrollToTop />
<Header isGitHubAuthEnabled={IS_GITHUB_AUTH_ENABLED} />
<BreadCrumbsWrapper />
<main className="flex flex-1 flex-col justify-center">{children}</main>
<main id="main" className="flex flex-1 flex-col justify-center">{children}</main>

Copy link
Copy Markdown
Contributor

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
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/app/layout.tsx, line 83:

<comment>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.</comment>

<file context>
@@ -71,13 +71,16 @@ export default function RootLayout({
               <Header isGitHubAuthEnabled={IS_GITHUB_AUTH_ENABLED} />
               <BreadCrumbsWrapper />
-              <main className="flex flex-1 flex-col justify-center">{children}</main>
+              <main id="main" className="flex flex-1 flex-col justify-center">{children}</main>
               <Footer />
               <ScrollToTop />
</file context>
Suggested change
<main id="main" className="flex flex-1 flex-col justify-center">{children}</main>
+ <main id="main" tabIndex={-1} className="flex flex-1 flex-col justify-center">{children}</main>

<Footer />
<ScrollToTop />
</div>
Expand Down
42 changes: 40 additions & 2 deletions frontend/src/components/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import Image from 'next/image'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Focus restoration fallback references a non-existent #mobile-menu-toggle element

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/Header.tsx, line 74:

<comment>Focus restoration fallback references a non-existent `#mobile-menu-toggle` element</comment>

<file context>
@@ -46,12 +47,44 @@ export default function Header({ isGitHubAuthEnabled }: { readonly isGitHubAuthE
+        if (prev) {
+          prev.focus()
+        } else {
+          const toggle = document.getElementById('mobile-menu-toggle') as HTMLElement | null
+          toggle?.focus()
+        }
</file context>

import Link from 'next/link'
import { usePathname } from 'next/navigation'
import { useEffect, useState } from 'react'
import { useEffect, useRef, useState } from 'react'
import {
FaRegHeart,
FaRegStar,
Expand All @@ -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 = () => {
Expand All @@ -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

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This assertion is unnecessary since it does not change the type of the expression.

See more on https://sonarcloud.io/project/issues?id=OWASP_Nest&issues=AZ7OBIJwxrtxLwevCip1&open=AZ7OBIJwxrtxLwevCip1&pullRequest=4935
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])

Expand Down Expand Up @@ -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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add id="mobile-menu-toggle" to enable fallback focus restoration.

The focus-restore logic at line 74 falls back to document.getElementById('mobile-menu-toggle') when previousActiveElement.current is null, but the toggle button lacks this id attribute. If the originally-focused element becomes unavailable (e.g., removed from DOM), focus will be lost.

Proposed fix
           <Button
+            id="mobile-menu-toggle"
             onPress={toggleMobileMenu}
             aria-expanded={mobileMenuOpen}
             aria-controls="mobile-navigation"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/Header.tsx` around lines 173 - 181, Add the id
attribute "mobile-menu-toggle" to the Button component with
onPress={toggleMobileMenu}. The focus-restore logic references this id as a
fallback when the previously focused element is no longer available in the DOM,
so without it, focus restoration will fail when the originally-focused element
becomes unavailable.

</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

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use <nav> instead of the "navigation" role to ensure accessibility across all devices.

See more on https://sonarcloud.io/project/issues?id=OWASP_Nest&issues=AZ7OBIJwxrtxLwevCip2&open=AZ7OBIJwxrtxLwevCip2&pullRequest=4935
<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">
Expand Down