Move change trigger to global event.

extensions/amp-form/0.1/amp-form.js

@@ -137,7 +137,7 @@ export class AmpForm {
 
     this.actions_.installActionHandler(
         this.form_, this.actionHandler_.bind(this));
-    this.installSubmitHandler_();
+    this.installEventHandlers_();
   }
 
   /**
@@ -151,7 +151,7 @@ export class AmpForm {
   }
 
   /** @private */
-  installSubmitHandler_() {
+  installEventHandlers_() {
     this.form_.addEventListener('submit', e => this.handleSubmit_(e), true);
     this.form_.addEventListener('blur', e => {
       onInputInteraction_(e);
@@ -161,16 +161,6 @@ export class AmpForm {
       onInputInteraction_(e);
       this.validator_.onInput(e);
     });
-    this.form_.addEventListener('change', this.changeEventHandler_.bind(this));
-  }
-
-  /**
-   * @param {!Event} e
-   * @private
-   */
-  changeEventHandler_(e) {
-    // TODO(mkhatib, #5702): Consider a debounced-input event for text-type inputs.
-    this.actions_.trigger(dev().assertElement(e.target), 'change', null);
   }
 
   /**

extensions/amp-form/0.1/test/test-amp-form.js

@@ -122,7 +122,6 @@ describe('amp-form', () => {
     expect(form.addEventListener).to.be.calledWith('submit');
     expect(form.addEventListener).to.be.calledWith('blur');
     expect(form.addEventListener).to.be.calledWith('input');
-    expect(form.addEventListener).to.be.calledWith('change');
     expect(form.className).to.contain('-amp-form');
   });
 
@@ -135,7 +134,7 @@ describe('amp-form', () => {
       target: form,
       preventDefault: sandbox.spy(),
     };
-    sandbox.spy(ampForm.xhr_, 'fetchJson');
+    sandbox.stub(ampForm.xhr_, 'fetchJson').returns(Promise.resolve());
     sandbox.spy(form, 'checkValidity');
     ampForm.handleSubmit_(event);
     expect(event.stopImmediatePropagation.called).to.be.true;
@@ -152,7 +151,7 @@ describe('amp-form', () => {
       target: form,
       preventDefault: sandbox.spy(),
     };
-    sandbox.spy(ampForm.xhr_, 'fetchJson');
+    sandbox.stub(ampForm.xhr_, 'fetchJson').returns(Promise.resolve());
     sandbox.spy(form, 'checkValidity');
     expect(() => ampForm.handleSubmit_(event)).to.throw(
         /Only XHR based \(via action-xhr attribute\) submissions are support/);
@@ -169,6 +168,7 @@ describe('amp-form', () => {
     emailInput.setAttribute('required', '');
     form.appendChild(emailInput);
     const ampForm = new AmpForm(form);
+    sandbox.stub(ampForm.xhr_, 'fetchJson').returns(Promise.resolve());
     const event = {
       stopImmediatePropagation: sandbox.spy(),
       target: form,
@@ -206,7 +206,7 @@ describe('amp-form', () => {
       emailInput.setAttribute('required', '');
       form.appendChild(emailInput);
       sandbox.spy(form, 'checkValidity');
-      sandbox.spy(ampForm.xhr_, 'fetchJson');
+      sandbox.stub(ampForm.xhr_, 'fetchJson').returns(Promise.resolve());
 
       const event = {
         stopImmediatePropagation: sandbox.spy(),
@@ -467,22 +467,16 @@ describe('amp-form', () => {
       const newRender = document.createElement('div');
       newRender.innerText = 'New Success: What What';
 
-      let fetchJsonResolver;
       sandbox.stub(ampForm.xhr_, 'fetchJson')
-          .returns(new Promise(resolve => {
-            fetchJsonResolver = resolve;
-          }));
+          .returns(Promise.resolve({'message': 'What What'}));
       sandbox.stub(ampForm.templates_, 'findAndRenderTemplate')
-          .returns(new Promise(resolve => {
-            resolve(newRender);
-          }));
+          .returns(Promise.resolve(newRender));
       const event = {
         stopImmediatePropagation: sandbox.spy(),
         target: form,
         preventDefault: sandbox.spy(),
       };
       ampForm.handleSubmit_(event);
-      fetchJsonResolver({'message': 'What What'});
       return timer.promise(5).then(() => {
         expect(ampForm.templates_.findAndRenderTemplate.called).to.be.true;
         expect(ampForm.templates_.findAndRenderTemplate.calledWith(
@@ -828,27 +822,12 @@ describe('amp-form', () => {
     const actions = actionServiceForDoc(form.ownerDocument);
     sandbox.stub(actions, 'installActionHandler');
     const ampForm = new AmpForm(form);
+    sandbox.stub(ampForm.xhr_, 'fetchJson').returns(Promise.resolve());
     expect(actions.installActionHandler).to.be.calledWith(form);
     sandbox.spy(ampForm, 'handleSubmit_');
     ampForm.actionHandler_({method: 'anything'});
     expect(ampForm.handleSubmit_).to.have.not.been.called;
     ampForm.actionHandler_({method: 'submit'});
     expect(ampForm.handleSubmit_).to.have.been.called;
   });
-
-  it('should trigger change events on inputs', () => {
-    return getAmpForm(true).then(ampForm => {
-      const form = ampForm.form_;
-      const actions = actionServiceForDoc(form.ownerDocument);
-      sandbox.stub(actions, 'trigger');
-      const emailInput = document.createElement('input');
-      emailInput.setAttribute('name', 'email');
-      emailInput.setAttribute('type', 'email');
-      form.appendChild(emailInput);
-      ampForm.changeEventHandler_({target: emailInput});
-      expect(actions.trigger).to.have.been.called;
-      expect(actions.trigger).to.have.been.calledWith(emailInput, 'change');
-    });
-  });
-
 });

src/service/action-impl.js

@@ -106,6 +106,8 @@ export class ActionService {
     // Add core events.
     this.addEvent('tap');
     this.addEvent('submit');
+    // TODO(mkhatib, #5702): Consider a debounced-input event for text-type inputs.
+    this.addEvent('change');
   }
 
   /**
@@ -122,9 +124,9 @@ export class ActionService {
           this.trigger(dev().assertElement(event.target), 'tap', event);
         }
       });
-    } else if (name == 'submit') {
-      this.ampdoc.getRootNode().addEventListener('submit', event => {
-        this.trigger(dev().assertElement(event.target), 'submit', event);
+    } else if (name == 'submit' || name == 'change') {
+      this.ampdoc.getRootNode().addEventListener(name, event => {
+        this.trigger(dev().assertElement(event.target), name, event);
       });
     }
   }
@@ -268,23 +270,12 @@ export class ActionService {
       return;
     }
 
-    // Special elements with AMP ID.
-    if (target.id && target.id.substring(0, 4) == 'amp-') {
-      if (!target[ACTION_QUEUE_]) {
-        target[ACTION_QUEUE_] = [];
-      }
-      target[ACTION_QUEUE_].push(invocation);
-      return;
-    }
-
-    // Check if this is an element has a known action.
+    // Special elements with AMP ID or known supported actions.
     const supportedActions = ELEMENTS_ACTIONS_MAP_[lowerTagName];
-    if (supportedActions && supportedActions.indexOf(method) != -1) {
+    if ((target.id && target.id.substring(0, 4) == 'amp-') ||
+        (supportedActions && supportedActions.indexOf(method) != -1)) {
       if (!target[ACTION_QUEUE_]) {
-        this.actionInfoError_(
-            'Action queue not initialized, probably tried to execute before ' +
-            'calling installActionHandler.', actionInfo, target);
-        return;
+        target[ACTION_QUEUE_] = [];
       }
       target[ACTION_QUEUE_].push(invocation);
       return;

test/functional/test-action.js

@@ -544,3 +544,52 @@ describe('Action common handler', () => {
     expect(target['__AMP_ACTION_QUEUE__']).to.not.exist;
   });
 });
+
+
+describe('Core events', () => {
+  let sandbox;
+  let action;
+  let target;
+
+  beforeEach(() => {
+    sandbox = sinon.sandbox.create();
+    sandbox.stub(window.document, 'addEventListener');
+    action = new ActionService(new AmpDocSingle(window));
+    sandbox.stub(action, 'trigger');
+    target = document.createElement('target');
+    target.setAttribute('id', 'amp-test-1');
+
+    action.vsync_ = {mutate: callback => callback()};
+  });
+
+  afterEach(() => {
+    sandbox.restore();
+  });
+
+  it('should trigger tap event', () => {
+    expect(window.document.addEventListener).to.have.been.calledWith('click');
+    const handler = window.document.addEventListener.getCall(0).args[1];
+    const element = {tagName: 'target1', nodeType: 1};
+    const event = {target: element};
+    handler(event);
+    expect(action.trigger).to.have.been.calledWith(element, 'tap', event);
+  });
+
+  it('should trigger submit event', () => {
+    expect(window.document.addEventListener).to.have.been.calledWith('submit');
+    const handler = window.document.addEventListener.getCall(1).args[1];
+    const element = {tagName: 'target1', nodeType: 1};
+    const event = {target: element};
+    handler(event);
+    expect(action.trigger).to.have.been.calledWith(element, 'submit', event);
+  });
+
+  it('should trigger change event', () => {
+    expect(window.document.addEventListener).to.have.been.calledWith('change');
+    const handler = window.document.addEventListener.getCall(2).args[1];
+    const element = {tagName: 'target2', nodeType: 1};
+    const event = {target: element};
+    handler(event);
+    expect(action.trigger).to.have.been.calledWith(element, 'change', event);
+  });
+});