Skip to content

Commit

Permalink
fix: fixing click issues when using calcite-label with calcite-checkb…
Browse files Browse the repository at this point in the history 
…ox and enabling interoperability with native labels and inputs (#1268)
  • Loading branch information
@eriklharper
eriklharper authored Nov 13, 2020
1 parent 151c75e commit 6081b26
Show file tree
Hide file tree
Showing 6 changed files with 285 additions and 38 deletions.
119 changes: 117 additions & 2 deletions src/components/calcite-checkbox/calcite-checkbox.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,17 @@ describe("calcite-checkbox", () => {
const input = await page.find("input");

expect(calciteCheckbox).not.toHaveAttribute("checked");
expect(await calciteCheckbox.getProperty("checked")).toBe(false);
expect(input).not.toHaveAttribute("checked");
expect(await input.getProperty("checked")).toBe(false);

calciteCheckbox.click();

await page.waitForChanges();

expect(calciteCheckbox).toHaveAttribute("checked");
expect(input).toHaveAttribute("checked");
expect(await calciteCheckbox.getProperty("checked")).toBe(true);
expect(await input.getProperty("checked")).toBe(true);
});

it("appropriately triggers the custom change event", async () => {
Expand Down Expand Up @@ -118,12 +121,124 @@ describe("calcite-checkbox", () => {
const input = await page.find("input");
const paragraph = await page.find("p");

expect(calciteCheckbox).not.toHaveAttribute("checked");
expect(await calciteCheckbox.getProperty("checked")).toBe(false);
expect(input).not.toHaveAttribute("checked");
expect(await input.getProperty("checked")).toBe(false);

paragraph.click();

await page.waitForChanges();

expect(calciteCheckbox).toHaveAttribute("checked");
expect(input).toHaveAttribute("checked");
expect(await calciteCheckbox.getProperty("checked")).toBe(true);
expect(await input.getProperty("checked")).toBe(true);
});

it("toggles when the wrapping label with for is clicked", async () => {
const page = await newE2EPage();
await page.setContent(`
<label for="checky">
<calcite-checkbox id="checky"></calcite-checkbox>
<p>hello!</p>
</label>
`);

const calciteCheckbox = await page.find("calcite-checkbox");
const input = await page.find("input");
const label = await page.find("label");

expect(calciteCheckbox).not.toHaveAttribute("checked");
expect(await calciteCheckbox.getProperty("checked")).toBe(false);
expect(input).not.toHaveAttribute("checked");
expect(await input.getProperty("checked")).toBe(false);

label.click();

await page.waitForChanges();

expect(calciteCheckbox).toHaveAttribute("checked");
expect(await calciteCheckbox.getProperty("checked")).toBe(true);
expect(await input.getProperty("checked")).toBe(true);
});

it("toggles when the wrapping calcite-label is clicked", async () => {
const page = await newE2EPage();
await page.setContent(`
<calcite-label>
<calcite-checkbox id="wrapped-in-calcite-label" name="wrapped-in-calcite-label"></calcite-checkbox>
wrapped in calcite-label
</calcite-label>
`);

const calciteCheckbox = await page.find("calcite-checkbox");
const input = await page.find("input");
const label = await page.find("label");

expect(calciteCheckbox).not.toHaveAttribute("checked");
expect(await calciteCheckbox.getProperty("checked")).toBe(false);
expect(input).not.toHaveAttribute("checked");
expect(await input.getProperty("checked")).toBe(false);

label.click();

await page.waitForChanges();

expect(calciteCheckbox).toHaveAttribute("checked");
expect(await calciteCheckbox.getProperty("checked")).toBe(true);
expect(await input.getProperty("checked")).toBe(true);
});

it("toggles when the wrapping calcite-label with for is clicked", async () => {
const page = await newE2EPage();
await page.setContent(`
<calcite-label for="wrapped-in-calcite-label-with-for">
<calcite-checkbox id="wrapped-in-calcite-label-with-for" name="wrapped-in-calcite-label-with-for"></calcite-checkbox>
wrapped in calcite-label with for
</calcite-label>
`);

const calciteCheckbox = await page.find("calcite-checkbox");
const input = await page.find("input");
const label = await page.find("label");

expect(calciteCheckbox).not.toHaveAttribute("checked");
expect(await calciteCheckbox.getProperty("checked")).toBe(false);
expect(input).not.toHaveAttribute("checked");
expect(await input.getProperty("checked")).toBe(false);

label.click();

await page.waitForChanges();

expect(calciteCheckbox).toHaveAttribute("checked");
expect(await calciteCheckbox.getProperty("checked")).toBe(true);
expect(await input.getProperty("checked")).toBe(true);
});

it("toggles when sibling label is clicked", async () => {
const page = await newE2EPage();
await page.setContent(`
<calcite-label for="sibling-calcite-label">sibling calcite-label</calcite-label>
<calcite-checkbox id="sibling-calcite-label" name="sibling-calcite-label"></calcite-checkbox>
`);

const calciteCheckbox = await page.find("calcite-checkbox");
const input = await page.find("input");
const label = await page.find("label");

expect(calciteCheckbox).not.toHaveAttribute("checked");
expect(await calciteCheckbox.getProperty("checked")).toBe(false);
expect(input).not.toHaveAttribute("checked");
expect(await input.getProperty("checked")).toBe(false);

label.click();

await page.waitForChanges();

expect(calciteCheckbox).toHaveAttribute("checked");
expect(await calciteCheckbox.getProperty("checked")).toBe(true);
expect(await input.getProperty("checked")).toBe(true);
});

it("removing a checkbox also removes the hidden <input type=checkbox> element", async () => {
Expand Down
27 changes: 18 additions & 9 deletions src/components/calcite-checkbox/calcite-checkbox.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export class CalciteCheckbox {

@Watch("checked")
checkedWatcher(newChecked: boolean): void {
newChecked ? this.input.setAttribute("checked", "") : this.input.removeAttribute("checked");
this.input.checked = newChecked;
}

/** True if the checkbox is disabled */
Expand Down Expand Up @@ -143,15 +143,13 @@ export class CalciteCheckbox {
//--------------------------------------------------------------------------

@Listen("click")
onClick({ currentTarget, target }: MouseEvent): void {
// prevent duplicate click events that occur
// when the component is wrapped in a label and checkbox is clicked
if (
(this.el.closest("label") && target === this.input) ||
(!this.el.closest("label") && currentTarget === this.el)
) {
this.toggle();
onClick(event: MouseEvent): void {
// This line prevents double-triggering when wrapped inside either a <label> or a <calcite-label>
// by preventing the browser default behavior, which is to click the label's first input child element
if (event.target === this.el) {
event.preventDefault();
}
this.toggle();
}

@Listen("keydown")
Expand All @@ -177,6 +175,15 @@ export class CalciteCheckbox {
this.checked = this.initialChecked;
};

private nativeLabelClickHandler = (event: MouseEvent): void => {
if (!this.el.closest("calcite-label") && (event.target as HTMLElement).nodeName === "LABEL") {
const target = event.target as HTMLLabelElement;
if (this.el.id && target.htmlFor === this.el.id) {
this.toggle();
}
}
};

private onInputBlur() {
this.focused = false;
this.calciteCheckboxFocusedChange.emit();
Expand All @@ -201,6 +208,7 @@ export class CalciteCheckbox {
if (form) {
form.addEventListener("reset", this.formResetHandler);
}
document.addEventListener("click", this.nativeLabelClickHandler);
}

disconnectedCallback(): void {
Expand All @@ -209,6 +217,7 @@ export class CalciteCheckbox {
if (form) {
form.removeEventListener("reset", this.formResetHandler);
}
document.removeEventListener("click", this.nativeLabelClickHandler);
}

// --------------------------------------------------------------------------
Expand Down
42 changes: 39 additions & 3 deletions src/components/calcite-label/calcite-label.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,49 @@ describe("calcite-label", () => {
</calcite-label>
`);
const label = await page.find("calcite-label");
const checkbox = await page.find("input");
const checkboxClass = checkbox["_elmHandle"]["_remoteObject"].description;
const input = await page.find("input");
const checkboxClass = input["_elmHandle"]["_remoteObject"].description;
await label.click();
const activeEl = await page.evaluateHandle(() => document.activeElement);
const activeElClass = activeEl["_remoteObject"].description;
expect(activeElClass).toEqual(checkboxClass);
expect(await input.getProperty("checked")).toBe(true);
});

it("focuses and checks a wrapped native checkbox when clicked", async () => {
const page = await newE2EPage();
await page.setContent(`
<calcite-label>
Label text
<input type="checkbox">
</calcite-label>
`);
const label = await page.find("calcite-label");
const input = await page.find("input");
const checkboxClass = input["_elmHandle"]["_remoteObject"].description;
await label.click();
const activeEl = await page.evaluateHandle(() => document.activeElement);
const activeElClass = activeEl["_remoteObject"].description;
expect(activeElClass).toEqual(checkboxClass);
expect(await input.getProperty("checked")).toBe(true);
});

it("focuses and checks a wrapped native checkbox with for when clicked", async () => {
const page = await newE2EPage();
await page.setContent(`
<calcite-label for="native">
Label text
<input id="native" type="checkbox">
</calcite-label>
`);
const label = await page.find("calcite-label");
const input = await page.find("input");
const checkboxClass = input["_elmHandle"]["_remoteObject"].description;
await label.click();
const activeEl = await page.evaluateHandle(() => document.activeElement);
const activeElClass = activeEl["_remoteObject"].description;
expect(activeElClass).toEqual(checkboxClass);
expect(checkbox).toHaveAttribute("checked");
expect(await input.getProperty("checked")).toBe(true);
});

it("focuses but does not check a wrapped checkbox when tabbed to", async () => {
Expand Down
49 changes: 30 additions & 19 deletions src/components/calcite-label/calcite-label.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,25 @@ export class CalciteLabel {
//--------------------------------------------------------------------------

@Listen("click")
onClick(event: MouseEvent): void {
const forAttr = this.el.getAttribute("for");
this.calciteLabelFocus.emit({
labelEl: this.el,
interactedEl: event.target as HTMLElement,
requestedInput: forAttr
});
if (forAttr) {
document.getElementById(forAttr).click();
onClick({ target }: MouseEvent): void {
if (
(target === this.el) ||
(target === this.labelEl) ||
(target === this.spanEl)
) {
const forAttr = this.el.getAttribute("for");
this.calciteLabelFocus.emit({
labelEl: this.el,
interactedEl: target as HTMLElement,
requestedInput: forAttr
});
const inputForThisLabel: HTMLElement = forAttr ? document.getElementById(forAttr) : this.el.querySelector("input");
if (
(inputForThisLabel && inputForThisLabel.nodeName.startsWith("CALCITE-")) ||
(inputForThisLabel && inputForThisLabel.nodeName === "INPUT" && target === this.el)
) {
inputForThisLabel.click();
}
}
}

Expand Down Expand Up @@ -118,14 +128,12 @@ export class CalciteLabel {
}

componentDidLoad(): void {
const labelNode = this.el.querySelector("label");
labelNode.childNodes.forEach((childNode) => {
this.labelEl.childNodes.forEach((childNode) => {
if (childNode.nodeName === "#text" && childNode.textContent.trim().length > 0) {
const newChildNode = document.createElement("span");
newChildNode.classList.add("calcite-label-text");
const newChildNodeText = document.createTextNode(childNode.textContent.trim());
newChildNode.appendChild(newChildNodeText);
childNode.parentNode.replaceChild(newChildNode, childNode);
this.spanEl = document.createElement("span");
this.spanEl.classList.add("calcite-label-text");
this.spanEl.appendChild(document.createTextNode(childNode.textContent.trim()));
childNode.parentNode.replaceChild(this.spanEl, childNode);
}
});
if (this.disabled) this.setDisabledControls();
Expand All @@ -136,7 +144,7 @@ export class CalciteLabel {
const dir = getElementDir(this.el);
return (
<Host dir={dir}>
<label {...attributes} ref={(el) => (this.childLabelEl = el)}>
<label {...attributes} ref={(el) => (this.labelEl = el)}>
<slot />
</label>
</Host>
Expand All @@ -149,7 +157,10 @@ export class CalciteLabel {
//--------------------------------------------------------------------------

// the rendered wrapping label element
private childLabelEl: HTMLLabelElement;
private labelEl: HTMLLabelElement;

// the span element that contains the computed label text
private spanEl: HTMLSpanElement;

//--------------------------------------------------------------------------
//
Expand All @@ -158,7 +169,7 @@ export class CalciteLabel {
//--------------------------------------------------------------------------

private setDisabledControls() {
this.childLabelEl?.childNodes.forEach((item) => {
this.labelEl?.childNodes.forEach((item) => {
if (item.nodeName.includes("CALCITE")) {
(item as HTMLElement).setAttribute("disabled", "");
}
Expand Down
Loading

0 comments on commit 6081b26

Please sign in to comment.