From c02072ba5450f272e84ab6c5d7e06547a8c9da69 Mon Sep 17 00:00:00 2001 From: Sam Richardson Date: Thu, 26 Oct 2023 16:28:23 -0400 Subject: [PATCH] fix(label): adjust reactivity --- src/dev/pages/label/label.ejs | 20 +++++- src/dev/pages/label/label.scss | 10 +++ src/lib/checkbox/checkbox.test.ts | 1 - src/lib/label/_configuration.scss | 0 src/lib/label/_core.scss | 6 -- src/lib/label/index.scss | 1 - src/lib/label/index.ts | 4 +- src/lib/label/label-adapter.ts | 8 +++ src/lib/label/label-aware.ts | 5 +- src/lib/label/label-component-delegate.ts | 18 ------ src/lib/label/label-constants.ts | 5 +- src/lib/label/label-foundation.ts | 32 ++++++---- src/lib/label/label.test.ts | 77 +++++++++++++++++++++++ src/lib/label/label.ts | 14 ++--- src/lib/switch/switch-adapter.ts | 28 +++++++++ src/lib/switch/switch-foundation.ts | 10 +++ src/lib/switch/switch.ts | 11 +++- 17 files changed, 196 insertions(+), 54 deletions(-) delete mode 100644 src/lib/label/_configuration.scss delete mode 100644 src/lib/label/label-component-delegate.ts diff --git a/src/dev/pages/label/label.ejs b/src/dev/pages/label/label.ejs index 2176b22dd..2acab809a 100644 --- a/src/dev/pages/label/label.ejs +++ b/src/dev/pages/label/label.ejs @@ -16,12 +16,28 @@
-

Frozen

- +

Dynamic

+ Label
+ +
+

Aligned list

+
+ nwiuh + + wefoinqwo + + qwdc + + oerncqeoicuh + + wed + +
+
\ No newline at end of file diff --git a/src/dev/pages/label/label.scss b/src/dev/pages/label/label.scss index b4b6ee6ce..36cd2acf6 100644 --- a/src/dev/pages/label/label.scss +++ b/src/dev/pages/label/label.scss @@ -2,3 +2,13 @@ forge-label::part(label) { display: flex; align-items: center; } + +.list { + display: grid; + grid-template-columns: auto 1fr; + align-items: center; + + forge-label { + width: 100%; + } +} diff --git a/src/lib/checkbox/checkbox.test.ts b/src/lib/checkbox/checkbox.test.ts index c9a6207bf..90f424d1d 100644 --- a/src/lib/checkbox/checkbox.test.ts +++ b/src/lib/checkbox/checkbox.test.ts @@ -9,7 +9,6 @@ import { TestHarness } from '../../test/utils/test-harness'; import { CHECKBOX_CONSTANTS, ICheckboxComponent } from '../checkbox'; import { IFocusIndicatorComponent } from '../focus-indicator'; import { IStateLayerComponent } from '../state-layer'; -import exp from 'constants'; class CheckboxHarness extends TestHarness { public rootElement: HTMLElement; diff --git a/src/lib/label/_configuration.scss b/src/lib/label/_configuration.scss deleted file mode 100644 index e69de29bb..000000000 diff --git a/src/lib/label/_core.scss b/src/lib/label/_core.scss index 337b6e829..6c65575f3 100644 --- a/src/lib/label/_core.scss +++ b/src/lib/label/_core.scss @@ -1,10 +1,4 @@ @use '../core/styles/typography'; -@use '../core/styles/utils'; -@use '../core/styles/tokens/label/tokens'; - -@mixin provide-theme($theme) { - @include utils.provide(tokens.$tokens, $theme, label); -} @mixin host { display: inline-block; diff --git a/src/lib/label/index.scss b/src/lib/label/index.scss index c78abe8eb..98a38c0d9 100644 --- a/src/lib/label/index.scss +++ b/src/lib/label/index.scss @@ -1,2 +1 @@ -@forward './configuration'; @forward './core'; diff --git a/src/lib/label/index.ts b/src/lib/label/index.ts index 3384ec42e..f46b00104 100644 --- a/src/lib/label/index.ts +++ b/src/lib/label/index.ts @@ -2,9 +2,11 @@ import { defineCustomElement } from '@tylertech/forge-core'; import { LabelComponent } from './label'; +export * from './label-adapter'; export * from './label-constants'; +export * from './label-foundation'; export * from './label'; -export * from './label-component-delegate'; +export * from './label-aware'; export function defineLabelComponent(): void { defineCustomElement(LabelComponent); diff --git a/src/lib/label/label-adapter.ts b/src/lib/label/label-adapter.ts index 7eb9ab5b7..c280c8840 100644 --- a/src/lib/label/label-adapter.ts +++ b/src/lib/label/label-adapter.ts @@ -1,3 +1,4 @@ +import { getShadowElement } from '@tylertech/forge-core'; import { BaseAdapter, IBaseAdapter } from '../core'; import { ILabelComponent } from './label'; import { ILabelAware, isLabelAware } from './label-aware'; @@ -11,16 +12,19 @@ export interface ILabelAdapter extends IBaseAdapter { trySetTarget(value: string | null): void; clickTarget(): void; updateTargetLabel(): void; + addSlotChangeListener(callback: EventListener): void; addMutationObserver(callback: MutationCallback): void; removeMutationObserver(): void; } export class LabelAdapter extends BaseAdapter implements ILabelAdapter { + private _slotElement: HTMLElement; private _targetElement: ILabelAware & HTMLElement | null = null; private _mutationObserver?: MutationObserver; constructor(component: ILabelComponent) { super(component); + this._slotElement = getShadowElement(component, LABEL_CONSTANTS.selectors.SLOT); } public destroy(): void { @@ -63,6 +67,10 @@ export class LabelAdapter extends BaseAdapter implements ILabel this._targetElement?.labelChangedCallback(value); } + public addSlotChangeListener(callback: EventListener): void { + this._slotElement.addEventListener('slotchange', callback); + } + public addMutationObserver(callback: MutationCallback): void { this._mutationObserver = new MutationObserver(callback); this._mutationObserver.observe(this._component, { diff --git a/src/lib/label/label-aware.ts b/src/lib/label/label-aware.ts index c4e487f07..00b41a955 100644 --- a/src/lib/label/label-aware.ts +++ b/src/lib/label/label-aware.ts @@ -9,8 +9,5 @@ export interface ILabelAware { * @returns True if the object is label aware, false otherwise. */ export const isLabelAware = (obj: any): obj is ILabelAware => { - return obj.labelClickedCallback && - typeof obj.labelClickedCallback === 'function' && - obj.labelChangedCallback && - typeof obj.labelChangedCallback === 'function'; + return typeof obj.labelClickedCallback === 'function' && typeof obj.labelChangedCallback === 'function'; }; diff --git a/src/lib/label/label-component-delegate.ts b/src/lib/label/label-component-delegate.ts deleted file mode 100644 index 355a3fc48..000000000 --- a/src/lib/label/label-component-delegate.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { BaseComponentDelegate, IBaseComponentDelegateConfig, IBaseComponentDelegateOptions } from '../core'; -import { ILabelComponent } from './label'; -import { LABEL_CONSTANTS } from './label-constants'; - -export type LabelComponentDelegateProps = Partial; -export interface ILabelComponentDelegateOptions extends IBaseComponentDelegateOptions {} -export interface ILabelComponentDelegateConfig extends IBaseComponentDelegateConfig {} - -export class LabelComponentDelegate extends BaseComponentDelegate { - constructor(config?: ILabelComponentDelegateConfig) { - super(config); - } - - protected _build(): ILabelComponent { - const label = document.createElement(LABEL_CONSTANTS.elementName); - return label; - } -} diff --git a/src/lib/label/label-constants.ts b/src/lib/label/label-constants.ts index 15e588de0..0325fdb09 100644 --- a/src/lib/label/label-constants.ts +++ b/src/lib/label/label-constants.ts @@ -6,11 +6,12 @@ const elementName: keyof HTMLElementTagNameMap = `${COMPONENT_NAME_PREFIX}label` const attributes = { FOR: 'for', - FREEZE: 'freeze' + DYNAMIC: 'dynamic' }; const selectors = { - ROOT: '.forge-label' + ROOT: '.forge-label', + SLOT: 'slot' }; const labelableChildSelectors = [ diff --git a/src/lib/label/label-foundation.ts b/src/lib/label/label-foundation.ts index 1ba25d753..61aaacb8f 100644 --- a/src/lib/label/label-foundation.ts +++ b/src/lib/label/label-foundation.ts @@ -5,7 +5,7 @@ import { LABEL_CONSTANTS } from './label-constants'; export interface ILabelFoundation extends ICustomElementFoundation { for: string | null | undefined; forElement: HTMLElement | null | undefined; - freeze: boolean; + dynamic: boolean; disconnect(): void; update(): void; } @@ -14,19 +14,22 @@ export class LabelFoundation implements ILabelFoundation { // State private _for: string | null | undefined; private _forElement: HTMLElement | null | undefined; - private _freeze = false; + private _dynamic = false; private _isConnected = false; // Listeners private readonly _clickListener: EventListener; + private readonly _slotChangeListener: EventListener; private readonly _mutationCallback: MutationCallback; constructor(private _adapter: ILabelAdapter) { this._clickListener = (evt: PointerEvent) => this._handleClick(evt); + this._slotChangeListener = () => this._handleSlotChange(); this._mutationCallback = () => this._handleMutation(); } public initialize(): void { + this._adapter.addSlotChangeListener(this._slotChangeListener); this._adapter.trySetTarget(null); if (this._adapter.hasTargetElement()) { this._connect(); @@ -50,13 +53,20 @@ export class LabelFoundation implements ILabelFoundation { this._adapter.clickTarget(); } + private _handleSlotChange(): void { + if (!this._for && !this._forElement) { + this._adapter.trySetTarget(null); + this._tryConnect(); + } + } + private _handleMutation(): void { this._adapter.updateTargetLabel(); } private _connect(): void { this._adapter.addHostListener('click', this._clickListener); - if (!this._freeze) { + if (this._dynamic) { this._adapter.addMutationObserver(this._mutationCallback); } this._isConnected = true; @@ -99,16 +109,16 @@ export class LabelFoundation implements ILabelFoundation { } } - public get freeze(): boolean { - return this._freeze; + public get dynamic(): boolean { + return this._dynamic; } - public set freeze(value: boolean) { - if (this._freeze !== value) { - this._freeze = value; - this._adapter.toggleHostAttribute(LABEL_CONSTANTS.attributes.FREEZE, this._freeze); - if (this._freeze) { + public set dynamic(value: boolean) { + if (this._dynamic !== value) { + this._dynamic = value; + this._adapter.toggleHostAttribute(LABEL_CONSTANTS.attributes.DYNAMIC, this._dynamic); + + if (!this._dynamic) { this._adapter.removeMutationObserver(); - this._adapter.updateTargetLabel(); } else if (this._adapter.hasTargetElement()) { this._adapter.addMutationObserver(this._mutationCallback); } diff --git a/src/lib/label/label.test.ts b/src/lib/label/label.test.ts index e69de29bb..8996bbf0a 100644 --- a/src/lib/label/label.test.ts +++ b/src/lib/label/label.test.ts @@ -0,0 +1,77 @@ +import { expect } from '@esm-bundle/chai'; +import { fixture, html } from '@open-wc/testing'; +import { getShadowElement } from '@tylertech/forge-core'; +import { sendMouse } from '@web/test-runner-commands'; +import { TestHarness } from '../../test/utils/test-harness'; +import { ILabelComponent } from './label'; +import { ILabelAware } from './label-aware'; +import { LABEL_CONSTANTS } from './label-constants'; + +import './label'; + +class LabelHarness extends TestHarness { + public rootElement: HTMLElement; + public labelAwareElement: HTMLElement & ILabelAware; + + constructor(el: ILabelComponent) { + super(el); + } + + public initElementRefs(): void { + this.rootElement = getShadowElement(this.element, LABEL_CONSTANTS.selectors.ROOT); + this.labelAwareElement = Object.create(document.createElement('div'), { + id: { + value: 'test' + }, + labelClickedCallback: { + value: () => { }, + }, + labelChangedCallback: { + value: () => { } + } + }) as HTMLElement & ILabelAware; + } + + public async clickElement(el: HTMLElement): Promise { + const { x, y, width, height } = el.getBoundingClientRect(); + + await sendMouse({ type: 'click', position: [ + Math.floor(x + window.scrollX + width / 2), + Math.floor(y + window.scrollY + height / 2) + ]}) + } +} + +describe('Label', () => { + it('should contain shadow root', async () => { + const el = await fixture(html``); + expect(el.shadowRoot).to.not.be.null; + }); + + it('should be accessible', async () => { + const el = await fixture(html``); + await expect(el).to.be.accessible(); + }); + + it('should render with correct default values', async () => { + const el = await fixture(html``); + + expect(el.for).to.be.undefined; + expect(el.forElement).to.be.undefined; + expect(el.dynamic).to.be.false; + }); + + it('should accept for', async () => { + const el = await fixture(html``); + expect(el.for).to.equal('test'); + }); + + it('should accept forElement', async () => { + const el = await fixture(html``); + const ctx = new LabelHarness(el); + + + + expect(el.forElement).to.not.be.null; + }); +}) \ No newline at end of file diff --git a/src/lib/label/label.ts b/src/lib/label/label.ts index 7cbf88286..90e0dc55e 100644 --- a/src/lib/label/label.ts +++ b/src/lib/label/label.ts @@ -10,7 +10,7 @@ import style from './label.scss'; export interface ILabelComponent extends IBaseComponent { for: string | null | undefined; forElement: HTMLElement | null | undefined; - freeze: boolean; + dynamic: boolean; update(): void; } @@ -28,10 +28,10 @@ declare global { * * @property {string | null | undefined} for - The id of the associated element. * @property {HTMLElement | null | undefined} forElement - The associated element. - * @property {boolean} freeze - Prevents the label from monitoring changes to its content. Set this if the label's content will never change. + * @property {boolean} dynamic - Propagates changes in the label's text content to the associated element. * * @attribute {string} for - The id of the associated form component. - * @attribute {boolean} freeze - Prevents the label from monitoring changes to its content. + * @attribute {boolean} dynamic - Propagates changes in the label's text content to the associated element. * * @method update - Updates the targetted element with the label's current text content. * @@ -44,7 +44,7 @@ export class LabelComponent extends BaseComponent implements ILabelComponent { public static get observedAttributes(): string[] { return [ LABEL_CONSTANTS.attributes.FOR, - LABEL_CONSTANTS.attributes.FREEZE + LABEL_CONSTANTS.attributes.DYNAMIC ]; } @@ -69,8 +69,8 @@ export class LabelComponent extends BaseComponent implements ILabelComponent { case LABEL_CONSTANTS.attributes.FOR: this.for = newValue; break; - case LABEL_CONSTANTS.attributes.FREEZE: - this.freeze = coerceBoolean(newValue); + case LABEL_CONSTANTS.attributes.DYNAMIC: + this.dynamic = coerceBoolean(newValue); break; } } @@ -82,7 +82,7 @@ export class LabelComponent extends BaseComponent implements ILabelComponent { public forElement: HTMLElement | null | undefined; @FoundationProperty() - public freeze: boolean; + public dynamic: boolean; /** * Updates the targetted element with the label's current text content. diff --git a/src/lib/switch/switch-adapter.ts b/src/lib/switch/switch-adapter.ts index 006736e2b..638002d85 100644 --- a/src/lib/switch/switch-adapter.ts +++ b/src/lib/switch/switch-adapter.ts @@ -17,6 +17,8 @@ export interface ISwitchAdapter extends IBaseAdapter { addRootListener(event: string, callback: EventListener): void; addInputSlotListener(callback: EventListener): void; detectInputElement(): void; + proxyClick(): void; + proxyLabel(value: string | null): void; syncValue(value: string | null): void; syncValidity(hasCustomValidityError: boolean): void; setValidity(flags?: ValidityStateFlags | undefined, message?: string | undefined): void; @@ -32,6 +34,8 @@ export class SwitchAdapter extends BaseAdapter implements ISwi private readonly _stateLayerElement: StateLayerComponent; private readonly _inputAdapter: InputAdapter; private _forwardObserver?: MutationObserver; + private _forwardedAriaLabel?: string; + private _labelElementText?: string; private get _activeInputElement(): HTMLInputElement { return this._inputAdapter.el ?? this._inputElement; @@ -127,6 +131,17 @@ export class SwitchAdapter extends BaseAdapter implements ISwi } } + public proxyClick(): void { + this._activeInputElement.click(); + // TODO: use `{ focusVisble: false }` when supported. + this._activeInputElement.focus(); + } + + public proxyLabel(value: string | null): void { + this._labelElementText = value ?? undefined; + this._setAriaLabel(); + } + public syncValue(value: string | null): void { if (value === null) { this._component.internals.setFormValue(null, SWITCH_CONSTANTS.state.OFF); @@ -154,7 +169,20 @@ export class SwitchAdapter extends BaseAdapter implements ISwi private _initializeForwardObserver(el: HTMLElement): void { this._forwardObserver = forwardAttributes(this._component, SWITCH_CONSTANTS.forwardedAttributes, (name, value) => { + // Use the connected label element as a fallback if aria-label is removed. Store the value so + // it can be used to determine whether an updated label element should take effect. + if (name === 'aria-label') { + this._forwardedAriaLabel = value ?? undefined; + this._setAriaLabel(); + return; + } + // Otherwise forward the attribute to the element. toggleAttribute(el, !!value, name, value ?? undefined); }); } + + private _setAriaLabel(): void { + const hasAriaLabel = !!this._forwardedAriaLabel || !!this._labelElementText; + toggleAttribute(this._activeInputElement, hasAriaLabel, 'aria-label', this._forwardedAriaLabel ?? this._labelElementText); + } } diff --git a/src/lib/switch/switch-foundation.ts b/src/lib/switch/switch-foundation.ts index 8fe65ec01..69cf67dcf 100644 --- a/src/lib/switch/switch-foundation.ts +++ b/src/lib/switch/switch-foundation.ts @@ -13,6 +13,8 @@ export interface ISwitchFoundation extends ICustomElementFoundation { readonly: boolean; icon: SwitchIconVisibility; labelPosition: SwitchLabelPosition; + proxyClick(): void; + proxyLabel(value: string | null): void; syncValidity(hasCustomValidityError: boolean): void; setValidity(flags?: ValidityStateFlags | undefined, message?: string | undefined): void; } @@ -50,6 +52,14 @@ export class SwitchFoundation implements ISwitchFoundation { this._adapter.syncValue(this._submittedValue); } + public proxyClick(): void { + this._adapter.proxyClick(); + } + + public proxyLabel(value: string | null): void { + this._adapter.proxyLabel(value); + } + public syncValidity(hasCustomValidityError: boolean): void { this._adapter.syncValidity(hasCustomValidityError); } diff --git a/src/lib/switch/switch.ts b/src/lib/switch/switch.ts index ea9e0a45d..254fd0655 100644 --- a/src/lib/switch/switch.ts +++ b/src/lib/switch/switch.ts @@ -8,8 +8,9 @@ import { SwitchFoundation } from './switch-foundation'; import template from './switch.html'; import styles from './switch.scss'; +import { ILabelAware } from '../label/label-aware'; -export interface ISwitchComponent extends IBaseFormComponent { +export interface ISwitchComponent extends IBaseFormComponent, ILabelAware { on: boolean; /** * @deprecated use `on` instead @@ -294,6 +295,14 @@ export class SwitchComponent extends BaseFormComponent implements ISwitchCompone this.disabled = isDisabled; } + public labelClickedCallback(): void { + this._foundation.proxyClick(); + } + + public labelChangedCallback(value: string | null): void { + this._foundation.proxyLabel(value); + } + @FoundationProperty() public declare on: boolean;