From c2839c7b1008bf96ed0d645008443963c19a213e Mon Sep 17 00:00:00 2001 From: Tommmaso Menga Date: Fri, 4 Oct 2024 14:54:15 +0200 Subject: [PATCH 01/30] feat(sbb-radio-button): form association - first implementation --- src/elements/core/mixins.ts | 1 + .../core/mixins/form-associated-mixin.ts | 7 +- .../form-associated-radio-button-mixin.ts | 222 ++++++++++++++++++ .../common/radio-button-common.scss | 4 +- .../common/radio-button-common.ts | 105 +++------ 5 files changed, 260 insertions(+), 79 deletions(-) create mode 100644 src/elements/core/mixins/form-associated-radio-button-mixin.ts diff --git a/src/elements/core/mixins.ts b/src/elements/core/mixins.ts index dfb85b5223..3524825a79 100644 --- a/src/elements/core/mixins.ts +++ b/src/elements/core/mixins.ts @@ -2,6 +2,7 @@ export * from './mixins/constructor.js'; export * from './mixins/disabled-mixin.js'; export * from './mixins/form-associated-checkbox-mixin.js'; export * from './mixins/form-associated-mixin.js'; +export * from './mixins/form-associated-radio-button-mixin.js'; export * from './mixins/hydration-mixin.js'; export * from './mixins/named-slot-list-mixin.js'; export * from './mixins/negative-mixin.js'; diff --git a/src/elements/core/mixins/form-associated-mixin.ts b/src/elements/core/mixins/form-associated-mixin.ts index 7ede1464f7..04f9d92842 100644 --- a/src/elements/core/mixins/form-associated-mixin.ts +++ b/src/elements/core/mixins/form-associated-mixin.ts @@ -122,9 +122,12 @@ export const SbbFormAssociatedMixin = >( old: string | null, value: string | null, ): void { - if (name !== 'name' || old !== value) { - super.attributeChangedCallback(name, old, value); + // For the 'name' changes we have to handle updates manually + if (name === 'name') { + this.requestUpdate(name, old); + return; } + super.attributeChangedCallback(name, old, value); } /** diff --git a/src/elements/core/mixins/form-associated-radio-button-mixin.ts b/src/elements/core/mixins/form-associated-radio-button-mixin.ts new file mode 100644 index 0000000000..10c80a5974 --- /dev/null +++ b/src/elements/core/mixins/form-associated-radio-button-mixin.ts @@ -0,0 +1,222 @@ +import type { LitElement, PropertyValues } from 'lit'; +import { property } from 'lit/decorators.js'; + +import type { Constructor } from './constructor.js'; +import { SbbDisabledMixin, type SbbDisabledMixinType } from './disabled-mixin.js'; +import { + type FormRestoreReason, + type FormRestoreState, + SbbFormAssociatedMixin, + type SbbFormAssociatedMixinType, +} from './form-associated-mixin.js'; +import { SbbRequiredMixin, type SbbRequiredMixinType } from './required-mixin.js'; + +export declare class SbbFormAssociatedRadioButtonMixinType + extends SbbFormAssociatedMixinType + implements Partial, Partial +{ + public checked: boolean; + public disabled: boolean; + public required: boolean; + + public formResetCallback(): void; + public formStateRestoreCallback(state: FormRestoreState | null, reason: FormRestoreReason): void; + + protected isDisabledExternally(): boolean; + protected isRequiredExternally(): boolean; + protected withUserInteraction?(): void; +} + +/** + * TODO add docs (maybe move to new file) + * @internal + */ +export class RadioButtonRegistry { + private static _registry: { [x: string]: SbbFormAssociatedRadioButtonMixinType[] } = {}; + + private constructor() {} + + public static addRadioToGroup( + radio: SbbFormAssociatedRadioButtonMixinType, + groupName: string, + ): void { + if (!this._registry[groupName]) { + this._registry[groupName] = []; + } + this._registry[groupName].push(radio); + } + + public static removeRadioFromGroup( + radio: SbbFormAssociatedRadioButtonMixinType, + groupName: string, + ): void { + this._registry[groupName].splice(this._registry[groupName].indexOf(radio), 1); + } + + public static getRadios(groupName: string): SbbFormAssociatedRadioButtonMixinType[] { + return this._registry[groupName]; + } +} + +/** + * The SbbFormAssociatedRadioButtonMixin enables native form support for radio controls. + */ +// eslint-disable-next-line @typescript-eslint/naming-convention +export const SbbFormAssociatedRadioButtonMixin = >( + superClass: T, +): Constructor & T => { + class SbbFormAssociatedRadioButtonElement + extends SbbDisabledMixin(SbbRequiredMixin(SbbFormAssociatedMixin(superClass))) + implements Partial + { + /** + * Whether the radio button is checked. + */ + @property({ type: Boolean }) + public set checked(value: boolean) { + this._checked = value; + } + public get checked(): boolean { + return this._checked; + } + private _checked: boolean = false; + + private _initialized: boolean = false; + + protected constructor() { + super(); + /** @internal */ + this.internals.role = 'radio'; + } + + public override connectedCallback(): void { + super.connectedCallback(); + this._connectToRegistry(); + } + + public override disconnectedCallback(): void { + super.disconnectedCallback(); + this._disconnectFromRegistry(); + } + + /** + * Is called whenever the form is being reset. + * @internal + */ + public override formResetCallback(): void { + this.checked = this.hasAttribute('checked'); + } + + /** + * Called when the browser is trying to restore element’s state to state in which case + * reason is “restore”, or when the browser is trying to fulfill autofill on behalf of + * user in which case reason is “autocomplete”. + * In the case of “restore”, state is a string, File, or FormData object + * previously set as the second argument to setFormValue. + * + * @internal + */ + public override formStateRestoreCallback( + state: FormRestoreState | null, + _reason: FormRestoreReason, + ): void { + if (state) { + this.checked = state === this.value; + } + } + + protected override willUpdate(changedProperties: PropertyValues): void { + super.willUpdate(changedProperties); + + if (changedProperties.has('disabled')) { + // this.internals.ariaDisabled = this.disabled.toString(); // TODO probably not needed + } + if (changedProperties.has('required')) { + this.internals.ariaRequired = this.required.toString(); + } + + // On 'name' change, move 'this' to the new registry + if (changedProperties.has('name') && this._initialized) { + const oldName = changedProperties.get('name')!; + this._disconnectFromRegistry(oldName); + this._connectToRegistry(); + if (this.checked) { + this._deselectGroupedRadios(); + } + } + + if (changedProperties.has('checked')) { + this.toggleAttribute('data-checked', this.checked); + this.internals.ariaChecked = this.checked.toString(); + this.updateFormValueOnCheckedChange(); + if (this.checked) { + this._deselectGroupedRadios(); + } + } + } + + protected override firstUpdated(changedProperties: PropertyValues): void { + super.firstUpdated(changedProperties); + this._initialized = true; + } + + /** + * Called on `value` change + * If I'm checked, update the value. Otherwise, do nothing. + */ + protected override updateFormValue(): void { + if (this.checked) { + this.internals.setFormValue(this.value); + } + } + + /** + * Called on `checked` change + * If I'm checked, set the value. Otherwise, reset it. + */ + protected updateFormValueOnCheckedChange(): void { + this.internals.setFormValue(this.checked ? this.value : null); + } + + /** + * Add `this` to the radioButton registry + */ + private _connectToRegistry(name = this.name): void { + if (!name) { + return; + } + RadioButtonRegistry.addRadioToGroup( + this as unknown as SbbFormAssociatedRadioButtonMixinType, + name, + ); + } + + /** + * Remove `this` from the radioButton registry + */ + private _disconnectFromRegistry(name = this.name): void { + if (!name) { + return; + } + RadioButtonRegistry.removeRadioFromGroup( + this as unknown as SbbFormAssociatedRadioButtonMixinType, + name, + ); + } + + /** + * Deselect other radio of the same group + */ + private _deselectGroupedRadios(): void { + RadioButtonRegistry.getRadios(this.name) + .filter((r) => r !== (this as unknown as SbbFormAssociatedRadioButtonMixinType)) + .forEach((r) => (r.checked = false)); + } + + // TODO Keyboard interaction + // Suggestion: query for (this.form ?? document).queryAll('radio-button[name="this.name"]') and cycle through them + } + + return SbbFormAssociatedRadioButtonElement as unknown as Constructor & + T; +}; diff --git a/src/elements/radio-button/common/radio-button-common.scss b/src/elements/radio-button/common/radio-button-common.scss index 12ff6e68d2..8d5c017fb0 100644 --- a/src/elements/radio-button/common/radio-button-common.scss +++ b/src/elements/radio-button/common/radio-button-common.scss @@ -30,7 +30,7 @@ } } -:host([checked]) { +:host([data-checked]) { --sbb-radio-button-inner-circle-color: var(--sbb-color-red); --sbb-radio-button-background-fake-border-width: calc( (var(--sbb-radio-button-dimension) - var(--sbb-radio-button-inner-circle-dimension)) / 2 @@ -43,7 +43,7 @@ } // Disabled definitions have to be after checked definitions -:host([disabled]) { +:host(:disabled) { --sbb-radio-button-label-color: var(--sbb-color-granite); --sbb-radio-button-background-color: var(--sbb-color-milk); --sbb-radio-button-border-style: dashed; diff --git a/src/elements/radio-button/common/radio-button-common.ts b/src/elements/radio-button/common/radio-button-common.ts index ac4dcf2bc3..e21e476d13 100644 --- a/src/elements/radio-button/common/radio-button-common.ts +++ b/src/elements/radio-button/common/radio-button-common.ts @@ -2,15 +2,18 @@ import type { LitElement, PropertyValues } from 'lit'; import { property } from 'lit/decorators.js'; import { SbbConnectedAbortController } from '../../core/controllers.js'; -import { hostAttributes } from '../../core/decorators.js'; -import { setOrRemoveAttribute } from '../../core/dom.js'; import { EventEmitter, HandlerRepository, formElementHandlerAspect } from '../../core/eventing.js'; import type { SbbCheckedStateChange, SbbDisabledStateChange, SbbStateChange, } from '../../core/interfaces.js'; -import type { AbstractConstructor } from '../../core/mixins.js'; +import { + type AbstractConstructor, + type Constructor, + SbbFormAssociatedRadioButtonMixin, + type SbbFormAssociatedRadioButtonMixinType, +} from '../../core/mixins.js'; import type { SbbRadioButtonGroupElement } from '../radio-button-group.js'; export type SbbRadioButtonSize = 'xs' | 's' | 'm'; @@ -20,29 +23,19 @@ export type SbbRadioButtonStateChange = Extract< SbbDisabledStateChange | SbbCheckedStateChange >; -export declare class SbbRadioButtonCommonElementMixinType { +export declare class SbbRadioButtonCommonElementMixinType extends SbbFormAssociatedRadioButtonMixinType { public get allowEmptySelection(): boolean; public set allowEmptySelection(boolean); - public value?: string; - public get disabled(): boolean; - public set disabled(boolean); - public get required(): boolean; - public set required(boolean); public get group(): SbbRadioButtonGroupElement | null; - public get checked(): boolean; - public set checked(boolean); public select(): void; } // eslint-disable-next-line @typescript-eslint/naming-convention -export const SbbRadioButtonCommonElementMixin = >( +export const SbbRadioButtonCommonElementMixin = >( superClass: T, ): AbstractConstructor & T => { - @hostAttributes({ - role: 'radio', - }) abstract class SbbRadioButtonCommonElement - extends superClass + extends SbbFormAssociatedRadioButtonMixin(superClass) implements Partial { public static readonly events = { @@ -61,35 +54,6 @@ export const SbbRadioButtonCommonElementMixin = ): void { super.willUpdate(changedProperties); if (changedProperties.has('checked')) { - this.setAttribute('aria-checked', `${this.checked}`); if (this.checked !== changedProperties.get('checked')!) { this._stateChange.emit({ type: 'checked', checked: this.checked }); } } if (changedProperties.has('disabled')) { - setOrRemoveAttribute(this, 'aria-disabled', this.disabled ? 'true' : null); if (this.disabled !== changedProperties.get('disabled')!) { this._stateChange.emit({ type: 'disabled', disabled: this.disabled }); } } - if (changedProperties.has('required')) { - this.setAttribute('aria-required', `${this.required}`); - } + } + + protected override isDisabledExternally(): boolean { + return this.group?.disabled ?? false; + } + + protected override isRequiredExternally(): boolean { + return this.group?.required ?? false; } private _handleClick(event: Event): void { From 283bfc8c1a2d1e6276a48ebe866d13186f0c1c4f Mon Sep 17 00:00:00 2001 From: Tommmaso Menga Date: Mon, 7 Oct 2024 16:25:51 +0200 Subject: [PATCH 02/30] feat(sbb-radio-button): implement standalone focus and keyboard handling --- .../core/mixins/form-associated-mixin.ts | 2 +- .../form-associated-radio-button-mixin.ts | 97 ++++++++++++++++--- .../common/radio-button-common.ts | 4 +- 3 files changed, 86 insertions(+), 17 deletions(-) diff --git a/src/elements/core/mixins/form-associated-mixin.ts b/src/elements/core/mixins/form-associated-mixin.ts index 04f9d92842..3712d90816 100644 --- a/src/elements/core/mixins/form-associated-mixin.ts +++ b/src/elements/core/mixins/form-associated-mixin.ts @@ -3,7 +3,7 @@ import { property, state } from 'lit/decorators.js'; import type { Constructor } from './constructor.js'; -export declare abstract class SbbFormAssociatedMixinType { +export declare abstract class SbbFormAssociatedMixinType extends LitElement { public get form(): HTMLFormElement | null; public get name(): string; public set name(value: string); diff --git a/src/elements/core/mixins/form-associated-radio-button-mixin.ts b/src/elements/core/mixins/form-associated-radio-button-mixin.ts index 10c80a5974..a1b12da58f 100644 --- a/src/elements/core/mixins/form-associated-radio-button-mixin.ts +++ b/src/elements/core/mixins/form-associated-radio-button-mixin.ts @@ -1,6 +1,9 @@ import type { LitElement, PropertyValues } from 'lit'; import { property } from 'lit/decorators.js'; +import { getNextElementIndex, isArrowKeyPressed } from '../a11y.js'; +import { SbbConnectedAbortController } from '../controllers.js'; + import type { Constructor } from './constructor.js'; import { SbbDisabledMixin, type SbbDisabledMixinType } from './disabled-mixin.js'; import { @@ -19,6 +22,8 @@ export declare class SbbFormAssociatedRadioButtonMixinType public disabled: boolean; public required: boolean; + protected abort: SbbConnectedAbortController; + public formResetCallback(): void; public formStateRestoreCallback(state: FormRestoreState | null, reason: FormRestoreReason): void; @@ -43,6 +48,10 @@ export class RadioButtonRegistry { if (!this._registry[groupName]) { this._registry[groupName] = []; } + // Check for duplicates + if (this._registry[groupName].indexOf(radio) !== -1) { + return; + } this._registry[groupName].push(radio); } @@ -50,7 +59,15 @@ export class RadioButtonRegistry { radio: SbbFormAssociatedRadioButtonMixinType, groupName: string, ): void { - this._registry[groupName].splice(this._registry[groupName].indexOf(radio), 1); + const index = this._registry[groupName]?.indexOf(radio); + if (!this._registry[groupName] || index === -1) { + return; + } + this._registry[groupName].splice(index, 1); + + if (this._registry[groupName].length === 0) { + delete this._registry[groupName]; + } } public static getRadios(groupName: string): SbbFormAssociatedRadioButtonMixinType[] { @@ -81,7 +98,7 @@ export const SbbFormAssociatedRadioButtonMixin = this._handleArrowKeyDown(e), { signal }); } public override disconnectedCallback(): void { @@ -111,9 +131,6 @@ export const SbbFormAssociatedRadioButtonMixin = ): void { - super.firstUpdated(changedProperties); - this._initialized = true; - } - /** * Called on `value` change * If I'm checked, update the value. Otherwise, do nothing. @@ -213,8 +228,64 @@ export const SbbFormAssociatedRadioButtonMixin = (r.checked = false)); } - // TODO Keyboard interaction - // Suggestion: query for (this.form ?? document).queryAll('radio-button[name="this.name"]') and cycle through them + /** + * Return the grouped radios in DOM order + */ + private _orderedGrouperRadios(groupName = this.name): SbbFormAssociatedRadioButtonElement[] { + return Array.from( + (this.form ?? document).querySelectorAll( + `:is(sbb-radio-button, sbb-radio-button-panel)[name="${groupName}"]`, + ), + ); + } + + /** + * The focusable radio is the checked one or the first in DOM order + * TODO handle radio-button panel exception (they are always focusable? check _hasSelectionExpansionPanelElement in the group) + */ + private _setFocusableRadio(): void { + const radios = this._orderedGrouperRadios(); + const checkedIndex = radios.findIndex( + (radio) => radio.checked && !radio.disabled && !radio.formDisabled, + ); + + radios.forEach((r) => r.removeAttribute('tabindex')); + radios[checkedIndex !== -1 ? checkedIndex : 0].tabIndex = 0; + } + + private async _handleArrowKeyDown(evt: KeyboardEvent): Promise { + if (!isArrowKeyPressed(evt)) { + return; + } + + const enabledRadios = this._orderedGrouperRadios().filter( + (r) => !r.disabled && !r.formDisabled, + ); + const current: number = enabledRadios.indexOf(this); + const nextIndex: number = getNextElementIndex(evt, current, enabledRadios.length); + + // if ( + // !enabledRadios || + // !enabledRadios.length || + // // don't trap nested handling + // ((evt.target as HTMLElement) !== this && + // (evt.target as HTMLElement).parentElement !== this && + // (evt.target as HTMLElement).parentElement?.localName !== 'sbb-selection-expansion-panel') + // ) { + // return; + // } + + // TODO + // if (!this._hasSelectionExpansionPanelElement) { + // enabledRadios[nextIndex].checked = true; + // } + + enabledRadios[nextIndex].checked = true; + evt.preventDefault(); + + await enabledRadios[nextIndex].updateComplete; + enabledRadios[nextIndex].focus(); + } } return SbbFormAssociatedRadioButtonElement as unknown as Constructor & diff --git a/src/elements/radio-button/common/radio-button-common.ts b/src/elements/radio-button/common/radio-button-common.ts index e21e476d13..36660f5948 100644 --- a/src/elements/radio-button/common/radio-button-common.ts +++ b/src/elements/radio-button/common/radio-button-common.ts @@ -1,7 +1,6 @@ import type { LitElement, PropertyValues } from 'lit'; import { property } from 'lit/decorators.js'; -import { SbbConnectedAbortController } from '../../core/controllers.js'; import { EventEmitter, HandlerRepository, formElementHandlerAspect } from '../../core/eventing.js'; import type { SbbCheckedStateChange, @@ -62,7 +61,6 @@ export const SbbRadioButtonCommonElementMixin = this._handleClick(e), { signal }); this.addEventListener('keydown', (e) => this._handleKeyDown(e), { signal }); this._handlerRepository.connect(); From fa7d7f421442072b51cb699169aa17222a25c4a2 Mon Sep 17 00:00:00 2001 From: Tommmaso Menga Date: Wed, 9 Oct 2024 12:01:11 +0200 Subject: [PATCH 03/30] feat(sbb-radio-button): remove native input --- .../radio-button-panel/radio-button-panel.ts | 28 ++++++++++++------- .../radio-button/radio-button/radio-button.ts | 12 +------- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/elements/radio-button/radio-button-panel/radio-button-panel.ts b/src/elements/radio-button/radio-button-panel/radio-button-panel.ts index e5cf286c57..136d6d7101 100644 --- a/src/elements/radio-button/radio-button-panel/radio-button-panel.ts +++ b/src/elements/radio-button/radio-button-panel/radio-button-panel.ts @@ -11,6 +11,7 @@ import { customElement, property } from 'lit/decorators.js'; import { slotState } from '../../core/decorators.js'; import { panelCommonStyle, + RadioButtonRegistry, SbbPanelMixin, type SbbPanelSize, SbbUpdateSchedulerMixin, @@ -53,6 +54,13 @@ export class SbbRadioButtonPanelElement extends SbbPanelMixin( } private _size: SbbPanelSize = 'm'; + private _hasSelectionExpansionPanelElement: boolean = false; + + public override connectedCallback(): void { + super.connectedCallback(); + this._hasSelectionExpansionPanelElement = !!this.closest?.('sbb-selection-expansion-panel'); + } + protected override async willUpdate(changedProperties: PropertyValues): Promise { super.willUpdate(changedProperties); @@ -61,6 +69,16 @@ export class SbbRadioButtonPanelElement extends SbbPanelMixin( } } + /** + * As an exception, panels with an expansion-panel attached are always focusable + */ + protected override updateFocusableRadios(): void { + super.updateFocusableRadios(); + const radios = RadioButtonRegistry.getRadios(this.name) as SbbRadioButtonPanelElement[]; + + radios.filter((r) => r._hasSelectionExpansionPanelElement).forEach((r) => (r.tabIndex = 0)); + } + protected override render(): TemplateResult { return html`