From af1e4401037bfb1a4fd5be2739d6987cefc7d36a Mon Sep 17 00:00:00 2001 From: Cory LaViska Date: Wed, 1 Jun 2022 09:23:06 -0400 Subject: [PATCH] fixes #772 --- docs/resources/changelog.md | 3 +- src/components/input/input.test.ts | 35 +++++++++++++++++++++++ src/internal/form.ts | 46 ++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/docs/resources/changelog.md b/docs/resources/changelog.md index 9a70d74e8..70fdcfc35 100644 --- a/docs/resources/changelog.md +++ b/docs/resources/changelog.md @@ -8,7 +8,8 @@ _During the beta period, these restrictions may be relaxed in the event of a mis ## Next -- Fix focus rings for ``, ``, and `` in Safari since they don't use `:focus-visible` [#767](https://github.com/shoelace-style/shoelace/issues/767) +- Fixed focus rings for ``, ``, and `` in Safari since they don't use `:focus-visible` [#767](https://github.com/shoelace-style/shoelace/issues/767) +- Fixed a bug where calling `HTMLFormElement.reportValidity()` would skip Shoelace form controls [#772](https://github.com/shoelace-style/shoelace/issues/772) - Revert menu item caching due to regression [#766](https://github.com/shoelace-style/shoelace/issues/766) ## 2.0.0-beta.74 diff --git a/src/components/input/input.test.ts b/src/components/input/input.test.ts index 3c278713d..7c1fc35cb 100644 --- a/src/components/input/input.test.ts +++ b/src/components/input/input.test.ts @@ -123,4 +123,39 @@ describe('', () => { expect(submitHandler).to.not.have.been.called; }); }); + + describe('when calling HTMLFormElement.reportValidity()', () => { + it('should be invalid when the input is empty and form.reportValidity() is called', async () => { + const form = await fixture(html` +
+ + Submit +
+ `); + + expect(form.reportValidity()).to.be.false; + }); + + it('should be valid when the input is empty, reportValidity() is called, and the form has novalidate', async () => { + const form = await fixture(html` +
+ + Submit +
+ `); + + expect(form.reportValidity()).to.be.true; + }); + + it('should be invalid when a native input is empty and form.reportValidity() is called', async () => { + const form = await fixture(html` +
+ + Submit +
+ `); + + expect(form.reportValidity()).to.be.false; + }); + }); }); diff --git a/src/internal/form.ts b/src/internal/form.ts index f92da79dd..a82bc47f0 100644 --- a/src/internal/form.ts +++ b/src/internal/form.ts @@ -2,6 +2,8 @@ import './formdata-event-polyfill'; import type SlButton from '../components/button/button'; import type { ReactiveController, ReactiveControllerHost } from 'lit'; +const formOverloads: WeakMap boolean> = new WeakMap(); + export interface FormSubmitControllerOptions { /** A function that returns the form containing the form control. */ form: (input: unknown) => HTMLFormElement | null; @@ -37,6 +39,7 @@ export class FormSubmitController implements ReactiveController { }; this.handleFormData = this.handleFormData.bind(this); this.handleFormSubmit = this.handleFormSubmit.bind(this); + this.reportFormValidity = this.reportFormValidity.bind(this); } hostConnected() { @@ -45,6 +48,12 @@ export class FormSubmitController implements ReactiveController { if (this.form) { this.form.addEventListener('formdata', this.handleFormData); this.form.addEventListener('submit', this.handleFormSubmit); + + // Overload the form's reportValidity() method so it looks at Shoelace form controls + if (!formOverloads.has(this.form)) { + formOverloads.set(this.form, this.form.reportValidity); + this.form.reportValidity = () => this.reportFormValidity(); + } } } @@ -52,6 +61,13 @@ export class FormSubmitController implements ReactiveController { if (this.form) { this.form.removeEventListener('formdata', this.handleFormData); this.form.removeEventListener('submit', this.handleFormSubmit); + + // Remove the overload and restore the original method + if (formOverloads.has(this.form)) { + this.form.reportValidity = formOverloads.get(this.form)!; + formOverloads.delete(this.form); + } + this.form = undefined; } } @@ -82,6 +98,36 @@ export class FormSubmitController implements ReactiveController { } } + reportFormValidity() { + // + // Shoelace form controls work hard to act like regular form controls. They support the Constraint Validation API + // and its associated methods such as setCustomValidity() and reportValidity(). However, the HTMLFormElement also + // has a reportValidity() method that will trigger validation on all child controls. Since we're not yet using + // ElementInternals, we need to overload this method so it looks for any element with the reportValidity() method. + // + // We preserve the original method in a WeakMap, but we don't call it from the overload because that would trigger + // validations in an unexpected order. When the element disconnects, we revert to the original behavior. This won't + // be necessary once we can use ElementInternals. + // + // Note that we're also honoring the form's novalidate attribute. + // + if (this.form && !this.form.noValidate) { + // This seems sloppy, but checking all elements will cover native inputs, Shoelace inputs, and other custom + // elements that support the constraint validation API. + const elements = this.form.querySelectorAll('*'); + + for (const element of elements) { + if (typeof element.reportValidity === 'function') { + if (!element.reportValidity()) { + return false; + } + } + } + } + + return true; + } + /** Submits the form, triggering validation and form data injection. */ submit(submitter?: HTMLInputElement | SlButton) { // Calling form.submit() bypasses the submit event and constraint validation. To prevent this, we can inject a