refactor(combobox, combobox-item, combobox-group): getElementProp i…

…s refactored out across child components as an outdated pattern in favor of inheritable props set directly on parent (#7562) **Related Issue:** #6038 ## Summary `getElementProp` is refactored out across child components as an outdated pattern in favor of inheritable props set directly on the parent. Instead of the `combobox-item` and `combobox-group` looking up the parent for scale, these get set by the combobox parent. The logic for setting these props thus moves to the parent, getting rid of the `getElementProp altogether`. The parent component gets a mutation observer to do this as well as watchers for when it needs to modify the children. Inherited props addressed: - [x] `scale` - [x] `selectionMode`
Esri · Aug 23, 2023 · f8a1b91 · f8a1b91
1 parent 44da54d
commit f8a1b91
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 37 deletions.
diff --git a/packages/calcite-components/src/components/combobox-item-group/combobox-item-group.tsx b/packages/calcite-components/src/components/combobox-item-group/combobox-item-group.tsx
@@ -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";
@@ -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
@@ -42,7 +48,6 @@ export class ComboboxItemGroup {
 
   connectedCallback(): void {
     this.ancestors = getAncestors(this.el);
-    this.scale = getElementProp(this.el, "scale", this.scale);
   }
 
   // --------------------------------------------------------------------------
@@ -55,8 +60,6 @@ export class ComboboxItemGroup {
 
   guid: string = guid();
 
-  scale: Scale = "m";
-
   // --------------------------------------------------------------------------
   //
   //  Render Methods

diff --git a/packages/calcite-components/src/components/combobox-item/combobox-item.tsx b/packages/calcite-components/src/components/combobox-item/combobox-item.tsx
@@ -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,
@@ -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";
 
 /**
@@ -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
@@ -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
@@ -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);
   }
@@ -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;

diff --git a/packages/calcite-components/src/components/combobox/combobox.e2e.ts b/packages/calcite-components/src/components/combobox/combobox.e2e.ts
@@ -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";
@@ -29,6 +29,10 @@ describe("calcite-combobox", () => {
         propertyName: "clearDisabled",
         defaultValue: false,
       },
+      {
+        propertyName: "flipPlacements",
+        defaultValue: undefined,
+      },
       {
         propertyName: "overlayPositioning",
         defaultValue: "absolute",
@@ -37,6 +41,10 @@ describe("calcite-combobox", () => {
         propertyName: "flipPlacements",
         defaultValue: undefined,
       },
+      {
+        propertyName: "scale",
+        defaultValue: "m",
+      },
     ]);
   });
 
@@ -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");
 
@@ -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);
@@ -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`);
@@ -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");
+    });
+  });
 });
diff --git a/packages/calcite-components/src/components/combobox/combobox.tsx b/packages/calcite-components/src/components/combobox/combobox.tsx
@@ -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",
@@ -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;
 
@@ -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);
@@ -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();
     }