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

Remove unnecessary testing of support and add implement new syntax #57

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"@web/test-runner-commands": "^0.5.13",
"@web/test-runner-playwright": "^0.8.8",
"construct-style-sheets-polyfill": "^3.0.5",
"element-internals-polyfill": "^1.1.19",
"element-internals-polyfill": "^1.3.11",
"eslint": "^7.32.0",
"eslint-config-prettier": "^8.3.0",
"eslint-plugin-simple-import-sort": "^7.0.0",
Expand Down
6 changes: 3 additions & 3 deletions packages/form-control/demo/complex-demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ input {
font-size: 16px;
padding: 4px;
}
/** Default invalid state */
:host(:--show-error) input {
/** Default invalid state with fallback https://developer.mozilla.org/en-US/docs/Web/API/CustomStateSet#using_a_try...catch_block */
:host(:is(:--show-error, :state(show-error))) input {
border-color: red;
}
:host(:--show-error) span {
:host(:is(:--show-error, :state(show-error))) span {
color: red;
}

Expand Down
14 changes: 7 additions & 7 deletions packages/form-control/demo/switch.style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ sheet.replace(`
top: 1px;
left: 1px;
}
:host(:not(:--checked):hover), :host(:not(:--checked):focus) {
:host(:not(:is(:--checked, :state(checked))):hover), :host(:not(:is(:--checked, :state(checked))):focus) {
background: #cccccc;
}
:host(:not([state--checked]):hover), :host(:not([state--checked]):focus) {
background: #cccccc;
}
:host(:not(:--checked):active) {
:host(:not(:is(:--checked, :state(checked))):active) {
background: #bbbbbb;
}
:host(:not([state--checked]):active) {
Expand All @@ -44,31 +44,31 @@ sheet.replace(`
:host(:active)::after {
background: #eeeeee;
}
:host(:--checked) {
:host(:is(:--checked, :state(checked))) {
background: ForestGreen;
}
:host([state--checked]) {
background: ForestGreen;
}
:host(:--checked:hover) {
:host(:is(:--checked, :state(checked)):hover) {
background: Green;
}
:host([state--checked]:hover) {
background: Green;
}
:host(:--checked:focus) {
:host(:is(:--checked, :state(checked)):focus) {
background: Green;
}
:host([state--checked]:focus) {
background: Green;
}
:host(:--checked:active) {
:host(:is(:--checked, :state(checked)):active) {
background: DarkGreen;
}
:host([state--checked]:active) {
background: DarkGreen;
}
:host(:--checked)::after {
:host(:is(:--checked, :state(checked)))::after {
left: calc(100% - 24px);
}
:host([state--checked])::after {
Expand Down
17 changes: 14 additions & 3 deletions packages/form-control/demo/switch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,22 @@ export class DemoSwitch extends FormControlMixin(LitElement) {
}

protected updated(changed: Map<string, unknown>): void {
if (changed.has('value') || changed.has('checked')) {
const checked: any = 'checked';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove the type annotation it’s unnecessary and not accurate.

if (changed.has('value') || changed.has(checked)) {
this.setValue(this.value);
}
if (changed.has('checked')) {
this.internals.states[this.checked ? 'add' : 'delete']('--checked');
if (changed.has(checked)) {
const states = this.internals.states;
if(this.checked) {
try {
states.add(checked);
} catch {
states.add(`--${checked}`);
}
} else {
states.delete(checked);
states.delete(`--${checked}`);
}
}
}
}
53 changes: 30 additions & 23 deletions packages/form-control/src/FormControlMixin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export function FormControlMixin<
*/
declare static formControlValidators: Validator[];

/**
/**
* If set to true the control described should be evaluated and validated
* as part of a group. Like a radio, if any member of the group's validity
* changes the the other members should update as well.
Expand All @@ -35,7 +35,7 @@ export function FormControlMixin<
* validator will be evaluated whenever the host's required attribute
* is updated.
*/
static get observedAttributes(): string[] {
static get observedAttributes(): string[] {
const validatorAttributes = this.validators.map((validator) => validator.attribute).flat();

const observedAttributes = super.observedAttributes || [];
Expand Down Expand Up @@ -286,10 +286,10 @@ export function FormControlMixin<
*/
declare validityCallback: (validationKey: string) => string | void;

/**
* Called when the control's validationMessage should be changed
* @param message { string } - The new validation message
*/
/**
* Called when the control's validationMessage should be changed
* @param message { string } - The new validation message
*/
declare validationMessageCallback: (message: string) => void;

/**
Expand Down Expand Up @@ -318,7 +318,7 @@ export function FormControlMixin<

/**
* Check to see if an error should be shown. This method will also
* update the internals state object with the --show-error state
* update the internals state object with the show-error state
* if necessary.
* @private
*/
Expand All @@ -327,16 +327,23 @@ export function FormControlMixin<
return false;
}

const showError = this.#forceError || (this.#touched && !this.validity.valid && !this.#focused);
const showError = this.#forceError || (this.#touched && !this.validity.valid && !this.#focused),
states = this.internals.states,
// Casted to any, because the polyfill forces dashed-ident syntax https://github.com/calebdwilliams/element-internals-polyfill/issues/126
// TODO: Remove any cast when the issue is resolved
state: any = 'show-error';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove inaccurate type annotation.

Copy link
Author

Choose a reason for hiding this comment

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

There's a comment and a TODO explaining why it's necessary. To make it work I should remove the import { IElementInternals } from "element-internals-polyfill"; from form-control types.


/**
* At the time of writing Firefox doesn't support states
* TODO: Remove when check for states when fully support is in place
*/
if (showError && this.internals.states) {
this.internals.states.add('--show-error');
} else if (this.internals.states) {
this.internals.states.delete('--show-error');
// https://developer.mozilla.org/en-US/docs/Web/API/CustomStateSet#compability_with_dashed-ident_syntax

if (showError) {
try {
states.add(state);
} catch {
states.add(`--${state}`);
}
} else {
states.delete(state);
states.delete(`--${state}`);
}

return showError;
Expand Down Expand Up @@ -419,12 +426,12 @@ export function FormControlMixin<
/** Once all the async validators have settled, resolve validationComplete */
Promise.allSettled(asyncValidators)
.then(() => {
/** Don't resolve validations if the signal is aborted */
if (!abortController?.signal.aborted) {
this.#isValidationPending = false;
this.#validationCompleteResolver?.();
}
});
/** Don't resolve validations if the signal is aborted */
if (!abortController?.signal.aborted) {
this.#isValidationPending = false;
this.#validationCompleteResolver?.();
}
});

/**
* If async validators are present:
Expand Down Expand Up @@ -484,7 +491,7 @@ export function FormControlMixin<
}
}

/** Reset control state when the form is reset */
/** Reset control state when the form is reset */
formResetCallback() {
this.#touched = false;
this.#forceError = false;
Expand Down