-
Notifications
You must be signed in to change notification settings - Fork 856
fix(Select/InputMenu): handle focus via label inside a FormField #4696
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
UFormField
commit: |
src/theme/select.ts
Outdated
compoundVariants: [...(options.theme.colors || []).map((color: string) => ({ | ||
color, | ||
variant: ['outline', 'subtle'], | ||
class: `focus:ring-2 focus:ring-inset focus:ring-${color}` | ||
})), { | ||
color: 'neutral', | ||
variant: ['outline', 'subtle'], | ||
class: 'focus:ring-2 focus:ring-inset focus:ring-inverted' | ||
}] |
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.
browser behavior when focusing button via label will fire :focus instead of :focus-visible,
you can check this out https://stackblitz.com/edit/stackblitz-starters-ew5qzrui?description=HTML/CSS/JS%20Starter&file=script.js,styles.css,index.html&terminalHeight=10&title=Static%20Starter
UFormField
id
on the input to handle focus in a FormField
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.
@rdjanuar I've updated your PR to focus on the id
.
Would you mind opening a new issue/PR regarding the focus classes?
Okee @benjamincanac , i make other PR for handling focus classes |
Yeah I'd like a reproduction as I'm not sure to understand why you want to change this. However, we still need to fix the Select component to resolve the linked issue π€ |
you can check this, focusing button via label will fire |
It doesn't really matter as the focus would be taken by the popup if it worked anyway. You can check the SelectMenu on the playground: |
if you take a look for native |
Indeed, we could change the EDIT: Nevermind, it works but now I wonder if SelectMenu should not have the same behavior.. |
It's fix the issue, native |
for Edit: And i think it's follow Combobox WAI-ARIA design pattern. |
id
on the input to handle focus in a FormField
Ok now I understand why you added those classes in the first place, took a while sorry about that! What do you think of latest commit 69c64f3? |
Look good to me, the |
I still think it would be best for the Select to open when clicking on the label like on https://ui.shadcn.com/docs/components/select#form (it doesn't on https://www.shadcn-vue.com/docs/components/select.html#form) but that's an issue for Reka UI I guess. |
shadcn use ![]() |
you can take a look ark-ui @benjamincanac |
π Linked issue
Resolves #4690
β Type of change
π Description
π Checklist