Update inputs setting and change listener (#16)

* Make the inputs readonly, add mutationObserver for setting inputs, add change listener to inputs-wrapper * Update tests: get rid of slotchange event, add test for inputs
vaadin · Jan 31, 2019 · 604d2f2 · 604d2f2
1 parent 5abf02f
commit 604d2f2
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 57 deletions.
diff --git a/src/vaadin-custom-field-mixin.html b/src/vaadin-custom-field-mixin.html
@@ -14,9 +14,9 @@
         /**
          * Array of available input nodes
          */
-        __inputs: {
+        inputs: {
           type: Array,
-          value: []
+          readOnly: true
         },
 
         /**
@@ -65,21 +65,36 @@
       };
     }
 
-    ready() {
-      super.ready();
+    connectedCallback() {
+      super.connectedCallback();
+      this.__observeChildrenInputsChange();
+    }
 
-      this.__boundUpdateValue = this.__updateValue.bind(this);
+    disconnectedCallback() {
+      super.disconnectedCallback();
+      this.__childMutationObserver.disconnect();
+    }
 
-      this.$.slot.addEventListener('slotchange', e =>
-        this.__setInputsFromNodes(e.target.assignedNodes()));
+    ready() {
+      super.ready();
 
-      this.__setInputsFromNodes(this.childNodes);
+      this.__setInputsFromChildNodes();
 
       var uniqueId = Vaadin.CustomFieldMixin._uniqueId = 1 + Vaadin.CustomFieldMixin._uniqueId || 1;
       this.__errorId = `${this.constructor.is}-error-${uniqueId}`;
       this.__labelId = `${this.constructor.is}-label-${uniqueId}`;
     }
 
+    __observeChildrenInputsChange() {
+      const mutationObserverConfig = {
+        'childList': true,
+        'subtree': true
+      };
+
+      this.__childMutationObserver = new MutationObserver(this.__setInputsFromChildNodes.bind(this));
+      this.__childMutationObserver.observe(this, mutationObserverConfig);
+    }
+
     __updateValue(e) {
       // Stop native change events
       e && e.stopPropagation();
@@ -101,18 +116,9 @@
       this.__settingValue = false;
     }
 
-    __setInputsFromNodes(nodes) {
-      const inputs = Array.from(nodes).filter(node => node.validate || node.checkValidity);
-
-      const addedInputs = inputs.filter(node => this.__inputs.indexOf(node) < 0);
-      const removedInputs = this.__inputs.filter(node => inputs.indexOf(node) < 0);
-
-      addedInputs.forEach(input => input.addEventListener('change', this.__boundUpdateValue));
-      this.__inputs = this.__inputs.concat(addedInputs);
-
-      removedInputs.forEach(input => input.removeEventListener('change', this.__boundUpdateValue));
-      this.__inputs = this.__inputs.filter(input => removedInputs.indexOf(input) < 0);
-
+    __setInputsFromChildNodes() {
+      const inputs = Array.from(this.querySelectorAll('*')).filter(node => node.validate || node.checkValidity);
+      this._setInputs(inputs);
       this.__setValue();
     }
 
@@ -132,9 +138,5 @@
         this.validate();
       }
     }
-
-    get inputs() {
-      return this.__inputs;
-    }
   };
 </script>
diff --git a/src/vaadin-custom-field.html b/src/vaadin-custom-field.html
@@ -33,7 +33,7 @@
 
     <div class="container">
       <label part="label" on-click="focus" id="[[__labelId]]">[[label]]</label>
-      <div class="inputs-wrapper">
+      <div class="inputs-wrapper" on-change="__updateValue">
         <slot id="slot"></slot>
       </div>
       <div part="error-message"
@@ -153,7 +153,7 @@
           return [
             '__getActiveErrorId(invalid, errorMessage, __errorId)',
             '__getActiveLabelId(label, __labelId)',
-            '__updateRequired(required, __inputs)'
+            '__updateRequired(required, inputs)'
           ];
         }
 

diff --git a/test/custom-field-basic-test.html b/test/custom-field-basic-test.html
@@ -39,6 +39,16 @@
         }
       });
 
+      describe('inputs property', function() {
+
+        it('should be readOnly', function() {
+          expect(customField.inputs.length).to.be.above(0);
+          customField.inputs = [];
+          expect(customField.inputs.length).to.be.above(0);
+        });
+
+      });
+
       describe('value property', function() {
 
         it('should be falsy by default', function() {

diff --git a/test/custom-field-slot-test.html b/test/custom-field-slot-test.html
@@ -29,47 +29,46 @@
         inputElement = window.document.createElement('input');
       });
 
-      it('should add new input to the input list when adding to the DOM', function() {
+      it('should add new input to the input list when adding to the DOM', function(done) {
         customField.appendChild(inputElement);
-        dispatchSlotChange(customField);
-        expect(customField.inputs.length).to.equal(3);
-        expect(customField.inputs[2]).to.eql(inputElement);
+        setTimeout(() => {
+          expect(customField.inputs.length).to.equal(3);
+          expect(customField.inputs[2]).to.eql(inputElement);
+          done();
+        });
       });
 
-      it('should remove input from the input list when removing from the DOM', function() {
+      it('should remove input from the input list when removing from the DOM', function(done) {
         // Remove one of the inputs from the DOM
         customField.removeChild(customField.children[1]);
-        dispatchSlotChange(customField);
-        expect(customField.inputs.length).to.equal(1);
-      });
-
-      it('should remove listener from input which is removed', function() {
-        const firstInput = customField.children[1];
-        customField.removeChild(firstInput);
-        dispatchSlotChange(customField);
-        const spy = sinon.spy(customField, '__setValue');
-        dispatchChange(firstInput);
-        expect(spy.called).to.be.false;
+        setTimeout(() => {
+          expect(customField.inputs.length).to.equal(1);
+          done();
+        });
       });
 
       describe('value property', function() {
 
-        it('should update value when input is added', function() {
+        it('should update value when input is added', function(done) {
           inputElement.value = 'foo';
           customField.appendChild(inputElement);
-          dispatchSlotChange(customField);
-          // Two empty spaces are from empty inputs
-          expect(customField.value).to.equal('  foo');
+          setTimeout(() => {
+            // Two empty spaces are from empty inputs
+            expect(customField.value).to.equal('  foo');
+            done();
+          });
         });
 
-        it('should update value when input is removed', function() {
+        it('should update value when input is removed', function(done) {
           const secondInput = customField.children[1];
           secondInput.value = '1';
           dispatchChange(secondInput);
           expect(customField.value).to.equal(' 1');
           customField.removeChild(secondInput);
-          dispatchSlotChange(customField);
-          expect(customField.value).to.equal('');
+          setTimeout(() => {
+            expect(customField.value).to.equal('');
+            done();
+          });
         });
 
       });

diff --git a/test/custom-field-validation-test.html b/test/custom-field-validation-test.html
@@ -123,7 +123,6 @@
       beforeEach(function() {
         iform = fixture('iform');
         customField = iform.querySelector('vaadin-custom-field');
-        dispatchSlotChange(customField);
       });
 
       it('should call validate', function(done) {

diff --git a/test/helpers.html b/test/helpers.html
@@ -1,12 +1,6 @@
 <script>
-  window.dispatchSlotChange = function(customField) {
-    // NOTE(yuriy): Workaround not to use timeouts
-    const evt = new CustomEvent('slotchange');
-    customField.shadowRoot.querySelector('slot').dispatchEvent(evt);
-  };
-
   window.dispatchChange = function(el) {
-    const evt = new CustomEvent('change');
+    const evt = new CustomEvent('change', {bubbles: true});
     el.dispatchEvent(evt);
   };
 </script>