From 907ac0087ae1e73bb1be4fdfecc81fe9accdf214 Mon Sep 17 00:00:00 2001 From: Steven Spriggs Date: Tue, 5 Mar 2024 12:07:18 -0500 Subject: [PATCH 01/19] fix(switch): implement internals controller --- elements/pf-switch/BaseSwitch.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/elements/pf-switch/BaseSwitch.ts b/elements/pf-switch/BaseSwitch.ts index 826c93b75b..6c583bafc1 100644 --- a/elements/pf-switch/BaseSwitch.ts +++ b/elements/pf-switch/BaseSwitch.ts @@ -1,7 +1,9 @@ import { LitElement, html } from 'lit'; import { property } from 'lit/decorators/property.js'; -import styles from './BaseSwitch.css'; +import { InternalsController } from '@patternfly/pfe-core/controllers/internals-controller.js'; + +import styles from './BaseSwitch.css'; /** * Switch */ @@ -14,7 +16,7 @@ export abstract class BaseSwitch extends LitElement { declare shadowRoot: ShadowRoot; - #internals = this.attachInternals(); + #internals = InternalsController.of(this, { role: 'switch' }); #initiallyDisabled = this.hasAttribute('disabled'); @@ -32,7 +34,7 @@ export abstract class BaseSwitch extends LitElement { override connectedCallback(): void { super.connectedCallback(); - this.setAttribute('role', 'checkbox'); + this.addEventListener('click', this.#onClick); this.addEventListener('keyup', this.#onKeyup); this.#updateLabels(); @@ -53,7 +55,7 @@ export abstract class BaseSwitch extends LitElement { `; } - override updated() { + override willUpdate() { this.#internals.ariaChecked = String(this.checked); this.#internals.ariaDisabled = String(this.disabled); } From 58ccc155f9e5e76a83931632ec276c55c76b2c12 Mon Sep 17 00:00:00 2001 From: Steven Spriggs Date: Thu, 21 Mar 2024 14:13:00 -0400 Subject: [PATCH 02/19] fix(switch): add stopPropagation to stop window scroll --- elements/pf-switch/BaseSwitch.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/elements/pf-switch/BaseSwitch.ts b/elements/pf-switch/BaseSwitch.ts index 6c583bafc1..e51a375c04 100644 --- a/elements/pf-switch/BaseSwitch.ts +++ b/elements/pf-switch/BaseSwitch.ts @@ -81,6 +81,7 @@ export abstract class BaseSwitch extends LitElement { case ' ': case 'Enter': event.preventDefault(); + event.stopPropagation(); this.#toggle(); } } From ec93e11859949af055a1acb6e11d371c372fc701 Mon Sep 17 00:00:00 2001 From: Steven Spriggs Date: Thu, 21 Mar 2024 14:16:59 -0400 Subject: [PATCH 03/19] fix(switch): move stopPropagation to keyDown --- elements/pf-switch/BaseSwitch.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/elements/pf-switch/BaseSwitch.ts b/elements/pf-switch/BaseSwitch.ts index e51a375c04..0ded7dd535 100644 --- a/elements/pf-switch/BaseSwitch.ts +++ b/elements/pf-switch/BaseSwitch.ts @@ -37,6 +37,7 @@ export abstract class BaseSwitch extends LitElement { this.addEventListener('click', this.#onClick); this.addEventListener('keyup', this.#onKeyup); + this.addEventListener('keydown', this.#onKeyDown); this.#updateLabels(); } @@ -86,6 +87,14 @@ export abstract class BaseSwitch extends LitElement { } } + #onKeyDown(event: KeyboardEvent) { + switch (event.key) { + case ' ': + event.preventDefault(); + event.stopPropagation(); + } + } + #toggle() { if (this.disabled) { return; From a45715dd0a472fb88d0cd1a64a55baa3c49ceadc Mon Sep 17 00:00:00 2001 From: Steven Spriggs Date: Thu, 21 Mar 2024 14:20:43 -0400 Subject: [PATCH 04/19] test(switch): update role to switch --- elements/pf-switch/test/pf-switch.spec.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/elements/pf-switch/test/pf-switch.spec.ts b/elements/pf-switch/test/pf-switch.spec.ts index 3670571b66..80f65e2b6f 100644 --- a/elements/pf-switch/test/pf-switch.spec.ts +++ b/elements/pf-switch/test/pf-switch.spec.ts @@ -30,10 +30,10 @@ describe('', function() { .to.be.an.instanceOf(PfSwitch); }); it('has accessible role', function() { - expect(snapshot.role).to.equal('checkbox'); + expect(snapshot.role).to.equal('switch'); }); it('has accessible checked field', function() { - expect(snapshot.role).to.equal('checkbox'); + expect(snapshot.role).to.equal('switch'); }); it('requires accessible name', function() { // Double negative - this would fail an accessibility audit, @@ -59,7 +59,7 @@ describe('', function() { }); it('is accessible', function() { - expect(snapshot.role).to.equal('checkbox'); + expect(snapshot.role).to.equal('switch'); expect(snapshot.name).to.be.ok; expect(snapshot.checked).to.be.false; }); @@ -68,7 +68,7 @@ describe('', function() { expect(snapshot.name).to.equal('Message when off'); }); - describe('clicking the checkbox', function() { + describe('clicking the switch', function() { beforeEach(async function() { element.click(); await element.updateComplete; From 9ccd718a0b16738f66d88e8cb3e9b706d43b7321 Mon Sep 17 00:00:00 2001 From: Steven Spriggs Date: Thu, 21 Mar 2024 14:34:46 -0400 Subject: [PATCH 05/19] fix(switch): update demo to use spans in a single label --- elements/pf-switch/demo/pf-switch.html | 30 +++++++++++++++++--------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/elements/pf-switch/demo/pf-switch.html b/elements/pf-switch/demo/pf-switch.html index 23d8f022d3..5407c73a3a 100644 --- a/elements/pf-switch/demo/pf-switch.html +++ b/elements/pf-switch/demo/pf-switch.html @@ -4,8 +4,10 @@
Option A - - +
@@ -16,20 +18,28 @@
Form Disabled State - - + - - + - - + - - +
From cc1de74f1106819a4e6ec2dfcbeef8a7e6a23963 Mon Sep 17 00:00:00 2001 From: Steven Spriggs Date: Thu, 21 Mar 2024 14:46:46 -0400 Subject: [PATCH 06/19] fix(switch): updateLabel now updates child spans in the label --- elements/pf-switch/BaseSwitch.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/elements/pf-switch/BaseSwitch.ts b/elements/pf-switch/BaseSwitch.ts index 0ded7dd535..0ef3e95d70 100644 --- a/elements/pf-switch/BaseSwitch.ts +++ b/elements/pf-switch/BaseSwitch.ts @@ -107,10 +107,13 @@ export abstract class BaseSwitch extends LitElement { #updateLabels() { const labelState = this.checked ? 'on' : 'off'; - if (this.labels.length > 1) { - for (const label of this.labels) { - label.hidden = label.dataset.state !== labelState; - } - } + this.labels.forEach(label => { + const spans = label.querySelectorAll('span'); + spans.forEach(span => { + if (span) { + span.hidden = span.dataset.state !== labelState; + } + }); + }); } } From f397f54d891aa38aef07092e86f143d52ea01f3d Mon Sep 17 00:00:00 2001 From: Steven Spriggs Date: Thu, 21 Mar 2024 14:50:28 -0400 Subject: [PATCH 07/19] fix(switch): use data-state attr as selector instead of span --- elements/pf-switch/BaseSwitch.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/elements/pf-switch/BaseSwitch.ts b/elements/pf-switch/BaseSwitch.ts index 0ef3e95d70..25fa8b9027 100644 --- a/elements/pf-switch/BaseSwitch.ts +++ b/elements/pf-switch/BaseSwitch.ts @@ -108,10 +108,10 @@ export abstract class BaseSwitch extends LitElement { #updateLabels() { const labelState = this.checked ? 'on' : 'off'; this.labels.forEach(label => { - const spans = label.querySelectorAll('span'); - spans.forEach(span => { - if (span) { - span.hidden = span.dataset.state !== labelState; + const states = label.querySelectorAll('[data-state]'); + states.forEach(state => { + if (state) { + state.hidden = state.dataset.state !== labelState; } }); }); From 23cc3c2b1d8deb93eeb3440bf75d24028f666512 Mon Sep 17 00:00:00 2001 From: Steven Spriggs Date: Thu, 21 Mar 2024 14:53:52 -0400 Subject: [PATCH 08/19] fix(switch): update all demos to use single label --- elements/pf-switch/demo/checked.html | 6 ++++-- elements/pf-switch/demo/disabled.html | 12 ++++++++---- elements/pf-switch/demo/reversed.html | 6 ++++-- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/elements/pf-switch/demo/checked.html b/elements/pf-switch/demo/checked.html index cb009030d3..c4e718db5f 100644 --- a/elements/pf-switch/demo/checked.html +++ b/elements/pf-switch/demo/checked.html @@ -3,8 +3,10 @@
Checked with label - - +
diff --git a/elements/pf-switch/demo/disabled.html b/elements/pf-switch/demo/disabled.html index 7cd8459733..28f3862fb0 100644 --- a/elements/pf-switch/demo/disabled.html +++ b/elements/pf-switch/demo/disabled.html @@ -3,13 +3,17 @@
Checked and Disabled - - +
- - +
diff --git a/elements/pf-switch/demo/reversed.html b/elements/pf-switch/demo/reversed.html index 3cd351e6c6..d98da7665e 100644 --- a/elements/pf-switch/demo/reversed.html +++ b/elements/pf-switch/demo/reversed.html @@ -2,8 +2,10 @@
Reversed - - +
From 37bf1f545cfcd864cdf4a03f0301f4c255cb387e Mon Sep 17 00:00:00 2001 From: Steven Spriggs Date: Thu, 21 Mar 2024 15:01:36 -0400 Subject: [PATCH 09/19] fix(switch): add aria-hidden to svg --- elements/pf-switch/BaseSwitch.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elements/pf-switch/BaseSwitch.ts b/elements/pf-switch/BaseSwitch.ts index 25fa8b9027..5fd6d6f068 100644 --- a/elements/pf-switch/BaseSwitch.ts +++ b/elements/pf-switch/BaseSwitch.ts @@ -49,7 +49,7 @@ export abstract class BaseSwitch extends LitElement { override render() { return html`
- +
From 05b5c917677c76bd648851e9684e2d381fe7b67a Mon Sep 17 00:00:00 2001 From: Steven Spriggs Date: Thu, 21 Mar 2024 15:12:04 -0400 Subject: [PATCH 10/19] test(switch): update tests to use single label --- elements/pf-switch/test/pf-switch.spec.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/elements/pf-switch/test/pf-switch.spec.ts b/elements/pf-switch/test/pf-switch.spec.ts index 80f65e2b6f..8741d7b8dd 100644 --- a/elements/pf-switch/test/pf-switch.spec.ts +++ b/elements/pf-switch/test/pf-switch.spec.ts @@ -50,8 +50,10 @@ describe('', function() { const container = await createFixture(html`
- - +
`); element = container.querySelector('pf-switch')!; @@ -106,8 +108,10 @@ describe('', function() { const container = await createFixture(html`
- - +
`); element = container.querySelector('pf-switch')!; @@ -125,8 +129,10 @@ describe('', function() { beforeEach(async function() { element = await createFixture(html` - - + `); }); it('should display a check icon', async function() { From 5dbdb14da9b3c10c46dabb5206c81b9e54738950 Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Sun, 24 Mar 2024 10:40:57 +0200 Subject: [PATCH 11/19] fix(switch): try to improve SR support --- elements/pf-switch/BaseSwitch.ts | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/elements/pf-switch/BaseSwitch.ts b/elements/pf-switch/BaseSwitch.ts index 5fd6d6f068..0c3c985700 100644 --- a/elements/pf-switch/BaseSwitch.ts +++ b/elements/pf-switch/BaseSwitch.ts @@ -10,23 +10,21 @@ import styles from './BaseSwitch.css'; export abstract class BaseSwitch extends LitElement { static readonly styles = [styles]; - static readonly shadowRootOptions = { ...LitElement.shadowRootOptions, delegatesFocus: true, }; - static readonly formAssociated = true; + static override readonly shadowRootOptions = { ...LitElement.shadowRootOptions, delegatesFocus: true }; + declare shadowRoot: ShadowRoot; #internals = InternalsController.of(this, { role: 'switch' }); - #initiallyDisabled = this.hasAttribute('disabled'); - @property({ reflect: true }) label?: string; @property({ reflect: true, type: Boolean, attribute: 'show-check-icon' }) showCheckIcon = false; @property({ reflect: true, type: Boolean }) checked = false; - disabled = this.#initiallyDisabled; + @property({ reflect: true, type: Boolean }) disabled = false; get labels(): NodeListOf { return this.#internals.labels as NodeListOf; @@ -34,7 +32,6 @@ export abstract class BaseSwitch extends LitElement { override connectedCallback(): void { super.connectedCallback(); - this.addEventListener('click', this.#onClick); this.addEventListener('keyup', this.#onKeyup); this.addEventListener('keydown', this.#onKeyDown); @@ -43,22 +40,27 @@ export abstract class BaseSwitch extends LitElement { formDisabledCallback(disabled: boolean) { this.disabled = disabled; - this.requestUpdate(); } override render() { return html`
-
`; } override willUpdate() { - this.#internals.ariaChecked = String(this.checked); - this.#internals.ariaDisabled = String(this.disabled); + this.ariaChecked = String(!!this.checked); + this.ariaDisabled = String(!!this.disabled); } #onClick(event: Event) { From ef26ff436d98aa246ca146cb324960c0ddec136b Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Wed, 27 Mar 2024 16:43:24 +0200 Subject: [PATCH 12/19] chore: debug demos --- elements/pf-switch/demo/pf-switch.html | 3 ++- elements/pf-switch/demo/without-label.html | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/elements/pf-switch/demo/pf-switch.html b/elements/pf-switch/demo/pf-switch.html index 5407c73a3a..69f552659d 100644 --- a/elements/pf-switch/demo/pf-switch.html +++ b/elements/pf-switch/demo/pf-switch.html @@ -45,7 +45,8 @@ From ae1e72d8169061b0cefda0e167532585648be338 Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Wed, 27 Mar 2024 16:56:37 +0200 Subject: [PATCH 13/19] chore(switch): debug tabindex --- elements/pf-switch/BaseSwitch.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/elements/pf-switch/BaseSwitch.ts b/elements/pf-switch/BaseSwitch.ts index 0c3c985700..c41df99de1 100644 --- a/elements/pf-switch/BaseSwitch.ts +++ b/elements/pf-switch/BaseSwitch.ts @@ -32,6 +32,7 @@ export abstract class BaseSwitch extends LitElement { override connectedCallback(): void { super.connectedCallback(); + this.tabIndex = 0; this.addEventListener('click', this.#onClick); this.addEventListener('keyup', this.#onKeyup); this.addEventListener('keydown', this.#onKeyDown); @@ -44,7 +45,7 @@ export abstract class BaseSwitch extends LitElement { override render() { return html` -
+
Date: Wed, 27 Mar 2024 17:12:42 +0200 Subject: [PATCH 14/19] fix(switch): tabindex attr on host --- elements/pf-switch/BaseSwitch.css | 2 +- elements/pf-switch/BaseSwitch.ts | 19 +++++++------------ elements/pf-switch/pf-switch.css | 2 +- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/elements/pf-switch/BaseSwitch.css b/elements/pf-switch/BaseSwitch.css index 26fa4864bd..e56b7c7dd8 100644 --- a/elements/pf-switch/BaseSwitch.css +++ b/elements/pf-switch/BaseSwitch.css @@ -19,7 +19,7 @@ svg { cursor: not-allowed; } -:host(:disabled:focus-within) #container { +:host(:disabled:is(:focus,:focus-within)) { outline: none; } diff --git a/elements/pf-switch/BaseSwitch.ts b/elements/pf-switch/BaseSwitch.ts index c41df99de1..932fe5a550 100644 --- a/elements/pf-switch/BaseSwitch.ts +++ b/elements/pf-switch/BaseSwitch.ts @@ -12,8 +12,6 @@ export abstract class BaseSwitch extends LitElement { static readonly formAssociated = true; - static override readonly shadowRootOptions = { ...LitElement.shadowRootOptions, delegatesFocus: true }; - declare shadowRoot: ShadowRoot; #internals = InternalsController.of(this, { role: 'switch' }); @@ -81,20 +79,17 @@ export abstract class BaseSwitch extends LitElement { } #onKeyup(event: KeyboardEvent) { - switch (event.key) { - case ' ': - case 'Enter': - event.preventDefault(); - event.stopPropagation(); - this.#toggle(); + if (event.key === ' ' || event.key === 'Enter') { + event.preventDefault(); + event.stopPropagation(); + this.#toggle(); } } #onKeyDown(event: KeyboardEvent) { - switch (event.key) { - case ' ': - event.preventDefault(); - event.stopPropagation(); + if (event.key === ' ') { + event.preventDefault(); + event.stopPropagation(); } } diff --git a/elements/pf-switch/pf-switch.css b/elements/pf-switch/pf-switch.css index d7703185ae..f17ddf83b2 100644 --- a/elements/pf-switch/pf-switch.css +++ b/elements/pf-switch/pf-switch.css @@ -82,7 +82,7 @@ var(--pf-c-switch__toggle--before--Transition, translate .25s ease 0s)); ; } -:host(:focus-within) #container { +:host(:is(:focus,:focus-within)) #container { outline: var(--pf-c-switch__input--focus__toggle--OutlineWidth, var(--pf-global--BorderWidth--md, 2px)) solid var(--pf-c-switch__input--focus__toggle--OutlineColor, var(--pf-global--primary-color--100, #06c)); From adc92c243a6710d9ae5c7f9b0344fb05055a08d2 Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Wed, 27 Mar 2024 17:13:48 +0200 Subject: [PATCH 15/19] fix(switch): focus outline --- elements/pf-switch/pf-switch.css | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/elements/pf-switch/pf-switch.css b/elements/pf-switch/pf-switch.css index f17ddf83b2..99f73cfc63 100644 --- a/elements/pf-switch/pf-switch.css +++ b/elements/pf-switch/pf-switch.css @@ -82,6 +82,10 @@ var(--pf-c-switch__toggle--before--Transition, translate .25s ease 0s)); ; } +:host { + outline: none; +} + :host(:is(:focus,:focus-within)) #container { outline: var(--pf-c-switch__input--focus__toggle--OutlineWidth, var(--pf-global--BorderWidth--md, 2px)) solid var(--pf-c-switch__input--focus__toggle--OutlineColor, From 24e188d02cc82df00622aa9a4afa7fc207d2f058 Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Wed, 27 Mar 2024 17:18:58 +0200 Subject: [PATCH 16/19] docs(switch): restore elementinternals polyfill --- elements/pf-switch/demo/pf-switch.html | 3 +-- elements/pf-switch/demo/without-label.html | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/elements/pf-switch/demo/pf-switch.html b/elements/pf-switch/demo/pf-switch.html index 69f552659d..5407c73a3a 100644 --- a/elements/pf-switch/demo/pf-switch.html +++ b/elements/pf-switch/demo/pf-switch.html @@ -45,8 +45,7 @@ Message when on + + From 13f43c324b088e4182d181f5bec0ff38f34360c0 Mon Sep 17 00:00:00 2001 From: Steven Spriggs Date: Wed, 27 Mar 2024 13:11:22 -0400 Subject: [PATCH 18/19] test(switch): remove duplicate when checked test for show icon, improve checked/unchecked --- elements/pf-switch/test/pf-switch.spec.ts | 36 +++++++++++++++++++---- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/elements/pf-switch/test/pf-switch.spec.ts b/elements/pf-switch/test/pf-switch.spec.ts index 8741d7b8dd..992953f6f8 100644 --- a/elements/pf-switch/test/pf-switch.spec.ts +++ b/elements/pf-switch/test/pf-switch.spec.ts @@ -89,19 +89,43 @@ describe('', function() { describe('when checked attr is present', function() { let element: PfSwitch; + let snapshot: A11yTreeSnapshot; beforeEach(async function() { element = await createFixture(html` - + `); + + await element.updateComplete; + await nextFrame(); + snapshot = await a11ySnapshot({ selector: '#switch' }); }); - it('should display a check icon', async function() { - // TODO: can we test this without inspecting the private shadowRoot? - const svg = element.shadowRoot.querySelector('svg'); - expect(svg).to.be.ok; - expect(svg?.hasAttribute('hidden')).to.be.false; + + it('should be checked', function() { + expect(element.checked).to.be.true; + expect(snapshot.checked).to.be.true; }); }); + describe('when checked attr is not present', function() { + let element: PfSwitch; + let snapshot: A11yTreeSnapshot; + beforeEach(async function() { + element = await createFixture(html` + + `); + + await element.updateComplete; + await nextFrame(); + snapshot = await a11ySnapshot({ selector: '#switch' }); + }); + + it('should be checked', function() { + expect(element.checked).to.be.false; + expect(snapshot.checked).to.be.false; + }); + }); + + describe('when checked and show-check-icon attrs are present', function() { let element: PfSwitch; beforeEach(async function() { From 3b5283978e647f065a82628043a974a24fe04636 Mon Sep 17 00:00:00 2001 From: Steven Spriggs Date: Wed, 27 Mar 2024 13:21:25 -0400 Subject: [PATCH 19/19] docs(switch): update doc example labels --- elements/pf-switch/docs/pf-switch.md | 31 ++++++++++++++++++---------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/elements/pf-switch/docs/pf-switch.md b/elements/pf-switch/docs/pf-switch.md index 4dedb27bef..236833b15d 100644 --- a/elements/pf-switch/docs/pf-switch.md +++ b/elements/pf-switch/docs/pf-switch.md @@ -5,16 +5,20 @@ provide a more explicit, visible representation on a setting. - - + {% endrenderOverview %} {% band header="Usage" %} ### Basic {% htmlexample %} - - + {% endhtmlexample %} ### Without label @@ -25,8 +29,10 @@ ### Checked with label {% htmlexample %} - - + {% endhtmlexample %} ### Disabled @@ -35,14 +41,17 @@
Checked and Disabled - - +
-
- - +
{% endhtmlexample %}