-
Notifications
You must be signed in to change notification settings - Fork 187
Pass the required prop to the combobox FieldLabel #690
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -250,7 +251,11 @@ export const Combobox = ({ | |||
{value} | |||
</VisuallyHidden> | |||
<Stack spacing={label || hint || error ? 1 : 0}> | |||
{label && <FieldLabel id={labelId}>{label}</FieldLabel>} | |||
{label && ( | |||
<FieldLabel required={required} id={labelId}> |
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.
Good catch! Could you also check if the labelAction
is passed through as action={labelAction}
? It seems to be missing too.
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.
Nice catch! Pushed 👍🏽
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.
Signed-off-by: HichamELBSI <[email protected]>
a75be4d
to
42add10
Compare
onClear, | ||
placeholder, | ||
required, | ||
value, |
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.
ℹ️ Just sorting the props list 😅
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.
💡 We need an eslint rule for that.
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.
Looks good to me. We should keep in mind that I think we need to create a ComboboxInput
component to be consistent with the rest of the input fields at some point.
Well done - thanks!
Signed-off-by: HichamELBSI [email protected]
What does it do?
Allow the combobox to display the required state.