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
fix: add id to checkbox, add link to profiles #1449
Changes from 1 commit
9a7afd7
5331847
42a0442
655b2ac
de21e67
9a90431
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -9,31 +9,34 @@ interface CheckboxProps extends React.ComponentPropsWithoutRef<typeof CheckboxPr | |||||||
} | ||||||||
|
||||||||
const Checkbox = React.forwardRef<React.ElementRef<typeof CheckboxPrimitive.Root>, CheckboxProps>( | ||||||||
({ className, label, id = "checkbox", ...props }, ref) => ( | ||||||||
<div className="flex items-center"> | ||||||||
<CheckboxPrimitive.Root | ||||||||
ref={ref} | ||||||||
className={clsx( | ||||||||
"peer h-4 w-4 shrink-0 rounded-[4px] cursor-pointer bg-white border border-light-slate-8 hover:border-orange-500 focus:outline-none disabled:cursor-not-allowed disabled:opacity-50 data-[state=checked]:border-orange-500 data-[state=checked]:bg-orange-500", | ||||||||
className | ||||||||
)} | ||||||||
{...props} | ||||||||
id={id} | ||||||||
> | ||||||||
<CheckboxPrimitive.Indicator className={clsx("flex items-center justify-center text-white")}> | ||||||||
<FiCheck className="w-full h-full" /> | ||||||||
</CheckboxPrimitive.Indicator> | ||||||||
</CheckboxPrimitive.Root> | ||||||||
{label && ( | ||||||||
<label | ||||||||
htmlFor={id} | ||||||||
className="ml-3 text-sm font-medium leading-none cursor-pointer text-light-slate-12 peer-disabled:cursor-not-allowed peer-disabled:opacity-70" | ||||||||
({ className, label, id, ...props }, ref) => { | ||||||||
id = id ? id : label?.replaceAll(" ", "_").toLowerCase(); | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is the line that I changed here, plus the ID call in the function, yet git did not detect it only (not sure why) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should turn this to a function that returns the conditional statements here
Suggested change
Then in the component attributes, set it to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am going to implement this, but was wondering why we would take this approach? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variable name needs updating to avoid conflicts with the reserved We could have easily said |
||||||||
return ( | ||||||||
<div className="flex items-center"> | ||||||||
<CheckboxPrimitive.Root | ||||||||
ref={ref} | ||||||||
className={clsx( | ||||||||
"peer h-4 w-4 shrink-0 rounded-[4px] cursor-pointer bg-white border border-light-slate-8 hover:border-orange-500 focus:outline-none disabled:cursor-not-allowed disabled:opacity-50 data-[state=checked]:border-orange-500 data-[state=checked]:bg-orange-500", | ||||||||
a0m0rajab marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
className | ||||||||
)} | ||||||||
{...props} | ||||||||
id={id} | ||||||||
> | ||||||||
{label} | ||||||||
</label> | ||||||||
)} | ||||||||
</div> | ||||||||
) | ||||||||
<CheckboxPrimitive.Indicator className={clsx("flex items-center justify-center text-white")}> | ||||||||
<FiCheck className="w-full h-full" /> | ||||||||
</CheckboxPrimitive.Indicator> | ||||||||
</CheckboxPrimitive.Root> | ||||||||
{label && ( | ||||||||
<label | ||||||||
htmlFor={id} | ||||||||
className="ml-3 text-sm font-medium leading-none cursor-pointer text-light-slate-12 peer-disabled:cursor-not-allowed peer-disabled:opacity-70" | ||||||||
> | ||||||||
{label} | ||||||||
</label> | ||||||||
)} | ||||||||
</div> | ||||||||
); | ||||||||
} | ||||||||
); | ||||||||
Checkbox.displayName = CheckboxPrimitive.Root.displayName; | ||||||||
|
||||||||
|
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.
the
id
had checkbox as default value, I removed it from here.