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

✨ FEATURE: AMP form dirtiness indicator class #22640

Merged
merged 9 commits into from Jun 4, 2019
168 changes: 168 additions & 0 deletions extensions/amp-form/0.1/form-dirtiness.js
@@ -0,0 +1,168 @@
/**
* Copyright 2019 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import {dev} from '../../../src/log';
import {isDisabled, isFieldDefault, isFieldEmpty} from '../../../src/form';

export const DIRTINESS_INDICATOR_CLASS = 'amp-form-dirty';

export class FormDirtiness {
xrav3nz marked this conversation as resolved.
Show resolved Hide resolved
/**
* @param {!HTMLFormElement} form
*/
constructor(form) {
/** @private @const {!HTMLFormElement} */
this.form_ = form;

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

/** @private {!Object<string, boolean>} */
this.isFieldNameDirty_ = {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we need a map rather than a Set()?
In that way we can also get rid of the counter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right I should've left a comment. Set is not allowed here

requirement: {
type: BANNED_NAME
error_message: 'Set is not allowed'
value: 'Set'
}

I think it's probably due to compatibility (#6551). I don't think we need to polyfill since its effects can be emulated with an object.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use the src/object.js#map type here instead of object literals. It creates a new prototypeless object, which decreases the cost of property lookups under the hood.

this.isFieldNameDirty_ = map();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL {} vs Object.create thanks!


/** @private {boolean} */
this.isSubmitting_ = false;

this.installEventHandlers_();
}

/**
* Processes dirtiness state when a form is being submitted.
*/
onSubmitting() {
this.isSubmitting_ = true;
this.updateDirtinessClass_();
}

/**
* Processes dirtiness state when the form submission fails.
*/
onSubmitError() {
this.isSubmitting_ = false;
this.updateDirtinessClass_();
}

/**
* Processes dirtiness state when the form submission succeeds.
*/
onSubmitSuccess() {
this.isSubmitting_ = false;
this.clearDirtyFields_();
this.updateDirtinessClass_();
}

/**
* @private
*/
updateDirtinessClass_() {
if (this.dirtyFieldCount_ == 0 || this.isSubmitting_) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be simplified into

const isDirty = this.dirtyFieldCount_ > 0 && !this.isSubmitting_;
this.form_.classList.toggle(DIRTINESS_INDICATOR_CLASS, isDirty);

// `Element.classList.remove` will not throw an error if the class does
// not already exist
this.form_.classList.remove(DIRTINESS_INDICATOR_CLASS);
} else {
// `Element.classList.add` will no-op if the class already exists
this.form_.classList.add(DIRTINESS_INDICATOR_CLASS);
}
}

/**
* @private
*/
installEventHandlers_() {
this.form_.addEventListener('input', this.onInput_.bind(this));
this.form_.addEventListener('reset', this.onReset_.bind(this));
}

/**
* @param {!Event} event
* @private
*/
onInput_(event) {
const field = dev().assertElement(event.target);
this.checkDirtinessAfterUserInteraction_(field);
this.updateDirtinessClass_();
}

/**
* @param {!Event} unusedEvent
* @private
*/
onReset_(unusedEvent) {
GoTcWang marked this conversation as resolved.
Show resolved Hide resolved
this.clearDirtyFields_();
this.updateDirtinessClass_();
}

/**
* @param {!Element} field
* @private
*/
checkDirtinessAfterUserInteraction_(field) {
if (shouldSkipDirtinessCheck(field)) {
return;
}

if (isFieldEmpty(field) || isFieldDefault(field)) {
this.removeDirtyField_(field.name);
} else {
this.addDirtyField_(field.name);
}
}

/**
* @param {string} fieldName
* @private
*/
addDirtyField_(fieldName) {
if (!this.isFieldNameDirty_[fieldName]) {
this.isFieldNameDirty_[fieldName] = true;
++this.dirtyFieldCount_;
}
}

/**
* @param {string} fieldName
* @private
*/
removeDirtyField_(fieldName) {
if (this.isFieldNameDirty_[fieldName]) {
this.isFieldNameDirty_[fieldName] = false;
--this.dirtyFieldCount_;
}
}

/**
* @private
*/
clearDirtyFields_() {
this.isFieldNameDirty_ = {};
this.dirtyFieldCount_ = 0;
}
}

/**
* @param {!Element} field
xrav3nz marked this conversation as resolved.
Show resolved Hide resolved
* @return {boolean}
*/
function shouldSkipDirtinessCheck(field) {
const {type, name, hidden} = field;

// TODO: add support for radio buttons, checkboxes, and dropdown menus
if (type !== 'text' && type !== 'textarea') {
xrav3nz marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

return !name || hidden || isDisabled(field);
}
183 changes: 183 additions & 0 deletions extensions/amp-form/0.1/test/test-form-dirtiness.js
@@ -0,0 +1,183 @@
/**
* Copyright 2019 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import {DIRTINESS_INDICATOR_CLASS, FormDirtiness} from '../form-dirtiness';

function getForm(doc) {
const form = doc.createElement('form');
form.setAttribute('method', 'POST');
doc.body.appendChild(form);

return form;
}

function changeInput(element, value) {
xrav3nz marked this conversation as resolved.
Show resolved Hide resolved
element.value = value;
const event = new Event('input', {bubbles: true});
element.dispatchEvent(event);
}

describes.realWin('form-dirtiness', {}, env => {
let doc, form, dirtinessHandler;

beforeEach(() => {
doc = env.win.document;
form = getForm(doc);
dirtinessHandler = new FormDirtiness(form);
});

describe('ignored elements', () => {
it('does not add dirtiness class if a nameless element changes', () => {
const nameless = doc.createElement('input');
form.appendChild(nameless);

changeInput(nameless, 'changed');

expect(form).to.not.have.class(DIRTINESS_INDICATOR_CLASS);
});

it('does not add dirtiness class if a hidden element changes', () => {
const hidden = doc.createElement('input');
hidden.name = 'name';
hidden.hidden = true;
form.appendChild(hidden);

changeInput(hidden, 'changed');

expect(form).to.not.have.class(DIRTINESS_INDICATOR_CLASS);
});

it('does not add dirtiness class if a disabled element changes', () => {
const disabled = doc.createElement('input');
disabled.name = 'name';
disabled.disabled = true;
form.appendChild(disabled);

changeInput(disabled, 'changed');
expect(form).to.not.have.class(DIRTINESS_INDICATOR_CLASS);
});
});

describe('text field changes', () => {
let textField;

beforeEach(() => {
// Element is inserted as HTML so that the `defaultValue` property is
// generated correctly, since it returns "the default value as
// **originally specified in the HTML** that created this object."
// https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement#Properties
const html = '<input name="name" type="text" value="default">';
form.insertAdjacentHTML('afterbegin', html);
textField = form.querySelector('input');
});

it('removes dirtiness class when text field is in default state', () => {
changeInput(textField, textField.defaultValue);
expect(form).to.not.have.class(DIRTINESS_INDICATOR_CLASS);
});

it('removes dirtiness class when text field is empty', () => {
changeInput(textField, '');
expect(form).to.not.have.class(DIRTINESS_INDICATOR_CLASS);
});

it('adds dirtiness class when text field is changed', () => {
changeInput(textField, 'changed');
expect(form).to.have.class(DIRTINESS_INDICATOR_CLASS);
});
});

describe('textarea changes', () => {
let textarea;

beforeEach(() => {
const html = '<textarea name="comment">default</textarea>';
form.insertAdjacentHTML('afterbegin', html);
textarea = form.querySelector('textarea');
});

it('removes dirtiness class when textarea is in default state', () => {
changeInput(textarea, textarea.defaultValue);
expect(form).to.not.have.class(DIRTINESS_INDICATOR_CLASS);
});

it('removes dirtiness class when textarea is empty', () => {
changeInput(textarea, '');
expect(form).to.not.have.class(DIRTINESS_INDICATOR_CLASS);
});

it('adds dirtiness class when textarea is changed', () => {
changeInput(textarea, 'changed');
expect(form).to.have.class(DIRTINESS_INDICATOR_CLASS);
});
});

describe('#onSubmitting', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, what does this # stands for, the method name?

Copy link
Contributor Author

@xrav3nz xrav3nz Jun 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh an old habit. # denotes instance methods while . or :: are for static methods. I couldn't remember/find the source of this convention tho. Just grep'ed the AMP code base -

it('clears the dirtiness class', () => {
const input = doc.createElement('input');
input.type = 'text';
input.name = 'text';
form.appendChild(input);

changeInput(input, 'changed');
dirtinessHandler.onSubmitting();

expect(form).to.not.have.class(DIRTINESS_INDICATOR_CLASS);
});
});

describe('#onSubmitError', () => {
let input;

beforeEach(() => {
input = doc.createElement('input');
input.type = 'text';
input.name = 'text';
form.appendChild(input);
});

it('adds the dirtiness class if the form was dirty before submitting', () => {
changeInput(input, 'changed');
dirtinessHandler.onSubmitting();
dirtinessHandler.onSubmitError();

expect(form).to.have.class(DIRTINESS_INDICATOR_CLASS);
});

it('does not add the dirtiness class if the form was clean before submitting', () => {
changeInput(input, '');
dirtinessHandler.onSubmitting();
dirtinessHandler.onSubmitError();

expect(form).to.have.not.class(DIRTINESS_INDICATOR_CLASS);
});
});

describe('#onSubmitSuccess', () => {
it('clears the dirtiness class', () => {
const input = doc.createElement('input');
input.type = 'text';
input.name = 'text';
form.appendChild(input);

changeInput(input, 'changed');
dirtinessHandler.onSubmitting();
dirtinessHandler.onSubmitSuccess();

expect(form).to.not.have.class(DIRTINESS_INDICATOR_CLASS);
});
});
});