Skip to content
Open
Show file tree
Hide file tree
Changes from 13 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
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { describe } from "vitest";
import { describe, expect, it, vi } from "vitest";
import { mount } from "@arcgis/lumina-compiler/testing";
import { disabled, focusable, hidden, renders } from "../../tests/commonTests/browser";
import { focusable, hidden, renders } from "../../tests/commonTests/browser";
import { afterNextTask } from "../../tests/utils/timing";

describe("is focusable", () => {
focusable(() => mount(`calcite-dropdown-item`));
Expand All @@ -15,5 +16,37 @@ describe("renders", () => {
});

describe("disabled", () => {
disabled(() => mount(`calcite-dropdown-item`));
it("prevents selection event when disabled", async () => {
const { el, reRender } = await mount("calcite-dropdown-item");
const selectSpy = vi.fn();
el.addEventListener("calciteDropdownItemSelect", selectSpy);

el.disabled = true;
await reRender();
await afterNextTask();

el.click();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer userEvent.click(el); for simulating user interactions.


expect(selectSpy).toHaveBeenCalledTimes(0);
expect(el.getAttribute("aria-disabled")).toBe("true");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: expect(el).toHaveAttribute("aria-disabled", "true"); as an idiomatic alternative.

});

it("allows selection event again after enabling", async () => {
const { el, reRender } = await mount("calcite-dropdown-item");
const selectSpy = vi.fn();
el.addEventListener("calciteDropdownItemSelect", selectSpy);

el.disabled = true;
await reRender();
await afterNextTask();

el.disabled = false;
await reRender();
await afterNextTask();

el.click();

expect(selectSpy).toHaveBeenCalledTimes(1);
expect(el.getAttribute("aria-disabled")).toBeNull();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,6 @@ it("should emit calciteDropdownItemSelect", async () => {
await calciteDropdownItemSelectEventSpy.next();

expect(itemChangeSpy).toHaveReceivedEventTimes(1);

await element.callMethod("setFocus");
await page.waitForChanges();
await page.keyboard.press("Enter");
await page.waitForChanges();
await calciteDropdownItemSelectEventSpy.next();

expect(itemChangeSpy).toHaveReceivedEventTimes(2);

await element.callMethod("setFocus");
await page.waitForChanges();
await page.keyboard.press("Space");
await page.waitForChanges();
await calciteDropdownItemSelectEventSpy.next();

expect(itemChangeSpy).toHaveReceivedEventTimes(3);
});

describe("theme", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@
}

//focus
:host(:focus) {
:host(:focus),
:host([active-descendant]) {
.container {
@apply focus-inset no-underline;
}
Expand Down
58 changes: 22 additions & 36 deletions packages/components/src/components/dropdown-item/dropdown-item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
setAttribute,
} from "@arcgis/lumina";
import { toAriaBoolean } from "../../utils/aria";
import { ItemKeyboardEvent } from "../dropdown/interfaces";
import { RequestedItem } from "../dropdown-group/interfaces";
import { FlipContext, Scale, SelectionMode } from "../interfaces";
import { getIconScale } from "../../utils/component";
Expand Down Expand Up @@ -59,6 +58,13 @@ export class DropdownItem extends LitElement {
/** When `true`, prevents interaction and decreases the component's opacity. */
@property({ reflect: true }) disabled = false;

/**
* When `true`, the component appears as if it is focused.
*
* @private
*/
@property({ reflect: true }) activeDescendant = false;

/**
* Specifies the URL of the linked resource, which can be set as an absolute or relative path.
*
Expand Down Expand Up @@ -120,19 +126,27 @@ export class DropdownItem extends LitElement {
return this.focusSetter(() => this.el, options);
}

/**
* Activates the component as if it were clicked.
*
* @private
*/
@method()
async activateItem(): Promise<void> {
this.emitRequestedItem();

if (this.href) {
this.childLinkRef.value?.click();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note: we might want to revisit this since it would cause an extra click event that might be unexpected.

}
}

//#endregion

//#region Events

/** Fires when the component is selected. */
calciteDropdownItemSelect = createEvent({ cancelable: false });

/** @private */
calciteInternalDropdownCloseRequest = createEvent({ cancelable: false });

/** @private */
calciteInternalDropdownItemKeyEvent = createEvent<ItemKeyboardEvent>({ cancelable: false });

/** @private */
calciteInternalDropdownItemSelect = createEvent<RequestedItem>({ cancelable: false });

Expand All @@ -143,7 +157,6 @@ export class DropdownItem extends LitElement {
constructor() {
super();
this.listen("click", this.onClick);
this.listen("keydown", this.keyDownHandler);
this.listenOn<CustomEvent>(
document.body,
"calciteInternalDropdownItemChange",
Expand All @@ -167,33 +180,6 @@ export class DropdownItem extends LitElement {
this.emitRequestedItem();
}

private keyDownHandler(event: KeyboardEvent): void {
switch (event.key) {
case " ":
case "Enter":
this.emitRequestedItem();
if (this.href) {
this.childLinkRef.value.click();
}
event.preventDefault();
break;
case "Escape":
this.calciteInternalDropdownCloseRequest.emit();
event.preventDefault();
break;
case "Tab":
this.calciteInternalDropdownItemKeyEvent.emit({ keyboardEvent: event });
break;
case "ArrowUp":
case "ArrowDown":
case "Home":
case "End":
event.preventDefault();
this.calciteInternalDropdownItemKeyEvent.emit({ keyboardEvent: event });
break;
}
}

private updateActiveItemOnChange(event: CustomEvent): void {
const parentEmittedChange = event.composedPath().includes(this.parentDropdownGroupEl);

Expand Down Expand Up @@ -313,7 +299,7 @@ export class DropdownItem extends LitElement {
/* TODO: [MIGRATION] This used <Host> before. In Stencil, <Host> props overwrite user-provided props. If you don't wish to overwrite user-values, replace "=" here with "??=" */
this.el.role = itemRole;
/* TODO: [MIGRATION] This used <Host> before. In Stencil, <Host> props overwrite user-provided props. If you don't wish to overwrite user-values, add a check for this.el.hasAttribute() before calling setAttribute() here */
setAttribute(this.el, "tabIndex", disabled ? -1 : 0);
setAttribute(this.el, "tabIndex", -1);

return (
<this.interactiveContainer disabled={disabled}>
Expand Down
130 changes: 128 additions & 2 deletions packages/components/src/components/dropdown/dropdown.browser.e2e.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { h } from "@arcgis/lumina";
import { mount } from "@arcgis/lumina-compiler/testing";
import { describe } from "vitest";
import { describe, expect, it } from "vitest";
import { page, userEvent } from "vitest/browser";
import { JsxNode } from "@arcgis/lumina";
import {
defaults,
Expand All @@ -12,7 +13,9 @@ import {
disabled,
topLayer,
} from "../../tests/commonTests/browser";
import { afterNextTask } from "../../tests/utils/timing";
import { CSS } from "./resources";
import { Dropdown } from "./dropdown";

describe("defaults", () => {
defaults(() => mount("calcite-dropdown"), {
Expand Down Expand Up @@ -125,7 +128,7 @@ describe("disabled", () => {
focusTarget: {
tab: "calcite-button",
click: {
pointer: "calcite-dropdown-item",
pointer: "calcite-button",
method: "body",
},
},
Expand All @@ -136,3 +139,126 @@ describe("disabled", () => {
describe("top layer placement", () => {
topLayer(() => mount("calcite-dropdown"));
});

describe("hover type", () => {
function createHoverDropdownHTML(): JsxNode {
return (
<calcite-dropdown type="hover">
<calcite-action id="trigger" slot="trigger">
Open dropdown
</calcite-action>
<calcite-dropdown-group selection-mode="single">
<calcite-dropdown-item id="item-1"> Dropdown Item Content</calcite-dropdown-item>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: there's leading whitespace in the item's text. 🤓

<calcite-dropdown-item id="item-2" selected>
Dropdown Item Content
</calcite-dropdown-item>
</calcite-dropdown-group>
</calcite-dropdown>
);
}

it("opens on focusin", async () => {
const { el } = await mount<Dropdown>(createHoverDropdownHTML);
const trigger = page.getByText("Open dropdown");

expect(el.open).toBe(false);

await expect.element(trigger).toBeInTheDocument();
await userEvent.click(trigger);
await afterNextTask();

expect(el.open).toBe(true);
});

it("does not toggle closed on click when type is hover", async () => {
const { el } = await mount<Dropdown>(createHoverDropdownHTML);
const trigger = page.getByText("Open dropdown");

expect(el.open).toBe(false);

await userEvent.click(trigger);
await afterNextTask();

expect(el.open).toBe(true);

await userEvent.click(trigger);
await afterNextTask();

expect(el.open).toBe(true);
});

it("closes when focus leaves trigger with Tab", async () => {
const { el } = await mount(
<div>
{createHoverDropdownHTML()}
<button id="next-focus-target" type="button">
Next
</button>
</div>,
);
const dropdownEl = el as Dropdown["el"];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can pass the type to mount for the main component:

const { el } = await mount<Dropdown>(/* ... */);

const trigger = page.getByText("Open dropdown");
const nextFocusTarget = page.getByRole("button", { name: "Next" });

await userEvent.click(trigger);
await afterNextTask();
expect(dropdownEl.open).toBe(true);

await userEvent.tab();
await afterNextTask();

await expect.element(nextFocusTarget).toHaveFocus();
expect(dropdownEl.open).toBe(false);
});
});

describe("ariaActiveDescendantElement", () => {
it("sets ariaActiveDescendantElement on the trigger slot when opened", async () => {
const { el } = await mount<Dropdown>(createSimpleDropdownHTML);
const trigger = page.getByText("Open dropdown");

await userEvent.click(trigger);
await afterNextTask();

const triggerSlot = el.shadowRoot.querySelector("slot[name='trigger']") as HTMLSlotElement;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use locators instead of querying the DOM?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can pass the type to querySelector:

const triggerSlot = el.shadowRoot.querySelector<HTMLSlotElement>("slot[name='trigger']");


expect(triggerSlot.ariaActiveDescendantElement?.id).toBe("item-2");
});

it("updates ariaActiveDescendantElement on keyboard navigation", async () => {
const { el } = await mount<Dropdown>(createSimpleDropdownHTML);
const trigger = page.getByText("Open dropdown");

await userEvent.click(trigger);
await afterNextTask();

const referenceEl = el.shadowRoot.querySelector(`.${CSS.triggerContainer}`) as HTMLDivElement;
const triggerSlot = el.shadowRoot.querySelector("slot[name='trigger']") as HTMLSlotElement;

referenceEl.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowDown", bubbles: true }));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this emit from the trigger?

// trigger focused in earlier click
await userEvent.keyboard({ArrowDown>}); // > presses without releasing - see https://testing-library.com/docs/user-event/keyboard/

Applies to other synthetic dispatching.

await afterNextTask();

expect(triggerSlot.ariaActiveDescendantElement?.id).toBe("item-3");
});

it("wraps ariaActiveDescendantElement on ArrowUp navigation", async () => {
const { el } = await mount<Dropdown>(createSimpleDropdownHTML);
const trigger = page.getByText("Open dropdown");

await userEvent.click(trigger);
await afterNextTask();

const referenceEl = el.shadowRoot.querySelector(`.${CSS.triggerContainer}`) as HTMLDivElement;
const triggerSlot = el.shadowRoot.querySelector("slot[name='trigger']") as HTMLSlotElement;

referenceEl.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowUp", bubbles: true }));
await afterNextTask();

expect(triggerSlot.ariaActiveDescendantElement?.id).toBe("item-1");

referenceEl.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowUp", bubbles: true }));
await afterNextTask();

expect(triggerSlot.ariaActiveDescendantElement?.id).toBe("item-3");
});
});
Loading
Loading