Skip to content

🎨 Palette: Header Accessibility and Tooltips #538

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.

Sign up for GitHub

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

Open
ngoiyaeric wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🎨 Palette: Header Accessibility and Tooltips #538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions .jules/palette.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

## 2026-02-20 - [Accessible Header Actions]
**Learning:** Icon-only buttons in the navigation header lack clear visual and screen-reader context. While ARIA labels solve accessibility, they don't provide visual guidance for sighted users on hover.
**Action:** Always pair ARIA labels with Tooltips for icon-only navigation elements. Ensure a global TooltipProvider is available in the root layout to support these components.
Comment on lines +1 to +4
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Two markdownlint warnings flagged by static analysis.

  1. MD022 — The ## heading on line 2 is not followed by a blank line; content starts immediately on line 3.
  2. MD041 — The first heading in the file should be a top-level # heading, not ##.
📝 Proposed fix 
-
-## 2026-02-20 - [Accessible Header Actions]
-**Learning:** Icon-only buttons in the navigation header lack clear visual and screen-reader context. While ARIA labels solve accessibility, they don't provide visual guidance for sighted users on hover.
-**Action:** Always pair ARIA labels with Tooltips for icon-only navigation elements. Ensure a global TooltipProvider is available in the root layout to support these components.
+# Palette Notes
+
+## 2026-02-20 - Accessible Header Actions
+
+**Learning:** Icon-only buttons in the navigation header lack clear visual and screen-reader context. While ARIA labels solve accessibility, they don't provide visual guidance for sighted users on hover.
+
+**Action:** Always pair ARIA labels with Tooltips for icon-only navigation elements. Ensure a global TooltipProvider is available in the root layout to support these components.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
## 2026-02-20 - [Accessible Header Actions]
**Learning:** Icon-only buttons in the navigation header lack clear visual and screen-reader context. While ARIA labels solve accessibility, they don't provide visual guidance for sighted users on hover.
**Action:** Always pair ARIA labels with Tooltips for icon-only navigation elements. Ensure a global TooltipProvider is available in the root layout to support these components.
# Palette Notes
## 2026-02-20 - Accessible Header Actions
**Learning:** Icon-only buttons in the navigation header lack clear visual and screen-reader context. While ARIA labels solve accessibility, they don't provide visual guidance for sighted users on hover.
**Action:** Always pair ARIA labels with Tooltips for icon-only navigation elements. Ensure a global TooltipProvider is available in the root layout to support these components.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)

[warning] 2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

[warning] 2-2: First line in a file should be a top-level heading

(MD041, first-line-heading, first-line-h1)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.jules/palette.md around lines 1 - 4, Change the initial second-level
heading to a top-level heading and add a blank line after it: replace the
leading "## 2026-02-20 - [Accessible Header Actions]" with "# 2026-02-20 -
[Accessible Header Actions]" and insert an empty line between that heading and
the following content to satisfy MD041 and MD022 (the heading symbol and the
blank-line-after-heading are the unique items to change).

15 changes: 9 additions & 6 deletions app/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { CalendarToggleProvider } from '@/components/calendar-toggle-context'
import { HistoryToggleProvider } from '@/components/history-toggle-context'
import { HistorySidebar } from '@/components/history-sidebar'
import { MapLoadingProvider } from '@/components/map-loading-context';
import { TooltipProvider } from '@/components/ui/tooltip'
import ConditionalLottie from '@/components/conditional-lottie';
import { MapProvider as MapContextProvider } from '@/components/map/map-context'

Expand Down Expand Up @@ -109,12 +110,14 @@ export default function RootLayout({
>
<MapContextProvider>
<MapLoadingProvider>
<Header />
<ConditionalLottie />
{children}
<HistorySidebar />
<Footer />
<Toaster />
<TooltipProvider>
<Header />
<ConditionalLottie />
{children}
<HistorySidebar />
<Footer />
<Toaster />
</TooltipProvider>
Comment on lines +113 to +120
Copy link

@charliecreates charliecreates bot Feb 20, 2026

Choose a reason for hiding this comment

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

TooltipProvider is mounted inside several other providers and fairly deep in the tree. That works for header tooltips, but it can cause unexpected behavior if other parts of the app render tooltips in portals that assume a top-level provider (e.g., modals/overlays mounted outside this subtree). Generally, TooltipProvider should be as close to the root of <body> as possible.

Suggestion

Consider moving <TooltipProvider> to wrap the entire app shell (ideally directly under <body>), so all routes/portals/modals share a single provider context.

Example:

<body>
  <TooltipProvider>
    {/* all existing providers/components */}
  </TooltipProvider>
</body>

This reduces the risk of tooltip context gaps when components render outside the current provider subtree. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this refactor.

</MapLoadingProvider>
</MapContextProvider>
</ThemeProvider>
Expand Down
84 changes: 65 additions & 19 deletions components/header.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ import { useUsageToggle } from './usage-toggle-context'
import { useProfileToggle } from './profile-toggle-context'
import { useHistoryToggle } from './history-toggle-context'
import { useState, useEffect } from 'react'
import {
Tooltip,
TooltipContent,
TooltipTrigger
} from '@/components/ui/tooltip'

export const Header = () => {
const { toggleCalendar } = useCalendarToggle()
Expand Down Expand Up @@ -52,15 +57,26 @@ export const Header = () => {
</div>

<div className="absolute left-1 flex items-center">
<Button variant="ghost" size="icon" onClick={toggleHistory} data-testid="logo-history-toggle">
<Image
src="/images/logo.svg"
alt="Logo"
width={20}
height={20}
className="h-5 w-auto"
/>
</Button>
<Tooltip>
<TooltipTrigger asChild>
<Button
variant="ghost"
size="icon"
onClick={toggleHistory}
data-testid="logo-history-toggle"
aria-label="Toggle history"
>
Comment on lines +62 to +68
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, locate the header component and read it
find . -name "header.tsx" -o -name "header.ts" | head -5

Repository: QueueLab/QCX

Length of output: 79

🏁 Script executed:

# If found, read the file with line numbers, focusing on the relevant sections
wc -l components/header.tsx

Repository: QueueLab/QCX

Length of output: 81

🏁 Script executed:

# Check what the hooks (useHistoryToggle, useCalendarToggle, useUsageToggle) export
rg -n "export.*useHistoryToggle|export.*useCalendarToggle|export.*useUsageToggle" --type ts --type tsx -A 5

Repository: QueueLab/QCX

Length of output: 83

🏁 Script executed:

# Also search for hook definitions and what they return
rg -n "function useHistoryToggle|function useCalendarToggle|function useUsageToggle|const useHistoryToggle|const useCalendarToggle|const useUsageToggle" --type ts --type tsx -A 10

Repository: QueueLab/QCX

Length of output: 83

🏁 Script executed:

cat -n components/header.tsx

Repository: QueueLab/QCX

Length of output: 5561

🏁 Script executed:

# Search for hook definitions without type filtering
rg -n "useHistoryToggle|useCalendarToggle|useUsageToggle" -A 8

Repository: QueueLab/QCX

Length of output: 13070

🏁 Script executed:

# Search for where these hooks are imported and used in header.tsx
rg -n "const.*useHistory|const.*useCalendar|const.*useUsage" -A 3

Repository: QueueLab/QCX

Length of output: 3986

🏁 Script executed:

# Read the usage-toggle-context to see its interface
cat -n components/usage-toggle-context.tsx

Repository: QueueLab/QCX

Length of output: 1156

🏁 Script executed:

# Read the history-toggle-context to see its interface
cat -n components/history-toggle-context.tsx

Repository: QueueLab/QCX

Length of output: 1231

🏁 Script executed:

# Read the calendar-toggle-context to see its interface
cat -n components/calendar-toggle-context.tsx

Repository: QueueLab/QCX

Length of output: 1288

Toggle buttons are missing aria-expanded — WCAG 4.1.2 gap.

Buttons that open/close panels must communicate their current state to assistive technologies. Without aria-expanded, screen reader users cannot determine whether the history/usage/calendar panel is open or closed after activation.

All three toggle state values are available from their respective hooks and should be destructured and applied:

  • Usage buttons (lines 109–114, 130–135): isUsageOpen is already destructured at line 32. Add aria-expanded={isUsageOpen} to both usage toggle buttons.
  • History button (line 62–68): Add isHistoryOpen to the destructuring at line 34 (const { toggleHistory, isHistoryOpen } = useHistoryToggle()), then add aria-expanded={isHistoryOpen}.
  • Calendar button (line 92–98): Add isCalendarOpen to the destructuring at line 30 (const { toggleCalendar, isCalendarOpen } = useCalendarToggle()), then add aria-expanded={isCalendarOpen}.
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/header.tsx` around lines 62 - 68, The three toggle buttons are
missing aria-expanded props; update the component to destructure and use the
open-state flags from the hooks and pass them into each Button: add
isHistoryOpen to the existing destructure from useHistoryToggle() and set
aria-expanded={isHistoryOpen} on the history Button (onClick={toggleHistory}),
add isCalendarOpen to the destructure from useCalendarToggle() and set
aria-expanded={isCalendarOpen} on the calendar Button, and ensure both usage
toggle Buttons use the already-destructured isUsageOpen by adding
aria-expanded={isUsageOpen}; reference the functions/hooks toggleHistory,
useHistoryToggle, toggleCalendar, useCalendarToggle and the state names
isHistoryOpen, isCalendarOpen, isUsageOpen to locate where to apply these
changes.

<Image
src="/images/logo.svg"
alt="Logo"
width={20}
height={20}
className="h-5 w-auto"
/>
</Button>
</TooltipTrigger>
<TooltipContent side="bottom">Toggle history</TooltipContent>
</Tooltip>
Comment on lines +60 to +79
Copy link

@charliecreates charliecreates bot Feb 20, 2026

Choose a reason for hiding this comment

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

The logo button now has both an aria-label ("Toggle history") and an <Image alt="Logo" />. Depending on the underlying Button implementation and how accessible names are computed, the alt text can end up being part of the button’s accessible name and potentially conflict with/duplicate the explicit aria-label.

For purely decorative icons inside an already-labeled control, the icon/image should typically be hidden from assistive tech.

Suggestion

Make the logo image decorative within the button so the button’s accessible name is solely controlled by aria-label.

Options:

  1. If Next/Image supports it in your version, set an empty alt:
<Image src="/images/logo.svg" alt="" ... />
  1. Or add aria-hidden:
<Image ... alt="Logo" aria-hidden="true" />

(Prefer alt="" for decorative images.) Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.

<h1 className="text-2xl font-poppins font-semibold text-primary">
QCX
</h1>
Expand All @@ -71,15 +87,36 @@ export const Header = () => {

<MapToggle />

<Button variant="ghost" size="icon" onClick={toggleCalendar} title="Open Calendar" data-testid="calendar-toggle">
<CalendarDays className="h-[1.2rem] w-[1.2rem]" />
</Button>
<Tooltip>
<TooltipTrigger asChild>
<Button
variant="ghost"
size="icon"
onClick={toggleCalendar}
data-testid="calendar-toggle"
aria-label="Open calendar"
>
<CalendarDays className="h-[1.2rem] w-[1.2rem]" />
</Button>
</TooltipTrigger>
<TooltipContent side="bottom">Open calendar</TooltipContent>
</Tooltip>

<div id="header-search-portal" className="contents" />

<Button variant="ghost" size="icon" onClick={handleUsageToggle}>
<TentTree className="h-[1.2rem] w-[1.2rem]" />
</Button>
<Tooltip>
<TooltipTrigger asChild>
<Button
variant="ghost"
size="icon"
onClick={handleUsageToggle}
aria-label="Open usage"
>
Comment on lines +109 to +114
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 20, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Missing data-testid on usage toggle buttons.

The logo-history button (data-testid="logo-history-toggle") and calendar button (data-testid="calendar-toggle") have test IDs, but neither the desktop nor mobile usage toggle has one, making it harder to target these buttons in automated tests.

🔧 Proposed fix 
-            onClick={handleUsageToggle}
-            aria-label="Open usage"
+            data-testid="usage-toggle"
+            onClick={handleUsageToggle}
+            aria-label="Open usage"

Apply the same data-testid="usage-toggle" to both desktop (line 109) and mobile (line 130) usage buttons.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/header.tsx` around lines 109 - 114, The desktop and mobile usage
toggle buttons are missing a test id — add data-testid="usage-toggle" to the
Button that calls handleUsageToggle (the one with aria-label="Open usage") in
the header component, and also add the same data-testid to the mobile usage
toggle Button variant so both toggles can be targeted in tests.

<TentTree className="h-[1.2rem] w-[1.2rem]" />
</Button>
</TooltipTrigger>
<TooltipContent side="bottom">Open usage</TooltipContent>
</Tooltip>

<ModeToggle />

Expand All @@ -88,10 +125,19 @@ export const Header = () => {

{/* Mobile menu buttons */}
<div className="flex md:hidden gap-2">

<Button variant="ghost" size="icon" onClick={handleUsageToggle}>
<TentTree className="h-[1.2rem] w-[1.2rem]" />
</Button>
<Tooltip>
<TooltipTrigger asChild>
<Button
variant="ghost"
size="icon"
onClick={handleUsageToggle}
aria-label="Open usage"
>
<TentTree className="h-[1.2rem] w-[1.2rem]" />
</Button>
</TooltipTrigger>
<TooltipContent side="bottom">Open usage</TooltipContent>
</Tooltip>
Comment on lines +128 to +140
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Tooltip on mobile (md:hidden) is a no-op for touch users.

Radix UI's Tooltip fires on pointerenter / focus events; touch devices don't generate hover events, so the tooltip on the mobile usage button will never appear. The aria-label already covers screen-reader accessibility on mobile.

Consider whether this <Tooltip> wrapper adds value in the mobile section, or if it can be removed to keep the mobile markup leaner.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/header.tsx` around lines 128 - 140, The Tooltip wrapper around the
mobile usage Button (Tooltip, TooltipTrigger, TooltipContent) is a no-op on
touch devices; remove the Tooltip and TooltipTrigger and the TooltipContent in
the mobile section while keeping the Button, its props (variant="ghost"
size="icon" onClick={handleUsageToggle} aria-label="Open usage") and the
TentTree icon so the interaction and screen-reader text remain intact (update
the JSX around handleUsageToggle and TentTree accordingly).

<ProfileToggle/>
</div>
</header>
Expand Down
11 changes: 0 additions & 11 deletions server.log
View file
Open in desktop

This file was deleted.