* backport #1707 & #1708

* prettier

* fix prettier log level

* fix test

* backport #1707 & #1708
This commit is contained in:
Konnor Rogers
2023-11-08 15:20:06 -05:00
committed by GitHub
parent 1b654c7c85
commit b5d9b49b27
6 changed files with 124 additions and 24 deletions

View File

@@ -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 `<sl-color-picker>`. The `<sl-change>` 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]

View File

@@ -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",

View File

@@ -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');

21
src/internal/form.test.ts Normal file
View File

@@ -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<HTMLFormElement>(html`
<form>
<wa-input name="name" label="Name" required></wa-input>
<wa-textarea name="comment" label="Comment" required></wa-textarea>
</form>
`);
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);
});

View File

@@ -14,6 +14,7 @@ export const formCollections: WeakMap<HTMLFormElement, Set<WebAwesomeFormControl
// restore the original behavior when they disconnect.
//
const reportValidityOverloads: WeakMap<HTMLFormElement, () => boolean> = new WeakMap();
const checkValidityOverloads: WeakMap<HTMLFormElement, () => 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, `<sl-button>` 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<HTMLInputElement>('*');
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

View File

@@ -44,6 +44,7 @@ export function runFormControlBaseTests<T extends WebAwesomeFormControl = WebAwe
// - `.checkValidity()`
// - `.reportValidity()`
// - `.setCustomValidity(msg)`
// - `.getForm()`
//
function runAllValidityTests(
tagName: string, //
@@ -124,6 +125,27 @@ function runAllValidityTests(
const emittedEvents = checkEventEmissions(control, 'wa-invalid', () => 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(`<form id='${formId}'></form>`);
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(`<form id='${formId}'></form>`);
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