search

PolymerElements / iron-input

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

iron-input value and bindValue sync #76

Closed fl4p closed 8 years ago

fl4p commented 8 years ago

JSBIN (note that I used the paper-input layer to make it visibility, I think the actual issues lies in iron-input): http://jsbin.com/vetiwupele/1/edit?html,output

Expected outcome

No validation errors.

Actual outcome

Valdation Errors for textareas.

Explanation

I think setting the this.bindValue = this.value; should be done in a iron-input.attach, not in iron-input.ready. At first there might be no difference, but let me explain what I think about this single line: As I understand the intention of this update of bindValue, it shall trigger a change event. Looking at paper-input-container, it indeed triggers _handleValueAndAutoValidate, and this conditionally auto-validates the input. However at this point the value of autoValidate is always false, because bindings are not yet setup properly. This might never be an actual issue, but it can cause unexpected behaviour when creating own inputs. There is a significant difference in how iron-autogrow-textarea and iron-input get auto validated. Here is an example, on the paper-input layer though, to make it visible http://jsbin.com/vetiwupele/1/edit?html,output . As you can see, the textarea is invalidated, the paper-inputs not.

Note that there is another issue with paper-autogrow-textarea validation https://github.com/PolymerElements/paper-input/pull/315 . But even this pull request would not fix what I showed in the jsbin.

Just replacing the ready with attach will break validation of the paper-input, though. It has very big impact.

An alternative to make validation behavior of paper-autogrow-textarea similar to paper-input is to set this.bindValue = this.value; on ready, too. I consider this as a dirty fix, because it also relies on a this not-binded-yet bug in paper-input-container's _handleValueAndAutoValidate. The impact of this would be smaller.

Let me know if you think that this whole issue is somewhat important, I can investigate this further.

notwaldorf commented 8 years ago

We had a discussion a bunch of time ago (reflected on how the gold-* elements behave), and the conclusion was that we should validate on blur/change, but not in ready. It's weird that when you load a form, for example, all of your fields (that say, are just required) are actually automatically invalid.

So i think paper-input is actually working correctly, and paper-textarea is the culprit :)

If you would like to send a PR to fix paper-textarea, that would be awesome!

notwaldorf commented 8 years ago

Oh I think the fix for paper-textarea might be in https://github.com/PolymerElements/paper-input/pull/315!

(which I've noticed is your PR).

notwaldorf commented 8 years ago

I think this can be closed, since the paper-textarea PR has been already merged.