fix: input focus on click #494
Conversation
✅ Deploy Preview for quantum-deploy-previews ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -107,7 +107,7 @@ const Input = React.forwardRef<HTMLInputElement, InputProps>( | |||
| const rightSideComponent = rightSideChild ?? <BsX strokeWidth={0.6} /> | |||
|
|
|||
| return ( | |||
| <div className={cn(formVariants({ variant }), formClassName)}> | |||
| <label className={cn(formVariants({ variant }), formClassName)}> | |||
There was a problem hiding this comment.
Maybe I'm wrong, but I don't know if using a label here is the right approach.
What happen in a context where we compose a form using the Label component and the input component?
Normally, to create a relationship, a Label component could have an Input component as child, or use the for attribute to indicate the id of the Input
Maybe you can create a story to test this scenario.
There was a problem hiding this comment.
I did test all these scenarios before using it like this (tested that label around input works without for, tested with label wrapping the component and also tested with label alongside the component - with htmlFor) and it works in terms of getting the focus. It is true that it "breaks" the html specification...
There was a problem hiding this comment.
Yeah, my only concern is about a11y. But if you tested all the scenario is good! Thx
|
Thanks Paul for this change. I'm not an expert about designing accessible components either. The only thing I can think of, have you checked how other UI libraries do this? E.g. Material UI. |
Yeah I did check a few libraries. Different approaches. Material UI for example adds a --focused on the parent. Other libraries style the input and position absolute the elements. Overall I haven't seen a generic approach to this. I should have write the findings in the description, let me add those now maybe someone will make use of them in the future. |
|
Yes please, and thanks for checking. Overall LGTM, the behavior in storybook is fine 👌 |
Closes #486
The bug was down to the current implementation where most of the input component styles are applied on a div surrounding the html input (along the left/right elements). The focus was received only when the actual input was clicked.
Initially I implemented a solution where a click on the parent div would have focused the input (useImperativeHandle) but currently we have a smarter solution (thanks @scottrippey for the suggestion).
LE:
I did look at what other libraries are doing (searched react input component and looked at a bunch) and I haven't seen a generic approach. Some libs simply add a class on the parent div (but then you have to take care of removing it) while others style the input and position other elements absolute.
The current approach is a simple and smart one but it might not suit well in certain circumstances, but given the status of the project it's a rather good one.