Skip to content
Merged
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
5 changes: 5 additions & 0 deletions .changeset/dsw-4058-associated-label-mixin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@justeattakeaway/pie-webc-core": minor
---

[Added] - New `AssociatedLabelMixin`, extracted from `pie-switch`, for mirroring the text content of a form-associated custom element's associated `<label>`(s) onto an `associatedLabelText` property (used as a Safari `aria-label` fallback)
6 changes: 6 additions & 0 deletions .changeset/dsw-4058-switch-label-announcement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@justeattakeaway/pie-switch": patch
---

[Fixed] - Announce external labels properly by screen readers in Safari
[Fixed] - component styles flicker on SSR
6 changes: 6 additions & 0 deletions .changeset/fruity-deserts-nail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@justeattakeaway/pie-button": patch
"@justeattakeaway/pie-storybook": patch
---

[Fixed] - update docs
2 changes: 1 addition & 1 deletion apps/pie-storybook/stories/pie-checkbox.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ const ExampleFormTemplate: TemplateFunction<CheckboxProps> = ({
<form id="testForm">
<pie-checkbox
.value="${value}"
?name="${ifDefined(name)}"
name="${ifDefined(name)}"
?checked="${checked}"
?defaultChecked="${defaultChecked}"
?disabled="${disabled}"
Expand Down
2 changes: 1 addition & 1 deletion packages/components/pie-button/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ import '@justeattakeaway/pie-webc/components/button.js'
```

```html
<pie-button type="reset" isFullWidth="true" onclick="e => console.log(e)">
<pie-button type="reset" isFullWidth onclick="e => console.log(e)">
Click me!
</pie-button>

Expand Down
20 changes: 13 additions & 7 deletions packages/components/pie-switch/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {
html, unsafeCSS, nothing,
html, unsafeCSS, nothing, type PropertyValues,
} from 'lit';
import { html as staticHtml, unsafeStatic } from 'lit/static-html.js';
import { PieElement } from '@justeattakeaway/pie-webc-core/src/internals/PieElement';
Expand All @@ -13,6 +13,7 @@ import {
validPropertyValues,
FormControlMixin,
DelegatesFocusMixin,
AssociatedLabelMixin,
wrapNativeEvent,
safeCustomElement,
type PIEInputElement,
Expand All @@ -32,7 +33,7 @@ const componentSelector = 'pie-switch';
* @event {CustomEvent} change - when the switch checked state is changed.
*/
@safeCustomElement('pie-switch')
export class PieSwitch extends FormControlMixin(DelegatesFocusMixin(PieElement)) implements SwitchProps, PIEInputElement {
export class PieSwitch extends AssociatedLabelMixin(FormControlMixin(DelegatesFocusMixin(PieElement))) implements SwitchProps, PIEInputElement {
@property({ type: String })
public label: SwitchProps['label'];

Expand Down Expand Up @@ -75,13 +76,15 @@ export class PieSwitch extends FormControlMixin(DelegatesFocusMixin(PieElement))
@state()
private _isAnimationAllowed = false;

protected firstUpdated (): void {
protected firstUpdated (changedProperties: PropertyValues): void {
super.firstUpdated(changedProperties);
const { signal } = this._abortController;
this.handleFormAssociation();
// This ensures that invalid events triggered by checkValidity() are propagated to the custom element
// for consumers to listen to: https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement/checkValidity
this.input.addEventListener('invalid', (event) => {
this.dispatchEvent(new Event('invalid', event));
});
}, { signal });
}

connectedCallback (): void {
Expand Down Expand Up @@ -260,8 +263,11 @@ export class PieSwitch extends FormControlMixin(DelegatesFocusMixin(PieElement))
disabled,
required,
_isAnimationAllowed,
associatedLabelText,
} = this;

const ariaLabel = aria?.label || label || associatedLabelText;

const classes = {
'c-switch-wrapper': true,
'c-switch-wrapper--allow-animation': _isAnimationAllowed,
Expand All @@ -283,11 +289,11 @@ export class PieSwitch extends FormControlMixin(DelegatesFocusMixin(PieElement))
role="switch"
type="checkbox"
class="c-switch-input"
.required=${required}
?required=${required}
.checked="${live(checked)}"
.disabled="${disabled}"
?disabled="${disabled}"
@change="${this.handleChange}"
aria-label="${ifDefined(aria?.label || label)}"
aria-label="${ifDefined(ariaLabel)}"
aria-describedby="${aria?.describedBy ? 'switch-description' : nothing}">
<div class="c-switch-control">
${checked ? html`<icon-check></icon-check>` : nothing}
Expand Down
6 changes: 3 additions & 3 deletions packages/components/pie-switch/src/switch.scss
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
vertical-align: top;
}

.c-switch-input:checked + & {
.c-switch[checked] & {
transform: translateX(var(--switch-translation));

.c-pieIcon--check {
Expand Down Expand Up @@ -143,7 +143,7 @@
.c-switch-control {
@include switch-transition(transform);

.c-switch-input:checked + & {
.c-switch[checked] & {
@include switch-transition(transform);

.c-pieIcon--check {
Expand All @@ -160,7 +160,7 @@
}

.c-switch-wrapper:dir(rtl) {
.c-switch-input:checked + .c-switch-control {
.c-switch[checked] .c-switch-control {
transform: translateX(calc(-1 * var(--switch-translation)));
}
}
1 change: 1 addition & 0 deletions packages/components/pie-webc-core/src/functions/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export * from './wrapNativeEvent';
export * from './dispatchCustomEvent';
export * from './isSafari';
6 changes: 6 additions & 0 deletions packages/components/pie-webc-core/src/functions/isSafari.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* Detects whether the current browser is Safari.
*/
export function isSafari (): boolean {
return typeof navigator !== 'undefined' && /^((?!chrome|android).)*safari/i.test(navigator.userAgent);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
import { type LitElement, type PropertyValues } from 'lit';
import type { GenericConstructor } from '../types/GenericConstructor';
import { isSafari } from '../../functions/isSafari';

/**
* Joins and normalises the text content of a set of `<label>` elements into a single string.
*/
const getLabelText = (labels: NodeList): string | undefined => {
const labelText = Array.from(labels)
.map((label) => label.textContent?.replace(/\s+/g, ' ').trim() ?? '')
.filter(Boolean)
.join(' ')
.trim();

return labelText || undefined;
};

/**
* An interface representing the structure of a class that has been extended
* with the AssociatedLabelMixin.
* @interface
*/
export interface AssociatedLabelInterface {
/** The text content of the element's associated `<label>`(s) */
readonly associatedLabelText?: string;
}

/**
* A mixin that mirrors the text content of a form-associated custom element's associated
* `<label>`(s) onto an `associatedLabelText` property.
*
* Safari doesn't compute the accessible name of a form-associated
* custom element from its associated `<label>` the way others do, so we need to use
* `associatedLabelText` as a fallback `aria-label`.
* https://bugs.webkit.org/show_bug.cgi?id=259124
*
*
* The class this mixin is applied to must already provide `_internals: ElementInternals`
* (for example via `FormControlMixin`), and must call `super.firstUpdated()` if it overrides
* `firstUpdated`.
*
* @param superClass - The LitElement class (with `_internals`) to extend.
* @returns A new class extending both the provided LitElement and AssociatedLabelInterface.
*
* @example
* ```typescript
* import { html } from 'lit';
* import { FormControlMixin, AssociatedLabelMixin } from '@justeattakeaway/pie-webc-core';
*
* class MyFormElement extends AssociatedLabelMixin(FormControlMixin(LitElement)) {
* render () {
* return html`<input aria-label="${this.associatedLabelText}">`;
* }
* }
* ```
*/
export const AssociatedLabelMixin =
<T extends GenericConstructor<LitElement & { _internals: ElementInternals }>>(superClass: T) => {
/**
* Class representing a LitElement with associated label handling.
* @extends {LitElement}
* @implements {AssociatedLabelInterface}
*/
class AssociatedLabelElement extends superClass implements AssociatedLabelInterface {
private _associatedLabelText?: string;

get associatedLabelText () {
return this._associatedLabelText;
}

private _labelMutationObserver?: MutationObserver;

protected firstUpdated (changedProperties: PropertyValues) {
super.firstUpdated(changedProperties);
this.observeAssociatedLabels();
}

disconnectedCallback () {
super.disconnectedCallback();
this._labelMutationObserver?.disconnect();
}

private updateAssociatedLabelText () : void {
this._associatedLabelText = getLabelText(this._internals.labels);
this.requestUpdate();
}

private observeAssociatedLabels () : void {
const { labels } = this._internals;

if (!isSafari() || !labels.length) {
return;
}

this.updateAssociatedLabelText();

this._labelMutationObserver = new MutationObserver(() => this.updateAssociatedLabelText());

labels.forEach((label) => {
this._labelMutationObserver?.observe(label, { childList: true, characterData: true, subtree: true });
});
}
}

return AssociatedLabelElement as GenericConstructor<AssociatedLabelInterface> & T;
};
1 change: 1 addition & 0 deletions packages/components/pie-webc-core/src/mixins/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from './rtl/rtlMixin';
export * from './formControl/formControlMixin';
export * from './delegatesFocus/delegatesFocusMixin';
export * from './associatedLabel/associatedLabelMixin';

export * from './types/GenericConstructor';
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import { LitElement, html } from 'lit';
import { customElement } from 'lit/decorators.js';

import {
describe,
it,
expect,
beforeEach,
afterEach,
vi,
} from 'vitest';

import { AssociatedLabelMixin, type GenericConstructor } from '../../../index';

const safariUserAgent = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Safari/605.1.15';
const chromeUserAgent = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36';

describe('AssociatedLabelMixin', () => {
const componentSelector = 'associated-label-mixin-mock';

@customElement(componentSelector)
class MockComponent extends AssociatedLabelMixin(LitElement as unknown as GenericConstructor<LitElement & { _internals: ElementInternals }>) {
// A minimal stand-in for the `_internals` provided by `FormControlMixin` in real usage,
// set directly by tests before the element is connected to the document.
public _internals = { labels: [] } as unknown as ElementInternals;

render () {
return html`<div>Mock Component</div>`;
}
}

async function createConnectedMockInstance (labels: HTMLLabelElement[]): Promise<MockComponent> {
const instance = document.createElement(componentSelector) as MockComponent;
instance._internals = { labels } as unknown as ElementInternals;
document.body.appendChild(instance);
await instance.updateComplete;
return instance;
}

let userAgentSpy: ReturnType<typeof vi.spyOn>;

beforeEach(() => {
document.body.innerHTML = '';
userAgentSpy = vi.spyOn(window.navigator, 'userAgent', 'get').mockReturnValue(safariUserAgent);
});

afterEach(() => {
userAgentSpy.mockRestore();
});

describe('when running in Safari', () => {
it('should mirror the text content of a single associated label', async () => {
// Arrange
const label = document.createElement('label');
label.textContent = 'Marketing emails';

// Act
const component = await createConnectedMockInstance([label]);

// Assert
expect(component.associatedLabelText).toBe('Marketing emails');
});

it('should join and normalise whitespace across multiple associated labels', async () => {
// Arrange
const labelOne = document.createElement('label');
labelOne.textContent = ' Marketing \n emails ';
const labelTwo = document.createElement('label');
labelTwo.textContent = 'for offers';

// Act
const component = await createConnectedMockInstance([labelOne, labelTwo]);

// Assert
expect(component.associatedLabelText).toBe('Marketing emails for offers');
});

it('should leave associatedLabelText undefined when there are no associated labels', async () => {
// Act
const component = await createConnectedMockInstance([]);

// Assert
expect(component.associatedLabelText).toBeUndefined();
});

it('should leave associatedLabelText undefined when the only associated label has no text', async () => {
// Arrange
const label = document.createElement('label');

// Act
const component = await createConnectedMockInstance([label]);

// Assert
expect(component.associatedLabelText).toBeUndefined();
});

it('should update associatedLabelText when an associated label mutates', async () => {
// Arrange
const label = document.createElement('label');
label.textContent = 'Original text';
const component = await createConnectedMockInstance([label]);
expect(component.associatedLabelText).toBe('Original text');

// Act
label.textContent = 'Updated text';
await vi.waitFor(() => expect(component.associatedLabelText).toBe('Updated text'));
});

it('should stop observing associated labels once disconnected', async () => {
// Arrange
const label = document.createElement('label');
label.textContent = 'Original text';
const component = await createConnectedMockInstance([label]);
expect(component.associatedLabelText).toBe('Original text');

// Act
component.remove();
label.textContent = 'Updated text';

// Assert - give any pending MutationObserver callback a chance to (not) fire
await new Promise((resolve) => { setTimeout(resolve, 0); });
expect(component.associatedLabelText).toBe('Original text');
});
});

describe('when not running in Safari', () => {
it('should leave associatedLabelText undefined even when associated labels exist', async () => {
// Arrange
userAgentSpy.mockReturnValue(chromeUserAgent);
const label = document.createElement('label');
label.textContent = 'Marketing emails';

// Act
const component = await createConnectedMockInstance([label]);

// Assert
expect(component.associatedLabelText).toBeUndefined();
});
});
});
Loading