-
-
Notifications
You must be signed in to change notification settings - Fork 112
add error state when !isNumber to Numeric component #823
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
Because NumericInput is the base widget used in all numerics (DualNumeric, derived components) adding validation to it should be working for anywhere there's numerics involved. @dmt0 @nicolaskruchten ready for review |
|
||
this.state = {value: props.value}; | ||
this.state = { | ||
value: props.value, |
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 know this is not a new code addition, but assigning props to state is such an antipattern.
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.
that's our way of not having plotly.js reject the value and override with its default
Can I merge this one?
…On Wed, Jan 9, 2019, 3:08 PM Nicolas Kruchten ***@***.*** wrote:
On my screen the little gray fuzzy focus style hides the red...
[image: image]
<https://user-images.githubusercontent.com/203523/50922231-f9d80d00-1417-11e9-879e-be5a3937f4e1.png>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#823 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AED3JN0H7Hg-nutCT_FQDWIyusjndQl_ks5vBkw9gaJpZM4Z30gQ>
.
|
same comments as the other validation one: there's no easy "back out to defaults" mode... once it's red it stays red and deleting it doesn't undo... Can we have it accept |
💃 |
eb8e93f
to
9bd9bfd
Compare
closes: #822
same style as in this pr:
#811 (comment)