Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨[amp-form] allow form attributes for form elements outside of amp-form #33095

Merged
merged 24 commits into from
May 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions build-system/test-configs/forbidden-terms.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,8 @@ 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: 137 additions & 23 deletions extensions/amp-form/0.1/amp-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,16 @@ import {SsrTemplateHelper} from '../../../src/ssr-template-helper';
import {
ancestorElementsByTag,
childElementByAttr,
closestAncestorElementBySelector,
createElementWithAttributes,
iterateCursor,
matches,
removeElement,
tryFocus,
} from '../../../src/dom';
import {createCustomEvent} from '../../../src/event-helper';
import {createCustomEvent, listen} from '../../../src/event-helper';
import {createFormDataWrapper} from '../../../src/form-data-wrapper';
import {deepMerge, dict} from '../../../src/core/types/object';
import {deepMerge, dict, hasOwn} from '../../../src/core/types/object';
import {dev, devAssert, user, userAssert} from '../../../src/log';
import {escapeCssSelectorIdent} from '../../../src/core/dom/css';
import {
Expand All @@ -60,6 +62,7 @@ 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 @@ -144,6 +147,9 @@ 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 @@ -390,6 +396,9 @@ 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 @@ -416,14 +425,16 @@ export class AmpForm {
tryFocus(autofocus);
}
});

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

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

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

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

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

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

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

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

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

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

Expand Down Expand Up @@ -1445,14 +1461,45 @@ 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 = form.querySelectorAll('input,select,textarea,fieldset');
iterateCursor(elements, (element) => checkUserValidity(element));
const elements = formElementsQuerySelectorAll(
form,
'input,select,textarea,fieldset'
);
elements.forEach((element) => checkUserValidity(element));
return checkUserValidity(form);
}

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

/**
* Bootstraps the amp-form elements
* @implements {../../src/service.Disposable}
*/
export class AmpFormService {
/**
Expand All @@ -1586,6 +1635,15 @@ 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 @@ -1666,6 +1724,62 @@ 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: 25 additions & 4 deletions extensions/amp-form/0.1/form-dirtiness.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@
*/

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} from '../../../src/log';
import {dev, devAssert} 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 @@ -30,6 +32,9 @@ const SUPPORTED_TAG_NAMES = {
'TEXTAREA': true,
};

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

export class FormDirtiness {
/**
* @param {!HTMLFormElement} form
Expand All @@ -42,6 +47,12 @@ 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 @@ -130,12 +141,22 @@ export class FormDirtiness {
* @private
*/
installEventHandlers_() {
this.form_.addEventListener('input', this.onInput_.bind(this));
this.form_.addEventListener('reset', this.onReset_.bind(this));
devAssert(this.ampFormService_);
this.ampFormService_.addFormEventListener(
this.form_,
'input',
this.onInput_.bind(this)
);
this.ampFormService_.addFormEventListener(
this.form_,
'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.form_.addEventListener(
this.ampFormService_.addFormEventListener(
this.form_,
AmpEvents.FORM_VALUE_CHANGE,
this.onInput_.bind(this)
);
Expand Down
Loading