fix(a11y): add keyboard navigation and ARIA labels to filter controls - #2053
fix(a11y): add keyboard navigation and ARIA labels to filter controls#2053waterWang wants to merge 1 commit into
Conversation
- Add aria-expanded and aria-controls to mobile filter/language dropdown buttons - Add role=menu and aria-hidden to mobile filter/language panels - Add role=menuitem to mobile filter/language options - Implement Escape key to close mobile dropdowns with focus return - Add focus-visible styles for filter chips, pills, and mobile options - Add focus management: auto-focus first item when dropdown opens - Remove remaining blank line Closes S3DFX-CYBER#1969
|
@waterWang is attempting to deploy a commit to the s3dfx-cyber's projects Team on Vercel. A member of the Team first needs to authorize it. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
👋 Thanks for opening a PR, @waterWang!Your PR has entered the 🚦 PR Review Pipeline.
🔄 Review Flow
A pipeline status comment may appear automatically as your PR progresses. ✅ Contributor Checklist
|
💬 Faster Reviews & AssignmentsHi @waterWang, for faster coordination and smoother communication, consider joining our Discord community: 👉 https://discord.gg/jYrbHKtn8t Useful Channels
|
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds visible focus indicators and accessibility semantics to filter controls. Mobile filter and language dropdowns now synchronize ARIA state, manage focus, close on outside clicks or Escape, and restore focus to their triggers. ChangesFilter accessibility
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant FilterTrigger as Mobile filter trigger
participant LanguageTrigger as Language dropdown trigger
participant DropdownMenu as Mobile dropdown menu
participant Document as Document events
FilterTrigger->>DropdownMenu: Open filter menu
DropdownMenu->>LanguageTrigger: Close language menu
DropdownMenu->>DropdownMenu: Set ARIA state
DropdownMenu->>DropdownMenu: Focus first option
Document->>DropdownMenu: Handle outside click or Escape
DropdownMenu->>FilterTrigger: Restore trigger focus
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚦 PR Review Pipeline
Last updated: Mon, 03 Aug 2026 02:19:29 GMT |
|
🤖 TENET Agent Review📋 SummaryThis pull request significantly enhances the accessibility of the mobile filter and language dropdown controls by implementing proper ARIA attributes ( 🔐 Security FindingsNo security issues found. The changes are purely frontend accessibility enhancements and do not interact with security-sensitive logic, data, or LLM components of the TENET AI system. 🧹 Code Quality
✅ What's Done Well
📝 Overall VerdictAPPROVE - Excellent work on improving accessibility; minor code quality points do not impede functionality or security. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
index.html (1)
2753-2772: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce the outside-click handler complexity.
SonarCloud reports cognitive complexity 16, above the configured limit of 15. Extract the repeated panel, trigger, ARIA, and chevron reset logic into one
closeMobileDropdownhelper. Reuse that helper from the toggle, outside-click, and Escape paths.🤖 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 `@index.html` around lines 2753 - 2772, Reduce the click-handler complexity by introducing a shared closeMobileDropdown helper that resets the specified panel, trigger aria-expanded state, and chevron transform. Replace the duplicated closing logic in the outside-click handler and reuse the helper from the mobile dropdown toggle and Escape-key paths, preserving existing behavior and element identifiers.Source: Linters/SAST tools
🤖 Prompt for all review comments with 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.
Inline comments:
In `@index.html`:
- Around line 2719-2721: Update the dropdown-opening focus logic around the
isOpen checks to avoid deferring focus with an uncancellable setTimeout: focus
the first matching item synchronously after opening, or track the timer and
cancel it from every close path, including the corresponding logic near the
other affected block. Ensure delayed callbacks cannot focus a hidden panel or
override focus restoration after Escape or another dropdown opens.
- Around line 1189-1193: Update the clear-filters focus rule to target the
control by its existing clearFiltersBtn identifier instead of the unused
clear-filters-btn class, preserving the current focus styling.
- Around line 1502-1546: Replace the menu semantics on mobileFilterPanel and
mobileLangPanel with a labeled role="group", and remove the menu/menuitem roles
from their options. Add synchronized aria-pressed states to every mobile filter
and language button, updating them in toggleMobileChip and toggleMobileLangPill
whenever selections change.
---
Nitpick comments:
In `@index.html`:
- Around line 2753-2772: Reduce the click-handler complexity by introducing a
shared closeMobileDropdown helper that resets the specified panel, trigger
aria-expanded state, and chevron transform. Replace the duplicated closing logic
in the outside-click handler and reuse the helper from the mobile dropdown
toggle and Escape-key paths, preserving existing behavior and element
identifiers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6a3bc69f-16ee-47fd-907b-187f9e58ae3a
📒 Files selected for processing (1)
index.html
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: cubic · AI code reviewer
⚠️ CI failures not shown inline (1)
Commit Status: Vercel: Vercel
Conclusion: failure
Authorization required to deploy.
🧰 Additional context used
🪛 ast-grep (0.45.0)
index.html
[error] 2720-2720: React's useState should not be directly called
Context: setTimeout(() => firstItem.focus(), 50)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 2747-2747: React's useState should not be directly called
Context: setTimeout(() => firstItem.focus(), 50)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
🪛 GitHub Check: SonarCloud Code Analysis
index.html
[failure] 2753-2753: Refactor this function to reduce its Cognitive Complexity from 16 to the 15 allowed.
| .clear-filters-btn:focus-visible { | ||
| outline: 2px solid var(--orange, #f46b0e); | ||
| outline-offset: 2px; | ||
| border-radius: 12px; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match the clear-filters focus selector to the control.
Line 1595 uses id="clearFiltersBtn". It does not use the .clear-filters-btn class. The added custom focus style does not apply to this button.
Proposed fix
-.clear-filters-btn:focus-visible {
+#clearFiltersBtn:focus-visible {📝 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.
| .clear-filters-btn:focus-visible { | |
| outline: 2px solid var(--orange, #f46b0e); | |
| outline-offset: 2px; | |
| border-radius: 12px; | |
| } | |
| `#clearFiltersBtn`:focus-visible { | |
| outline: 2px solid var(--orange, `#f46b0e`); | |
| outline-offset: 2px; | |
| border-radius: 12px; | |
| } |
🤖 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 `@index.html` around lines 1189 - 1193, Update the clear-filters focus rule to
target the control by its existing clearFiltersBtn identifier instead of the
unused clear-filters-btn class, preserving the current focus styling.
| <div id="mobileFilterPanel" role="menu" aria-hidden="true" class="hidden absolute left-0 top-full mt-1 bg-white dark:bg-zinc-800 rounded-xl shadow-lg border border-zinc-100 dark:border-zinc-700 z-30 p-2 flex flex-col gap-1" style="min-width:180px"> | ||
| <button id="mob-chip-veteran" data-mobile-chip="chip-veteran" onclick="toggleMobileChip(this)" role="menuitem" class="mobile-filter-option flex items-center gap-2 px-3 py-2 rounded-lg text-xs font-medium hover:bg-surface-container-low transition-colors text-left focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary"> | ||
| <span class="material-symbols-outlined text-sm">military_tech</span> Veterans Only | ||
| </button> | ||
| <button id="mob-chip-newcomer" data-mobile-chip="chip-newcomer" onclick="toggleMobileChip(this)" class="mobile-filter-option flex items-center gap-2 px-3 py-2 rounded-lg text-xs font-medium hover:bg-surface-container-low transition-colors text-left"> | ||
| <button id="mob-chip-newcomer" data-mobile-chip="chip-newcomer" onclick="toggleMobileChip(this)" role="menuitem" class="mobile-filter-option flex items-center gap-2 px-3 py-2 rounded-lg text-xs font-medium hover:bg-surface-container-low transition-colors text-left focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary"> | ||
| <span class="material-symbols-outlined text-sm">fiber_new</span> Newcomers | ||
| </button> | ||
| <button id="mob-chip-hot" data-mobile-chip="chip-hot" onclick="toggleMobileChip(this)" class="mobile-filter-option flex items-center gap-2 px-3 py-2 rounded-lg text-xs font-medium hover:bg-surface-container-low transition-colors text-left"> | ||
| <button id="mob-chip-hot" data-mobile-chip="chip-hot" onclick="toggleMobileChip(this)" role="menuitem" class="mobile-filter-option flex items-center gap-2 px-3 py-2 rounded-lg text-xs font-medium hover:bg-surface-container-low transition-colors text-left focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary"> | ||
| <span class="material-symbols-outlined text-sm">local_fire_department</span> High Competition | ||
| </button> | ||
| <button id="mob-chip-chill" data-mobile-chip="chip-chill" onclick="toggleMobileChip(this)" class="mobile-filter-option flex items-center gap-2 px-3 py-2 rounded-lg text-xs font-medium hover:bg-surface-container-low transition-colors text-left"> | ||
| <button id="mob-chip-chill" data-mobile-chip="chip-chill" onclick="toggleMobileChip(this)" role="menuitem" class="mobile-filter-option flex items-center gap-2 px-3 py-2 rounded-lg text-xs font-medium hover:bg-surface-container-low transition-colors text-left focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary"> | ||
| <span class="material-symbols-outlined text-sm">sentiment_satisfied</span> Low Competition | ||
| </button> | ||
| <button id="mob-chip-active" data-mobile-chip="chip-active" onclick="toggleMobileChip(this)" class="mobile-filter-option flex items-center gap-2 px-3 py-2 rounded-lg text-xs font-medium hover:bg-surface-container-low transition-colors text-left"> | ||
| <button id="mob-chip-active" data-mobile-chip="chip-active" onclick="toggleMobileChip(this)" role="menuitem" class="mobile-filter-option flex items-center gap-2 px-3 py-2 rounded-lg text-xs font-medium hover:bg-surface-container-low transition-colors text-left focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary"> | ||
| <span class="material-symbols-outlined text-sm">bolt</span> Actively Maintained | ||
| </button> | ||
| <button id="mob-chip-bookmarked" data-mobile-chip="chip-bookmarked" onclick="toggleMobileChip(this)" class="mobile-filter-option flex items-center gap-2 px-3 py-2 rounded-lg text-xs font-medium hover:bg-surface-container-low transition-colors text-left"> | ||
| <button id="mob-chip-bookmarked" data-mobile-chip="chip-bookmarked" onclick="toggleMobileChip(this)" role="menuitem" class="mobile-filter-option flex items-center gap-2 px-3 py-2 rounded-lg text-xs font-medium hover:bg-surface-container-low transition-colors text-left focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary"> | ||
| <span class="material-symbols-outlined text-sm icon-fill" style="color:#f59e0b">star</span> Bookmarked | ||
| </button> | ||
| </div> | ||
| </div> | ||
|
|
||
| <!-- Mobile: Languages dropdown (replaces language pills) --> | ||
| <div class="relative flex-1" id="mobileLangDropdownWrap"> | ||
| <button id="mobileLangBtn" onclick="toggleMobileLangDropdown()" class="w-full flex items-center justify-between px-3 py-2.5 bg-surface-container-highest rounded-xl text-xs font-bold text-on-surface border border-transparent hover:border-primary transition-colors"> | ||
| <button id="mobileLangBtn" onclick="toggleMobileLangDropdown()" aria-expanded="false" aria-controls="mobileLangPanel" class="w-full flex items-center justify-between px-3 py-2.5 bg-surface-container-highest rounded-xl text-xs font-bold text-on-surface border border-transparent hover:border-primary transition-colors focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary"> | ||
| <span class="flex items-center gap-1.5"> | ||
| <span class="material-symbols-outlined text-sm text-primary">code</span> | ||
| <span id="mobileLangLabel">Languages</span> | ||
| </span> | ||
| <span class="material-symbols-outlined text-sm transition-transform" id="mobileLangChevron">expand_more</span> | ||
| </button> | ||
| <div id="mobileLangPanel" class="hidden absolute right-0 top-full mt-1 bg-white dark:bg-zinc-800 rounded-xl shadow-lg border border-zinc-100 dark:border-zinc-700 z-30 p-2" style="min-width:240px"> | ||
| <div id="mobileLangPanel" role="menu" aria-hidden="true" class="hidden absolute right-0 top-full mt-1 bg-white dark:bg-zinc-800 rounded-xl shadow-lg border border-zinc-100 dark:border-zinc-700 z-30 p-2" style="min-width:240px"> | ||
| <div class="flex flex-wrap gap-1.5 p-1"> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors" data-lang="Python" onclick="toggleMobileLangPill(this)">Python</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors" data-lang="JavaScript" onclick="toggleMobileLangPill(this)">JavaScript</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors" data-lang="TypeScript" onclick="toggleMobileLangPill(this)">TypeScript</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors" data-lang="C/C++" onclick="toggleMobileLangPill(this)">C/C++</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors" data-lang="Java" onclick="toggleMobileLangPill(this)">Java</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors" data-lang="Rust" onclick="toggleMobileLangPill(this)">Rust</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors" data-lang="Go" onclick="toggleMobileLangPill(this)">Go</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors" data-lang="Ruby" onclick="toggleMobileLangPill(this)">Ruby</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors" data-lang="Haskell" onclick="toggleMobileLangPill(this)">Haskell</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors" data-lang="Scala" onclick="toggleMobileLangPill(this)">Scala</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors" data-lang="ML/AI" onclick="toggleMobileLangPill(this)">ML/AI</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors" data-lang="Robotics" onclick="toggleMobileLangPill(this)">Robotics</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary" data-lang="Python" onclick="toggleMobileLangPill(this)">Python</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary" data-lang="JavaScript" onclick="toggleMobileLangPill(this)">JavaScript</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary" data-lang="TypeScript" onclick="toggleMobileLangPill(this)">TypeScript</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary" data-lang="C/C++" onclick="toggleMobileLangPill(this)">C/C++</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary" data-lang="Java" onclick="toggleMobileLangPill(this)">Java</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary" data-lang="Rust" onclick="toggleMobileLangPill(this)">Rust</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary" data-lang="Go" onclick="toggleMobileLangPill(this)">Go</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary" data-lang="Ruby" onclick="toggleMobileLangPill(this)">Ruby</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary" data-lang="Haskell" onclick="toggleMobileLangPill(this)">Haskell</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary" data-lang="Scala" onclick="toggleMobileLangPill(this)">Scala</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary" data-lang="ML/AI" onclick="toggleMobileLangPill(this)">ML/AI</button> | ||
| <button class="mobile-lang-pill px-3 py-1.5 rounded-full bg-surface-container-high border border-zinc-200 text-xs font-medium hover:border-primary hover:text-primary transition-colors focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary" data-lang="Robotics" onclick="toggleMobileLangPill(this)">Robotics</button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'toggleMobileChip|toggleMobileLangPill|aria-pressed|aria-checked|ArrowDown|ArrowUp|Home|End|mobileFilterPanel|mobileLangPanel' index.htmlRepository: S3DFX-CYBER/GSoC-Org-Finder-
Length of output: 24803
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '2799,2852p' index.html
sed -n '2620,2692p' index.htmlRepository: S3DFX-CYBER/GSoC-Org-Finder-
Length of output: 4942
Use toggle semantics for the mobile filter controls.
mobileFilterPanel and mobileLangPanel use role="menu" with role="menuitem" options, but these controls work as multi-select toggles. Use a labeled role="group" and add aria-pressed on each filter/language button updated by toggleMobileChip and toggleMobileLangPill; or replace the entire menu model with menuitemcheckbox with synchronized aria-checked.
🤖 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 `@index.html` around lines 1502 - 1546, Replace the menu semantics on
mobileFilterPanel and mobileLangPanel with a labeled role="group", and remove
the menu/menuitem roles from their options. Add synchronized aria-pressed states
to every mobile filter and language button, updating them in toggleMobileChip
and toggleMobileLangPill whenever selections change.
| if (!isOpen) { | ||
| const firstItem = panel.querySelector('button, [tabindex]:not([tabindex="-1"])'); | ||
| if (firstItem) setTimeout(() => firstItem.focus(), 50); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not defer focus without cancellation.
The setTimeout callback can run after Escape or after the other dropdown opens. It can then move focus into a hidden panel and override the focus restoration behavior.
Focus the first option synchronously after opening, or store and cancel the pending timer from every close path.
Proposed fix
- if (firstItem) setTimeout(() => firstItem.focus(), 50);
+ if (firstItem) firstItem.focus();Also applies to: 2746-2748
🧰 Tools
🪛 ast-grep (0.45.0)
[error] 2720-2720: React's useState should not be directly called
Context: setTimeout(() => firstItem.focus(), 50)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
🤖 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 `@index.html` around lines 2719 - 2721, Update the dropdown-opening focus logic
around the isOpen checks to avoid deferring focus with an uncancellable
setTimeout: focus the first matching item synchronously after opening, or track
the timer and cancel it from every close path, including the corresponding logic
near the other affected block. Ensure delayed callbacks cannot focus a hidden
panel or override focus restoration after Escape or another dropdown opens.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 1 file
Confidence score: 3/5
- In
index.html, the deferred focus call (setTimeout(() => firstItem.focus(), 50)) can still fire after the panel is closed or another dropdown opens, causing focus to jump to hidden/stale UI and confusing keyboard users; store and clear the timeout on close/Escape/switch paths to prevent late focus moves. - In
index.html, applyingrole="menu"/role="menuitem"without full menu keyboard behavior (single tab stop + Arrow navigation) creates an accessibility mismatch that can mislead assistive tech and break expected interaction patterns; either implement the ARIA menu pattern fully or use semantics that match simple filter controls. - In
index.html, the Clear Filters control lacks a visible focus style because the selector does not matchid="clearFiltersBtn", which leaves keyboard focus hard to track; update the selector to target the existing ID (or add the expected class). - In
index.html, the two toggle handlers now duplicate roughly the same open/close, ARIA, chevron, sibling-close, and focus logic, increasing drift risk when future fixes are made; extract shared behavior into one helper so bug fixes land consistently in both paths.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="index.html">
<violation number="1" location="index.html:1189">
P3: Clear Filters still has no visible focus indicator because this selector does not match its `id="clearFiltersBtn"`. Target the existing ID (or add the class) so keyboard focus receives the intended style.</violation>
<violation number="2" location="index.html:1502">
P2: The new `role="menu"` / `role="menuitem"` markup doesn't match a real ARIA menu: per the WAI-ARIA menu pattern, a menu requires menuitem children, only one tab stop, and Arrow-key navigation — but these filter items are still plain buttons reached via Tab, with no arrow-key handling or `aria-haspopup` on the trigger. The result is that screen readers announce "menu/menuitem" semantics that don't match how the control actually behaves, which undercuts the accessibility goal of this PR. Additionally, the language panel has `role="menu"` while its child language pills were left as plain `<button>`s (no menuitem role), so that menu has no valid menuitem children at all. Recommend removing the menu/menuitem roles and relying on the native `<button>` names combined with the `aria-expanded`/`aria-controls` you already added (which is the simpler, correct pattern for a group of toggle options); or, if a true menu is wanted, implement the full pattern (aria-haspopup, roving tabindex, Arrow-key navigation, and menuitem roles on every item including the language pills).</violation>
<violation number="3" location="index.html:2721">
P3: The two toggle functions are now near-identical (~25 duplicated lines each): same isOpen computation, hidden-class toggle, aria-expanded/aria-hidden updates, chevron rotation, sibling-panel close, and focus-first-item block. This duplication makes the ARIA state logic drift-prone — the only differences are the element IDs. Consider extracting a single helper, e.g. `toggleDropdown(panelId, btnId, chevronId)` plus `closeDropdown(...)`, and have both triggers (and the click-outside/Escape handlers) call it, so the aria-expanded/aria-hidden/chevron/focus sync lives in one place.</violation>
<violation number="4" location="index.html:2721">
P2: The deferred `setTimeout(() => firstItem.focus(), 50)` call used to auto-focus the first item is never cancelled. If Escape is pressed or the panel is closed (or the other dropdown is opened) within that 50ms window, this pending timeout can still fire and move focus into a now-hidden panel, breaking the focus-restoration behavior added by the Escape handler. Focus synchronously, or store and clear the timer id in every close path.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| </button> | ||
| <div id="mobileFilterPanel" class="hidden absolute left-0 top-full mt-1 bg-white dark:bg-zinc-800 rounded-xl shadow-lg border border-zinc-100 dark:border-zinc-700 z-30 p-2 flex flex-col gap-1" style="min-width:180px"> | ||
| <button id="mob-chip-veteran" data-mobile-chip="chip-veteran" onclick="toggleMobileChip(this)" class="mobile-filter-option flex items-center gap-2 px-3 py-2 rounded-lg text-xs font-medium hover:bg-surface-container-low transition-colors text-left"> | ||
| <div id="mobileFilterPanel" role="menu" aria-hidden="true" class="hidden absolute left-0 top-full mt-1 bg-white dark:bg-zinc-800 rounded-xl shadow-lg border border-zinc-100 dark:border-zinc-700 z-30 p-2 flex flex-col gap-1" style="min-width:180px"> |
There was a problem hiding this comment.
P2: The new role="menu" / role="menuitem" markup doesn't match a real ARIA menu: per the WAI-ARIA menu pattern, a menu requires menuitem children, only one tab stop, and Arrow-key navigation — but these filter items are still plain buttons reached via Tab, with no arrow-key handling or aria-haspopup on the trigger. The result is that screen readers announce "menu/menuitem" semantics that don't match how the control actually behaves, which undercuts the accessibility goal of this PR. Additionally, the language panel has role="menu" while its child language pills were left as plain <button>s (no menuitem role), so that menu has no valid menuitem children at all. Recommend removing the menu/menuitem roles and relying on the native <button> names combined with the aria-expanded/aria-controls you already added (which is the simpler, correct pattern for a group of toggle options); or, if a true menu is wanted, implement the full pattern (aria-haspopup, roving tabindex, Arrow-key navigation, and menuitem roles on every item including the language pills).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At index.html, line 1502:
<comment>The new `role="menu"` / `role="menuitem"` markup doesn't match a real ARIA menu: per the WAI-ARIA menu pattern, a menu requires menuitem children, only one tab stop, and Arrow-key navigation — but these filter items are still plain buttons reached via Tab, with no arrow-key handling or `aria-haspopup` on the trigger. The result is that screen readers announce "menu/menuitem" semantics that don't match how the control actually behaves, which undercuts the accessibility goal of this PR. Additionally, the language panel has `role="menu"` while its child language pills were left as plain `<button>`s (no menuitem role), so that menu has no valid menuitem children at all. Recommend removing the menu/menuitem roles and relying on the native `<button>` names combined with the `aria-expanded`/`aria-controls` you already added (which is the simpler, correct pattern for a group of toggle options); or, if a true menu is wanted, implement the full pattern (aria-haspopup, roving tabindex, Arrow-key navigation, and menuitem roles on every item including the language pills).</comment>
<file context>
@@ -1469,58 +1492,58 @@ <h2 class="text-4xl md:text-5xl font-extrabold font-headline tracking-tighter mt
</button>
- <div id="mobileFilterPanel" class="hidden absolute left-0 top-full mt-1 bg-white dark:bg-zinc-800 rounded-xl shadow-lg border border-zinc-100 dark:border-zinc-700 z-30 p-2 flex flex-col gap-1" style="min-width:180px">
- <button id="mob-chip-veteran" data-mobile-chip="chip-veteran" onclick="toggleMobileChip(this)" class="mobile-filter-option flex items-center gap-2 px-3 py-2 rounded-lg text-xs font-medium hover:bg-surface-container-low transition-colors text-left">
+ <div id="mobileFilterPanel" role="menu" aria-hidden="true" class="hidden absolute left-0 top-full mt-1 bg-white dark:bg-zinc-800 rounded-xl shadow-lg border border-zinc-100 dark:border-zinc-700 z-30 p-2 flex flex-col gap-1" style="min-width:180px">
+ <button id="mob-chip-veteran" data-mobile-chip="chip-veteran" onclick="toggleMobileChip(this)" role="menuitem" class="mobile-filter-option flex items-center gap-2 px-3 py-2 rounded-lg text-xs font-medium hover:bg-surface-container-low transition-colors text-left focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary">
<span class="material-symbols-outlined text-sm">military_tech</span> Veterans Only
</file context>
| // Focus first item when opening | ||
| if (!isOpen) { | ||
| const firstItem = panel.querySelector('button, [tabindex]:not([tabindex="-1"])'); | ||
| if (firstItem) setTimeout(() => firstItem.focus(), 50); |
There was a problem hiding this comment.
P2: The deferred setTimeout(() => firstItem.focus(), 50) call used to auto-focus the first item is never cancelled. If Escape is pressed or the panel is closed (or the other dropdown is opened) within that 50ms window, this pending timeout can still fire and move focus into a now-hidden panel, breaking the focus-restoration behavior added by the Escape handler. Focus synchronously, or store and clear the timer id in every close path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At index.html, line 2721:
<comment>The deferred `setTimeout(() => firstItem.focus(), 50)` call used to auto-focus the first item is never cancelled. If Escape is pressed or the panel is closed (or the other dropdown is opened) within that 50ms window, this pending timeout can still fire and move focus into a now-hidden panel, breaking the focus-restoration behavior added by the Escape handler. Focus synchronously, or store and clear the timer id in every close path.</comment>
<file context>
@@ -2674,43 +2697,102 @@ <h3 style="font-size:10px;font-weight:700;color:var(--muted);letter-spacing:.1em
+ // Focus first item when opening
+ if (!isOpen) {
+ const firstItem = panel.querySelector('button, [tabindex]:not([tabindex="-1"])');
+ if (firstItem) setTimeout(() => firstItem.focus(), 50);
+ }
};
</file context>
| outline-offset: 1px; | ||
| border-radius: 8px; | ||
| } | ||
| .clear-filters-btn:focus-visible { |
There was a problem hiding this comment.
P3: Clear Filters still has no visible focus indicator because this selector does not match its id="clearFiltersBtn". Target the existing ID (or add the class) so keyboard focus receives the intended style.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At index.html, line 1189:
<comment>Clear Filters still has no visible focus indicator because this selector does not match its `id="clearFiltersBtn"`. Target the existing ID (or add the class) so keyboard focus receives the intended style.</comment>
<file context>
@@ -1174,6 +1174,29 @@
+ outline-offset: 1px;
+ border-radius: 8px;
+}
+.clear-filters-btn:focus-visible {
+ outline: 2px solid var(--orange, #f46b0e);
+ outline-offset: 2px;
</file context>
| .clear-filters-btn:focus-visible { | |
| #clearFiltersBtn:focus-visible { |
| // Focus first item when opening | ||
| if (!isOpen) { | ||
| const firstItem = panel.querySelector('button, [tabindex]:not([tabindex="-1"])'); | ||
| if (firstItem) setTimeout(() => firstItem.focus(), 50); |
There was a problem hiding this comment.
P3: The two toggle functions are now near-identical (~25 duplicated lines each): same isOpen computation, hidden-class toggle, aria-expanded/aria-hidden updates, chevron rotation, sibling-panel close, and focus-first-item block. This duplication makes the ARIA state logic drift-prone — the only differences are the element IDs. Consider extracting a single helper, e.g. toggleDropdown(panelId, btnId, chevronId) plus closeDropdown(...), and have both triggers (and the click-outside/Escape handlers) call it, so the aria-expanded/aria-hidden/chevron/focus sync lives in one place.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At index.html, line 2721:
<comment>The two toggle functions are now near-identical (~25 duplicated lines each): same isOpen computation, hidden-class toggle, aria-expanded/aria-hidden updates, chevron rotation, sibling-panel close, and focus-first-item block. This duplication makes the ARIA state logic drift-prone — the only differences are the element IDs. Consider extracting a single helper, e.g. `toggleDropdown(panelId, btnId, chevronId)` plus `closeDropdown(...)`, and have both triggers (and the click-outside/Escape handlers) call it, so the aria-expanded/aria-hidden/chevron/focus sync lives in one place.</comment>
<file context>
@@ -2674,43 +2697,102 @@ <h3 style="font-size:10px;font-weight:700;color:var(--muted);letter-spacing:.1em
+ // Focus first item when opening
+ if (!isOpen) {
+ const firstItem = panel.querySelector('button, [tabindex]:not([tabindex="-1"])');
+ if (firstItem) setTimeout(() => firstItem.focus(), 50);
+ }
};
</file context>
S3DFX-CYBER
left a comment
S3DFX-CYBER
left a comment
There was a problem hiding this comment.
This adds keyboard navigation and ARIA attributes to the mobile filter and language dropdowns — Escape-to-close, focus management, aria-expanded/aria-controls on the trigger buttons, and focus-visible outlines on the chips and pills. The direction is solid and the intent is right, but there are a few things that need fixing before this can merge.
First, there's no linked issue in the PR body. The repo requires a Closes #N (or fix/resolve variant) — add one so the PR satisfies the hard rules and CI doesn't flag it.
The bigger problem is the ARIA semantics. The panels get role="menu" and the filter options get role="menuitem", but the actual interaction doesn't implement the WAI-ARIA menu pattern — there's no roving tabindex, no arrow-key navigation, and the trigger has no aria-haspopup="menu". Worse, the language panel gets role="menu" but its child language pills are left as plain <button> with no role="menuitem", so that menu has no valid menuitem children at all. Screen readers will announce "menu" semantics that don't match how the control actually behaves, which undercuts the accessibility goal. The simpler and correct approach for a group of toggle filters is to drop the menu/menuitem roles entirely and rely on the native <button> names plus the aria-expanded/aria-controls you already added (which is correct). If you want a true menu, you'd need to implement the full pattern — but that's overkill here.
The focus management has a race condition. Both toggle functions call setTimeout(() => firstItem.focus(), 50) to focus the first item after opening, but that timeout is never tracked or cancelled. If the user presses Escape or clicks outside within that 50ms window — or opens the other dropdown — the pending timeout can still fire and move focus into a now-hidden panel, breaking the focus restoration the Escape handler tries to do. Focus the first item synchronously after toggling hidden, or store the timer ID and clear it in every close path (Escape, click-outside, sibling-open).
Finally, the .clear-filters-btn:focus-visible rule at the top of the diff targets a class that doesn't exist on the element — the Clear Filters button uses id="clearFiltersBtn", not a .clear-filters-btn class. So that focus indicator will never apply. Target the existing ID instead.
One thing to verify: the toggleMobileFilterDropdown and toggleMobileLangDropdown functions live in the index.html inline script. If the same functions exist in src/js/app.js (the test mirror), they need to be updated there too — the repo's architecture requires the two stay in sync. If they don't exist in app.js, then this is fine as-is.
| </button> | ||
| <div id="mobileFilterPanel" class="hidden absolute left-0 top-full mt-1 bg-white dark:bg-zinc-800 rounded-xl shadow-lg border border-zinc-100 dark:border-zinc-700 z-30 p-2 flex flex-col gap-1" style="min-width:180px"> | ||
| <button id="mob-chip-veteran" data-mobile-chip="chip-veteran" onclick="toggleMobileChip(this)" class="mobile-filter-option flex items-center gap-2 px-3 py-2 rounded-lg text-xs font-medium hover:bg-surface-container-low transition-colors text-left"> | ||
| <div id="mobileFilterPanel" role="menu" aria-hidden="true" class="hidden absolute left-0 top-full mt-1 bg-white dark:bg-zinc-800 rounded-xl shadow-lg border border-zinc-100 dark:border-zinc-700 z-30 p-2 flex flex-col gap-1" style="min-width:180px"> |
There was a problem hiding this comment.
role="menu" / role="menuitem" without the full ARIA menu pattern (roving tabindex, arrow-key nav, aria-haspopup on trigger) creates a semantics/behavior mismatch. The language panel below has role="menu" but its child pills have no role="menuitem". Recommend dropping these roles and relying on the native button semantics + aria-expanded/aria-controls you already added.
| outline-offset: 1px; | ||
| border-radius: 8px; | ||
| } | ||
| .clear-filters-btn:focus-visible { |
There was a problem hiding this comment.
This selector targets .clear-filters-btn but the element uses id="clearFiltersBtn" — this focus style will never apply. Use #clearFiltersBtn:focus-visible instead.
| // Focus first item when opening | ||
| if (!isOpen) { | ||
| const firstItem = panel.querySelector('button, [tabindex]:not([tabindex="-1"])'); | ||
| if (firstItem) setTimeout(() => firstItem.focus(), 50); |
There was a problem hiding this comment.
This setTimeout is never cancelled. If Escape or click-outside fires within 50ms, the callback still runs and focuses an element inside a hidden panel. Store the timer ID and clear it in every close path, or focus synchronously after toggling hidden.
⏰ Stale PR WarningThis PR has been inactive for 7+ days with unresolved issues. If there is no activity, it will be automatically closed in 7 days. Please push updates or comment if you need more time. |
2 participants
Summary
Fixes accessibility issues with the organization filter controls:
Changes
HTML (index.html)
aria-expandedandaria-controlsattributesrole="menu"andaria-hiddenfor screen reader supportrole="menuitem"to each chip buttonaria-expandedandaria-controlsattributesrole="menu"andaria-hiddenfocus-visible:outline-*Tailwind classes to all mobile filter buttonsJavaScript (inline)
aria-expandedandaria-hiddenfor both filter and language panelsCSS (inline stylesheet)
.filter-chip:focus-visible,.pill:focus-visiblestyles with orange outline.mobile-filter-option:focus-visiblestyle for mobile dropdown items.clear-filters-btn:focus-visibleand#loadMoreBtn:focus-visiblestylesTesting
Closes #1969