Skip to content

feat: add ARIA attributes to plugin components#130

Open
quemsah wants to merge 3 commits into
mainfrom
feature/aria-attributes
Open

feat: add ARIA attributes to plugin components#130
quemsah wants to merge 3 commits into
mainfrom
feature/aria-attributes

Conversation

@quemsah

@quemsah quemsah commented Jun 14, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add ARIA attributes to PluginCard and PluginList components for improved accessibility
  • Use semantic HTML elements (<section> instead of <div>)

Changes

  • PluginCard.tsx: Wrap content in <article> with aria-label
  • PluginList.tsx: Replace div with section, add aria-labelledby and heading id
  • PluginHeader.tsx: Fix version span placement inside h3

Summary by CodeRabbit

  • Improvements
    • Improved accessibility for plugin information by using more semantic HTML and adding appropriate ARIA labeling.
    • Updated plugin list layout semantics and connected headings to their containers for clearer screen reader navigation.

quemsah added 2 commits June 14, 2026 23:14
- 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
@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7300773d-6414-4e50-96a1-9fb8e29752a5

📥 Commits

Reviewing files that changed from the base of the PR and between 877856f and 842b2f8.

📒 Files selected for processing (1)
  • ui/src/components/repo/PluginList.tsx

📝 Walkthrough

Walkthrough

Two UI components receive ARIA landmark improvements. PluginCard wraps its content block in an article element with an aria-label from plugin.name. PluginList replaces its outer div with a section element, adds a generated id to its h6 heading, and links them via aria-labelledby.

Changes

ARIA Landmark Improvements

Layer / File(s) Summary
PluginCard article wrapper with aria-label
ui/src/components/repo/PluginCard.tsx
PluginCard wraps its description, metadata, and lists in an article element with aria-label={plugin.name ?? "Unnamed Plugin"} for semantic document structure.
PluginList section container with aria-labelledby and heading id
ui/src/components/repo/PluginList.tsx
PluginList replaces its outer div with section, introduces a slugify(title) helper and useId() hook to generate a stable per-instance headingId, assigns that id to the h6 element, and links the section via aria-labelledby.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐇 Hop, hop through the DOM I go,
Wrapping cards in article rows,
Sections labeled, headings too,
Screen readers now know what is who.
Accessibility in every burrow! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: add ARIA attributes to plugin components' clearly and concisely summarizes the main changes—adding ARIA attributes and semantic HTML to plugin UI components for accessibility improvements.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/aria-attributes

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

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.

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.

Comment thread ui/src/components/repo/PluginList.tsx Outdated
Comment on lines 12 to 14
<section aria-labelledby={`${title}-heading`} className="plugin-list">
<h6 className="mb-0.5 font-medium text-sm" id={`${title}-heading`}>
{title} ({items.length})

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.

high

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.

Suggested change
<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})

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
ui/src/components/repo/PluginList.tsx (1)

12-13: ⚡ Quick win

Sanitize title for ID generation.

The ID ${title}-heading can 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

📥 Commits

Reviewing files that changed from the base of the PR and between 11833c7 and 877856f.

📒 Files selected for processing (2)
  • ui/src/components/repo/PluginCard.tsx
  • ui/src/components/repo/PluginList.tsx

Comment thread ui/src/components/repo/PluginList.tsx Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant