Skip to content

Commit

Permalink
Revert "✨[amp-form] allow form attributes for form elements outside…
Browse files Browse the repository at this point in the history 
… of `amp-form` (#33095)" (#34640)

This reverts commit be956c9.
  • Loading branch information
Micajuine Ho authored Jun 1, 2021
1 parent c774d98 commit 7c2af00
Show file tree
Hide file tree
Showing 12 changed files with 417 additions and 921 deletions.
2 changes: 0 additions & 2 deletions build-system/test-configs/forbidden-terms.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,8 +304,6 @@ const forbiddenTermsGlobal = {
'extensions/amp-a4a/0.1/amp-ad-template-helper.js',
'extensions/amp-analytics/0.1/instrumentation.js',
'extensions/amp-analytics/0.1/variables.js',
'extensions/amp-form/0.1/amp-form.js', // References service defined in amp-form.
'extensions/amp-form/0.1/form-dirtiness.js', // References service defined in amp-form.
'extensions/amp-fx-collection/0.1/providers/fx-provider.js',
'extensions/amp-gwd-animation/0.1/amp-gwd-animation.js',
'src/chunk.js',
Expand Down
160 changes: 23 additions & 137 deletions extensions/amp-form/0.1/amp-form.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,14 @@ import {SsrTemplateHelper} from '../../../src/ssr-template-helper';
import {
ancestorElementsByTag,
childElementByAttr,
closestAncestorElementBySelector,
createElementWithAttributes,
iterateCursor,
matches,
removeElement,
tryFocus,
} from '../../../src/dom';
import {createCustomEvent, listen} from '../../../src/event-helper';
import {createCustomEvent} from '../../../src/event-helper';
import {createFormDataWrapper} from '../../../src/form-data-wrapper';
import {deepMerge, dict, hasOwn} from '../../../src/core/types/object';
import {deepMerge, dict} from '../../../src/core/types/object';
import {dev, devAssert, user, userAssert} from '../../../src/log';
import {escapeCssSelectorIdent} from '../../../src/core/dom/css';
import {
Expand All @@ -62,7 +60,6 @@ import {
} from '../../../src/form';
import {getFormValidator, isCheckValiditySupported} from './form-validators';
import {getMode} from '../../../src/mode';
import {getServiceForDocOrNull} from '../../../src/service';
import {installFormProxy} from './form-proxy';
import {installStylesForDoc} from '../../../src/style-installer';
import {isAmp4Email} from '../../../src/format';
Expand Down Expand Up @@ -147,9 +144,6 @@ export class AmpForm {
/** @const @private {!../../../src/service/ampdoc-impl.AmpDoc} */
this.ampdoc_ = Services.ampdoc(this.form_);

/** @const @private {?AmpFormService} */
this.ampFormService_ = getServiceForDocOrNull(this.ampdoc_, TAG);

/** @private {?Promise} */
this.dependenciesPromise_ = null;

Expand Down Expand Up @@ -396,9 +390,6 @@ export class AmpForm {
* Returns a promise that will be resolved when all dependencies used inside
* the form tag are loaded and built (e.g. amp-selector) or 2 seconds timeout
* - whichever is first.
*
* NOTE: amp-form allows elements that are not descendants of itself, but
* not <amp-selector>s. See https://go.amp.dev/issue/33891
* @return {!Promise}
* @private
*/
Expand All @@ -425,16 +416,14 @@ export class AmpForm {
tryFocus(autofocus);
}
});
devAssert(this.ampFormService_);
this.ampFormService_.addFormEventListener(
this.form_,

this.form_.addEventListener(
'submit',
this.handleSubmitEvent_.bind(this),
true
);

this.ampFormService_.addFormEventListener(
this.form_,
this.form_.addEventListener(
'blur',
(e) => {
checkUserValidityAfterInteraction_(dev().assertElement(e.target));
Expand All @@ -443,8 +432,7 @@ export class AmpForm {
true
);

this.ampFormService_.addFormEventListener(
this.form_,
this.form_.addEventListener(
AmpEvents.FORM_VALUE_CHANGE,
(e) => {
checkUserValidityAfterInteraction_(dev().assertElement(e.target));
Expand All @@ -455,7 +443,7 @@ export class AmpForm {

// Form verification is not supported when SSRing templates is enabled.
if (!this.ssrTemplateHelper_.isEnabled()) {
this.ampFormService_.addFormEventListener(this.form_, 'change', (e) => {
this.form_.addEventListener('change', (e) => {
this.verifier_.onCommit().then((updatedErrors) => {
const {errors, updatedElements} = updatedErrors;
updatedElements.forEach(checkUserValidityAfterInteraction_);
Expand All @@ -482,7 +470,7 @@ export class AmpForm {
});
}

this.ampFormService_.addFormEventListener(this.form_, 'change', (e) => {
this.form_.addEventListener('input', (e) => {
checkUserValidityAfterInteraction_(dev().assertElement(e.target));
this.validator_.onInput(e);
});
Expand Down Expand Up @@ -550,11 +538,10 @@ export class AmpForm {
this.form_.classList.remove('user-valid');
this.form_.classList.remove('user-invalid');

const validityElements = formElementsQuerySelectorAll(
this.form_,
const validityElements = this.form_.querySelectorAll(
'.user-valid, .user-invalid'
);
validityElements.forEach((element) => {
iterateCursor(validityElements, (element) => {
element.classList.remove('user-valid');
element.classList.remove('user-invalid');
});
Expand Down Expand Up @@ -709,15 +696,12 @@ export class AmpForm {

/**
* Get form fields that require variable substitutions
* @return {!Array<!HTMLInputElement>}
* @return {!IArrayLike<!HTMLInputElement>}
* @private
*/
getVarSubsFields_() {
// Fields that support var substitutions.
return formElementsQuerySelectorAll(
this.form_,
'[type="hidden"][data-amp-replace]'
);
return this.form_.querySelectorAll('[type="hidden"][data-amp-replace]');
}

/**
Expand Down Expand Up @@ -894,7 +878,7 @@ export class AmpForm {

/**
* Perform asynchronous variable substitution on the fields that require it.
* @param {!Array<!HTMLInputElement>} varSubsFields
* @param {!IArrayLike<!HTMLInputElement>} varSubsFields
* @return {!Promise}
* @private
*/
Expand Down Expand Up @@ -954,9 +938,10 @@ export class AmpForm {
* @private
*/
doVerifyXhr_() {
const noVerifyFields = formElementsQuerySelectorAll(
this.form_,
`[${escapeCssSelectorIdent(FORM_VERIFY_OPTOUT)}]`
const noVerifyFields = toArray(
this.form_.querySelectorAll(
`[${escapeCssSelectorIdent(FORM_VERIFY_OPTOUT)}]`
)
);
const denylist = noVerifyFields.map((field) => field.name || field.id);

Expand Down Expand Up @@ -1142,14 +1127,13 @@ export class AmpForm {
* @private
*/
assertNoSensitiveFields_() {
const fields = formElementsQuerySelectorAll(
this.form_,
const fields = this.form_.querySelectorAll(
'input[type=password],input[type=file]'
);
userAssert(
fields.length == 0,
'input[type=password] or input[type=file] ' +
'may only appear with form[method=post]'
'may only appear in form[method=post]'
);
}

Expand Down Expand Up @@ -1461,45 +1445,14 @@ export class AmpForm {
}
}

/**
* Returns all elements of form.elements
* that match the selectors.
* @param {!HTMLFormElement} form
* @param {string} query
* @return {!Array<HTMLElement>}
*/
export function formElementsQuerySelectorAll(form, query) {
return Array.from(form.elements).filter((element) => matches(element, query));
}

/**
* Returns the first element for the form.elements
* that match the selectors.
* @param {!HTMLFormElement} form
* @param {string} query
* @return {?HTMLElement}
*/
export function formElementsQuerySelector(form, query) {
for (let i = 0; i < form.elements.length; i++) {
const element = form.elements[i];
if (matches(element, query)) {
return element;
}
}
return null;
}

/**
* Checks user validity for all inputs, fieldsets and the form.
* @param {!HTMLFormElement} form
* @return {boolean} Whether the form is currently valid or not.
*/
function checkUserValidityOnSubmission(form) {
const elements = formElementsQuerySelectorAll(
form,
'input,select,textarea,fieldset'
);
elements.forEach((element) => checkUserValidity(element));
const elements = form.querySelectorAll('input,select,textarea,fieldset');
iterateCursor(elements, (element) => checkUserValidity(element));
return checkUserValidity(form);
}

Expand Down Expand Up @@ -1539,11 +1492,10 @@ function updateInvalidTypesClasses(element) {
function removeValidityStateClasses(form) {
const dummyInput = document.createElement('input');
for (const validityState in dummyInput.validity) {
const elements = formElementsQuerySelectorAll(
form,
const elements = form.querySelectorAll(
`.${escapeCssSelectorIdent(validityState)}`
);
elements.forEach((element) => {
iterateCursor(elements, (element) => {
dev().assertElement(element).classList.remove(validityState);
});
}
Expand Down Expand Up @@ -1623,7 +1575,6 @@ export function checkUserValidityAfterInteraction_(input) {

/**
* Bootstraps the amp-form elements
* @implements {../../src/service.Disposable}
*/
export class AmpFormService {
/**
Expand All @@ -1635,15 +1586,6 @@ export class AmpFormService {
this.installHandlers_(ampdoc)
);

/** @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc */
this.ampdoc_ = ampdoc;

/** @const @private {!Array<UnlistenDef>} */
this.unlisteners_ = [];

/** @const @private {!Object<string, WeakMap<HTMLFormElement, Array<function(!Event)>>>} */
this.eventHandlers_ = {};

// Dispatch a test-only event for integration tests.
if (getMode().test) {
this.whenInitialized_.then(() => {
Expand Down Expand Up @@ -1724,62 +1666,6 @@ export class AmpFormService {
});
}

/** @override */
dispose() {
while (this.unlisteners_.length > 0) {
const unlisten = this.unlisteners_.pop();
unlisten();
}
}

/**
* Adds handler for the form for a given type, when the
* rootNode gets the signal.
* @param {!HTMLFormElement} form
* @param {string} type
* @param {function(!Event)} handler
* @param {boolean=} opt_options
*/
addFormEventListener(form, type, handler, opt_options) {
if (!hasOwn(this.eventHandlers_, type)) {
this.eventHandlers_[type] = new WeakMap();
this.unlisteners_.push(
listen(
this.ampdoc_.getRootNode(),
type,
(e) => {
let {form} = e.target;

// If it's an AMP element that does not have a native form attribute,
// then find the form by either querySelector for based upon 'form'
// attribute on the element or traversing up.
if (!form) {
dev().assertElement(e.target);
const formId = e.target.getAttribute('form');
form = formId
? this.ampdoc_.getRootNode().querySelector(formId)
: closestAncestorElementBySelector(e.target, 'form');
}

// Only call handlers if the element has a registered form.
if (form && this.eventHandlers_[type].has(form)) {
this.eventHandlers_[type].get(form).forEach((handlerForForm) => {
handlerForForm(e);
});
}
},
opt_options
)
);
}
const handlersArr = this.eventHandlers_[type].get(form) || [];
handlersArr.push(handler);
this.eventHandlers_[type].set(form, handlersArr);
this.unlisteners_.push(() => {
this.eventHandlers_[type].delete(form);
});
}

/**
* Listen for Ctrl/Cmd + Enter in textarea elements
* to trigger form submission when relevant.
Expand Down
29 changes: 4 additions & 25 deletions extensions/amp-form/0.1/form-dirtiness.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,10 @@
*/

import {AmpEvents} from '../../../src/core/constants/amp-events';
import {Services} from '../../../src/services';
import {createCustomEvent} from '../../../src/event-helper';
import {createFormDataWrapper} from '../../../src/form-data-wrapper';
import {dev, devAssert} from '../../../src/log';
import {dev} from '../../../src/log';
import {dict, map} from '../../../src/core/types/object';
import {getServiceForDocOrNull} from '../../../src/service';
import {isDisabled, isFieldDefault, isFieldEmpty} from '../../../src/form';

export const DIRTINESS_INDICATOR_CLASS = 'amp-form-dirty';
Expand All @@ -32,9 +30,6 @@ const SUPPORTED_TAG_NAMES = {
'TEXTAREA': true,
};

/** @const {string} */
const TAG = 'amp-form';

export class FormDirtiness {
/**
* @param {!HTMLFormElement} form
Expand All @@ -47,12 +42,6 @@ export class FormDirtiness {
/** @private @const {!Window} */
this.win_ = win;

/** @const {!../../../src/service/ampdoc-impl.AmpDoc} */
const ampdoc = Services.ampdoc(this.form_);

/** @const @private {?AmpFormService} */
this.ampFormService_ = getServiceForDocOrNull(ampdoc, TAG);

/** @private {number} */
this.dirtyFieldCount_ = 0;

Expand Down Expand Up @@ -141,22 +130,12 @@ export class FormDirtiness {
* @private
*/
installEventHandlers_() {
devAssert(this.ampFormService_);
this.ampFormService_.addFormEventListener(
this.form_,
'input',
this.onInput_.bind(this)
);
this.ampFormService_.addFormEventListener(
this.form_,
'reset',
this.onReset_.bind(this)
);
this.form_.addEventListener('input', this.onInput_.bind(this));
this.form_.addEventListener('reset', this.onReset_.bind(this));

// `amp-bind` dispatches the custom event `FORM_VALUE_CHANGE` when it
// mutates the value of a form field (e.g. textarea, input, etc)
this.ampFormService_.addFormEventListener(
this.form_,
this.form_.addEventListener(
AmpEvents.FORM_VALUE_CHANGE,
this.onInput_.bind(this)
);
Expand Down
Loading

0 comments on commit 7c2af00

Please sign in to comment.