-
Notifications
You must be signed in to change notification settings - Fork 734
The new TextField (Incubator) #854
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
Hey guys! I found out TextField is typed as any in the current release. Would this PR just replace the old one, would new TextField have correct typings, so it would be unnecessary to fix the old one? |
Hi @egorshulga |
|
||
export interface CharCounterProps { | ||
showCharCounter?: boolean; | ||
maxLength: number; |
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.
Do you want to revert the change here and keep it optional?
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.
Changed it back to optional
and moved the check for maxLength
to CharCounter
Recreate the TextField component basing on these guidelines
Features
hint
)Notes for Review
It should be much more readable to go over the code. And in general that's how I see we should write complex components.
I decided not to fix them - I believe fixing them lead us to lots of repeating spacings issues.