Skip to content

Commit

Permalink
Revert "🐛async-input: Fixed NonXHR GET on amp-form with Async Input E…
Browse files Browse the repository at this point in the history
…lements (#20362)" (#20601)

This reverts commit de0ea8e.
  • Loading branch information
torch2424 authored and Enriqe committed Jan 30, 2019
1 parent 80c5d52 commit f97d40d
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 48 deletions.
19 changes: 9 additions & 10 deletions extensions/amp-form/0.1/amp-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ export class AmpForm {
this.urlReplacement_.expandInputValueSync(varSubsFields[i]);
}

this.handleNonXhrGet_(/*shouldSubmitFormElement*/false);
this.handleNonXhrGet_(event);
return Promise.resolve();
}

Expand All @@ -548,7 +548,7 @@ export class AmpForm {
presubmitPromises,
SUBMIT_TIMEOUT
).then(
() => this.handlePresubmitSuccess_(trust),
() => this.handlePresubmitSuccess_(trust, event),
error => this.handlePresubmitError_(/**@type {!Error}*/(error))
);
}
Expand All @@ -566,15 +566,16 @@ export class AmpForm {
/**
* Handle successful presubmit tasks
* @param {!ActionTrust} trust
* @param {?Event} event
* @return {!Promise}
*/
handlePresubmitSuccess_(trust) {
handlePresubmitSuccess_(trust, event) {
if (this.xhrAction_) {
return this.handleXhrSubmit_(trust);
} else if (this.method_ == 'POST') {
this.handleNonXhrPost_();
} else if (this.method_ == 'GET') {
this.handleNonXhrGet_(/*shouldSubmitFormElement*/true);
this.handleNonXhrGet_(event);
}

return Promise.resolve();
Expand Down Expand Up @@ -888,14 +889,12 @@ export class AmpForm {

/**
* Triggers Submit Analytics,
* and Form Element submit if passed by param.
* shouldSubmitFormElement should ONLY be true
* If the submit event.preventDefault was called
* @param {boolean} shouldSubmitFormElement
* and Form Element submit if not triggered by an event
* @param {?Event} event
*/
handleNonXhrGet_(shouldSubmitFormElement) {
handleNonXhrGet_(event) {
this.triggerFormSubmitInAnalytics_('amp-form-submit');
if (shouldSubmitFormElement) {
if (!event) {
this.form_.submit();
}
this.setState_(FormState.INITIAL);
Expand Down
49 changes: 11 additions & 38 deletions extensions/amp-form/0.1/test/test-amp-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -1833,7 +1833,7 @@ describes.repeated('', {
sandbox.stub(ampForm.urlReplacement_, 'expandInputValueAsync');
sandbox.spy(ampForm.urlReplacement_, 'expandInputValueSync');

sandbox.stub(ampForm, 'handleNonXhrGet_')
sandbox.stub(ampForm, 'handleXhrSubmitSuccess_')
.returns(Promise.resolve());
const submitActionPromise =
ampForm.handleSubmitAction_(/* invocation */ {});
Expand All @@ -1846,9 +1846,8 @@ describes.repeated('', {
expect(ampForm.urlReplacement_.expandInputValueSync)
.to.have.been.calledWith(canonicalUrlField);


return whenCalled(ampForm.handleNonXhrGet_).then(() => {
expect(form.submit).to.not.have.been.called;
return whenCalled(form.submit).then(() => {
expect(form.submit).to.have.been.calledOnce;
expect(clientIdField.value).to.equal('');
expect(canonicalUrlField.value).to.equal(
'https%3A%2F%2Fexample.com%2Famps.html');
Expand Down Expand Up @@ -2003,8 +2002,6 @@ describes.repeated('', {
it('should execute form submit when not triggered through event', () => {
return getAmpForm(getForm()).then(ampForm => {
const form = ampForm.form_;

// Non XHR Get
ampForm.method_ = 'GET';
ampForm.xhrAction_ = null;
sandbox.stub(form, 'submit');
Expand All @@ -2016,7 +2013,7 @@ describes.repeated('', {
const submitActionPromise =
ampForm.handleSubmitAction_(/* invocation */ {});

expect(form.submit).to.have.not.been.called;
expect(form.submit).to.have.been.called;

return submitActionPromise;
});
Expand Down Expand Up @@ -2100,7 +2097,6 @@ describes.repeated('', {
unnamedInput.setAttribute('value', 'unnamed');
form.appendChild(unnamedInput);

// Non XHR Get
ampForm.method_ = 'GET';
ampForm.xhrAction_ = null;
sandbox.stub(form, 'submit');
Expand All @@ -2118,7 +2114,7 @@ describes.repeated('', {
'formFields[name]': 'John Miller',
'formFields[email]': 'j@hnmiller.com',
};
expect(form.submit).to.have.not.been.called;
expect(form.submit).to.have.been.called;
expect(ampForm.analyticsEvent_).to.be.calledWith(
'amp-form-submit',
expectedFormData
Expand Down Expand Up @@ -2231,8 +2227,6 @@ describes.repeated('', {
return getAmpForm(getForm()).then(ampForm => {
const form = ampForm.form_;
form.id = 'registration';

// Non XHR GET
ampForm.method_ = 'GET';
ampForm.xhrAction_ = null;
const clientIdField = createElement('input');
Expand Down Expand Up @@ -2266,7 +2260,6 @@ describes.repeated('', {
'amp-form-submit',
expectedFormData
);

expect(ampForm.urlReplacement_.expandInputValueAsync)
.to.not.have.been.called;
expect(ampForm.urlReplacement_.expandInputValueSync)
Expand All @@ -2276,10 +2269,12 @@ describes.repeated('', {
expect(ampForm.urlReplacement_.expandInputValueSync)
.to.have.been.calledWith(canonicalUrlField);

expect(form.submit).to.not.have.been.called;
expect(clientIdField.value).to.equal('');
expect(canonicalUrlField.value).to.equal(
'https%3A%2F%2Fexample.com%2Famps.html');
return whenCalled(form.submit).then(() => {
expect(form.submit).to.have.been.calledOnce;
expect(clientIdField.value).to.equal('');
expect(canonicalUrlField.value).to.equal(
'https%3A%2F%2Fexample.com%2Famps.html');
});
});
});

Expand All @@ -2301,28 +2296,6 @@ describes.repeated('', {
});
});

it('NonXHRGet should submit async if Async Input', () => {
return getAmpFormWithAsyncInput().then(response => {
const {ampForm, getValueStub} = response;

// Make the form a NonXHRGet
ampForm.method_ = 'GET';
ampForm.xhrAction_ = null;

const formElementSubmitSpy =
sandbox.spy(ampForm.form_, 'submit');

const mockEvent = {
preventDefault: () => {},
};

return ampForm.submit_(ActionTrust.HIGH, mockEvent).then(() => {
expect(getValueStub).to.be.called;
expect(formElementSubmitSpy).to.be.called;
});
});
});

it('should call getValue() on Async Input Elements', () => {
return getAmpFormWithAsyncInput().then(response => {
const {ampForm, getValueStub} = response;
Expand Down

0 comments on commit f97d40d

Please sign in to comment.