Skip to content
Open
Show file tree
Hide file tree
Changes from 5 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
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,51 @@ 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.getBySelector("calcite-action[slot='trigger']");

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.getBySelector("calcite-action[slot='trigger']");

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);
});
});
Loading
Loading