Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(label): associate label to component when for prop is set after initialization #8309

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
23 changes: 21 additions & 2 deletions packages/calcite-components/src/components/label/label.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
import { Component, Element, Event, EventEmitter, h, Host, Prop, VNode } from "@stencil/core";
import { labelConnectedEvent, labelDisconnectedEvent } from "../../utils/label";
import {
Component,
Element,
Event,
EventEmitter,
h,
Host,
Prop,
VNode,
Watch,
} from "@stencil/core";
import {
associateExplicitLabelToUnlabeledComponent,
labelConnectedEvent,
labelDisconnectedEvent,
} from "../../utils/label";
import { Alignment, Scale } from "../interfaces";
import { CSS } from "./resources";

Expand All @@ -24,6 +38,11 @@ export class Label {
/** Specifies the `id` of the component the label is bound to. Use when the component the label is bound to does not reside within the component. */
@Prop({ reflect: true }) for: string;

@Watch("for")
handleForChange(): void {
associateExplicitLabelToUnlabeledComponent(this.el);
}

/** Specifies the size of the component. */
@Prop({ reflect: true }) scale: Scale = "m";

Expand Down
24 changes: 24 additions & 0 deletions packages/calcite-components/src/tests/commonTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,30 @@ export function labelable(
shadowFocusTargetSelector,
});
});

it("is labelable when label's for is set after initialization", async () => {
const siblingHtml = html`
<calcite-label>${labelTitle}</calcite-label>
${componentHtml}
`;
const siblingPage: E2EPage = await newE2EPage();
beforeContent?.(siblingPage);

await siblingPage.setContent(siblingHtml);
await siblingPage.waitForChanges();

const label = await siblingPage.find("calcite-label");
label.setProperty("for", id);
await siblingPage.waitForChanges();

await assertLabelable({
page: siblingPage,
componentTag,
propertyToToggle,
focusTargetSelector,
shadowFocusTargetSelector,
});
});
});
}

Expand Down
41 changes: 40 additions & 1 deletion packages/calcite-components/src/utils/component.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { getIconScale } from "./component";
import { componentOnReady, getIconScale } from "./component";
import { html } from "../../support/formatting";
import { HTMLStencilElement } from "@stencil/core/internal";

describe("getIconScale", () => {
it('should return "m" when input is "l"', () => {
Expand All @@ -10,3 +12,40 @@ describe("getIconScale", () => {
expect(getIconScale("s")).toBe("s");
});
});

describe("componentOnReady", () => {
let requestAnimationFrameSpy: jest.SpyInstance;
let fakeComponent: HTMLElement;

beforeEach(() => {
document.body.innerHTML = html` <fake-component></fake-component> `;
fakeComponent = document.querySelector<HTMLElement>("fake-component");

const originalRaf = globalThis.requestAnimationFrame;
requestAnimationFrameSpy = jest
.spyOn(globalThis, "requestAnimationFrame")
.mockImplementation((callback) => originalRaf(callback));
});

afterEach(() => requestAnimationFrameSpy.mockRestore());

it("should call componentOnReady if it exists on the element (lazy-loaded)", async () => {
const componentOnReadyStub = ((fakeComponent as HTMLStencilElement).componentOnReady = jest.fn());

const promise = componentOnReady(fakeComponent);
expect(promise).toBeInstanceOf(Promise);

await promise;
expect(componentOnReadyStub).toHaveBeenCalled();
});

it("waits for an animation frame if componentOnReady does not exist on the element", async () => {
expect(requestAnimationFrameSpy).not.toHaveBeenCalled();

const promise = componentOnReady(fakeComponent);
expect(promise).toBeInstanceOf(Promise);

await promise;
expect(requestAnimationFrameSpy).toHaveBeenCalled();
});
});
18 changes: 18 additions & 0 deletions packages/calcite-components/src/utils/component.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,23 @@
import { Scale } from "../components/interfaces";
import { HTMLStencilElement } from "@stencil/core/internal";

export function getIconScale(componentScale: Scale): Extract<Scale, "s" | "m"> {
return componentScale === "l" ? "m" : "s";
}

/**
* This util helps us wait for a component to be ready for both lazy-loading (`dist` output) and non-lazy-loading (`components` output) components.
*
* Based on https://github.com/ionic-team/ionic-framework/blob/1a8bd6d/core/src/utils/helpers.ts#L60C1-L79C3
*
* @param el - the host element to wait for
*/
export async function componentOnReady(el: HTMLElement): Promise<void> {
await (isStencilEl(el)
? el.componentOnReady()
: new Promise<void>((resolve) => requestAnimationFrame(() => resolve())));
}

function isStencilEl(el: HTMLElement): el is HTMLStencilElement {
return typeof (el as HTMLStencilElement).componentOnReady === "function";
}
43 changes: 41 additions & 2 deletions packages/calcite-components/src/utils/label.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { closestElementCrossShadowBoundary, isBefore, queryElementRoots } from "./dom";
import { componentOnReady } from "./component";

export interface LabelableComponent {
/**
Expand Down Expand Up @@ -41,7 +42,7 @@ const labelToLabelables = new WeakMap<HTMLCalciteLabelElement, LabelableComponen
const onLabelClickMap = new WeakMap<HTMLCalciteLabelElement, typeof onLabelClick>();
const onLabelConnectedMap = new WeakMap<LabelableComponent, typeof onLabelConnected>();
const onLabelDisconnectedMap = new WeakMap<LabelableComponent, typeof onLabelDisconnected>();
const unlabeledComponents = new WeakSet<LabelableComponent>();
const unlabeledComponents = new Set<LabelableComponent>();

const findLabelForComponent = (componentEl: HTMLElement): HTMLCalciteLabelElement | null => {
const { id } = componentEl;
Expand Down Expand Up @@ -70,7 +71,7 @@ function hasAncestorCustomElements(label: HTMLCalciteLabelElement, componentEl:
let traversedElements: HTMLElement[];
const customElementAncestorCheckEventType = "custom-element-ancestor-check";

const listener = (event) => {
const listener = (event: CustomEvent) => {
event.stopImmediatePropagation();
const composedPath = event.composedPath() as HTMLElement[];
traversedElements = composedPath.slice(composedPath.indexOf(componentEl), composedPath.indexOf(label));
Expand All @@ -94,6 +95,10 @@ function hasAncestorCustomElements(label: HTMLCalciteLabelElement, componentEl:
* @param component
*/
export function connectLabel(component: LabelableComponent): void {
if (!component) {
return;
}

const labelEl = findLabelForComponent(component.el);

if (
Expand Down Expand Up @@ -132,6 +137,10 @@ export function connectLabel(component: LabelableComponent): void {
* @param component
*/
export function disconnectLabel(component: LabelableComponent): void {
if (!component) {
return;
}

unlabeledComponents.delete(component);
document.removeEventListener(labelConnectedEvent, onLabelConnectedMap.get(component));
document.removeEventListener(labelDisconnectedEvent, onLabelDisconnectedMap.get(component));
Expand Down Expand Up @@ -202,3 +211,33 @@ function onLabelDisconnected(this: LabelableComponent): void {
onLabelConnectedMap.set(this, boundOnLabelConnected);
document.addEventListener(labelConnectedEvent, boundOnLabelConnected);
}

/**
* Helper to associate an explicit label (i.e., using `for`) with a labelable component that does not have an associated label.
*
* @param label - the label element
*/
export async function associateExplicitLabelToUnlabeledComponent(label: HTMLCalciteLabelElement): Promise<void> {
await componentOnReady(label);

const alreadyLabeled = labelToLabelables.has(label);

if (alreadyLabeled) {
return;
}

const forComponentEl = label.ownerDocument?.getElementById(label.for);

if (!forComponentEl) {
return;
}

requestAnimationFrame(() => {
for (const labelable of unlabeledComponents) {
if (labelable.el === forComponentEl) {
connectLabel(labelable);
break;
}
}
});
}
Loading