forked from NousResearch/hermes-agent
-
Notifications
You must be signed in to change notification settings - Fork 0
๐จ Palette: Add ARIA label to external docs link button #166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
MillionthOdin16
wants to merge
1
commit into
main
Choose a base branch
from
palette/a11y-oauth-providers-card-10602061536119515160
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
๐ฏ Functional Correctness | ๐ Major | โก Quick win
๐งฉ Analysis chain
๐ Script executed:
Repository: MillionthOdin16/hermes-agent
Length of output: 24472
๐ Script executed:
Repository: MillionthOdin16/hermes-agent
Length of output: 50384
๐ Script executed:
Repository: MillionthOdin16/hermes-agent
Length of output: 26909
๐ Script executed:
Repository: 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:
๐ Web query:
"@nous-research/ui" 0.18.2 button source Slot.Root๐ก Result:
In the context of the
@nous-research/uilibrary 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/uiintroduced 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/uibuilds upon these established primitives [1][2].Citations:
Avoid nesting the icon button inside the docs link. Put the
aria-labelon 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