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

Conversation

Totati
Copy link

@Totati Totati commented Jul 25, 2024

Fixes #56
I've migrated the selectors to support both the new :state(checked) and the old :--checked selector leaving the polyfilled syntax intact.

Copy link
Collaborator

@calebdwilliams calebdwilliams left a comment

Choose a reason for hiding this comment

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

Some nits.

@@ -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.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ElementInternals states are now widely supported
2 participants