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(list): fix keyboard navigation after a listItem's disabled or closed property changes #7275

Merged
merged 7 commits into from
Jul 3, 2023
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
20 changes: 20 additions & 0 deletions packages/calcite-components/src/components/list-item/list-item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ export class ListItem
/** When `true`, hides the component. */
@Prop({ reflect: true, mutable: true }) closed = false;

@Watch("closed")
handleClosedChange(): void {
this.emitCalciteInternalListItemChange();
}

/**
* A description for the component. Displays below the label text.
*/
Expand All @@ -96,6 +101,11 @@ export class ListItem
*/
@Prop({ reflect: true }) disabled = false;

@Watch("disabled")
handleDisabledChange(): void {
this.emitCalciteInternalListItemChange();
}

/**
* The label text of the component. Displays above the description text.
*/
Expand Down Expand Up @@ -210,6 +220,12 @@ export class ListItem
*/
@Event({ cancelable: false }) calciteInternalFocusPreviousItem: EventEmitter<void>;

/**
*
* @internal
*/
@Event({ cancelable: false }) calciteInternalListItemChange: EventEmitter<void>;

// --------------------------------------------------------------------------
//
// Private Properties
Expand Down Expand Up @@ -541,6 +557,10 @@ export class ListItem
//
// --------------------------------------------------------------------------

private emitCalciteInternalListItemChange(): void {
this.calciteInternalListItemChange.emit();
}

closeClickHandler = (): void => {
this.closed = true;
this.calciteListItemClose.emit();
Expand Down
120 changes: 97 additions & 23 deletions packages/calcite-components/src/components/list/list.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { newE2EPage } from "@stencil/core/testing";
import { debounceTimeout } from "./resources";
import { CSS } from "../list-item/resources";
import { DEBOUNCE_TIMEOUT as FILTER_DEBOUNCE_TIMEOUT } from "../filter/resources";
import { isElementFocused } from "../../tests/utils";

const placeholder = placeholderImage({
width: 140,
Expand Down Expand Up @@ -209,13 +210,14 @@ describe("calcite-list", () => {
await page.waitForChanges();
await page.waitForTimeout(listDebounceTimeout);

const list = await page.find("calcite-list");
const items = await page.findAll("calcite-list-item");

expect(await items[0].getProperty("active")).toBe(true);
expect(await items[1].getProperty("active")).toBe(false);
expect(await items[2].getProperty("active")).toBe(false);

const eventSpy = await page.spyOnEvent("calciteInternalListItemActive");
const eventSpy = await list.spyOnEvent("calciteInternalListItemActive");

await items[1].click();

Expand All @@ -229,13 +231,12 @@ describe("calcite-list", () => {
});

it("should prevent de-selection of selected item in single-persist mode", async () => {
const page = await newE2EPage({
html: html`<calcite-list selection-mode="single-persist">
<calcite-list-item id="item-1" label="hello" description="world"></calcite-list-item>
<calcite-list-item id="item-2" label="hello 2" description="world 2"></calcite-list-item>
<calcite-list-item id="item-3" selected label="hello 3" description="world 3"></calcite-list-item>
</calcite-list>`
});
const page = await newE2EPage();
await page.setContent(html`<calcite-list selection-mode="single-persist">
<calcite-list-item id="item-1" label="hello" description="world"></calcite-list-item>
<calcite-list-item id="item-2" label="hello 2" description="world 2"></calcite-list-item>
<calcite-list-item id="item-3" selected label="hello 3" description="world 3"></calcite-list-item>
</calcite-list>`);

await page.waitForChanges();
await page.waitForTimeout(listDebounceTimeout);
Expand All @@ -260,13 +261,12 @@ describe("calcite-list", () => {
});

it("should correctly allow de-selection and change of selected item in single mode", async () => {
const page = await newE2EPage({
html: html`<calcite-list selection-mode="single">
<calcite-list-item id="item-1" label="hello" description="world"></calcite-list-item>
<calcite-list-item id="item-2" label="hello 2" description="world 2"></calcite-list-item>
<calcite-list-item id="item-3" selected label="hello 3" description="world 3"></calcite-list-item>
</calcite-list>`
});
const page = await newE2EPage();
await page.setContent(html`<calcite-list selection-mode="single">
<calcite-list-item id="item-1" label="hello" description="world"></calcite-list-item>
<calcite-list-item id="item-2" label="hello 2" description="world 2"></calcite-list-item>
<calcite-list-item id="item-3" selected label="hello 3" description="world 3"></calcite-list-item>
</calcite-list>`);

await page.waitForChanges();
await page.waitForTimeout(listDebounceTimeout);
Expand Down Expand Up @@ -301,14 +301,13 @@ describe("calcite-list", () => {
});

it("should emit calciteListChange on selection change", async () => {
const page = await newE2EPage({
html: html`
<calcite-list selection-mode="single">
<calcite-list-item value="one" label="One" description="hello world"></calcite-list-item>
<calcite-list-item value="two" label="Two" description="hello world"></calcite-list-item>
</calcite-list>
`
});
const page = await newE2EPage();
await page.setContent(html`
<calcite-list selection-mode="single">
<calcite-list-item value="one" label="One" description="hello world"></calcite-list-item>
<calcite-list-item value="two" label="Two" description="hello world"></calcite-list-item>
</calcite-list>
`);
await page.waitForChanges();
const list = await page.find("calcite-list");
const listItemOne = await page.find(`calcite-list-item[value=one]`);
Expand Down Expand Up @@ -339,4 +338,79 @@ describe("calcite-list", () => {
expect(await listItemOne.getProperty("selected")).toBe(false);
expect(await list.getProperty("selectedItems")).toHaveLength(0);
});

describe("keyboard navigation", () => {
it("should navigate via ArrowUp, ArrowDown, Home, and End", async () => {
const page = await newE2EPage();
await page.setContent(html`
<calcite-list>
<calcite-list-item id="one" value="one" label="One" description="hello world"></calcite-list-item>
<calcite-list-item id="two" value="two" label="Two" description="hello world"></calcite-list-item>
<calcite-list-item
disabled
id="three"
value="three"
label="three"
description="hello world"
></calcite-list-item>
<calcite-list-item
closable
closed
id="four"
value="four"
label="four"
description="hello world"
></calcite-list-item>
</calcite-list>
`);
await page.waitForChanges();
const list = await page.find("calcite-list");
await list.callMethod("setFocus");
await page.waitForChanges();

await isElementFocused(page, "#one");

await list.press("ArrowDown");

await isElementFocused(page, "#two");

await list.press("ArrowDown");

await isElementFocused(page, "#two");

await list.press("ArrowUp");

await isElementFocused(page, "#one");

await list.press("ArrowDown");

await isElementFocused(page, "#two");

const listItemThree = await page.find("#three");
listItemThree.setProperty("disabled", false);
await page.waitForChanges();
await page.waitForTimeout(listDebounceTimeout);

await list.press("ArrowDown");

await isElementFocused(page, "#three");

const listItemFour = await page.find("#four");
listItemFour.setProperty("closed", false);
await page.waitForChanges();
await page.waitForTimeout(listDebounceTimeout);

await list.press("ArrowDown");

await isElementFocused(page, "#four");

await list.press("Home");

await isElementFocused(page, "#one");

await list.press("End");

await isElementFocused(page, "#four");
});
});
});
7 changes: 5 additions & 2 deletions packages/calcite-components/src/components/list/list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ export class List implements InteractiveComponent, LoadableComponent {

@Listen("calciteInternalListItemActive")
handleCalciteInternalListItemActive(event: CustomEvent): void {
event.stopPropagation();
const target = event.target as HTMLCalciteListItemElement;
const { listItems } = this;

Expand All @@ -190,6 +191,7 @@ export class List implements InteractiveComponent, LoadableComponent {

@Listen("calciteInternalListItemSelect")
handleCalciteInternalListItemSelect(event: CustomEvent): void {
event.stopPropagation();
const target = event.target as HTMLCalciteListItemElement;
const { listItems, selectionMode } = this;

Expand All @@ -200,8 +202,9 @@ export class List implements InteractiveComponent, LoadableComponent {
this.updateSelectedItems();
}

@Listen("calciteListItemClose")
handleCalciteListItemClose(): void {
@Listen("calciteInternalListItemChange")
handleCalciteInternalListItemChange(event: CustomEvent): void {
event.stopPropagation();
this.updateListItems(true);
}

Expand Down