Skip to content
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

Please fix/document the validation contract of iron-form-element-behavior #28

Closed
fejesjoco opened this issue Jul 18, 2016 · 5 comments
Closed

Comments

@fejesjoco
Copy link

I think the distinction between form element and validatable element is not clear here. See paper-input: it is a form element, but not validatable, it contains an iron-input child, which is validatable, but not a form element. That should be reflected here.

iron-form used to only validate form elements that were required, which did not make sense (the validation doesn't happen at this level) and luckily that was fixed in iron-form 1.1.0. But iron-form-element-behavior still has this required property. So I think this property is now obsolete, as this behavior doesn't do validation anymore and the form doesn't expect it.

Also it should be documented that elements using iron-form-element-behavior should (must?) have a validate() function, because iron-form expects it. As an incentive, an abstract version of this function might be added to the behavior itself and do console.warn('validation is not implemented, please fix me').

Here's a basic guideline that I think should be recommended here. If the element is a form element and validatable at the same time, it clearly implements the validate() function through the validatable behavior, happy end. In the case of more advanced elements, where form element and validation don't happen at the same level, eg. paper-input, validate() should be implemented manually and call into the validate() function(s) of the validatable children (that's exactly what paper-input does anyway, I don't know about the other paper elements).

@ghost
Copy link

ghost commented Sep 5, 2016

I think this might be related to your comments. I just tried to create an extended element that is both a validatable and form element, and it doesn't work.

    Polymer({
      is: 'goturl-url-input',
      extends: 'input',
      listeners: {
        'input': '_onInput'
      },
      behaviors: [
        Polymer.IronFormElementBehavior,
        Polymer.IronValidatableBehavior
      ],
      _onInput: function(e) {
        console.log('_onInput', this.value)
      },
      _getValidity: function(value) {
        console.log('_getValidity', this.value, value)
        return (value)
      }
    })

If you have both IronFormElementBehavior and IronValidatableBehavior on the element, then this.value is always undefined. If you remove IronFormElementBehavior, then this.value is now correct (in the input listener), but validation doesn't fire. Of course, if you remove IronValidatableBehavior then you don't have any validation.

To create a simple validating input control for forms, it seems like these should be able to work together.

@notwaldorf
Copy link
Contributor

I think there's a couple of problems with that code snippet. Here's an example one: http://jsbin.com/heleyo/edit?html,console,output

I think the fundamental problem is that IronValidatableBehavior has a validate method that takes a value parameter and uses that, but iron-form just calls the no-arg validate() form, so nothing is ever validated. I think the problem is with IronValidatableBehavior, and its signature should change -- unfortunately that's a breaking change and cannot be done quite right now.
@fejesjoco: would that be an acceptable solution?

@chrismbeckett in the meantime, you should implement the validate() method yourself as a workaround -- see the JSBin I posted for an example.

@fejesjoco
Copy link
Author

Removing the value parameter is another issue I filed: PolymerElements/iron-validatable-behavior#27 (And yes I agree normally it would be breaking, however not in this case and not in JS. Handling of value parameter is pretty bad right now, not taking it as a parameter would make things much better and would not break existing clients, JS will ignore the new parameter.)

This issue was about removing the required property and adding documentation/contract about the validate() method in iron-form-element-behavior. This would basically just document the status quo because it's already working like that. Delegating validation from iron-form-element-behavior to the real validatable elements makes perfect sense.

@ghost
Copy link

ghost commented Sep 7, 2016

@notwaldorf I appreciate your example, but if you look at my code sample, I am extending the native input element. My goal seems pretty simple. I want a custom validating input element. I could use an iron-input element, but then my validator has to be seperate. Why would I want to have to code my validator seperately? Here is a codepen updated to demonstrate that once you extend the native input control then it no longer works - http://codepen.io/chrismbeckett/full/bwdgyX/.

@valdrinkoshi
Copy link
Member

Closing this since iron-form 2.x doesn't rely anymore on this behavior. More info about the "contract" to participate to form serialization & validation at https://github.com/PolymerElements/iron-form/#changes-in-20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants