-
Notifications
You must be signed in to change notification settings - Fork 503
feat(@clayui/css): LPD-53483 Adds inline-text-input #6056
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
base: master
Are you sure you want to change the base?
Conversation
Shouldn't we be adding this to our docs site too? https://clayui.com/docs/components/input |
de0a1e1
to
23e1d85
Compare
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.
I added some comments but overall it looks good.
packages/clay-form/src/Input.tsx
Outdated
}: IInlineTextProps) => { | ||
const inputRef = React.useRef<HTMLDivElement>(null); | ||
|
||
const [inputValue, setInputValue] = React.useState(''); |
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.
To follow the pattern of other components we need the state to be controlled or uncontrolled you can see how we implemented this in Autocomplete for example.
packages/clay-form/src/Input.tsx
Outdated
const InlineText = React.forwardRef< | ||
HTMLDivElement | HTMLSpanElement | HTMLLabelElement, | ||
IInlineTextProps | ||
>( | ||
({ | ||
children, | ||
className, | ||
id, | ||
placeholder, | ||
readOnly, | ||
spritemap, | ||
...otherProps | ||
}: IInlineTextProps) => { |
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 to pass the ref
forward to the span element of the component and in this case we just need to set the ref type to HTMLSpanElement
or just pass it to the root element which might be better for the developer.
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.
Hey @pat270 , I think with this current implementation we don't have anyway to display a saved value in the inline input on the first load? So, the inline input will always be empty when it loads even if a value is passed to it. As long as we use contentEditable I don't think this will work, since content editable does not play nicely with React.
What was the reason for using content editable instead of an unstyled input?
@ethib137 Good catch, I'll update it. Standard inputs are a fixed width and can't grow with its content. There is some discussion at https://liferay.atlassian.net/browse/LPD-53482?focusedCommentId=2911231 |
@ethib137 I've been trying to get the value to display in the contenteditable element, but I'm running into some React quirks. Using
I was hoping Lexicon could revisit the design / interactions and accommodate the limitations of the standard HTML input. |
https://liferay.atlassian.net/browse/LPD-53483