🎨 Palette: Fix redundant tab stop and missing ARIA label on OAuth Docs button - #143
MillionthOdin16 wants to merge 1 commit into
Conversation
Added `tabIndex={-1}` and an `aria-label` to the docs button component within the OAuth providers card to fix redundant tab stops and provide accessibility labels for screen readers.
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 documentation icon button now sets ChangesOAuth provider accessibility
Estimated code review effort: 1 (Trivial) | ~2 minutes 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.
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`:
- Line 225: Update the OAuthProvidersCard link/button markup around the Button
rendering so interactive elements are not nested. Move the accessible label onto
the anchor and render the documentation icon non-interactively, or use Button’s
asChild mode so the anchor receives the button styling while remaining the sole
interactive 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: 820b3afb-1b1d-4d00-be56-8b406148e102
📒 Files selected for processing (1)
web/src/components/OAuthProvidersCard.tsx
| title={`Open ${p.name} docs`} | ||
| > | ||
| <Button ghost size="icon"> | ||
| <Button ghost size="icon" tabIndex={-1} aria-label={`Open ${p.name} docs`}> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant component and find the shared Button implementation.
git ls-files 'web/src/components/OAuthProvidersCard.tsx' 'web/src/components/**' 'web/src/**' | sed -n '1,120p'
echo
rg -n --glob 'web/src/**' --glob '!**/node_modules/**' 'export .*Button|function Button|const Button|forwardRef\\(.*Button|asChild|tabIndex=\\{-1\\}|aria-label=\\{`Open .* docs`\\}' web/src
echo
# Read the relevant file around the reported line.
wc -l web/src/components/OAuthProvidersCard.tsx
sed -n '200,245p' web/src/components/OAuthProvidersCard.tsx
echo
# Try to locate the shared Button source.
fd -a 'Button.tsx' web/src
fd -a 'button.tsx' web/src
fd -a 'Button.*' web/srcRepository: MillionthOdin16/hermes-agent
Length of output: 3913
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find likely Button implementations and show their definitions.
for f in $(fd -a 'button.tsx' web/src 2>/dev/null; fd -a 'Button.tsx' web/src 2>/dev/null; fd -a 'Button.ts' web/src 2>/dev/null); do
echo "### $f"
wc -l "$f"
sed -n '1,220p' "$f"
echo
done
echo "### OAuthProvidersCard.tsx excerpt"
sed -n '205,235p' web/src/components/OAuthProvidersCard.tsxRepository: MillionthOdin16/hermes-agent
Length of output: 1369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the Button component definition and inspect it.
fd -a 'Button.tsx' web src . 2>/dev/null || true
fd -a 'button.tsx' web src . 2>/dev/null || true
fd -a 'button.ts' web src . 2>/dev/null || true
fd -a 'Button.*' web src . 2>/dev/null || true
echo
# Search for likely definitions without regex mistakes.
rg -n --glob 'web/src/**' 'asChild|forwardRef|export (const|function) Button|function Button|const Button =|className=.*button' web/src
echo
# If a candidate file is found, print it with line numbers.
for f in $(fd -a 'Button.tsx' web/src 2>/dev/null; fd -a 'button.tsx' web/src 2>/dev/null; fd -a 'button.ts' web/src 2>/dev/null); do
echo "### $f"
wc -l "$f"
sed -n '1,220p' "$f"
echo
doneRepository: MillionthOdin16/hermes-agent
Length of output: 1898
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the imports and top-of-file for OAuthProvidersCard.tsx.
sed -n '1,80p' web/src/components/OAuthProvidersCard.tsx
echo
# Find the Button import across the web source tree.
rg -n --glob 'web/src/**' 'from .*Button|from .*button|import .*Button' web/src/components web/src
echo
# If the import resolves to a local file, print that file's contents.
# First, list candidate button files in web/src.
fd -a 'button.tsx' web/src 2>/dev/null || true
fd -a 'Button.tsx' web/src 2>/dev/null || true
fd -a 'button.ts' web/src 2>/dev/null || true
fd -a 'Button.ts' web/src 2>/dev/null || trueRepository: MillionthOdin16/hermes-agent
Length of output: 8045
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the shared button component that matches the import path.
fd -a 'button.tsx' . 2>/dev/null | rg '/ui/components/button\.tsx$|/components/button\.tsx$|/ui/components/.*/button\.tsx$' || true
fd -a 'button.ts' . 2>/dev/null | rg '/ui/components/button\.ts$|/components/button\.ts$|/ui/components/.*/button\.ts$' || true
fd -a 'button.*' . 2>/dev/null | rg '/ui/components/' || true
echo
# Read likely matches if found.
for f in $(fd -a 'button.tsx' . 2>/dev/null | rg '/ui/components/button\.tsx$|/components/button\.tsx$|/ui/components/.*/button\.tsx$'); do
echo "### $f"
wc -l "$f"
sed -n '1,220p' "$f"
echo
done
for f in $(fd -a 'button.ts' . 2>/dev/null | rg '/ui/components/button\.ts$|/components/button\.ts$|/ui/components/.*/button\.ts$'); do
echo "### $f"
wc -l "$f"
sed -n '1,220p' "$f"
echo
doneRepository: MillionthOdin16/hermes-agent
Length of output: 3888
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the exact file behind the Button import and inspect it.
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' '`@nous-research/ui/ui/components/button`' .
echo
fd -a 'button.tsx' . 2>/dev/null | head -200
echo
fd -a 'button.ts' . 2>/dev/null | head -200Repository: MillionthOdin16/hermes-agent
Length of output: 4979
Avoid nesting the icon button inside the link. Button renders a native <button> here, so this creates invalid interactive nesting and leaves the link without the accessible name. Put the label on the <a> and render the icon non-interactively, or switch to asChild so the anchor takes the button styling.
🤖 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` at line 225, Update the
OAuthProvidersCard link/button markup around the Button rendering so interactive
elements are not nested. Move the accessible label onto the anchor and render
the documentation icon non-interactively, or use Button’s asChild mode so the
anchor receives the button styling while remaining the sole interactive element.
There was a problem hiding this comment.
Code Review
This pull request adds tabIndex and an aria-label to a Button component nested inside an anchor tag in OAuthProvidersCard.tsx. The reviewer pointed out that nesting a button inside an anchor tag is invalid HTML because interactive content should not be nested, and suggested refactoring the components to avoid this.
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" tabIndex={-1} aria-label={`Open ${p.name} docs`}> |
There was a problem hiding this comment.
Nesting a <Button> (which renders a <button> element) inside an <a> anchor tag is invalid HTML. According to the HTML specification, interactive content must not be nested within other interactive content. This can cause unexpected behavior with screen readers and keyboard navigation.
To fix this while keeping the button styling, you can swap the elements and use the asChild prop on the Button component (assuming it is supported by your UI library) so that the Button styles are applied directly to the <a> tag:
<Button ghost size=
💡 What: Added
tabIndex={-1}andaria-labelto the external link button inside the OAuth providers card.🎯 Why: The parent
<a>tag already provides context and acts as the primary interactive element. The nested<Button>component created a redundant tab stop and lacked anaria-label, leading to accessibility issues.📸 Before/After: Visually identical; accessibility improvement only.
♿ Accessibility: Prevents redundant tab stops and provides explicit labeling for screen readers.
PR created automatically by Jules for task 6788417255366973737 started by @MillionthOdin16
Summary by CodeRabbit