🎨 Palette: Add ARIA label and fix tabIndex for nested docs button - #157
MillionthOdin16 wants to merge 1 commit into
Conversation
Added `aria-label` and `tabIndex={-1}` to the icon-only `<Button>` nested inside the `<a>` wrapper link for OAuth provider docs in `web/src/components/OAuthProvidersCard.tsx`.
Fixes accessibility issues by ensuring the inner button is correctly identified by screen readers and removed from the tab sequence to prevent redundant tab stops.
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThe OAuth provider docs icon button now has an explicit accessible label and is excluded from keyboard tab navigation. ChangesOAuth docs accessibility
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 updates the OAuthProvidersCard component to add an aria-label and tabIndex={-1} to a Button nested inside an anchor tag. The review feedback correctly points out that nesting a button inside an anchor tag is invalid HTML and can cause accessibility issues. The reviewer suggests a cleaner, semantically correct approach using the asChild prop on the Button component to render the button style directly on the anchor tag.
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.
| title={`Open ${p.name} docs`} | ||
| > | ||
| <Button ghost size="icon"> | ||
| <Button ghost size="icon" aria-label={`Open ${p.name} docs`} tabIndex={-1}> |
There was a problem hiding this comment.
Nesting an interactive element like <Button> (which renders a <button> element) inside another interactive element like <a> is invalid HTML according to the HTML5 specification. This nesting can confuse screen readers and disrupt keyboard navigation.
Instead of nesting them and using tabIndex={-1} to bypass the double-tabbing issue, a cleaner and semantically correct approach is to use the asChild prop on the Button component (assuming it is a standard Radix/shadcn-based button) to render the button style directly on the anchor tag.
Here is how you can refactor the block:
<Button ghost size="icon" asChild>
<a
href={p.docs_url}
target="_blank"
rel="noopener noreferrer"
className="inline-flex"
title={`Open ${p.name} docs`}
aria-label={`Open ${p.name} docs`}
>
<ExternalLink />
</a>
</Button>There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@web/src/components/OAuthProvidersCard.tsx`:
- Around line 225-227: Update the docs-link markup in OAuthProvidersCard so the
anchor is the sole interactive element: move the accessible label and
interaction-related attributes onto the link, and render ExternalLink directly
without the nested Button component.
🪄 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: e8742aa5-7023-4dfe-b6fa-0995f6f9302b
📒 Files selected for processing (1)
web/src/components/OAuthProvidersCard.tsx
| <Button ghost size="icon" aria-label={`Open ${p.name} docs`} tabIndex={-1}> | ||
| <ExternalLink /> | ||
| </Button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant component and the Button implementation it uses.
git ls-files web/src/components/OAuthProvidersCard.tsx web/src | sed -n '1,120p'
printf '\n--- OAuthProvidersCard excerpt ---\n'
sed -n '180,250p' web/src/components/OAuthProvidersCard.tsx
printf '\n--- ui Button candidates ---\n'
fd -a 'Button.*' web src . 2>/dev/null | sed -n '1,40p'
rg -n "export .*Button|const Button|function Button|forwardRef.*Button|asChild" web src -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'Repository: MillionthOdin16/hermes-agent
Length of output: 6865
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the actual Button component file if the first search was too broad.
fd -a 'button.*' web src . 2>/dev/null | sed -n '1,80p'
rg -n "aria-label=.*Open .*docs|ExternalLink|tabIndex=\{-1\}" web/src/components/OAuthProvidersCard.tsxRepository: MillionthOdin16/hermes-agent
Length of output: 1656
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the button implementation used by the web app and nearby composition patterns.
printf '%s\n' '--- apps/desktop/src/components/ui/button.tsx ---'
sed -n '1,240p' apps/desktop/src/components/ui/button.tsx
printf '\n%s\n' '--- search for asChild / polymorphic button usage ---'
rg -n "asChild|Slot|<Button[^>]*asChild|button.tsx" apps/desktop/src/components -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'
printf '\n%s\n' '--- OAuthProvidersCard surrounding lines ---'
sed -n '210,235p' web/src/components/OAuthProvidersCard.tsxRepository: MillionthOdin16/hermes-agent
Length of output: 11922
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Identify the Button import and its implementation in the web app.
sed -n '1,80p' web/src/components/OAuthProvidersCard.tsx
printf '\n--- Button definitions under web/src ---\n'
fd -a 'button.tsx' web/src web 2>/dev/null | sed -n '1,40p'
rg -n "export .*Button|function Button|const Button|asChild" web/src -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'Repository: MillionthOdin16/hermes-agent
Length of output: 2786
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,80p' web/src/components/OAuthProvidersCard.tsx
printf '\n--- Button definitions under web/src ---\n'
fd -a 'button.tsx' web/src web 2>/dev/null | sed -n '1,40p'
rg -n "export .*Button|function Button|const Button|asChild" web/src -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'Repository: MillionthOdin16/hermes-agent
Length of output: 2786
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the exact component behind the imported path.
rg -n "ui/components/button" -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' .
fd -a 'button.tsx' . 2>/dev/null | sed -n '1,80p'Repository: MillionthOdin16/hermes-agent
Length of output: 4267
Use a single interactive element for the docs link.
web/src/components/OAuthProvidersCard.tsx:225-227 nests a button inside an a; move the label/interaction to the link itself and render the icon without a nested control.
🤖 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 `@web/src/components/OAuthProvidersCard.tsx` around lines 225 - 227, Update the
docs-link markup in OAuthProvidersCard so the anchor is the sole interactive
element: move the accessible label and interaction-related attributes onto the
link, and render ExternalLink directly without the nested Button component.
aria-labelandtabIndex={-1}to the icon-only<Button>nested inside the<a>wrapper link for OAuth provider docs.<Button>element had noaria-labelortabIndexattributes. Now it has both, improving screen reader compatibility and keyboard navigation.PR created automatically by Jules for task 1020161125202482881 started by @MillionthOdin16
Summary by CodeRabbit