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

refactor(combobox, combobox-item, combobox-group): getElementProp is refactored out across child components as an outdated pattern in favor of inheritable props set directly on parent #7562

Merged
merged 9 commits into from
Aug 23, 2023
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Component, Element, h, Prop, VNode } from "@stencil/core";
import { getElementProp } from "../../utils/dom";
import { guid } from "../../utils/guid";
import { ComboboxChildElement } from "../combobox/interfaces";
import { getAncestors, getDepth } from "../combobox/utils";
Expand Down Expand Up @@ -34,6 +33,13 @@ export class ComboboxItemGroup {
/** Specifies the title of the component. */
@Prop() label!: string;

/**
* Specifies the size of the component inherited from the `calcite-combobox`, defaults to `m`.
*
* @internal
*/
@Prop() scale: Scale = "m";

// --------------------------------------------------------------------------
//
// Lifecycle
Expand All @@ -42,7 +48,6 @@ export class ComboboxItemGroup {

connectedCallback(): void {
this.ancestors = getAncestors(this.el);
this.scale = getElementProp(this.el, "scale", this.scale);
}

// --------------------------------------------------------------------------
Expand All @@ -55,8 +60,6 @@ export class ComboboxItemGroup {

guid: string = guid();

scale: Scale = "m";

// --------------------------------------------------------------------------
//
// Render Methods
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
connectConditionalSlotComponent,
disconnectConditionalSlotComponent,
} from "../../utils/conditionalSlot";
import { getElementProp, getSlotted } from "../../utils/dom";
import { getSlotted } from "../../utils/dom";
import { guid } from "../../utils/guid";
import {
connectInteractive,
Expand All @@ -24,7 +24,7 @@ import {
} from "../../utils/interactive";
import { ComboboxChildElement } from "../combobox/interfaces";
import { getAncestors, getDepth } from "../combobox/utils";
import { Scale } from "../interfaces";
import { Scale, SelectionMode } from "../interfaces";
import { CSS } from "./resources";

/**
Expand Down Expand Up @@ -81,6 +81,26 @@ export class ComboboxItem implements ConditionalSlotComponent, InteractiveCompon
*/
@Prop({ reflect: true }) filterDisabled: boolean;

/**
* Specifies the selection mode:
* - `multiple` allows any number of selected items (default),
* - `single` allows only one selection,
* - `ancestors` is like multiple, but shows ancestors of selected items as selected, with only deepest children shown in chips.
*
* @internal
*/
@Prop({ reflect: true }) selectionMode: Extract<
"single" | "ancestors" | "multiple",
SelectionMode
> = "multiple";

/**
* Specifies the size of the component inherited from the `calcite-combobox`, defaults to `m`.
*
* @internal
*/
@Prop() scale: Scale = "m";

// --------------------------------------------------------------------------
//
// Private Properties
Expand All @@ -91,9 +111,6 @@ export class ComboboxItem implements ConditionalSlotComponent, InteractiveCompon

isNested: boolean;

/** Specifies the scale of the combobox-item controlled by parent, defaults to m */
scale: Scale = "m";

// --------------------------------------------------------------------------
//
// Lifecycle
Expand All @@ -102,7 +119,6 @@ export class ComboboxItem implements ConditionalSlotComponent, InteractiveCompon

connectedCallback(): void {
this.ancestors = getAncestors(this.el);
this.scale = getElementProp(this.el, "scale", this.scale);
connectConditionalSlotComponent(this);
connectInteractive(this);
}
Expand Down Expand Up @@ -206,7 +222,8 @@ export class ComboboxItem implements ConditionalSlotComponent, InteractiveCompon
}

render(): VNode {
const isSingleSelect = getElementProp(this.el, "selection-mode", "multiple") === "single";
const isSingleSelect = this.selectionMode === "single";

const showDot = isSingleSelect && !this.disabled;
const defaultIcon = isSingleSelect ? "dot" : "check";
const iconPath = this.disabled ? "circle-disallowed" : defaultIcon;
Expand Down
60 changes: 41 additions & 19 deletions packages/calcite-components/src/components/combobox/combobox.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { E2EPage, E2EElement, newE2EPage } from "@stencil/core/testing";
import {
renders,
hidden,
accessible,
defaults,
labelable,
disabled,
floatingUIOwner,
formAssociated,
disabled,
t9n,
hidden,
labelable,
reflects,
renders,
t9n,
} from "../../tests/commonTests";

import { html } from "../../../support/formatting";
Expand All @@ -29,6 +29,10 @@ describe("calcite-combobox", () => {
propertyName: "clearDisabled",
defaultValue: false,
},
{
propertyName: "flipPlacements",
defaultValue: undefined,
},
{
propertyName: "overlayPositioning",
defaultValue: "absolute",
Expand All @@ -37,6 +41,10 @@ describe("calcite-combobox", () => {
propertyName: "flipPlacements",
defaultValue: undefined,
},
{
propertyName: "scale",
defaultValue: "m",
},
]);
});

Expand Down Expand Up @@ -586,13 +594,10 @@ describe("calcite-combobox", () => {
await input.click();
await input.press("K");
await input.press("Enter");
await input.press("Escape");
await page.waitForChanges();

const item = await page.find("calcite-combobox-item:last-child");
const label = await page.find("calcite-combobox >>> .label");

expect(label.textContent).toBe("K");
expect(await item.getProperty("textLabel")).toBe("K");

const combobox = await page.find("calcite-combobox");

Expand Down Expand Up @@ -622,9 +627,9 @@ describe("calcite-combobox", () => {

const item1 = await page.find("calcite-combobox-item#one");
const item2 = await page.find("calcite-combobox-item:last-child");
const label = await page.find("calcite-combobox >>> .label");

expect(label.textContent).toBe("K");
expect(await item2.getProperty("textLabel")).toBe("K");

expect((await combobox.getProperty("selectedItems")).length).toBe(1);
expect(await item1.getProperty("selected")).toBe(false);
expect(await item2.getProperty("selected")).toBe(true);
Expand Down Expand Up @@ -923,14 +928,14 @@ describe("calcite-combobox", () => {
// set body to overflow so we can test the scroll functionality;
// set default margin/padding to 0 to not have to adjust for it in position calculations
content: `body {
height: ${scrollablePageSizeInPx}px;
width: ${scrollablePageSizeInPx}px;
}
html, body {
margin: 0;
padding: 0;
}
`,
height: ${scrollablePageSizeInPx}px;
width: ${scrollablePageSizeInPx}px;
}
html, body {
margin: 0;
padding: 0;
}
`,
});
const combobox = await page.find("calcite-combobox");
await combobox.callMethod(`setFocus`);
Expand Down Expand Up @@ -1699,4 +1704,21 @@ describe("calcite-combobox", () => {
));
});
});

it("inheritable props: `selectionMode` and `scale` modified on the parent get passed to items", async () => {
const page = await newE2EPage();
await page.setContent(html`
<calcite-combobox label="Trees" value="Trees" scale="l" selection-mode="single">
<calcite-combobox-item-group label="Conifers">
<calcite-combobox-item value="Pine" text-label="Pine"></calcite-combobox-item>
</calcite-combobox-item-group>
</calcite-combobox>
`);
const comboboxItems = await page.findAll("calcite-combobox-items");

comboboxItems.forEach(async (item) => {
expect(await item.getProperty("selectionMode")).toBe("single");
expect(await item.getProperty("scale")).toBe("l");
});
});
});
30 changes: 23 additions & 7 deletions packages/calcite-components/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,10 @@ export class Combobox
@Prop({ reflect: true }) required = false;

/**
* specify the selection mode
* - multiple: allow any number of selected items (default)
* - single: only one selection)
* - ancestors: like multiple, but show ancestors of selected items as selected, only deepest children shown in chips
* Specifies the selection mode:
* - `multiple` allows any number of selected items (default),
* - `single` allows only one selection,
* - `ancestors` is like multiple, but shows ancestors of selected items as selected, with only deepest children shown in chips.
*/
@Prop({ reflect: true }) selectionMode: Extract<
"single" | "ancestors" | "multiple",
Expand All @@ -210,6 +210,12 @@ export class Combobox
/** Specifies the size of the component. */
@Prop({ reflect: true }) scale: Scale = "m";

@Watch("selectionMode")
@Watch("scale")
handlePropsChange(): void {
this.updateItems();
}

/** The component's value(s) from the selected `calcite-combobox-item`(s). */
@Prop({ mutable: true }) value: string | string[] = null;

Expand Down Expand Up @@ -386,14 +392,18 @@ export class Combobox
connectInteractive(this);
connectLocalized(this);
connectMessages(this);
connectLabel(this);
connectForm(this);

this.internalValueChangeFlag = true;
this.value = this.getValue();
this.internalValueChangeFlag = false;
this.mutationObserver?.observe(this.el, { childList: true, subtree: true });
connectLabel(this);
connectForm(this);

this.updateItems();
this.setFilteredPlacements();
this.reposition(true);

if (this.open) {
this.openHandler();
onToggleOpenCloseComponent(this);
Expand Down Expand Up @@ -954,13 +964,19 @@ export class Combobox
);
}

updateItems = (): void => {
private updateItems = (): void => {
this.items = this.getItems();
this.groupItems = this.getGroupItems();
this.data = this.getData();
this.selectedItems = this.getSelectedItems();
this.filteredItems = this.getFilteredItems();
this.needsIcon = this.getNeedsIcon();

this.items.forEach((item) => {
item.selectionMode = this.selectionMode;
item.scale = this.scale;
});

if (!this.allowCustomValues) {
this.setMaxScrollerHeight();
}
Expand Down