diff --git a/docs/pages/resources/changelog.md b/docs/pages/resources/changelog.md index 17ce362c4..f692203ab 100644 --- a/docs/pages/resources/changelog.md +++ b/docs/pages/resources/changelog.md @@ -22,6 +22,9 @@ New versions of Web Awesome are released as-needed and generally occur when a cr ## Next +- Added the ability to call `form.checkValidity()` and it will use Shoelace's custom `checkValidity()` handler. [#1708] +- Fixed a bug with form controls removing the custom validity handlers from the form. [#1708] +- Fixed a bug in form control components that used a `form` property, but not an attribute. [#1707] - Fixed a bug with bundled components using CDN builds not having translations on initial connect [#1696] - Fixed a bug where the `"sl-change"` event would always fire simultaneously with `"sl-input"` event in ``. The `` event now only fires when a user stops dragging a slider or stops dragging on the color canvas. [#1689] - Updated the copy icon in the system library [#1702] diff --git a/package.json b/package.json index 48dc17402..e5ac09756 100644 --- a/package.json +++ b/package.json @@ -46,8 +46,8 @@ "build": "node scripts/build.js", "verify": "npm run prettier:check && npm run lint && npm run build && npm run test", "prepublishOnly": "npm run verify", - "prettier": "prettier --write --loglevel warn .", - "prettier:check": "prettier --check --loglevel warn .", + "prettier": "prettier --write --log-level warn .", + "prettier:check": "prettier --check --log-level warn .", "lint": "eslint src --max-warnings 0", "lint:fix": "eslint src --max-warnings 0 --fix", "create": "plop --plopfile scripts/plop/plopfile.js", diff --git a/src/components/button/button.component.ts b/src/components/button/button.component.ts index 7753116d2..f3686dfb6 100644 --- a/src/components/button/button.component.ts +++ b/src/components/button/button.component.ts @@ -60,18 +60,6 @@ export default class WaButton extends WebAwesomeElement implements WebAwesomeFor }; private readonly formControlController = new FormControlController(this, { - form: input => { - // Buttons support a form attribute that points to an arbitrary form, so if this attribute is set we need to query - // the form from the same root using its id - if (input.hasAttribute('form')) { - const doc = input.getRootNode() as Document | ShadowRoot; - const formId = input.getAttribute('form')!; - return doc.getElementById(formId) as HTMLFormElement; - } - - // Fall back to the closest containing form - return input.closest('form'); - }, assumeInteractionOn: ['click'] }); private readonly hasSlotController = new HasSlotController(this, '[default]', 'prefix', 'suffix'); diff --git a/src/internal/form.test.ts b/src/internal/form.test.ts new file mode 100644 index 000000000..382858e70 --- /dev/null +++ b/src/internal/form.test.ts @@ -0,0 +1,21 @@ +import '../../../dist/webawesome.js'; + +import { expect, fixture, html } from '@open-wc/testing'; + +// Reproduction of this issue: https://github.com/shoelace-style/shoelace/issues/1703 +it('Should still run form validations if an element is removed', async () => { + const form = await fixture(html` +
+ + +
+ `); + + expect(form.checkValidity()).to.equal(false); + expect(form.reportValidity()).to.equal(false); + + form.querySelector('wa-input')!.remove(); + + expect(form.checkValidity()).to.equal(false); + expect(form.reportValidity()).to.equal(false); +}); diff --git a/src/internal/form.ts b/src/internal/form.ts index 0525f7444..57c1cc993 100644 --- a/src/internal/form.ts +++ b/src/internal/form.ts @@ -14,6 +14,7 @@ export const formCollections: WeakMap boolean> = new WeakMap(); +const checkValidityOverloads: WeakMap boolean> = new WeakMap(); // // We store a Set of controls that users have interacted with. This allows us to determine the interaction state @@ -42,6 +43,12 @@ export interface FormControlControllerOptions { * prevent submission and trigger the browser's constraint violation warning. */ reportValidity: (input: WebAwesomeFormControl) => boolean; + + /** + * A function that maps to the form control's `checkValidity()` function. When the control is invalid, this will return false. + * this is helpful is you want to check validation without triggering the native browser constraint violation warning. + */ + checkValidity: (input: WebAwesomeFormControl) => boolean; /** A function that sets the form control's value */ setValue: (input: WebAwesomeFormControl, value: unknown) => void; /** @@ -61,12 +68,16 @@ export class FormControlController implements ReactiveController { this.options = { form: input => { // If there's a form attribute, use it to find the target form by id - if (input.hasAttribute('form') && input.getAttribute('form') !== '') { - const root = input.getRootNode() as Document | ShadowRoot; - const formId = input.getAttribute('form'); + // Controls may not always reflect the 'form' property. For example, `` doesn't reflect. + const formId = input.form; - if (formId) { - return root.getElementById(formId) as HTMLFormElement; + if (formId) { + const root = input.getRootNode() as Document | ShadowRoot; + + const form = root.getElementById(formId); + + if (form) { + return form as HTMLFormElement; } } @@ -77,6 +88,7 @@ export class FormControlController implements ReactiveController { defaultValue: input => input.defaultValue, disabled: input => input.disabled ?? false, reportValidity: input => (typeof input.reportValidity === 'function' ? input.reportValidity() : true), + checkValidity: input => (typeof input.checkValidity === 'function' ? input.checkValidity() : true), setValue: (input, value: string) => (input.value = value), assumeInteractionOn: ['wa-input'], ...options @@ -146,16 +158,34 @@ export class FormControlController implements ReactiveController { reportValidityOverloads.set(this.form, this.form.reportValidity); this.form.reportValidity = () => this.reportFormValidity(); } + + // Overload the form's checkValidity() method so it looks at Web Awesome form controls + if (!checkValidityOverloads.has(this.form)) { + checkValidityOverloads.set(this.form, this.form.checkValidity); + this.form.checkValidity = () => this.checkFormValidity(); + } } else { this.form = undefined; } } private detachForm() { - if (this.form) { - // Remove this element from the form's collection - formCollections.get(this.form)?.delete(this.host); + if (!this.form) return; + const formCollection = formCollections.get(this.form); + + if (!formCollection) { + return; + } + + // Remove this host from the form's collection + formCollection.delete(this.host); + + // Check to make sure there's no other form controls in the collection. If we do this + // without checking if any other controls are still in the collection, then we will wipe out the + // validity checks for all other elements. + // see: https://github.com/shoelace-style/shoelace/issues/1703 + if (formCollection.size <= 0) { this.form.removeEventListener('formdata', this.handleFormData); this.form.removeEventListener('submit', this.handleFormSubmit); this.form.removeEventListener('reset', this.handleFormReset); @@ -165,9 +195,17 @@ export class FormControlController implements ReactiveController { this.form.reportValidity = reportValidityOverloads.get(this.form)!; reportValidityOverloads.delete(this.form); } - } - this.form = undefined; + if (checkValidityOverloads.has(this.form)) { + this.form.checkValidity = checkValidityOverloads.get(this.form)!; + checkValidityOverloads.delete(this.form); + } + + // So it looks weird here to not always set the form to undefined. But I _think_ if we unattach this.form here, + // we end up in this fun spot where future validity checks don't have a reference to the form validity handler. + // First form element in sets the validity handler. So we can't clean up `this.form` until there are no other form elements in the form. + this.form = undefined; + } } private handleFormData = (event: FormDataEvent) => { @@ -226,6 +264,34 @@ export class FormControlController implements ReactiveController { } }; + private checkFormValidity = () => { + // + // This is very similar to the `reportFormValidity` function, but it does not trigger native constraint validation + // Allow the user to simply check if the form is valid and handling validity in their own way. + // + // 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, Web Awesome inputs, and other custom + // elements that support the constraint validation API. + const elements = this.form.querySelectorAll('*'); + + for (const element of elements) { + if (typeof element.checkValidity === 'function') { + if (!element.checkValidity()) { + return false; + } + } + } + } + + return true; + }; + private reportFormValidity = () => { // // Web Awesome form controls work hard to act like regular form controls. They support the Constraint Validation API diff --git a/src/internal/test/form-control-base-tests.ts b/src/internal/test/form-control-base-tests.ts index c3268d195..1e7df93de 100644 --- a/src/internal/test/form-control-base-tests.ts +++ b/src/internal/test/form-control-base-tests.ts @@ -44,6 +44,7 @@ export function runFormControlBaseTests control.reportValidity()); expect(emittedEvents.length).to.equal(0); }); + + it('Should find the correct form when given a form property', async () => { + const formId = 'test-form'; + const form = await fixture(`
`); + const control = await createControl(); + expect(control.getForm()).to.equal(null); + control.form = 'test-form'; + await control.updateComplete; + expect(control.getForm()).to.equal(form); + }); + + it('Should find the correct form when given a form attribute', async () => { + const formId = 'test-form'; + const form = await fixture(`
`); + const control = await createControl(); + expect(control.getForm()).to.equal(null); + control.setAttribute('form', 'test-form'); + + await control.updateComplete; + expect(control.getForm()).to.equal(form); + }); } // Run special tests depending on component type