feat: add ARIA attributes to plugin components#130
Conversation
- Wrap plugin content in article element with aria-label - Add aria-labelledby for PluginList sections - Add role='list' and role='listitem' to plugin lists - Fix PluginHeader structure for better semantics
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughTwo UI components receive ARIA landmark improvements. ChangesARIA Landmark Improvements
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request improves accessibility in the UI by wrapping plugin details in an <article> element with an aria-label in PluginCard.tsx, and by converting the container in PluginList.tsx to a <section> with an aria-labelledby attribute linked to the section's heading. A review comment correctly points out that the title prop in PluginList.tsx can contain spaces, which would result in an invalid HTML id attribute. It suggests sanitizing the title to replace spaces with hyphens to ensure valid IDs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| <section aria-labelledby={`${title}-heading`} className="plugin-list"> | ||
| <h6 className="mb-0.5 font-medium text-sm" id={`${title}-heading`}> | ||
| {title} ({items.length}) |
There was a problem hiding this comment.
The title prop can contain spaces (e.g., 'MCP Servers'), which results in an invalid HTML id attribute (e.g., id="MCP Servers-heading"). HTML id attributes must not contain spaces, and aria-labelledby will fail to associate correctly because it treats spaces as delimiters for multiple IDs.
We should sanitize the title to replace spaces with hyphens to ensure a valid ID.
| <section aria-labelledby={`${title}-heading`} className="plugin-list"> | |
| <h6 className="mb-0.5 font-medium text-sm" id={`${title}-heading`}> | |
| {title} ({items.length}) | |
| <section aria-labelledby={`${title.toLowerCase().replace(/\s+/g, '-')}-heading`} className="plugin-list"> | |
| <h6 className="mb-0.5 font-medium text-sm" id={`${title.toLowerCase().replace(/\s+/g, '-')}-heading`}> | |
| {title} ({items.length}) |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ui/src/components/repo/PluginList.tsx (1)
12-13: ⚡ Quick winSanitize title for ID generation.
The ID
${title}-headingcan contain spaces (e.g., "MCP Servers-heading"). While valid in HTML5, spaces in IDs can cause issues with CSS selectors and JavaScript queries. Consider sanitizing the title by replacing spaces and special characters.♻️ Proposed sanitization approach
export function PluginList({ title, items, repoPath, defaultBranch, pluginId }: PluginListProps) { if (!items?.length) return null - const headingId = pluginId ? `${pluginId}-${title}-heading` : `${title}-heading` + const sanitizedTitle = title.toLowerCase().replace(/\s+/g, '-') + const headingId = pluginId ? `${pluginId}-${sanitizedTitle}-heading` : `${sanitizedTitle}-heading` return (This transforms "MCP Servers" → "mcp-servers", making selectors more reliable.
🤖 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 `@ui/src/components/repo/PluginList.tsx` around lines 12 - 13, The IDs generated from the title variable in the section and h6 elements (aria-labelledby and id attributes both use ${title}-heading) may contain spaces and special characters, which can cause issues with CSS selectors and JavaScript queries. Create a sanitization function that converts the title to lowercase and replaces spaces and special characters with hyphens (for example, "MCP Servers" becomes "mcp-servers"), then apply this sanitized version when constructing both the aria-labelledby attribute on the section element and the id attribute on the h6 element.
🤖 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 `@ui/src/components/repo/PluginList.tsx`:
- Around line 12-13: The PluginList component creates duplicate IDs across
multiple plugin instances because it hardcodes the title in the aria-labelledby
and id attributes (e.g., `Commands-heading`, `Agents-heading`, `MCP
Servers-heading`). This violates HTML specifications and breaks accessibility.
Modify the PluginList component to accept a unique identifier parameter (such as
pluginId or a unique index) and incorporate it into the ID generation. Change
the id template from using just the title to including the unique identifier,
for example by prepending or appending the plugin ID. Then update the PluginCard
component that renders PluginList to pass the unique plugin identifier to each
PluginList instance so that each heading gets a distinct ID.
---
Nitpick comments:
In `@ui/src/components/repo/PluginList.tsx`:
- Around line 12-13: The IDs generated from the title variable in the section
and h6 elements (aria-labelledby and id attributes both use ${title}-heading)
may contain spaces and special characters, which can cause issues with CSS
selectors and JavaScript queries. Create a sanitization function that converts
the title to lowercase and replaces spaces and special characters with hyphens
(for example, "MCP Servers" becomes "mcp-servers"), then apply this sanitized
version when constructing both the aria-labelledby attribute on the section
element and the id attribute on the h6 element.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5a82f48a-6770-4e60-828a-e7f15d6d822e
📒 Files selected for processing (2)
ui/src/components/repo/PluginCard.tsxui/src/components/repo/PluginList.tsx
Summary
<section>instead of<div>)Changes
<article>with aria-labelSummary by CodeRabbit