feat: add accessibility showcase page #751
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughA new Accessibility page is introduced to the application, demonstrating best practices through interactive examples. The page includes landmarks and skip navigation, keyboard-friendly menu navigation, an accessible feedback form with inline validation, and an expandable FAQ section. Changes encompass the new component and stylesheet, routing integration, and navigation updates on the home page. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
use_cases/src/pages/AccessibilityPage.tsx (1)
239-325: Well-implemented accessible form with one minor redundancy.The form demonstrates excellent accessibility practices with proper labeling, validation, and error handling.
However, line 316 contains a redundancy:
<div className={styles.successMessage} role="status" aria-live="polite">The
aria-live="polite"attribute is redundant becauserole="status"already impliesaria-live="polite". While not harmful, it can be removed:- <div className={styles.successMessage} role="status" aria-live="polite"> + <div className={styles.successMessage} role="status">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
use_cases/src/App.tsx(3 hunks)use_cases/src/pages/AccessibilityPage.module.css(1 hunks)use_cases/src/pages/AccessibilityPage.tsx(1 hunks)use_cases/src/pages/HomePage.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-22T15:33:18.528Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf_apps/vue-cupertino-gallery/CLAUDE.md:0-0
Timestamp: 2025-09-22T15:33:18.528Z
Learning: Applies to webf_apps/vue-cupertino-gallery/src/**/*.vue : Use the custom RouterView component for page routing
Applied to files:
use_cases/src/App.tsx
🧬 Code graph analysis (2)
use_cases/src/pages/AccessibilityPage.tsx (1)
packages/react-core-ui/src/lib/src/html/listview.tsx (1)
WebFListView(168-202)
use_cases/src/App.tsx (1)
use_cases/src/pages/AccessibilityPage.tsx (1)
AccessibilityPage(25-371)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (c-cpp)
🔇 Additional comments (9)
use_cases/src/pages/HomePage.tsx (1)
102-114: LGTM! Consistent integration of the new Accessibility section.The new section follows the established pattern used throughout the HomePage, maintaining consistency in structure, styling, and navigation behavior.
use_cases/src/App.tsx (2)
39-39: LGTM! Import added correctly.The import follows the established pattern and integrates the new AccessibilityPage component into the application routing.
89-89: LGTM! Route registered correctly.The route registration follows the same pattern as other routes in the application, properly wiring the AccessibilityPage component to the
/accessibilitypath.use_cases/src/pages/AccessibilityPage.module.css (1)
1-367: LGTM! Well-structured CSS module with excellent focus management.The styling demonstrates strong accessibility practices:
- Skip link pattern correctly positions the link off-screen and reveals it on focus
- All interactive elements have clear focus-visible indicators using box-shadows and border colors
- Responsive design adapts the menu layout at appropriate breakpoints
- Color variables enable theme customization while maintaining readability
use_cases/src/pages/AccessibilityPage.tsx (5)
1-42: LGTM! Clean component setup with proper state and effect management.The component initialization is well-structured with clear type definitions, appropriate state management, and correct useEffect dependency tracking for focus management.
44-94: LGTM! Robust keyboard navigation implementation.The menu keyboard handling correctly implements the roving tabindex pattern with proper:
- Wrap-around navigation using modulo arithmetic
- Home/End key support for quick jumps
- Event prevention to avoid scrolling
- Live announcements for each navigation action
96-136: LGTM! Comprehensive form validation and accessible state management.The form validation provides clear, user-friendly error messages with proper announcements, and the FAQ toggle correctly manages state changes with appropriate live region updates.
138-195: LGTM! Excellent semantic structure with proper landmarks.The page structure demonstrates best practices:
- Skip link correctly targets the main content area
- Proper use of semantic HTML with complementary ARIA landmarks
- Internal navigation links correctly reference section IDs
- Clear role attributes and aria-labels for assistive technology
327-370: LGTM! Proper implementation of disclosure pattern and live announcements.The FAQ toggle correctly uses aria-expanded and aria-controls attributes, and the live region at the bottom provides appropriate status updates with proper ARIA attributes for announcements.
| <div className={styles.menuContainer}> | ||
| <div className={styles.menu} role="menubar" aria-label="Accessibility resources"> | ||
| {MENU_ITEMS.map((item, index) => ( | ||
| <button | ||
| key={item.label} | ||
| type="button" | ||
| role="menuitem" | ||
| tabIndex={focusIndex === index ? 0 : -1} | ||
| className={`${styles.menuButton} ${focusIndex === index ? styles.menuButtonActive : ''}`} | ||
| ref={(element) => { | ||
| menuRefs.current[index] = element; | ||
| }} | ||
| aria-describedby={`menu-item-desc-${index}`} | ||
| onClick={() => handleMenuSelect(item, index)} | ||
| onKeyDown={(event) => handleMenuKeyDown(event, index)} | ||
| > | ||
| {item.label} | ||
| </button> | ||
| ))} | ||
| </div> | ||
| <div className={styles.menuSummary} aria-live="polite"> | ||
| <h3 className={styles.menuSummaryTitle}>{activeMenu.label}</h3> | ||
| <p id={`menu-item-desc-${focusIndex}`} className={styles.menuSummaryBody}> | ||
| {activeMenu.description} | ||
| </p> | ||
| <p className={styles.menuHint}> | ||
| Tip: Use Arrow keys to move between items, Enter or Space to activate, Home and End to jump to the | ||
| first or last item. | ||
| </p> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Fix broken ARIA references and consider removing redundant live region.
The aria-describedby implementation has a critical issue:
Each button references aria-describedby="menu-item-desc-{index}" (line 218), but only one description element exists at any time with id="menu-item-desc-{focusIndex}" (line 228). This creates broken ARIA references for all non-focused buttons.
Recommended fix: Remove the aria-describedby attribute from the buttons since the active description is already announced via the separate live region mechanism:
<button
key={item.label}
type="button"
role="menuitem"
tabIndex={focusIndex === index ? 0 : -1}
className={`${styles.menuButton} ${focusIndex === index ? styles.menuButtonActive : ''}`}
ref={(element) => {
menuRefs.current[index] = element;
}}
- aria-describedby={`menu-item-desc-${index}`}
onClick={() => handleMenuSelect(item, index)}
onKeyDown={(event) => handleMenuKeyDown(event, index)}
>
{item.label}
</button>Additionally, the aria-live="polite" on the menuSummary (line 226) may cause redundant announcements since the announce() function already updates a separate live region. Consider removing it:
- <div className={styles.menuSummary} aria-live="polite">
+ <div className={styles.menuSummary}>📝 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.
| <div className={styles.menuContainer}> | |
| <div className={styles.menu} role="menubar" aria-label="Accessibility resources"> | |
| {MENU_ITEMS.map((item, index) => ( | |
| <button | |
| key={item.label} | |
| type="button" | |
| role="menuitem" | |
| tabIndex={focusIndex === index ? 0 : -1} | |
| className={`${styles.menuButton} ${focusIndex === index ? styles.menuButtonActive : ''}`} | |
| ref={(element) => { | |
| menuRefs.current[index] = element; | |
| }} | |
| aria-describedby={`menu-item-desc-${index}`} | |
| onClick={() => handleMenuSelect(item, index)} | |
| onKeyDown={(event) => handleMenuKeyDown(event, index)} | |
| > | |
| {item.label} | |
| </button> | |
| ))} | |
| </div> | |
| <div className={styles.menuSummary} aria-live="polite"> | |
| <h3 className={styles.menuSummaryTitle}>{activeMenu.label}</h3> | |
| <p id={`menu-item-desc-${focusIndex}`} className={styles.menuSummaryBody}> | |
| {activeMenu.description} | |
| </p> | |
| <p className={styles.menuHint}> | |
| Tip: Use Arrow keys to move between items, Enter or Space to activate, Home and End to jump to the | |
| first or last item. | |
| </p> | |
| </div> | |
| </div> | |
| <div className={styles.menuContainer}> | |
| <div className={styles.menu} role="menubar" aria-label="Accessibility resources"> | |
| {MENU_ITEMS.map((item, index) => ( | |
| <button | |
| key={item.label} | |
| type="button" | |
| role="menuitem" | |
| tabIndex={focusIndex === index ? 0 : -1} | |
| className={`${styles.menuButton} ${focusIndex === index ? styles.menuButtonActive : ''}`} | |
| ref={(element) => { | |
| menuRefs.current[index] = element; | |
| }} | |
| onClick={() => handleMenuSelect(item, index)} | |
| onKeyDown={(event) => handleMenuKeyDown(event, index)} | |
| > | |
| {item.label} | |
| </button> | |
| ))} | |
| </div> | |
| <div className={styles.menuSummary}> | |
| <h3 className={styles.menuSummaryTitle}>{activeMenu.label}</h3> | |
| <p id={`menu-item-desc-${focusIndex}`} className={styles.menuSummaryBody}> | |
| {activeMenu.description} | |
| </p> | |
| <p className={styles.menuHint}> | |
| Tip: Use Arrow keys to move between items, Enter or Space to activate, Home and End to jump to the | |
| first or last item. | |
| </p> | |
| </div> | |
| </div> |
🤖 Prompt for AI Agents
In use_cases/src/pages/AccessibilityPage.tsx around lines 206 to 236, the
buttons currently set aria-describedby={`menu-item-desc-${index}`} while only
one description element exists with id={`menu-item-desc-${focusIndex}`} which
breaks ARIA references for non‑focused buttons; remove the aria-describedby
attribute from the buttons and also remove the aria-live="polite" from the
menuSummary div (since announcements are handled by your separate live region)
so the description is not referenced incorrectly and you avoid redundant live
announcements.
Summary
Testing
Summary by CodeRabbit