Skip to content

feat(combobox): make combobox clearable #6972

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

Merged
merged 13 commits into from
May 18, 2023
Merged
Show file tree
Hide file tree
Changes from 8 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
58 changes: 58 additions & 0 deletions src/assets/styles/includes.scss
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,61 @@
pointer-events: none;
}
}

@mixin xButton() {
:host([scale="s"]) {
.x-button {
inline-size: theme("spacing.4");
block-size: theme("spacing.4");
}
}

:host([scale="m"]) {
.x-button {
inline-size: theme("spacing.6");
block-size: theme("spacing.6");
}
}

:host([scale="l"]) {
.x-button {
inline-size: theme("spacing.8");
block-size: theme("spacing.8");
}
}

.x-button {
@apply flex
self-center
focus-base
transition-default
text-color-3
m-0
border-2
cursor-pointer
items-center
bg-transparent;
-webkit-appearance: none;
display: flex;
border-radius: 50%;
align-content: center;
justify-content: center;
border-color: transparent;
background-color: var(--calcite-ui-foreground-2);

&:active,
&:hover {
@apply text-color-3;
background-color: var(--calcite-ui-foreground-3);
}

&:active {
@apply border-solid;
border-color: theme("borderColor.color-brand");
}

& calcite-icon {
color: inherit;
}
}
}
9 changes: 6 additions & 3 deletions src/components/combobox-item/combobox-item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,14 @@ export class ComboboxItem implements ConditionalSlotComponent, InteractiveCompon
//
// --------------------------------------------------------------------------

toggleSelected(coerce?: boolean): Promise<void> {
if (this.disabled) {
toggleSelected(): Promise<void> {
const isSingleSelect = getElementProp(this.el, "selection-mode", "multiple") === "single";
Copy link
Member

Choose a reason for hiding this comment

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

Just a note that getElementProp is deprecated since it actually only gets the attribute and not the prop. I know we'll need to refactor this at some point.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I remember. I'll submit a PR to rename it today, so it's not misleading when reading code.

Copy link
Member

Choose a reason for hiding this comment

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

related to #6038


if (this.disabled || (isSingleSelect && this.selected)) {
return;
}
this.selected = typeof coerce === "boolean" ? coerce : !this.selected;

this.selected = !this.selected;
}

itemClickHandler = (event: MouseEvent): void => {
Expand Down
1 change: 1 addition & 0 deletions src/components/combobox/assets/combobox/t9n/messages.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{
"clear": "Clear value",
"removeTag": "Remove tag"
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{
"clear": "Clear value",
"removeTag": "Remove tag"
}
200 changes: 176 additions & 24 deletions src/components/combobox/combobox.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import {
floatingUIOwner,
formAssociated,
disabled,
t9n
t9n,
reflects
} from "../../tests/commonTests";

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

describe("calcite-combobox", () => {
Expand All @@ -22,6 +24,10 @@ describe("calcite-combobox", () => {

it("defaults", async () =>
defaults("calcite-combobox", [
{
propertyName: "clearDisabled",
defaultValue: false
},
{
propertyName: "overlayPositioning",
defaultValue: "absolute"
Expand All @@ -32,6 +38,58 @@ describe("calcite-combobox", () => {
}
]));

it("reflects", async () =>
reflects("calcite-combobox", [
{
propertyName: "allowCustomValues",
value: true
},
{
propertyName: "clearDisabled",
value: true
},
{
propertyName: "disabled",
value: true
},
{
propertyName: "form",
value: "test-form"
},
{
propertyName: "maxItems",
value: 1
},
{
propertyName: "name",
value: "test-name"
},
{
propertyName: "open",
value: true
},
{
propertyName: "placeholderIcon",
value: "banana"
},
{
propertyName: "placeholderIconFlipRtl",
value: true
},
{
propertyName: "required",
value: true
},
{
propertyName: "scale",
value: "m"
},
{
propertyName: "selectionMode",
value: "single"
}
]));

describe("honors hidden attribute", () => {
hidden("calcite-combobox");
});
Expand Down Expand Up @@ -303,33 +361,86 @@ describe("calcite-combobox", () => {
});

describe("item selection", () => {
it("should add/remove item to the selected items when an item is clicked", async () => {
const page = await newE2EPage();
await page.setContent(
html`
<calcite-combobox>
<calcite-combobox-item value="one" text-label="one"></calcite-combobox-item>
<calcite-combobox-item value="two" text-label="two"></calcite-combobox-item>
</calcite-combobox>
`
);
const cbox = await page.find("calcite-combobox");
const openEvent = page.waitForEvent("calciteComboboxOpen");
await cbox.click();
await openEvent;
describe("toggling items", () => {
it("single-selection mode does not allow toggling selection once the selected item is clicked", async () => {
const page = await newE2EPage();
await page.setContent(
html`
<calcite-combobox>
<calcite-combobox-item value="one" text-label="one"></calcite-combobox-item>
<calcite-combobox-item value="two" text-label="two"></calcite-combobox-item>
</calcite-combobox>
`
);
const cbox = await page.find("calcite-combobox");
const openEvent = page.waitForEvent("calciteComboboxOpen");
await cbox.click();
await openEvent;

const item1 = await cbox.find("calcite-combobox-item[value=one]");

let item1 = await cbox.find("calcite-combobox-item[value=one]");
await item1.click();
await item1.click();
expect(await page.find("calcite-combobox >>> calcite-chip")).toBeDefined();

let chip = await page.find("calcite-combobox >>> calcite-chip");
expect(chip).toBeDefined();
await item1.click();
expect(await page.find("calcite-combobox >>> calcite-chip")).toBeDefined();
});

item1 = await cbox.find("calcite-combobox-item[value=one]");
await item1.click();
await page.waitForChanges();
it("multiple-selection mode allows toggling selection once the selected item is clicked", async () => {
const page = await newE2EPage();
await page.setContent(
html`
<calcite-combobox selection-mode="multiple">
<calcite-combobox-item value="one" text-label="one"></calcite-combobox-item>
<calcite-combobox-item value="two" text-label="two"></calcite-combobox-item>
</calcite-combobox>
`
);
const cbox = await page.find("calcite-combobox");
const openEvent = page.waitForEvent("calciteComboboxOpen");
await cbox.click();
await openEvent;

const item1 = await cbox.find("calcite-combobox-item[value=one]");

chip = await page.find("calcite-combobox >>> calcite-chip");
expect(chip).toBeNull();
await item1.click();
expect(await page.find("calcite-combobox >>> calcite-chip")).toBeDefined();

await item1.click();
expect(await page.find("calcite-combobox >>> calcite-chip")).toBeNull();

await item1.click();
expect(await page.find("calcite-combobox >>> calcite-chip")).toBeDefined();
});

it("ancestors-selection mode allows toggling selection once the selected item is clicked", async () => {
const page = await newE2EPage();
await page.setContent(
html`
<calcite-combobox selection-mode="ancestors">
<calcite-combobox-item value="one" text-label="parent">
<calcite-combobox-item value="two" text-label="child1"></calcite-combobox-item>
<calcite-combobox-item value="three" text-label="child2"></calcite-combobox-item>
</calcite-combobox-item>
</calcite-combobox>
`
);
const cbox = await page.find("calcite-combobox");
const openEvent = page.waitForEvent("calciteComboboxOpen");
await cbox.click();
await openEvent;

const item1 = await cbox.find("calcite-combobox-item[value=one]");

await item1.click();
expect(await page.find("calcite-combobox >>> calcite-chip")).toBeDefined();

await item1.click();
expect(await page.find("calcite-combobox >>> calcite-chip")).toBeNull();

await item1.click();
expect(await page.find("calcite-combobox >>> calcite-chip")).toBeDefined();
});
});

it("should select parent in ancestor selection mode", async () => {
Expand Down Expand Up @@ -540,6 +651,47 @@ describe("calcite-combobox", () => {
});
});

describe("clearing values", () => {
async function assertValueClearing(html: string): Promise<void> {
const page = await newE2EPage();
await page.setContent(html);
const combobox = await page.find("calcite-combobox");
const xButton = await page.find(`calcite-combobox >>> .${XButtonCSS.button}`);

await xButton.click();

expect(await combobox.getProperty("value")).toBe("");
}

it("clears the value in single-selection mode", async () =>
assertValueClearing(html`
<calcite-combobox selection-mode="single">
<calcite-combobox-item selected id="one" value="one" text-label="one"></calcite-combobox-item>
<calcite-combobox-item id="two" value="two" text-label="two"></calcite-combobox-item>
<calcite-combobox-item id="three" value="three" text-label="three"></calcite-combobox-item>
</calcite-combobox>
`));

it("clears the value in multiple-selection mode", async () =>
assertValueClearing(html`
<calcite-combobox selection-mode="multiple">
<calcite-combobox-item selected id="one" value="one" text-label="one"></calcite-combobox-item>
<calcite-combobox-item selected id="two" value="two" text-label="two"></calcite-combobox-item>
<calcite-combobox-item selected id="three" value="three" text-label="three"></calcite-combobox-item>
</calcite-combobox>
`));

it("clears the value in ancestors-selection mode", async () =>
assertValueClearing(html`
<calcite-combobox selection-mode="ancestors">
<calcite-combobox-item value="parent" text-label="parent">
<calcite-combobox-item value="child1" text-label="child1"></calcite-combobox-item>
<calcite-combobox-item selected value="child2" text-label="child2"></calcite-combobox-item>
</calcite-combobox-item>
</calcite-combobox>
`));
});

describe("keyboard navigation", () => {
let page: E2EPage;
const scrollablePageSizeInPx = 2400;
Expand Down
13 changes: 13 additions & 0 deletions src/components/combobox/combobox.scss
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,39 @@
}

@include disabled();
@include xButton();

:host([scale="s"]) {
@apply text-n2;
--calcite-combobox-item-spacing-unit-l: theme("spacing.2");
--calcite-combobox-item-spacing-unit-s: theme("spacing.1");
--calcite-combobox-input-height: theme("spacing.6");

.x-button {
margin-inline-end: theme("spacing.2");
}
}

:host([scale="m"]) {
@apply text-n1;
--calcite-combobox-item-spacing-unit-l: theme("spacing.3");
--calcite-combobox-item-spacing-unit-s: theme("spacing.2");
--calcite-combobox-input-height: theme("spacing.8");

.x-button {
margin-inline-end: theme("spacing.4");
}
}

:host([scale="l"]) {
@apply text-0;
--calcite-combobox-item-spacing-unit-l: theme("spacing.4");
--calcite-combobox-item-spacing-unit-s: theme("spacing.3");
--calcite-combobox-input-height: theme("spacing.11");

.x-button {
margin-inline-end: theme("spacing.6");
}
}

.wrapper {
Expand Down
Loading