fix(demos): Sanitize slider input values #2018
Conversation
Foundation's `min`, `max`, and `step` setters now accept string values, and parse them with `parseFloat()`. If a value is negative or non-finite (e.g., `NaN`, `Infinity`), a descriptive error is thrown. Without these checks, a random and seemingly unrelated error is thrown instead, which makes it harder for clients to debug (especially if they're using minified JS). The demo page now checks that the user's input value is finite before assigning it to any of the aforementioned setters. This prevents an error from being thrown when the user deletes all the text in one of the inputs (which is a fairly common thing to do before entering a new number).
This reverts commit 5261751.
Codecov Report
@@ Coverage Diff @@
## master #2018 +/- ##
=======================================
Coverage 99.43% 99.43%
=======================================
Files 84 84
Lines 3720 3720
Branches 485 485
=======================================
Hits 3699 3699
Misses 21 21Continue to review full report at Codecov.
|
There was a problem hiding this comment.
While I can understand the changes to the demo page, I don't understand why we're widening the type of public APIs to accept strings when their inputs are numeric by nature.
If we do indeed widen the APIs, we should probably add tests that pass strings, and update the README.
acdvorak
changed the title
|
Removed string parsing from the foundation. The reason I added it is that many standard DOM APIs automatically coerce string values to numbers (e.g., |
demos/slider.html
Outdated
| discreteWMarkerSlider.max = parseFloat(max.value); | ||
| customDiscreteWMarkerSlider.max = parseFloat(max.value); | ||
| maxInput.addEventListener('input', function() { | ||
| var minValue = parseFloat(maxInput.value); |
The demo page now checks that the user's input values are finite numbers before assigning them to any of the component's setter properties. This prevents an error from being thrown when the user deletes all the text in one of the inputs (which is a fairly common thing to do before entering a new number).
In addition, to make it clearer that the slider supports negative numbers for
minandmax, the demo page now allows those input values to go down to-100(previously they were bounded at0).