Skip to content

Commit

Permalink
fix(combobox): ensure most recent selected item is active when combob…
Browse files Browse the repository at this point in the history 
…ox is opened (#6973)

**Related Issue:** N/A

## Summary

This should help users recall the most recent selection when opening the
combobox. Previously, this was not ideal if there is a large list of
items since the active item index would start from the top when opened.
This is especially noticeable in single-selection mode where the input
showing the selected value is cleared on open.

Stems from #6947.
  • Loading branch information
@jcfranco
jcfranco committed May 18, 2023
1 parent 5112279 commit 8476595
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 15 deletions.
113 changes: 103 additions & 10 deletions src/components/combobox/combobox.e2e.ts
View file
Expand Up @@ -13,6 +13,7 @@ import {

import { html } from "../../../support/formatting";
import { CSS } from "./resources";
import { skipAnimations } from "../../tests/utils";

describe("calcite-combobox", () => {
describe("renders", () => {
Expand Down Expand Up @@ -699,17 +700,29 @@ describe("calcite-combobox", () => {
const element = await page.find("calcite-combobox");
await element.click();
expect(await item1.getProperty("active")).toBe(true);
expect(await item2.getProperty("active")).toBe(false);
expect(await item3.getProperty("active")).toBe(false);
await element.press("ArrowDown");
expect(await item1.getProperty("active")).toBe(false);
expect(await item2.getProperty("active")).toBe(true);
expect(await item3.getProperty("active")).toBe(false);
await element.press("ArrowUp");
expect(await item1.getProperty("active")).toBe(true);
expect(await item2.getProperty("active")).toBe(false);
expect(await item3.getProperty("active")).toBe(false);
await element.press("ArrowUp");
expect(await item3.getProperty("active")).toBe(true);
expect(await item1.getProperty("active")).toBe(false);
expect(await item2.getProperty("active")).toBe(false);
expect(await item3.getProperty("active")).toBe(true);
await element.press("ArrowUp");
expect(await item1.getProperty("active")).toBe(false);
expect(await item2.getProperty("active")).toBe(true);
expect(await item3.getProperty("active")).toBe(false);
await element.press("ArrowDown");
await element.press("ArrowDown");
expect(await item1.getProperty("active")).toBe(true);
expect(await item2.getProperty("active")).toBe(false);
expect(await item3.getProperty("active")).toBe(false);
await element.press("Enter");
expect(await item1.getProperty("selected")).toBe(true);
expect(eventSpy).toHaveReceivedEventTimes(1);
Expand Down Expand Up @@ -738,17 +751,17 @@ describe("calcite-combobox", () => {
});

it("should cycle through chips on left/right keys", async () => {
expect(chips[0]).not.toBeNull();
expect(chips[1]).not.toBeNull();
expect(chips[2]).not.toBeNull();

await element.click();
await page.waitForChanges();

await element.press("ArrowLeft");
expect(chips[0]).not.toHaveClass("chip--active");
expect(chips[1]).not.toHaveClass("chip--active");
expect(chips[2]).toHaveClass("chip--active");

await element.press("ArrowLeft");
expect(await chips[1]).toHaveClass("chip--active");
expect(chips[0]).not.toHaveClass("chip--active");
expect(chips[1]).toHaveClass("chip--active");
expect(chips[2]).not.toHaveClass("chip--active");

await element.press("Delete");
Expand All @@ -757,10 +770,6 @@ describe("calcite-combobox", () => {
});

it("should delete last chip on Delete", async () => {
expect(chips[0]).not.toBeNull();
expect(chips[1]).not.toBeNull();
expect(chips[2]).not.toBeNull();

await element.click();

await element.press("Backspace");
Expand Down Expand Up @@ -1316,6 +1325,7 @@ describe("calcite-combobox", () => {
await page.setContent(html` <calcite-combobox id="demoId">
<calcite-combobox-item value="test-value" text-label="test"> </calcite-combobox-item>
</calcite-combobox>`);
await skipAnimations(page);
const item = await page.find("calcite-combobox-item");
await item.click();
await page.waitForChanges();
Expand All @@ -1332,6 +1342,7 @@ describe("calcite-combobox", () => {
await page.setContent(html` <calcite-combobox id="demoId">
<calcite-combobox-item value="test-value" text-label="test"> </calcite-combobox-item>
</calcite-combobox>`);
await skipAnimations(page);
await page.keyboard.press("Tab");
await page.keyboard.press("ArrowDown");
await page.keyboard.press("Enter");
Expand All @@ -1340,4 +1351,86 @@ describe("calcite-combobox", () => {
const focusedId = await page.evaluate(() => document.activeElement.id);
expect(focusedId).toBe("demoId");
});

describe("active item when opened", () => {
async function assertActiveItem(html: string, expectedActiveItemValue: string): Promise<void> {
const page = await newE2EPage();
await skipAnimations(page);
await page.setContent(html);
await page.click("calcite-combobox");
await page.waitForChanges();

const activeItem = await page.find("calcite-combobox-item[active]");
expect(await activeItem.getProperty("value")).toBe(expectedActiveItemValue);
}

describe("single-selection", () => {
it("shows the first item as active if there is no previous selection", async () =>
assertActiveItem(
html`<calcite-combobox selection-mode="single">
<calcite-combobox-item value="item1" text-label="item1"></calcite-combobox-item>
<calcite-combobox-item value="item2" text-label="item2"></calcite-combobox-item>
</calcite-combobox>`,
"item1"
));

it("shows the selected item as active when opened", async () =>
assertActiveItem(
html`<calcite-combobox selection-mode="single">
<calcite-combobox-item value="item1" text-label="item1"></calcite-combobox-item>
<calcite-combobox-item value="item2" text-label="item2"></calcite-combobox-item>
<calcite-combobox-item value="item3" text-label="item3" selected></calcite-combobox-item>
</calcite-combobox>`,
"item3"
));
});

describe("multiple-selection", () => {
it("shows the first item as active if there is no previous selection", async () =>
assertActiveItem(
html` <calcite-combobox selection-mode="multiple">
<calcite-combobox-item value="item1" text-label="item1"></calcite-combobox-item>
<calcite-combobox-item value="item2" text-label="item2"></calcite-combobox-item>
<calcite-combobox-item value="item3" text-label="item3"></calcite-combobox-item>
</calcite-combobox>`,
"item1"
));

it("shows the last selected item as active", async () =>
assertActiveItem(
html` <calcite-combobox selection-mode="multiple">
<calcite-combobox-item selected value="item1" text-label="item1"></calcite-combobox-item>
<calcite-combobox-item value="item2" text-label="item2" selected></calcite-combobox-item>
<calcite-combobox-item selected value="item3" text-label="item3"></calcite-combobox-item>
</calcite-combobox>`,
"item3"
));
});

describe("ancestors-selection", () => {
it("shows the first item as active if there is no previous selection", async () =>
assertActiveItem(
html` <calcite-combobox selection-mode="ancestors">
<calcite-combobox-item value="item1" text-label="parent">
<calcite-combobox-item value="item1_1" text-label="item1_1"></calcite-combobox-item>
</calcite-combobox-item>
<calcite-combobox-item value="item2" text-label="item2"></calcite-combobox-item>
<calcite-combobox-item value="item3" text-label="item3"></calcite-combobox-item>
</calcite-combobox>`,
"item1"
));

it("shows the last selected item as active", async () =>
assertActiveItem(
html` <calcite-combobox selection-mode="ancestors">
<calcite-combobox-item value="item1" text-label="parent" selected>
<calcite-combobox-item value="item1_1" text-label="item1_1"></calcite-combobox-item>
</calcite-combobox-item>
<calcite-combobox-item value="item2" text-label="item2"></calcite-combobox-item>
<calcite-combobox-item value="item3" text-label="item3" selected></calcite-combobox-item>
</calcite-combobox>`,
"item3"
));
});
});
});
25 changes: 20 additions & 5 deletions src/components/combobox/combobox.tsx
View file
Expand Up @@ -558,17 +558,23 @@ export class Combobox
break;
case "ArrowUp":
event.preventDefault();
this.shiftActiveItemIndex(-1);
if (this.open) {
this.shiftActiveItemIndex(-1);
}

if (!this.comboboxInViewport()) {
this.el.scrollIntoView();
}
break;
case "ArrowDown":
event.preventDefault();
if (!this.open) {
if (this.open) {
this.shiftActiveItemIndex(1);
} else {
this.open = true;
this.ensureRecentSelectedItemIsActive();
}
this.shiftActiveItemIndex(1);

if (!this.comboboxInViewport()) {
this.el.scrollIntoView();
}
Expand Down Expand Up @@ -646,6 +652,7 @@ export class Combobox
};

onBeforeOpen(): void {
this.scrollToActiveItem();
this.calciteComboboxBeforeOpen.emit();
}

Expand Down Expand Up @@ -694,10 +701,17 @@ export class Combobox
return;
}
this.open = !this.open;
this.updateActiveItemIndex(0);
this.setFocus();
this.ensureRecentSelectedItemIsActive();
};

private ensureRecentSelectedItemIsActive(): void {
const { selectedItems } = this;
const targetIndex =
selectedItems.length === 0 ? 0 : this.items.indexOf(selectedItems[selectedItems.length - 1]);

this.updateActiveItemIndex(targetIndex);
}

setInactiveIfNotContained = (event: Event): void => {
const composedPath = event.composedPath();

Expand Down Expand Up @@ -1116,6 +1130,7 @@ export class Combobox
const single = selectionMode === "single";
const selectedItem = selectedItems[0];
const showLabel = !open && single && !!selectedItem;

return (
<span
class={{
Expand Down

0 comments on commit 8476595

Please sign in to comment.