-
Notifications
You must be signed in to change notification settings - Fork 11
Fix Inputs interactive area #600
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 ↗︎
|
4383178
to
627cb8b
Compare
error: { control: "text" }, | ||
disabled: { control: "boolean" }, | ||
placeholder: { control: "text" }, | ||
readOnly: { control: "boolean" }, |
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.
readOnly
disappeared from the parameters list, because it's actually a part of the native input API, not of the click-ui component API.
And rightfully so, because we don't actually support this property. If you set readOnly
together with clear
, then it's possible to clear the field, despite it being read-only.
We need to
- stop accepting any native property in component API. Because:
- we can't guarantee they will work, considering we have custom component logic (readOnly is an example)
- they clog up component API, making it hard to use
- it is not clear, which HTML element these props will be assigned to
- properly support hand-picked properties like readOnly in the component
- optionally, accept prop
controlProps
/controlAttrs
that would accept all native attributes and pass them directly to the control
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 also have a resize
in the params list now, that doesn't actually work! Even for TextArea!
But it's also correct, because components do accept this property. They don't process it in any way, but they do accept it.
padding: ${theme.click.field.space.y} 0; | ||
visibility: ${$show ? "visible" : "hidden"}; |
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.
is there any reason we were hiding icons using visibility rather than not rendering them?
mind accepted visual changes: https://www.chromatic.com/build?appId=6481a2f37ea081d3131e5219&number=3001 |
Why
Horizontal space in Inputs appear interactive, and should be interactive, but is not.
Clicking anywhere within visible input area should activate the field.
inactive-input-padding.mov
What
To address the issue:
TextField
: introduced propsstartContent
andendContent
, that accept anyReactNode
and render extra content before or after input controlSearchField
: removed all code and remade the component on top ofTextField
, because they are exactly the same; SearchField now usesstartContent
to render its iconPasswordField
: used the same utility component to render the eye button asTextField
uses to renderendContent
Also:
TextField
,SearchField
, andPasswordField
to be auto-generated; now they are more informative and up-to-date with the actual components API.Remaining issues
gap
to create space between elements inside InputWrapper, these gaps don't have proper cursor, and aren't interactive. To be fixed in next iterations.