diff --git a/.changeset/small-mugs-mix.md b/.changeset/small-mugs-mix.md new file mode 100644 index 0000000000..3ae302bc0a --- /dev/null +++ b/.changeset/small-mugs-mix.md @@ -0,0 +1,8 @@ +--- +"@justeattakeaway/pie-switch": minor +--- + +- [Added] - Wrapped the checked value with the [lit live directive ](https://lit.dev/docs/api/directives/#live) to ensure the DOM and component state stay in sync. +- [Added] - Missing `defaultChecked` prop and form rest logic +- [Changed] - Refactored the toggle between leading and trailing labels to be handled via CSS. +- [Changed] - Updated the component to use a `
` as the parent wrapper when the label prop isn’t provided. This avoids nested ` @@ -208,16 +223,16 @@ const ExternalLabelsTemplate: TemplateFunction = (props: SwitchProp data-test-id="external-switch-multi" name="${props.name || nothing}" value="${props.value || nothing}" + ?defaultChecked="${props.defaultChecked}" @change="${changeAction}"> + `; const createSwitchStory = createStory(Template, defaultArgs); const createSwitchStoryWithForm = createStory(FormTemplate, defaultArgs); -const createSwitchTestStoryWithForm = createStory(TestFormTemplate, defaultArgs); - const createSwitchStoryWithExternalLabels = createStory(ExternalLabelsTemplate, defaultArgs); const formIntegrationOnly = { @@ -236,8 +251,6 @@ export const Default = createSwitchStory({}, { export const FormIntegration = createSwitchStoryWithForm(); -export const TestFormIntegration = createSwitchTestStoryWithForm(); - export const ExternalLabels = createSwitchStoryWithExternalLabels(); const baseSharedPropsMatrix: Partial> = { diff --git a/packages/components/pie-switch/README.md b/packages/components/pie-switch/README.md index bdec80dfe0..eeeba53163 100644 --- a/packages/components/pie-switch/README.md +++ b/packages/components/pie-switch/README.md @@ -36,6 +36,7 @@ Ideally, you should install the component using the **`@justeattakeaway/pie-webc | Prop | Options | Description | Default | |------------------|----------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------|-------------| | `checked` | `true`, `false` | Same as the HTML `checked` attribute; indicates whether the switch is on or off. | `false` | +| `defaultChecked` | `true`, `false` | Sets the default checked state for the switch. This does not directly set the initial checked state when the page loads, use `checked` for that. If the switch is inside a form which is reset, the `checked` state will be updated to match `defaultChecked`. | `false` | | `disabled` | `true`, `false` | Same as the HTML `disabled` attribute; indicates whether the switch is disabled or not. | `false` | | `required` | `true`, `false` | Same as the HTML `required` attribute; indicates whether the switch must be turned on when submitted as part of a form. | `false` | | `label` | — | The label text for the switch. | — | diff --git a/packages/components/pie-switch/src/defs.ts b/packages/components/pie-switch/src/defs.ts index 5173bfb40d..46c01700d8 100644 --- a/packages/components/pie-switch/src/defs.ts +++ b/packages/components/pie-switch/src/defs.ts @@ -16,6 +16,11 @@ export interface SwitchProps { * Same as the HTML checked attribute - indicates whether the switch is on or off */ checked?: boolean; + /** + * The default checked state of the switch (not necessarily the same as the current checked state). + * Used when the switch is part of a form that is reset. + */ + defaultChecked?: boolean; /** * Same as the HTML required attribute - indicates whether the switch must be turned or not */ @@ -53,6 +58,7 @@ export type DefaultProps = ComponentDefaultProps { + const [source] = event.composedPath(); + + if (this.disabled || source === this.input) { + return; + } + // Only programmatically click the input if the explicit target // of the click was the host element itself (e.g., via an external label). // This ignores clicks bubbling up from the internal shadow DOM and prevents loops. - if (event.composedPath()[0] === this) { + // Also forward clicks from the visual switch body when no internal label exists. + const isInsideSwitchBody = !this.label && event.composedPath().includes(this.switchBody); + if (source === this || isInsideSwitchBody) { this.input.click(); } }, { signal }); @@ -166,6 +182,23 @@ export class PieSwitch extends FormControlMixin(DelegatesFocusMixin(PieElement)) this._internals.setValidity(this.validity, this.validationMessage, this.input); } + /** + * Called when the containing form is reset. + * Resets checked state back to defaultChecked and emits a change event when needed. + */ + public formResetCallback () : void { + if (this.checked === this.defaultChecked) { + return; + } + + this.checked = this.defaultChecked; + + const changeEvent = new Event('change', { bubbles: true, composed: true }); + this.dispatchEvent(changeEvent); + + this.handleFormAssociation(); + } + /** * (Read-only) returns a ValidityState with the validity states that this element is in. * https://developer.mozilla.org/en-US/docs/Web/API/HTMLObjectElement/validity @@ -183,36 +216,12 @@ export class PieSwitch extends FormControlMixin(DelegatesFocusMixin(PieElement)) return this.input.validationMessage; } - /** - * If a label is provided, renders it if `labelPlacement` matches the given position. - * If no label is provided, or `labelPlacement` does not match the given position, nothing is rendered. - * - * @private - */ - private renderSwitchLabel (placement : SwitchProps['labelPlacement']) { - const { label, labelPlacement } = this; - - if (!label || labelPlacement !== placement) { - return nothing; - } - - // Using aria-hidden here to prevent the label from potentially being narrated twice by screen readers such as Apple VoiceOver. - // Instead, we apply the label as an aria-label attribute on the input (if no aria.label prop is provided). - return html` - `; - } - private renderAriaDescription () { if (!this.aria?.describedBy) { return nothing; } - // we apply aria-hidden to the element containing the description because it prevent some screen readers such as Apple VoiceOver from announcing the description once + // we apply aria-hidden to the element containing the description because it prevents some screen readers such as Apple VoiceOver from announcing the description once // on the input and again separately. The description is still announced once, when the input is focused/selected. return html`
`; } + private renderSwitchLabel () { + const { label, labelPlacement } = this; + + if (!label) { + return nothing; + } + + // Using aria-hidden here to prevent the label from potentially being narrated twice by screen readers such as Apple VoiceOver. + // Instead, we apply the label as an aria-label attribute on the input (if no aria.label prop is provided). + return html` + `; + } + render () { const { label, + labelPlacement, aria, checked, disabled, @@ -237,13 +265,15 @@ export class PieSwitch extends FormControlMixin(DelegatesFocusMixin(PieElement)) const classes = { 'c-switch-wrapper': true, 'c-switch-wrapper--allow-animation': _isAnimationAllowed, + [`c-switch-wrapper--label-${labelPlacement}`]: true, }; - return html` -
- ${this.renderSwitchLabel('trailing')} + ${this.renderSwitchLabel()} ${this.renderAriaDescription()} - `; + `; } } diff --git a/packages/components/pie-switch/src/switch.scss b/packages/components/pie-switch/src/switch.scss index 7f6e4ec865..365f9981ff 100644 --- a/packages/components/pie-switch/src/switch.scss +++ b/packages/components/pie-switch/src/switch.scss @@ -121,6 +121,13 @@ @include p.visually-hidden; } +.c-switch-wrapper--label-leading { + flex-direction: row-reverse; +} + +.c-switch-wrapper--label-trailing { + flex-direction: row; +} .c-switch-wrapper--allow-animation { /* stylelint-disable-next-line no-descending-specificity */ diff --git a/packages/components/pie-switch/test/component/pie-switch.spec.ts b/packages/components/pie-switch/test/component/pie-switch.spec.ts index 367223f3c9..71aa51c7ef 100644 --- a/packages/components/pie-switch/test/component/pie-switch.spec.ts +++ b/packages/components/pie-switch/test/component/pie-switch.spec.ts @@ -268,7 +268,7 @@ test.describe('Component: `Pie switch`', () => { const switchInput = page.getByTestId(pieSwitch.selectors.input.dataTestId); // Assert - await expect(switchInput).toHaveAttribute('aria-describedBy', 'switch-description'); + await expect(switchInput).toHaveAttribute('aria-describedby', 'switch-description'); }); test('should render a description element with the correct text', async ({ page }) => { @@ -317,7 +317,7 @@ test.describe('Component: `Pie switch`', () => { const switchName = 'switch'; const switchValue = 'switchValue'; - const switchPage = new BasePage(page, 'switch--test-form-integration'); + const switchPage = new BasePage(page, 'switch--form-integration'); const props: Partial = { required: true, }; @@ -330,8 +330,8 @@ test.describe('Component: `Pie switch`', () => { await page.click('#submitButton'); // Assert - const formDataJson = await page.$eval('#formDataJson', (el) => el.textContent); - const formDataObj = JSON.parse(formDataJson || '{}'); + const formDataOutput = await page.$eval('#formDataOutput', (el) => el.textContent); + const formDataObj = JSON.parse(formDataOutput || '{}'); // Check if the switch value is in the form data expect(switchName in formDataObj).toBe(true); @@ -340,7 +340,7 @@ test.describe('Component: `Pie switch`', () => { test('form should be invalid and not submit if the switch is required but not set', async ({ page }) => { // // Arrange - const switchPage = new BasePage(page, 'switch--test-form-integration'); + const switchPage = new BasePage(page, 'switch--form-integration'); const props: Partial = { required: true, }; @@ -351,13 +351,13 @@ test.describe('Component: `Pie switch`', () => { await page.click('#submitButton'); // Assert - const formDataJsonElement = await page.$('#formDataJson'); - expect(formDataJsonElement).toBeNull(); + const formDataOutput = await page.$eval('#formDataOutput', (el) => el.textContent?.trim() ?? ''); + expect(formDataOutput).toBe(''); }); test('should not be included in the submitted form data if disabled and checked', async ({ page }) => { // Arrange - const switchPage = new BasePage(page, 'switch--test-form-integration'); + const switchPage = new BasePage(page, 'switch--form-integration'); const props: Partial = { checked: true, disabled: true, @@ -368,15 +368,15 @@ test.describe('Component: `Pie switch`', () => { await page.click('#submitButton'); // Assert - const formDataJson = await page.$eval('#formDataJson', (el) => el.textContent); - const formDataObj = JSON.parse(formDataJson || '{}'); + const formDataOutput = await page.$eval('#formDataOutput', (el) => el.textContent); + const formDataObj = JSON.parse(formDataOutput || '{}'); expect(formDataObj.switch).toBeUndefined(); }); test('should not be included in the submitted form data if disabled and not checked', async ({ page }) => { // Arrange - const switchPage = new BasePage(page, 'switch--test-form-integration'); + const switchPage = new BasePage(page, 'switch--form-integration'); const props: Partial = { disabled: true, }; @@ -386,10 +386,54 @@ test.describe('Component: `Pie switch`', () => { await page.click('#submitButton'); // Assert - const formDataJson = await page.$eval('#formDataJson', (el) => el.textContent); - const formDataObj = JSON.parse(formDataJson || '{}'); + const formDataOutput = await page.$eval('#formDataOutput', (el) => el.textContent); + const formDataObj = JSON.parse(formDataOutput || '{}'); expect(formDataObj.switch).toBeUndefined(); }); + + test('should reset checked state to defaultChecked true when form is reset', async ({ page }) => { + // Arrange + const switchPage = new BasePage(page, 'switch--form-integration'); + const props: Partial = { + checked: false, + defaultChecked: true, + }; + await switchPage.load({ ...props }); + + const switchEl = page.locator('#pie-switch'); + await expect(switchEl).not.toHaveAttribute('checked'); + + // Act + await page.$eval('#testForm', (form) => { + (form as HTMLFormElement).reset(); + }); + + // Assert + await expect(switchEl).toHaveAttribute('checked', ''); + }); + + test('should reset checked state to defaultChecked false when form is reset', async ({ page }) => { + // Arrange + const switchPage = new BasePage(page, 'switch--form-integration'); + const props: Partial = { + checked: false, + defaultChecked: false, + }; + await switchPage.load({ ...props }); + + const switchEl = page.locator('#pie-switch'); + + // Act + await page.getByTestId(pieSwitch.selectors.container.dataTestId).click(); + await expect(switchEl).toHaveAttribute('checked', ''); + + await page.$eval('#testForm', (form) => { + (form as HTMLFormElement).reset(); + }); + + // Assert + await expect(switchEl).not.toHaveAttribute('checked'); + }); }); });