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

Add a combination as-you-go and show-all-on-submit validator #10700

Merged
merged 3 commits into from Jul 31, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 24 additions & 0 deletions examples/forms.amp.html
Expand Up @@ -491,6 +491,30 @@ <h4>Show As You Go Messages On Submit</h4>
</fieldset>
</form>

<h4>Show As You Go Messages (with all messages on submit)</h4>
<form method="post"
action-xhr="/form/echo-json/post"
target="_blank"
custom-validation-reporting="interact-and-submit">
<fieldset>
<label>
<span>Your name</span>
<input type="text" name="name" id="name7" required pattern="\w+\s\w+">
<span visible-when-invalid="valueMissing" validation-for="name7"></span>
<span visible-when-invalid="patternMismatch" validation-for="name7">
Please enter your first and last name separated by a space (e.g. Jane Miller)
</span>
</label>
<label>
<span>Your email</span>
<input type="email" name="email" id="email7" required>
<span visible-when-invalid="valueMissing" validation-for="email7"></span>
<span visible-when-invalid="typeMismatch" validation-for="email7"></span>
</label>
<input type="submit" value="Subscribe">
</fieldset>
</form>

<h4 id="header1">on=change and form.submit examples</h4>
<button on="tap:header1.submit">Wrong Action - should throw an error</button>
<form method="post"
Expand Down
12 changes: 12 additions & 0 deletions extensions/amp-form/0.1/form-validators.js
Expand Up @@ -51,6 +51,7 @@ export function setCheckValiditySupportedForTesting(isSupported) {
const CustomValidationTypes = {
AsYouGo: 'as-you-go',
ShowAllOnSubmit: 'show-all-on-submit',
AsYouGoShowAllOnSubmit: 'interact-and-submit',
ShowFirstOnSubmit: 'show-first-on-submit',
};

Expand Down Expand Up @@ -337,6 +338,15 @@ export class AsYouGoValidator extends AbstractCustomValidator {
}


/** @private visible for testing */
export class InteractAndSubmitValidator extends ShowAllOnSubmitValidator {
/** @override */
shouldValidateOnInteraction(unusedInput) {
return true;
}
}


/**
* Returns the form validator instance.
*
Expand All @@ -351,6 +361,8 @@ export function getFormValidator(form) {
return new AsYouGoValidator(form);
case CustomValidationTypes.ShowAllOnSubmit:
return new ShowAllOnSubmitValidator(form);
case CustomValidationTypes.AsYouGoShowAllOnSubmit:
Copy link
Contributor

Choose a reason for hiding this comment

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

How about we make custom-validation-reporting take multiple space-separated values, create a unified validator that takes the type of the validation(s) as a parameter and get rid of InteractAndSubmitValidator. I think the infrastructure is already there for this to happen and that way the syntax would be custom-validation-reporting = 'as-you-go show-all-on-submit'.

Copy link
Contributor

Choose a reason for hiding this comment

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

some combinations may not make sense like show-first-on-submit show-all-on-submit but this would additionally allow show-first-on-submit as-you-go which is nice. @cvializ is this possible/easy to implement?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that should be ok, it would just show all on submit.

return new InteractAndSubmitValidator(form);
case CustomValidationTypes.ShowFirstOnSubmit:
return new ShowFirstOnSubmitValidator(form);
}
Expand Down
67 changes: 67 additions & 0 deletions extensions/amp-form/0.1/test/test-form-validators.js
Expand Up @@ -21,6 +21,7 @@ import {
PolyfillDefaultValidator,
AsYouGoValidator,
ShowAllOnSubmitValidator,
InteractAndSubmitValidator,
ShowFirstOnSubmitValidator,
} from '../form-validators';
import {ValidationBubble} from '../validation-bubble';
Expand Down Expand Up @@ -111,6 +112,10 @@ describes.realWin('form-validators', {amp: true}, env => {
'custom-validation-reporting', 'show-all-on-submit');
expect(getFormValidator(form)).to.be.instanceOf(
ShowAllOnSubmitValidator);
form.setAttribute(
'custom-validation-reporting', 'interact-and-submit');
expect(getFormValidator(form)).to.be.instanceOf(
InteractAndSubmitValidator);
form.setAttribute(
'custom-validation-reporting', 'show-first-on-submit');
expect(getFormValidator(form)).to.be.instanceOf(
Expand Down Expand Up @@ -366,6 +371,68 @@ describes.realWin('form-validators', {amp: true}, env => {
expect(validations[1].className).to.not.contain('visible');
expect(validations[2].className).to.not.contain('visible');
});

describe('InteractAndSubmitValidator', () => {
it('should report validation for input on interaction', () => {
const doc = env.win.document;
const form = getForm(doc, true);
const validations = doc.querySelectorAll('[visible-when-invalid]');
const validator = new AsYouGoValidator(form);
form.elements[0].validationMessage = 'Name is required';
form.elements[1].validationMessage = 'Email is required';

validator.onBlur({target: form.elements[0]});
expect(validations[0].className).to.contain('visible');
expect(validations[1].className).to.not.contain('visible');
expect(validations[2].className).to.not.contain('visible');

form.elements[0].value = 'John Miller';
validator.onInput({target: form.elements[0]});
expect(validations[0].className).to.not.contain('visible');
expect(validations[1].className).to.not.contain('visible');
expect(validations[2].className).to.not.contain('visible');

validator.onInput({target: form.elements[1]});
expect(validations[0].className).to.not.contain('visible');
expect(validations[1].className).to.contain('visible');
expect(validations[2].className).to.not.contain('visible');
expect(validations[1].textContent).to.equal('Email is required');

form.elements[1].value = 'invalidemail';
form.elements[1].validationMessage = 'Email format is wrong';
validator.onInput({target: form.elements[1]});
expect(validations[0].className).to.not.contain('visible');
expect(validations[1].className).to.not.contain('visible');
expect(validations[2].className).to.contain('visible');
expect(validations[2].textContent).to.not.equal(
'Email format is wrong');
expect(validations[2].textContent).to.equal(emailTypeValidationMsg);

form.elements[1].value = 'valid@email.com';
validator.onBlur({target: form.elements[1]});
expect(validations[0].className).to.not.contain('visible');
expect(validations[1].className).to.not.contain('visible');
expect(validations[2].className).to.not.contain('visible');
});

it('should report on interaction for non-active inputs on submit', () => {
const doc = env.win.document;
const form = getForm(doc, true);
const validations = doc.querySelectorAll('[visible-when-invalid]');
const validator = new ShowAllOnSubmitValidator(form);
validator.report();

expect(doc.activeElement).to.equal(form.elements[0]);
expect(validations[0].className).to.contain('visible');
expect(validations[1].className).to.contain('visible');
expect(validations[2].className).to.not.contain('visible');

expect(doc.activeElement).to.equal(form.elements[0]);
expect(validations[0].className).to.contain('visible');
expect(validations[1].className).to.contain('visible');
expect(validations[2].className).to.not.contain('visible');
});
});
});

});