🎨 Palette: Add ARIA label to external docs link button - #166
🎨 Palette: Add ARIA label to external docs link button#166MillionthOdin16 wants to merge 1 commit into
Conversation
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. |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughThe OAuth provider documentation icon button now has an explicit provider-specific accessible label and ChangesOAuth provider 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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
🤖 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-230: Update the docs-link rendering around the Button and its
enclosing anchor so the anchor carries the `aria-label` and is the only
interactive element. Remove the nested icon button and its `tabIndex={-1}`,
while preserving the existing icon appearance and link destination.
🪄 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: 5d6bdb36-52e1-47a9-850c-971f74f9323a
📒 Files selected for processing (1)
web/src/components/OAuthProvidersCard.tsx
| <Button | ||
| ghost | ||
| size="icon" | ||
| aria-label={`Open ${p.name} docs`} | ||
| tabIndex={-1} | ||
| > |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file structure first
ast-grep outline web/src/components/OAuthProvidersCard.tsx --view expanded || true
# Inspect the changed area with line numbers
sed -n '200,245p' web/src/components/OAuthProvidersCard.tsx
# Find the Button component implementation and any asChild/link composition support
fd -a 'Button.*' web src . 2>/dev/null | head -n 50
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 'asChild|Slot|forwardRef|button' web/src -g '*Button*' -g '*.tsx' -g '*.ts'
# Search for usages of Button wrapped in anchors or link composition patterns
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' '<a[^>]*>\s*<Button|<Button[^>]*asChild|aria-label=.*docs' web/srcRepository: MillionthOdin16/hermes-agent
Length of output: 24472
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the UI button implementation that web/src imports
fd -a 'button.tsx' .
fd -a 'Button.tsx' .
fd -a 'button.ts' .
# Read likely candidates from the UI package if present
for f in $(fd -a 'button.tsx' . | head -n 20); do
echo "### $f"
sed -n '1,260p' "$f"
done
for f in $(fd -a 'Button.tsx' . | head -n 20); do
echo "### $f"
sed -n '1,260p' "$f"
doneRepository: MillionthOdin16/hermes-agent
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find how the web app resolves the `@nous-research/ui` import
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' '`@nous-research/ui/ui/components/button`|`@nous-research/ui`' web packages apps -g 'tsconfig*.json' -g 'vite.config.*' -g 'webpack.config.*' -g 'package.json' -g '*.ts' -g '*.tsx'
# Inspect the web app package/workspace config for path aliases or workspace links
sed -n '1,240p' web/package.json 2>/dev/null || true
sed -n '1,260p' package.json 2>/dev/null || true
sed -n '1,260p' tsconfig.json 2>/dev/null || true
sed -n '1,260p' web/tsconfig.json 2>/dev/null || true
sed -n '1,260p' web/vite.config.* 2>/dev/null || true
# Search for a matching button implementation under any ui package path
fd -a 'button.tsx' web apps packages ui . 2>/dev/null | sed -n '1,80p'Repository: MillionthOdin16/hermes-agent
Length of output: 26909
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "### web/package.json"
sed -n '1,220p' web/package.json
echo "### web/vite.config.ts"
sed -n '1,220p' web/vite.config.ts
echo "### root package.json"
sed -n '1,260p' package.json
echo "### lockfile hints"
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' '"`@nous-research/ui`"|`@nous-research/ui`' package-lock.json pnpm-lock.yaml yarn.lock bun.lockb 2>/dev/null || trueRepository: MillionthOdin16/hermes-agent
Length of output: 7533
🌐 Web query:
@nour-research/ui 0.18.2 button asChild Slot.Root GitHub
💡 Result:
The use of Slot.Root in component patterns like Button with asChild is a common best practice in React applications, particularly when using Radix UI primitives, to ensure compatibility across different build environments and package resolutions [1][2]. In the context of a component like a Button, the asChild prop allows the component to merge its internal functionality (props, refs, event handlers) onto its child element instead of rendering a native HTML tag [1][3]. The Radix UI Slot component is used to perform this composition [1][3]. Using Slot.Root is specifically recommended when importing Slot from an umbrella package (like 'radix-ui') [2]. In some environments, simply importing Slot may result in an object namespace rather than the renderable component itself, leading to runtime errors like Element type is invalid [2]. Accessing Slot.Root explicitly ensures that the correct, renderable component is used [2]. While there is no specific package named @nour-research/ui, the pattern you are inquiring about is identical to the standard implementation found in widespread UI libraries like shadcn/ui [4][5] and various projects using the Radix Slot utility [1][6]. Implementation Pattern: import { Slot } from "radix-ui"; function Button({ asChild,...props }) { const Comp = asChild? Slot.Root: "button"; return <Comp {...props} />; }
Citations:
- 1: https://www.radix-ui.com/primitives/docs/utilities/slot
- 2: Button generated by CLI can fail at runtime with `Element type is invalid` when using `asChild` with umbrella Radix imports shadcn-ui/ui#10405
- 3: https://zenn.dev/tsuboi/articles/8abddb1ae3038f?locale=en
- 4: https://github.com/shadcn-ui/ui/blob/15ac1be9/apps/v4/registry/new-york-v4/ui/button.tsx
- 5: https://shadcn-ui-ui.mintlify.app/components/button
- 6: https://github.com/NousResearch/hermes-agent/blob/44ddc552/apps/desktop/src/components/ui/button.tsx
🌐 Web query:
"@nous-research/ui" 0.18.2 button source Slot.Root
💡 Result:
In the context of the @nous-research/ui library at version 0.18.2, the use of Slot.Root relates to the Radix UI Slot primitive, which is standardly used to implement an asChild API pattern [1][2][3]. The Slot.Root component merges its props onto its immediate child, allowing a custom component (such as a Button) to be polymorphic [1][4]. When a user passes the asChild prop to a component like Button, the component switches its underlying HTML element from the default (e.g., button) to the Slot.Root component [1][3]. Key points regarding its usage in this library: 1. Pattern: It is employed to facilitate the asChild pattern, enabling developers to render the Button component as a different HTML element or another component while still inheriting the Button's functionality and styling [1][3]. 2. Implementation: The standard implementation involves conditionally setting the component tag: const Comp = asChild? Slot.Root: 'button';, followed by returning <Comp {...props} /> [1][3]. 3. Breaking Changes: Version 0.18.2 of @nous-research/ui introduced breaking changes to the Button component's API, specifically removing several props (ghost, size, outlined, destructive) from ButtonProps [5]. Projects upgrading to this version often require updates to their usage of the Button component to align with the new design system primitives [6]. For detailed implementation logic, you can refer to the Radix UI Slot documentation, as @nous-research/ui builds upon these established primitives [1][2].
Citations:
- 1: https://www.radix-ui.com/primitives/docs/utilities/slot
- 2: https://github.com/radix-ui/website/blob/main/data/primitives/docs/utilities/slot.mdx
- 3: https://github.com/shadcn-ui/ui/blob/b59f68ec/apps/v4/styles/radix-lyra/ui/button.tsx
- 4: https://www.radix-ui.com/primitives/docs/utilities/slot.md
- 5: https://www.stepcodex.com/en/issue/web-ui-build-fails-after-update
- 6: fix: replace all buttons for design system buttons NousResearch/hermes-agent#17007
Avoid nesting the icon button inside the docs link. Put the aria-label on the <a> and render a single interactive element here; tabIndex={-1} only removes the extra tab stop, it doesn’t fix the nested control semantics.
🤖 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 - 230, Update the
docs-link rendering around the Button and its enclosing anchor so the anchor
carries the `aria-label` and is the only interactive element. Remove the nested
icon button and its `tabIndex={-1}`, while preserving the existing icon
appearance and link destination.
aria-labelandtabIndex={-1}to the innermost icon-only<Button>nested inside the<a>wrapper for the OAuth provider docs link.PR created automatically by Jules for task 10602061536119515160 started by @MillionthOdin16
Summary by CodeRabbit