fix(accordion, accordion-item): icon-position, icon-type, selection-mode and scale can now be set as props or attributes (#7191)

Elijbet · web-flow · commit 2b09abacf4f2 · 2023-07-07T19:08:01.000-07:00
**Related Issue:** #6199, #6038, #6200 ## Summary This should resolve #6199, where you're able to set `accordionEl.setAttribute("icon-position", "start")`, but not `accordionEl.iconPosition = "start"`, happening because of a logical issue within `getElementProp` function. `getElementProp` is being refactored across child components as an outdated pattern in #6038, so this will also kick off that issue as well. Instead of the `accordion-item` looking up the parent for `iconPosition, iconType, selectionMode, or scale`, these get set by the `accordion` 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. With the `mutationObserver` added, there is no longer a need for the `calciteInternalAccordionItemRegister` event logic.
diff --git a/packages/calcite-components/src/components/accordion-item/accordion-item.tsx b/packages/calcite-components/src/components/accordion-item/accordion-item.tsx
@@ -14,17 +14,11 @@ import {
   connectConditionalSlotComponent,
   disconnectConditionalSlotComponent
 } from "../../utils/conditionalSlot";
-import {
-  closestElementCrossShadowBoundary,
-  getElementDir,
-  getElementProp,
-  getSlotted,
-  toAriaBoolean
-} from "../../utils/dom";
+import { getElementDir, getSlotted, toAriaBoolean } from "../../utils/dom";
 import { CSS_UTILITY } from "../../utils/resources";
 import { SLOTS, CSS } from "./resources";
-import { FlipContext, Position, Scale } from "../interfaces";
-import { RegistryEntry, RequestedItem } from "./interfaces";
+import { FlipContext, Position, Scale, SelectionMode } from "../interfaces";
+import { RequestedItem } from "./interfaces";
 
 /**
  * @slot - A slot for adding custom content, including nested `calcite-accordion-item`s.
@@ -69,6 +63,40 @@ export class AccordionItem implements ConditionalSlotComponent {
   /** Displays the `iconStart` and/or `iconEnd` as flipped when the element direction is right-to-left (`"rtl"`). */
   @Prop({ reflect: true }) iconFlipRtl: FlipContext;
 
+  /**
+   * Specifies the placement of the icon in the header inherited from the `calcite-accordion`.
+   *
+   * @internal
+   */
+  @Prop() iconPosition: Position;
+
+  /** Specifies the type of the icon in the header inherited from the `calcite-accordion`.
+   *
+   * @internal
+   */
+  @Prop() iconType: "chevron" | "caret" | "plus-minus";
+
+  /**
+   * The containing `accordion` element.
+   *
+   * @internal
+   */
+  @Prop() accordionParent: HTMLCalciteAccordionElement;
+
+  /**
+   * Specifies the `selectionMode` of the component inherited from the `calcite-accordion`.
+   *
+   * @internal
+   */
+  @Prop() selectionMode: Extract<"single" | "single-persist" | "multiple", SelectionMode>;
+
+  /**
+   * Specifies the size of the component inherited from the `calcite-accordion`.
+   *
+   * @internal
+   */
+  @Prop() scale: Scale;
+
   //--------------------------------------------------------------------------
   //
   //  Events
@@ -85,34 +113,16 @@ export class AccordionItem implements ConditionalSlotComponent {
    */
   @Event({ cancelable: false }) calciteInternalAccordionItemClose: EventEmitter<void>;
 
-  /**
-   * @internal
-   */
-  @Event({ cancelable: false }) calciteInternalAccordionItemRegister: EventEmitter<RegistryEntry>;
-
   //--------------------------------------------------------------------------
   //
   //  Lifecycle
   //
   //--------------------------------------------------------------------------
 
   connectedCallback(): void {
-    this.parent = this.el.parentElement as HTMLCalciteAccordionElement;
-    this.iconType = getElementProp(this.el, "icon-type", "chevron");
-    this.iconPosition = getElementProp(this.el, "icon-position", this.iconPosition);
-    this.scale = getElementProp(this.el, "scale", this.scale);
-
     connectConditionalSlotComponent(this);
   }
 
-  componentDidLoad(): void {
-    this.itemPosition = this.getItemPosition();
-    this.calciteInternalAccordionItemRegister.emit({
-      parent: this.parent,
-      position: this.itemPosition
-    });
-  }
-
   disconnectedCallback(): void {
     disconnectConditionalSlotComponent(this);
   }
@@ -248,38 +258,19 @@ export class AccordionItem implements ConditionalSlotComponent {
   //
   //--------------------------------------------------------------------------
 
-  /** the containing accordion element */
-  private parent: HTMLCalciteAccordionElement;
-
-  /** position within parent */
-  private itemPosition: number;
-
   /** the latest requested item */
   private requestedAccordionItem: HTMLCalciteAccordionItemElement;
 
-  /** what selection mode is the parent accordion in */
-  private selectionMode: string;
-
-  /** what icon position does the parent accordion specify */
-  private iconPosition: Position = "end";
-
-  /** what icon type does the parent accordion specify */
-  private iconType: string;
-
   /** handle clicks on item header */
   private itemHeaderClickHandler = (): void => this.emitRequestedItem();
 
-  /** Specifies the scale of the `accordion-item` controlled by the parent, defaults to m */
-  scale: Scale = "m";
-
   //--------------------------------------------------------------------------
   //
   //  Private Methods
   //
   //--------------------------------------------------------------------------
 
   private determineActiveItem(): void {
-    this.selectionMode = getElementProp(this.el, "selection-mode", "multiple");
     switch (this.selectionMode) {
       case "multiple":
         if (this.el === this.requestedAccordionItem) {
@@ -302,14 +293,4 @@ export class AccordionItem implements ConditionalSlotComponent {
       requestedAccordionItem: this.el as HTMLCalciteAccordionItemElement
     });
   }
-
-  private getItemPosition(): number {
-    const { el } = this;
-
-    const items = closestElementCrossShadowBoundary(el, "calcite-accordion")?.querySelectorAll(
-      "calcite-accordion-item"
-    );
-
-    return items ? Array.from(items).indexOf(el) : -1;
-  }
 }
diff --git a/packages/calcite-components/src/components/accordion/accordion.e2e.ts b/packages/calcite-components/src/components/accordion/accordion.e2e.ts
@@ -1,5 +1,5 @@
 import { newE2EPage } from "@stencil/core/testing";
-import { accessible, renders, hidden } from "../../tests/commonTests";
+import { accessible, defaults, hidden, reflects, renders } from "../../tests/commonTests";
 import { html } from "../../../support/formatting";
 import { CSS } from "../accordion-item/resources";
 
@@ -14,6 +14,14 @@ describe("calcite-accordion", () => {
     <calcite-accordion-item heading="Accordion Title 3" id="3">Accordion Item Content </calcite-accordion-item>
   `;
 
+  const accordionContentInheritablePropsNonDefault = html`
+    <calcite-accordion-item heading="Accordion Title 1" id="1">
+      <calcite-action></calcite-action>Accordion Item Content<calcite-action></calcite-action>
+    </calcite-accordion-item>
+    <calcite-accordion-item heading="Accordion Title 1" id="2">Accordion Item Content </calcite-accordion-item>
+    <calcite-accordion-item heading="Accordion Title 3" id="3">Accordion Item Content </calcite-accordion-item>
+  `;
+
   describe("renders", () => {
     renders("calcite-accordion", { display: "block" });
   });
@@ -26,18 +34,70 @@ describe("calcite-accordion", () => {
     accessible(`<calcite-accordion>${accordionContent}</calcite-accordion>`);
   });
 
-  it("renders default props when none are provided", async () => {
+  describe("defaults", () => {
+    defaults("calcite-accordion", [
+      {
+        propertyName: "appearance",
+        defaultValue: "solid"
+      },
+      {
+        propertyName: "iconPosition",
+        defaultValue: "end"
+      },
+      {
+        propertyName: "scale",
+        defaultValue: "m"
+      },
+      {
+        propertyName: "selectionMode",
+        defaultValue: "multiple"
+      },
+      {
+        propertyName: "iconType",
+        defaultValue: "chevron"
+      }
+    ]);
+  });
+
+  describe("reflects", () => {
+    reflects("calcite-accordion", [
+      {
+        propertyName: "iconPosition",
+        value: "start"
+      },
+      {
+        propertyName: "iconPosition",
+        value: "end"
+      },
+      {
+        propertyName: "selectionMode",
+        value: "single-persist"
+      },
+      {
+        propertyName: "selectionMode",
+        value: "single"
+      },
+      {
+        propertyName: "selectionMode",
+        value: "multiple"
+      }
+    ]);
+  });
+
+  it("inheritable props: `iconPosition`, `iconType`, `selectionMode`, and `scale` modified on the parent get passed into items", async () => {
     const page = await newE2EPage();
     await page.setContent(`
-    <calcite-accordion>
-    ${accordionContent}
+    <calcite-accordion icon-position="start", icon-type="plus-minus", selection-mode="single-persist" scale="l">
+    ${accordionContentInheritablePropsNonDefault}
     </calcite-accordion>`);
-    const element = await page.find("calcite-accordion");
-    expect(element).toEqualAttribute("appearance", "solid");
-    expect(element).toEqualAttribute("icon-position", "end");
-    expect(element).toEqualAttribute("scale", "m");
-    expect(element).toEqualAttribute("selection-mode", "multiple");
-    expect(element).toEqualAttribute("icon-type", "chevron");
+    const accordionItems = await page.findAll("calcite-accordion-items");
+
+    accordionItems.forEach(async (item) => {
+      expect(await item.getProperty("iconPosition")).toBe("start");
+      expect(await item.getProperty("iconType")).toBe("plus-minus");
+      expect(await item.getProperty("selectionMode")).toBe("single-persist");
+      expect(await item.getProperty("scale")).toBe("l");
+    });
   });
 
   it("renders requested props when valid props are provided", async () => {
diff --git a/packages/calcite-components/src/components/accordion/accordion.tsx b/packages/calcite-components/src/components/accordion/accordion.tsx
@@ -1,5 +1,16 @@
-import { Component, Element, Event, EventEmitter, h, Listen, Prop, VNode } from "@stencil/core";
+import {
+  Component,
+  Element,
+  Event,
+  EventEmitter,
+  h,
+  Listen,
+  Prop,
+  VNode,
+  Watch
+} from "@stencil/core";
 import { Appearance, Position, Scale, SelectionMode } from "../interfaces";
+import { createObserver } from "../../utils/observers";
 import { RequestedItem } from "./interfaces";
 /**
  * @slot - A slot for adding `calcite-accordion-item`s. `calcite-accordion` cannot be nested, however `calcite-accordion-item`s can.
@@ -24,6 +35,9 @@ export class Accordion {
   //
   //--------------------------------------------------------------------------
 
+  /** Specifies the parent of the component. */
+  @Prop({ reflect: true }) accordionParent: HTMLCalciteAccordionElement;
+
   /** Specifies the appearance of the component. */
   @Prop({ reflect: true }) appearance: Extract<"solid" | "transparent", Appearance> = "solid";
 
@@ -45,6 +59,14 @@ export class Accordion {
     SelectionMode
   > = "multiple";
 
+  @Watch("iconPosition")
+  @Watch("iconType")
+  @Watch("scale")
+  @Watch("selectionMode")
+  handlePropsChange(): void {
+    this.updateAccordionItems();
+  }
+
   //--------------------------------------------------------------------------
   //
   //  Events
@@ -62,11 +84,13 @@ export class Accordion {
   //
   //--------------------------------------------------------------------------
 
-  componentDidLoad(): void {
-    if (!this.sorted) {
-      this.items = this.sortItems(this.items);
-      this.sorted = true;
-    }
+  connectedCallback(): void {
+    this.mutationObserver?.observe(this.el, { childList: true });
+    this.updateAccordionItems();
+  }
+
+  disconnectedCallback(): void {
+    this.mutationObserver?.disconnect();
   }
 
   render(): VNode {
@@ -89,19 +113,6 @@ export class Accordion {
   //
   //--------------------------------------------------------------------------
 
-  @Listen("calciteInternalAccordionItemRegister")
-  registerCalciteAccordionItem(event: CustomEvent): void {
-    const item = {
-      item: event.target as HTMLCalciteAccordionItemElement,
-      parent: event.detail.parent as HTMLCalciteAccordionElement,
-      position: event.detail.position as number
-    };
-    if (this.el === item.parent) {
-      this.items.push(item);
-    }
-    event.stopPropagation();
-  }
-
   @Listen("calciteInternalAccordionItemSelect")
   updateActiveItemOnChange(event: CustomEvent): void {
     this.requestedAccordionItem = event.detail.requestedAccordionItem;
@@ -117,11 +128,7 @@ export class Accordion {
   //
   //--------------------------------------------------------------------------
 
-  /** created list of Accordion items */
-  private items = [];
-
-  /** keep track of whether the items have been sorted so we don't re-sort */
-  private sorted = false;
+  mutationObserver = createObserver("mutation", () => this.updateAccordionItems());
 
   /** keep track of the requested item for multi mode */
   private requestedAccordionItem: HTMLCalciteAccordionItemElement;
@@ -132,6 +139,13 @@ export class Accordion {
   //
   //--------------------------------------------------------------------------
 
-  private sortItems = (items: any[]): any[] =>
-    items.sort((a, b) => a.position - b.position).map((a) => a.item);
+  private updateAccordionItems(): void {
+    this.el.querySelectorAll("calcite-accordion-item").forEach((item) => {
+      item.iconPosition = this.iconPosition;
+      item.iconType = this.iconType;
+      item.selectionMode = this.selectionMode;
+      item.scale = this.scale;
+      item.accordionParent = this.el;
+    });
+  }
 }
