-
Notifications
You must be signed in to change notification settings - Fork 65
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
Add validation support for text-input #664
Conversation
- add helperText property that will show a validity message if it exists - fix combobox code and tests - fix enhanced text input tests
Codecov Report
@@ Coverage Diff @@
## master #664 +/- ##
==========================================
- Coverage 99.05% 98.82% -0.24%
==========================================
Files 43 44 +1
Lines 3285 3320 +35
Branches 901 912 +11
==========================================
+ Hits 3254 3281 +27
- Misses 31 39 +8
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #664 +/- ##
==========================================
- Coverage 99.05% 99.03% -0.03%
==========================================
Files 43 44 +1
Lines 3285 3319 +34
Branches 901 912 +11
==========================================
+ Hits 3254 3287 +33
- Misses 31 32 +1
Continue to review full report at Codecov.
|
remove Omit usage and directly copy InputProperties into component properties
const { properties: { helperText }, _state: { valid, message } } = this; | ||
const text = message || helperText; | ||
|
||
return text ? v('div', { |
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.
should this perhaps be an aside
and the text itself a span
? could be wrong, but just thinking out loud.
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.
Either is fine with me. I was just using the same markup as in the example here.
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.
fair enough then!
@tomdye I made changes to how Combobox internally uses |
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.
Once you've addressed the enhanced-text-input validation this is all good.
Type: feature
The following has been addressed in the PR:
Description:
This adds validation support as partially described in #616. The user can provide a
validate
property to the widget, which iftrue
, will validate against thetype
("email"
, for example) and use the browser's built-in validation.Alternatively,
validate
can be a function that accepts the widget's current value as an argument and must return{ valid: boolean; message: string | undefined; }
. The user can also provide anonValidate(valid: boolean, message?: string)
which will be called to pass the validation value back up to the parent, along with any potential message.With the current behavior, if
validate
is a function, it is the primary validation, but the other validation (email, for example) is also being checked.Example:
Breaking changes
This PR removes the
invalid
property. The way to mark an input as invalid is to use custom validation. This also slightly affects combobox and enhanced text input as theirinvalid
properties will no longer be passed down to the underlying text-input widget.