PolymerElements / iron-form

Custom form element
https://www.webcomponents.org/element/PolymerElements/iron-form
63 stars 81 forks source link

Validation of inputs inside shadowRoot ? #218

Open ThibzThibz opened 6 years ago

ThibzThibz commented 6 years ago

Description

It seems that the iron-form components no longer validate inputs inside shadow DOM. I have a custom component with and iron-form element, which includes some other custom components that have some inputs (paper-input, paper-radio-group, native inputs...) nested in dom-if templates. The validation of the iron-form was working great with Polymer 1.9 when calling the validate() method, but after migrated to Polymer2.0 the method returns always true because it seems unable to find and validate inputs inside the custom components. Maybe the _findElements method could also find nodes inside Polymer.dom(parent.shadowRoot) ? Or am I doing something wrong somewhere ? Thanks for your help

Expected outcome

The component should find all the validatable elements in shadowDom too.

Actual outcome

The validate() method always return true because it can't find inputs in shadow Dom.

Browsers Affected

estene commented 6 years ago

I'm experiencing something similar, although my problem seems related nested components. I can get it to work with a custom element with an iron-input inside, but not nested custom components.

jstclair commented 6 years ago

I've debugged this issue and it seems that the entire validate function is broken. Validate calls var elements = this._getValidatableElements();. That is a wrapper around _findElements:

_getValidatableElements: function() {
  return this._findElements(this._form, true);
}

The check in findElements when the second parameter (ignoreNames) is true will always ignore custom elements:

_findElements: function(parent, ignoreName) {
        var nodes = Polymer.dom(parent).querySelectorAll('*');
        var submittable = [];

        for (var i = 0; i < nodes.length; i++) {
          var node = nodes[i];
          // An element is submittable if it is not disabled, and if it has a
          // 'name' attribute.
// NOTE: validate calls this with ignoreName == true; all non-disabled nodes will go down this path, 
// regardless if they have a .root or not. 
          if(!node.disabled && (ignoreName || node.name)) {
            submittable.push(node);
          }
// NOTE: one solution could be to remove this 'else' check - if the element has a root, recurse into it
          else {
            // This element has a root which could contain more submittable elements.
            if(node.root) {
              Array.prototype.push.apply(submittable, this._findElements(node.root, ignoreName));
            }
          }
        }
        return submittable;
      }

I'd have marked this issue as critical; as working validation code using iron-form is completely broken now.

(edit) Add additional thoughts on fix in second comment above

jstclair commented 6 years ago

The lack of basic comments on open issues in this repo is incredible - 2+ months, multiple related issues, and not even a simple ack that it's being looked at, or rejected, or whatever....

nickpelone commented 6 years ago

I unfortunately kinda want to agree with @jstclair ....hello? Everyone busy on Polymer 3 or something?

notwaldorf commented 6 years ago

Hiya! Yup, this is a bug, but there isn't a PR for it to fix it, and I have not had a chance to look at it.

ThibzThibz commented 6 years ago

Hi Notwaldorf,

Here is a simplified version of what I was trying to explain in that issue :

<my-page></my-page>

<dom-module id="my-page">
  <template>
    <style>
      paper-button{
        background-color: rgba(150,150,150,0.5);
      }
    </style>

    <h2>
      [[headerTxt]]
    </h2>
    <iron-form id="ironform">
      <form>
          <my-cell required item=[[cellItem]]></my-cell>
        <!-- 
        <paper-input id="secondInput" auto-validate required label='directly inside iron-form'></paper-input>
         -->
     </form>
    </iron-form>
    <paper-button on-tap='validateForm'>Validate</paper-button>
  </template>
  <script>
  Polymer({
      is: 'my-page',

      properties: {
        headerTxt: {type:String, value:"Page content here..."},
        cellItem :{type:Object, value: {label:'some label', value:''}}
      },

      validateForm:function(e){
        var isValid = this.$$("#ironform").validate();
        window.alert( isValid? 'ironForm is valid !': 'ironForm is NOT valid !');
      }
   });
  </script>
</dom-module>

<dom-module id="my-cell">
  <template>
    <paper-input label="[[item.label]]" value={{item.value::input}} auto-validate required="{{required}}" error-message='field required'></paper-input>
  </template>
  <script>
   Polymer({
      is: 'my-cell',
      properties: {
        item:{type:Object},
        required: {type:Boolean, value:false}
      }
   });
  </script>
</dom-module>

If you click on the 'validate' Button, the form is considered as valid, even if the paper-input nested in the component 'my-cell' is not valid.

I know about the iron-validatable-behavior which I could (should?) implement in my-cell component and add a validate() method that will be called by the iron-form when calling it's own validate method. But i think that it would be good if the iron-form was able to also look inside shadowRoot (in it's _findValidatable() method).