Radio Group #41
Radio Group #41
Conversation
src/components/Radio/Radio.tsx
Outdated
| <Field className="group flex items-center gap-2"> | ||
| <input | ||
| type="radio" | ||
| id={inputId} |
There was a problem hiding this comment.
headless handles this as long as the input is wrapped with a Field; can omit
|
@tom-vx51 I updated this for your comments but made a larger refactor to make better use of Headless UI (I previously used a native |
| !disabled && "cursor-pointer", | ||
| "appearance-none", | ||
| "border", | ||
| "border-content-text-tertiary", |
There was a problem hiding this comment.
@tom-vx51 I just wanted to note this, there is some inconsistency in figma I was going to ask Sejal about. It lists this "text-tertiary" as the border color in this case for example, it doesn't fit neatly into your color refactor.
There are some others here too that combine something like [TAILWIND SELECTOR]-[DESIGN-SYSTEM-NAME] like ring-action-primary-primary.
There was a problem hiding this comment.
yeah for borders specifically I ran into a similar case with Button - I updated it to use the defined colors. I think it's better that we're consistent, and easy enough to add/adjust colors in the future if we're not happy with how it all looks
tom-vx51
left a comment
tom-vx51
left a comment
There was a problem hiding this comment.
some linter errors, one small change to restrict enum types in radio
There was a problem hiding this comment.
guessing these files were reformatted by your IDE. I think they should be eslint-compliant after gavin's PR so can probably just omit these changes
| labelClassName?: string; | ||
| } | ||
|
|
||
| const textSizeStyles: Partial<Record<Size, string | null>> = { |
There was a problem hiding this comment.
let's do something similar to this https://github.com/voxel51/design-system/blob/develop/src/components/Pill/Pill.tsx#L17 to ensure type safety
Rationale
Adds radio group component
Changes
Testing
storybook, locally, unit tests
Screenshots