-
Notifications
You must be signed in to change notification settings - Fork 734
TextField - new features: helperText and validationIcon #3097
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
@@ -74,7 +76,7 @@ const TextField = (props: InternalTextFieldProps) => { | |||
enableErrors, // TODO: rename to enableValidation | |||
validationMessageStyle, | |||
validationMessagePosition = ValidationMessagePosition.BOTTOM, | |||
retainValidationSpace = true, | |||
retainValidationSpace = !helperText, |
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.
Just to make sure I understand
because helperText now renders under the input then we don't need to retain space if it exists?
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.
Yes. Otherwise there will be a gap btw the input and the helperText
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.
Approved. But see my comment about the fix in the example screen
Anyway the build fails
validationMessage="This field is required" | ||
containerStyle={{flexGrow: 1}} | ||
validationMessage="This field is required. That means you have to enter some value" | ||
containerStyle={{width: 300}} |
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.
It's not a good fix, we can't expect the user to do this, so we need to understand better the issue maybe it'll lead us to a problem in the TextField implmentation
We can do it separately since it's not related to the PR as you said
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.
fixed
Description
TextField - new features: helperText and validationIcon
Changelog
TextField - new features: helperText and validationIcon
Additional info
ticket 4173