PolymerElements / iron-input

An input with data binding
https://webcomponents.org/element/PolymerElements/iron-input
33 stars 45 forks source link

apply validation in the correct order #71

Closed notwaldorf closed 8 years ago

notwaldorf commented 8 years ago

Fixes https://github.com/PolymerElements/iron-input/issues/46

The issue was that if you type invalid things (like ---) in type="number, the browser sets the value to "". Since we were first checking required and the value, we actually returned true in this case.

I've rejiggered the function to:

It used to be that the validator was checked first. First of all, I'm not sure what it means to have both a validator and a browser validation. Take something like <input pattern="[1-9]*" validator="value-is-exactly-equal-to-cat">; should the string "cat" be valid? not be valid? who knows. Second, I am not even sure that's right, since inside a form, for example, the form will only look at the browser validity anyway and ignore the validator, so technically browser rules all. I am not opposed to leaving it the original way though.

Here's the kicker: I literally cannot add a test for this. Because security, you can't simulate typing inside and input, and because type=number is 🍌🍌🍌 , it does not let you programmatically set it to an invalid value. πŸ˜‚πŸ”«

cdata commented 8 years ago

This change looks good to me. However, it definitely deserves a test for the condition where the browser's validity pre-empts our validity.

cdata commented 8 years ago

Bump for test.

notwaldorf commented 8 years ago

Sorry, it's been low priority. Will get to it soon!

notwaldorf commented 8 years ago

@cdata added a test. PTAL.

cdata commented 8 years ago

LGTM :+1:

addyosmani commented 8 years ago

LGTM2. Nice work on getting the tests added :cookie:

addyosmani commented 8 years ago

Landed via https://github.com/PolymerElements/iron-input/commit/bd5ce7131245ca39c759ceeb178a933fdd5b5528 and https://github.com/PolymerElements/iron-input/commit/1a6df3d8387a5c64a0392d0d12cf681f82225021.

(Btw, if y'all have a preference on either waiting for squash or manually having the reviewers squash can do either for other PRs)