From d23b4ee7535d418f10e7e22bb7960513e37f1d59 Mon Sep 17 00:00:00 2001 From: eliza Date: Fri, 16 Jun 2023 16:57:41 -0700 Subject: [PATCH 01/13] refactor(accordion, accordion-item): as an outdated pattern --- .../accordion-item/accordion-item.tsx | 58 ++++++++++++------- .../src/components/accordion/accordion.e2e.ts | 21 +++++++ .../src/components/accordion/accordion.tsx | 45 +++++++++++++- 3 files changed, 101 insertions(+), 23 deletions(-) diff --git a/packages/calcite-components/src/components/accordion-item/accordion-item.tsx b/packages/calcite-components/src/components/accordion-item/accordion-item.tsx index 0cc20cc8516..b1ce27adeba 100644 --- a/packages/calcite-components/src/components/accordion-item/accordion-item.tsx +++ b/packages/calcite-components/src/components/accordion-item/accordion-item.tsx @@ -17,13 +17,12 @@ import { import { closestElementCrossShadowBoundary, getElementDir, - getElementProp, getSlotted, toAriaBoolean } from "../../utils/dom"; import { CSS_UTILITY } from "../../utils/resources"; import { SLOTS, CSS } from "./resources"; -import { FlipContext, Position, Scale } from "../interfaces"; +import { FlipContext, Position, Scale, SelectionMode } from "../interfaces"; import { RegistryEntry, RequestedItem } from "./interfaces"; /** @@ -69,6 +68,37 @@ 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({ reflect: false, mutable: false }) iconPosition: Position = "end"; + + /** Specifies the type of the icon in the header inherited from the `calcite-accordion`. + * + * @internal + */ + @Prop({ reflect: false, mutable: false }) iconType: "chevron" | "caret" | "plus-minus" = + "chevron"; + + /** + * Specifies the selectionMode of the component inherited from the `calcite-accordion`. + * + * @internal + */ + @Prop({ reflect: false, mutable: false }) selectionMode: Extract< + "single" | "single-persist" | "multiple", + SelectionMode + > = "multiple"; + + /** + * Specifies the size of the component inherited from the `calcite-accordion`. + * + * @internal + */ + @Prop({ reflect: false, mutable: false }) scale: Scale = "m"; + //-------------------------------------------------------------------------- // // Events @@ -98,10 +128,6 @@ export class AccordionItem implements ConditionalSlotComponent { 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); } @@ -238,6 +264,7 @@ export class AccordionItem implements ConditionalSlotComponent { if (this.el.parentNode !== this.requestedAccordionItem.parentNode) { return; } + this.determineActiveItem(); event.stopPropagation(); } @@ -248,30 +275,18 @@ export class AccordionItem implements ConditionalSlotComponent { // //-------------------------------------------------------------------------- - /** the containing accordion element */ - private parent: HTMLCalciteAccordionElement; - /** position within parent */ private itemPosition: number; + /** the containing accordion element */ + private parent: HTMLCalciteAccordionElement; + /** 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 @@ -279,7 +294,6 @@ export class AccordionItem implements ConditionalSlotComponent { //-------------------------------------------------------------------------- private determineActiveItem(): void { - this.selectionMode = getElementProp(this.el, "selection-mode", "multiple"); switch (this.selectionMode) { case "multiple": if (this.el === this.requestedAccordionItem) { diff --git a/packages/calcite-components/src/components/accordion/accordion.e2e.ts b/packages/calcite-components/src/components/accordion/accordion.e2e.ts index a1c8b43c55a..206abc8b31f 100644 --- a/packages/calcite-components/src/components/accordion/accordion.e2e.ts +++ b/packages/calcite-components/src/components/accordion/accordion.e2e.ts @@ -14,6 +14,14 @@ describe("calcite-accordion", () => { Accordion Item Content `; + const accordionContentInheritablePropsNonDefault = html` + + Accordion Item Content + + Accordion Item Content + Accordion Item Content + `; + describe("renders", () => { renders("calcite-accordion", { display: "block" }); }); @@ -40,6 +48,19 @@ describe("calcite-accordion", () => { expect(element).toEqualAttribute("icon-type", "chevron"); }); + it("renders inheritable props: `iconPosition`, `iconType`, `selectionMode`, and `scale` to match those set on parent (non-default)", async () => { + const page = await newE2EPage(); + await page.setContent(` + + ${accordionContentInheritablePropsNonDefault} + `); + const element = await page.find("calcite-accordion"); + expect(element).toEqualAttribute("icon-position", "start"); + expect(element).toEqualAttribute("icon-type", "plus-minus"); + expect(element).toEqualAttribute("selection-mode", "single-persist"); + expect(element).toEqualAttribute("scale", "l"); + }); + it("renders requested props when valid props are provided", async () => { const page = await newE2EPage(); await page.setContent(` diff --git a/packages/calcite-components/src/components/accordion/accordion.tsx b/packages/calcite-components/src/components/accordion/accordion.tsx index a5ab1caf5b9..294bc7ccf9c 100644 --- a/packages/calcite-components/src/components/accordion/accordion.tsx +++ b/packages/calcite-components/src/components/accordion/accordion.tsx @@ -1,5 +1,17 @@ -import { Component, Element, Event, EventEmitter, h, Listen, Prop, VNode } from "@stencil/core"; +import { + Component, + Element, + Event, + EventEmitter, + h, + Listen, + Prop, + State, + 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. @@ -45,6 +57,14 @@ export class Accordion { SelectionMode > = "multiple"; + @Watch("iconPosition") + @Watch("iconType") + @Watch("scale") + @Watch("selectionMode") + onInternalPropChange(): void { + this.passPropsToAccordionItems(); + } + //-------------------------------------------------------------------------- // // Events @@ -62,6 +82,11 @@ export class Accordion { // //-------------------------------------------------------------------------- + connectedCallback(): void { + this.passPropsToAccordionItems(); + this.mutationObserver?.observe(this.el, { childList: true, subtree: true }); + } + componentDidLoad(): void { if (!this.sorted) { this.items = this.sortItems(this.items); @@ -69,6 +94,10 @@ export class Accordion { } } + disconnectedCallback(): void { + this.mutationObserver?.disconnect(); + } + render(): VNode { const transparent = this.appearance === "transparent"; return ( @@ -116,6 +145,9 @@ export class Accordion { // Private State/Props // //-------------------------------------------------------------------------- + mutationObserver = createObserver("mutation", () => this.passPropsToAccordionItems()); + + @State() accordionItems: HTMLCalciteAccordionItemElement[] = []; /** created list of Accordion items */ private items = []; @@ -132,6 +164,17 @@ export class Accordion { // //-------------------------------------------------------------------------- + private passPropsToAccordionItems = (): void => { + this.accordionItems = Array.from(this.el.querySelectorAll("accordion-item")); + + this.accordionItems.forEach((accordionItem) => { + accordionItem.iconPosition = this.iconPosition; + accordionItem.iconType = this.iconType; + accordionItem.selectionMode = this.selectionMode; + accordionItem.scale = this.scale; + }); + }; + private sortItems = (items: any[]): any[] => items.sort((a, b) => a.position - b.position).map((a) => a.item); } From f0b283408c7a47da1035815ae7be8dfbc9d6c76e Mon Sep 17 00:00:00 2001 From: eliza Date: Sat, 17 Jun 2023 01:45:09 -0700 Subject: [PATCH 02/13] refactoring and renaming --- .../accordion-item/accordion-item.tsx | 7 +++--- .../src/components/accordion/accordion.tsx | 22 +++++++++++-------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/packages/calcite-components/src/components/accordion-item/accordion-item.tsx b/packages/calcite-components/src/components/accordion-item/accordion-item.tsx index b1ce27adeba..10af8768593 100644 --- a/packages/calcite-components/src/components/accordion-item/accordion-item.tsx +++ b/packages/calcite-components/src/components/accordion-item/accordion-item.tsx @@ -73,21 +73,20 @@ export class AccordionItem implements ConditionalSlotComponent { * * @internal */ - @Prop({ reflect: false, mutable: false }) iconPosition: Position = "end"; + @Prop({ reflect: false }) iconPosition: Position = "end"; /** Specifies the type of the icon in the header inherited from the `calcite-accordion`. * * @internal */ - @Prop({ reflect: false, mutable: false }) iconType: "chevron" | "caret" | "plus-minus" = - "chevron"; + @Prop({ reflect: false }) iconType: "chevron" | "caret" | "plus-minus" = "chevron"; /** * Specifies the selectionMode of the component inherited from the `calcite-accordion`. * * @internal */ - @Prop({ reflect: false, mutable: false }) selectionMode: Extract< + @Prop({ reflect: false }) selectionMode: Extract< "single" | "single-persist" | "multiple", SelectionMode > = "multiple"; diff --git a/packages/calcite-components/src/components/accordion/accordion.tsx b/packages/calcite-components/src/components/accordion/accordion.tsx index 294bc7ccf9c..6796ac63aff 100644 --- a/packages/calcite-components/src/components/accordion/accordion.tsx +++ b/packages/calcite-components/src/components/accordion/accordion.tsx @@ -6,7 +6,6 @@ import { h, Listen, Prop, - State, VNode, Watch } from "@stencil/core"; @@ -61,8 +60,8 @@ export class Accordion { @Watch("iconType") @Watch("scale") @Watch("selectionMode") - onInternalPropChange(): void { - this.passPropsToAccordionItems(); + handlePropsChange(): void { + this.updateAccordionItems(); } //-------------------------------------------------------------------------- @@ -83,7 +82,7 @@ export class Accordion { //-------------------------------------------------------------------------- connectedCallback(): void { - this.passPropsToAccordionItems(); + this.updateAccordionItems(); this.mutationObserver?.observe(this.el, { childList: true, subtree: true }); } @@ -145,12 +144,17 @@ export class Accordion { // Private State/Props // //-------------------------------------------------------------------------- - mutationObserver = createObserver("mutation", () => this.passPropsToAccordionItems()); - - @State() accordionItems: HTMLCalciteAccordionItemElement[] = []; + mutationObserver = createObserver("mutation", () => this.updateAccordionItems()); /** created list of Accordion items */ - private items = []; + accordionItems: HTMLCalciteAccordionItemElement[] = []; + + /** created list of Accordion item objects */ + private items: { + item: HTMLCalciteAccordionItemElement; + parent: HTMLCalciteAccordionElement; + position: number; + }[] = []; /** keep track of whether the items have been sorted so we don't re-sort */ private sorted = false; @@ -164,7 +168,7 @@ export class Accordion { // //-------------------------------------------------------------------------- - private passPropsToAccordionItems = (): void => { + private updateAccordionItems = (): void => { this.accordionItems = Array.from(this.el.querySelectorAll("accordion-item")); this.accordionItems.forEach((accordionItem) => { From 5751aec508cdac9a80f7efcb2ff85b779a82a79c Mon Sep 17 00:00:00 2001 From: eliza Date: Mon, 19 Jun 2023 23:34:25 -0700 Subject: [PATCH 03/13] fix failing tests --- .../src/components/accordion-item/accordion-item.tsx | 3 +-- .../calcite-components/src/components/accordion/accordion.tsx | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/calcite-components/src/components/accordion-item/accordion-item.tsx b/packages/calcite-components/src/components/accordion-item/accordion-item.tsx index 10af8768593..15878c193e8 100644 --- a/packages/calcite-components/src/components/accordion-item/accordion-item.tsx +++ b/packages/calcite-components/src/components/accordion-item/accordion-item.tsx @@ -96,7 +96,7 @@ export class AccordionItem implements ConditionalSlotComponent { * * @internal */ - @Prop({ reflect: false, mutable: false }) scale: Scale = "m"; + @Prop({ reflect: false }) scale: Scale = "m"; //-------------------------------------------------------------------------- // @@ -263,7 +263,6 @@ export class AccordionItem implements ConditionalSlotComponent { if (this.el.parentNode !== this.requestedAccordionItem.parentNode) { return; } - this.determineActiveItem(); event.stopPropagation(); } diff --git a/packages/calcite-components/src/components/accordion/accordion.tsx b/packages/calcite-components/src/components/accordion/accordion.tsx index 6796ac63aff..83d54dcc395 100644 --- a/packages/calcite-components/src/components/accordion/accordion.tsx +++ b/packages/calcite-components/src/components/accordion/accordion.tsx @@ -82,8 +82,8 @@ export class Accordion { //-------------------------------------------------------------------------- connectedCallback(): void { - this.updateAccordionItems(); this.mutationObserver?.observe(this.el, { childList: true, subtree: true }); + this.updateAccordionItems(); } componentDidLoad(): void { @@ -169,7 +169,7 @@ export class Accordion { //-------------------------------------------------------------------------- private updateAccordionItems = (): void => { - this.accordionItems = Array.from(this.el.querySelectorAll("accordion-item")); + this.accordionItems = Array.from(this.el.querySelectorAll("calcite-accordion-item")); this.accordionItems.forEach((accordionItem) => { accordionItem.iconPosition = this.iconPosition; From 9af21d20e83ca0af6c5d832935d0b1fdaf870fb2 Mon Sep 17 00:00:00 2001 From: eliza Date: Mon, 19 Jun 2023 23:59:26 -0700 Subject: [PATCH 04/13] for stability, refactor test to check for prop instead of the attribute and correct location of the attributes --- .../src/components/accordion/accordion.e2e.ts | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/packages/calcite-components/src/components/accordion/accordion.e2e.ts b/packages/calcite-components/src/components/accordion/accordion.e2e.ts index 206abc8b31f..6fab50f2b68 100644 --- a/packages/calcite-components/src/components/accordion/accordion.e2e.ts +++ b/packages/calcite-components/src/components/accordion/accordion.e2e.ts @@ -15,7 +15,7 @@ describe("calcite-accordion", () => { `; const accordionContentInheritablePropsNonDefault = html` - + Accordion Item Content Accordion Item Content @@ -41,24 +41,26 @@ describe("calcite-accordion", () => { ${accordionContent} `); 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"); + + expect(await element.getProperty("appearance")).toBe("solid"); + expect(await element.getProperty("iconPosition")).toBe("end"); + expect(await element.getProperty("scale")).toBe("m"); + expect(await element.getProperty("selectionMode")).toBe("multiple"); + expect(await element.getProperty("iconType")).toBe("chevron"); }); it("renders inheritable props: `iconPosition`, `iconType`, `selectionMode`, and `scale` to match those set on parent (non-default)", async () => { const page = await newE2EPage(); await page.setContent(` - + ${accordionContentInheritablePropsNonDefault} `); const element = await page.find("calcite-accordion"); - expect(element).toEqualAttribute("icon-position", "start"); - expect(element).toEqualAttribute("icon-type", "plus-minus"); - expect(element).toEqualAttribute("selection-mode", "single-persist"); - expect(element).toEqualAttribute("scale", "l"); + + expect(await element.getProperty("iconPosition")).toBe("start"); + expect(await element.getProperty("iconType")).toBe("plus-minus"); + expect(await element.getProperty("selectionMode")).toBe("single-persist"); + expect(await element.getProperty("scale")).toBe("l"); }); it("renders requested props when valid props are provided", async () => { From 4eea4b5bfb864b08b02b1e60d3387c79fd75432a Mon Sep 17 00:00:00 2001 From: eliza Date: Tue, 20 Jun 2023 18:10:22 -0700 Subject: [PATCH 05/13] refactor redundant items prop, owning accordion sets itself as parent on children, cleanup --- .../accordion-item/accordion-item.tsx | 26 +++++----- .../src/components/accordion/accordion.tsx | 47 ++++++++++++------- 2 files changed, 43 insertions(+), 30 deletions(-) diff --git a/packages/calcite-components/src/components/accordion-item/accordion-item.tsx b/packages/calcite-components/src/components/accordion-item/accordion-item.tsx index 15878c193e8..7808a94a672 100644 --- a/packages/calcite-components/src/components/accordion-item/accordion-item.tsx +++ b/packages/calcite-components/src/components/accordion-item/accordion-item.tsx @@ -73,30 +73,34 @@ export class AccordionItem implements ConditionalSlotComponent { * * @internal */ - @Prop({ reflect: false }) iconPosition: Position = "end"; + @Prop() iconPosition: Position; /** Specifies the type of the icon in the header inherited from the `calcite-accordion`. * * @internal */ - @Prop({ reflect: false }) iconType: "chevron" | "caret" | "plus-minus" = "chevron"; + @Prop() iconType: "chevron" | "caret" | "plus-minus"; /** - * Specifies the selectionMode of the component inherited from the `calcite-accordion`. + * The containing `accordion` element. * * @internal */ - @Prop({ reflect: false }) selectionMode: Extract< - "single" | "single-persist" | "multiple", - SelectionMode - > = "multiple"; + @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({ reflect: false }) scale: Scale = "m"; + @Prop() scale: Scale; //-------------------------------------------------------------------------- // @@ -126,14 +130,13 @@ export class AccordionItem implements ConditionalSlotComponent { //-------------------------------------------------------------------------- connectedCallback(): void { - this.parent = this.el.parentElement as HTMLCalciteAccordionElement; connectConditionalSlotComponent(this); } componentDidLoad(): void { this.itemPosition = this.getItemPosition(); this.calciteInternalAccordionItemRegister.emit({ - parent: this.parent, + parent: this.accordionParent, position: this.itemPosition }); } @@ -276,9 +279,6 @@ export class AccordionItem implements ConditionalSlotComponent { /** position within parent */ private itemPosition: number; - /** the containing accordion element */ - private parent: HTMLCalciteAccordionElement; - /** the latest requested item */ private requestedAccordionItem: HTMLCalciteAccordionItemElement; diff --git a/packages/calcite-components/src/components/accordion/accordion.tsx b/packages/calcite-components/src/components/accordion/accordion.tsx index 83d54dcc395..03b6942ffe7 100644 --- a/packages/calcite-components/src/components/accordion/accordion.tsx +++ b/packages/calcite-components/src/components/accordion/accordion.tsx @@ -35,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"; @@ -82,17 +85,21 @@ export class Accordion { //-------------------------------------------------------------------------- connectedCallback(): void { - this.mutationObserver?.observe(this.el, { childList: true, subtree: true }); + this.mutationObserver?.observe(this.el, { childList: true }); this.updateAccordionItems(); } componentDidLoad(): void { if (!this.sorted) { - this.items = this.sortItems(this.items); + this.accordionItems = this.sortItems(this.accordionItems); this.sorted = true; } } + // componentDidRender(): void { + + // } + disconnectedCallback(): void { this.mutationObserver?.disconnect(); } @@ -119,13 +126,15 @@ export class Accordion { @Listen("calciteInternalAccordionItemRegister") registerCalciteAccordionItem(event: CustomEvent): void { + this.accordionParent = event.detail.parent as HTMLCalciteAccordionElement; + const item = { - item: event.target as HTMLCalciteAccordionItemElement, - parent: event.detail.parent as HTMLCalciteAccordionElement, + accordionItem: event.target as HTMLCalciteAccordionItemElement, position: event.detail.position as number }; - if (this.el === item.parent) { - this.items.push(item); + + if (this.el === this.accordionParent) { + this.accordionItems.push(item); } event.stopPropagation(); } @@ -144,15 +153,12 @@ export class Accordion { // Private State/Props // //-------------------------------------------------------------------------- - mutationObserver = createObserver("mutation", () => this.updateAccordionItems()); - /** created list of Accordion items */ - accordionItems: HTMLCalciteAccordionItemElement[] = []; + mutationObserver = createObserver("mutation", () => this.updateAccordionItems()); - /** created list of Accordion item objects */ - private items: { - item: HTMLCalciteAccordionItemElement; - parent: HTMLCalciteAccordionElement; + /** list of `accordion-item`s */ + accordionItems: { + accordionItem: HTMLCalciteAccordionItemElement; position: number; }[] = []; @@ -169,16 +175,23 @@ export class Accordion { //-------------------------------------------------------------------------- private updateAccordionItems = (): void => { - this.accordionItems = Array.from(this.el.querySelectorAll("calcite-accordion-item")); + const accordionItemsQueryArray = Array.from(this.el.querySelectorAll("calcite-accordion-item")); + + this.accordionItems = accordionItemsQueryArray.map((_item, index) => { + return { accordionItem: accordionItemsQueryArray[index], position: null }; + }); - this.accordionItems.forEach((accordionItem) => { + this.accordionItems.forEach((object) => { + const accordionItem = object.accordionItem; accordionItem.iconPosition = this.iconPosition; accordionItem.iconType = this.iconType; accordionItem.selectionMode = this.selectionMode; accordionItem.scale = this.scale; + accordionItem.accordionParent = this.el; }); }; - private sortItems = (items: any[]): any[] => - items.sort((a, b) => a.position - b.position).map((a) => a.item); + private sortItems = ( + accordionItems: { accordionItem: HTMLCalciteAccordionItemElement; position: number }[] + ): any[] => accordionItems.sort((a, b) => a.position - b.position).map((a) => a.accordionItem); } From cd18c9b0f70c9d5e73b0f7b03d3820184a87342f Mon Sep 17 00:00:00 2001 From: eliza Date: Thu, 22 Jun 2023 19:34:44 -0700 Subject: [PATCH 06/13] cleanup --- .../calcite-components/src/components/accordion/accordion.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/calcite-components/src/components/accordion/accordion.tsx b/packages/calcite-components/src/components/accordion/accordion.tsx index 03b6942ffe7..25db0a61ec2 100644 --- a/packages/calcite-components/src/components/accordion/accordion.tsx +++ b/packages/calcite-components/src/components/accordion/accordion.tsx @@ -96,10 +96,6 @@ export class Accordion { } } - // componentDidRender(): void { - - // } - disconnectedCallback(): void { this.mutationObserver?.disconnect(); } From 4644b2518f1e75139f2c1b1c59cebec96e337bcb Mon Sep 17 00:00:00 2001 From: eliza Date: Fri, 23 Jun 2023 18:07:05 -0700 Subject: [PATCH 07/13] expand mutationObserver function to retrieve accordion item being added and it's position, which is now passed in as an argument instead of being an internal prop --- .../accordion-item/accordion-item.tsx | 35 +------------ .../src/components/accordion/accordion.tsx | 50 +++++++++++-------- 2 files changed, 30 insertions(+), 55 deletions(-) diff --git a/packages/calcite-components/src/components/accordion-item/accordion-item.tsx b/packages/calcite-components/src/components/accordion-item/accordion-item.tsx index 7808a94a672..1ae66013198 100644 --- a/packages/calcite-components/src/components/accordion-item/accordion-item.tsx +++ b/packages/calcite-components/src/components/accordion-item/accordion-item.tsx @@ -14,16 +14,11 @@ import { connectConditionalSlotComponent, disconnectConditionalSlotComponent } from "../../utils/conditionalSlot"; -import { - closestElementCrossShadowBoundary, - getElementDir, - 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, SelectionMode } from "../interfaces"; -import { RegistryEntry, RequestedItem } from "./interfaces"; +import { RequestedItem } from "./interfaces"; /** * @slot - A slot for adding custom content, including nested `calcite-accordion-item`s. @@ -118,11 +113,6 @@ export class AccordionItem implements ConditionalSlotComponent { */ @Event({ cancelable: false }) calciteInternalAccordionItemClose: EventEmitter; - /** - * @internal - */ - @Event({ cancelable: false }) calciteInternalAccordionItemRegister: EventEmitter; - //-------------------------------------------------------------------------- // // Lifecycle @@ -133,14 +123,6 @@ export class AccordionItem implements ConditionalSlotComponent { connectConditionalSlotComponent(this); } - componentDidLoad(): void { - this.itemPosition = this.getItemPosition(); - this.calciteInternalAccordionItemRegister.emit({ - parent: this.accordionParent, - position: this.itemPosition - }); - } - disconnectedCallback(): void { disconnectConditionalSlotComponent(this); } @@ -276,9 +258,6 @@ export class AccordionItem implements ConditionalSlotComponent { // //-------------------------------------------------------------------------- - /** position within parent */ - private itemPosition: number; - /** the latest requested item */ private requestedAccordionItem: HTMLCalciteAccordionItemElement; @@ -314,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.tsx b/packages/calcite-components/src/components/accordion/accordion.tsx index 25db0a61ec2..12aa5901168 100644 --- a/packages/calcite-components/src/components/accordion/accordion.tsx +++ b/packages/calcite-components/src/components/accordion/accordion.tsx @@ -120,21 +120,6 @@ export class Accordion { // //-------------------------------------------------------------------------- - @Listen("calciteInternalAccordionItemRegister") - registerCalciteAccordionItem(event: CustomEvent): void { - this.accordionParent = event.detail.parent as HTMLCalciteAccordionElement; - - const item = { - accordionItem: event.target as HTMLCalciteAccordionItemElement, - position: event.detail.position as number - }; - - if (this.el === this.accordionParent) { - this.accordionItems.push(item); - } - event.stopPropagation(); - } - @Listen("calciteInternalAccordionItemSelect") updateActiveItemOnChange(event: CustomEvent): void { this.requestedAccordionItem = event.detail.requestedAccordionItem; @@ -150,7 +135,20 @@ export class Accordion { // //-------------------------------------------------------------------------- - mutationObserver = createObserver("mutation", () => this.updateAccordionItems()); + mutationObserver = createObserver("mutation", (mutationList) => { + let addedAccordionItem: HTMLCalciteAccordionItemElement; + for (const mutation of mutationList) { + if (mutation.type === "childList") { + const addedNodes = Array.from(mutation.addedNodes); + addedAccordionItem = addedNodes.find( + (node) => node instanceof HTMLCalciteAccordionItemElement + ) as HTMLCalciteAccordionItemElement; + } + } + const childNodes = Array.from(this.el.childNodes); + const position = childNodes.indexOf(addedAccordionItem); + this.updateAccordionItems(addedAccordionItem, position); + }); /** list of `accordion-item`s */ accordionItems: { @@ -170,12 +168,20 @@ export class Accordion { // //-------------------------------------------------------------------------- - private updateAccordionItems = (): void => { - const accordionItemsQueryArray = Array.from(this.el.querySelectorAll("calcite-accordion-item")); - - this.accordionItems = accordionItemsQueryArray.map((_item, index) => { - return { accordionItem: accordionItemsQueryArray[index], position: null }; - }); + private updateAccordionItems = ( + addedAccordionItem?: HTMLCalciteAccordionItemElement, + position?: number + ): void => { + if (addedAccordionItem) { + this.accordionItems.push({ accordionItem: addedAccordionItem, position: position }); + } else { + const accordionItemsQueryArray = Array.from( + this.el.querySelectorAll("calcite-accordion-item") + ); + this.accordionItems = accordionItemsQueryArray.map((_item, index) => { + return { accordionItem: accordionItemsQueryArray[index], position: null }; + }); + } this.accordionItems.forEach((object) => { const accordionItem = object.accordionItem; From 5548bff2f8a1217814fb302fcaea709c9f46d71f Mon Sep 17 00:00:00 2001 From: eliza Date: Mon, 26 Jun 2023 10:16:33 -0700 Subject: [PATCH 08/13] accordion properties can be set as an attribute as well as a prop test coverage for #6199 --- .../src/components/accordion/accordion.e2e.ts | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/packages/calcite-components/src/components/accordion/accordion.e2e.ts b/packages/calcite-components/src/components/accordion/accordion.e2e.ts index 6fab50f2b68..e4fc3f2d818 100644 --- a/packages/calcite-components/src/components/accordion/accordion.e2e.ts +++ b/packages/calcite-components/src/components/accordion/accordion.e2e.ts @@ -63,6 +63,34 @@ describe("calcite-accordion", () => { expect(await element.getProperty("scale")).toBe("l"); }); + it("accordion properties can be set as an attribute as well as a prop", async () => { + const page = await newE2EPage(); + await page.setContent(` + + ${accordionContentInheritablePropsNonDefault} + `); + const element = await page.find("calcite-accordion"); + expect(await element.getProperty("iconPosition")).toBe("start"); + + element.setProperty("iconPosition", "end"); + await page.waitForChanges(); + expect(await element.getProperty("iconPosition")).toBe("end"); + + element.setAttribute("icon-position", "start"); + await page.waitForChanges(); + expect(await element.getProperty("iconPosition")).toBe("start"); + + expect(await element.getProperty("selectionMode")).toBe("single-persist"); + + element.setProperty("selectionMode", "single"); + await page.waitForChanges(); + expect(await element.getProperty("selectionMode")).toBe("single"); + + element.setAttribute("selection-mode", "multiple"); + await page.waitForChanges(); + expect(await element.getAttribute("selection-mode")).toBe("multiple"); + }); + it("renders requested props when valid props are provided", async () => { const page = await newE2EPage(); await page.setContent(` From 03ff0bc49886c2e7b38e4f805e1b037649139583 Mon Sep 17 00:00:00 2001 From: eliza Date: Mon, 26 Jun 2023 11:31:39 -0700 Subject: [PATCH 09/13] simplify by updating accordion items by their order in the DOM, remove the use of position --- .../src/components/accordion/accordion.tsx | 39 ++++--------------- 1 file changed, 8 insertions(+), 31 deletions(-) diff --git a/packages/calcite-components/src/components/accordion/accordion.tsx b/packages/calcite-components/src/components/accordion/accordion.tsx index 12aa5901168..6fa3a0ac370 100644 --- a/packages/calcite-components/src/components/accordion/accordion.tsx +++ b/packages/calcite-components/src/components/accordion/accordion.tsx @@ -89,13 +89,6 @@ export class Accordion { this.updateAccordionItems(); } - componentDidLoad(): void { - if (!this.sorted) { - this.accordionItems = this.sortItems(this.accordionItems); - this.sorted = true; - } - } - disconnectedCallback(): void { this.mutationObserver?.disconnect(); } @@ -145,19 +138,11 @@ export class Accordion { ) as HTMLCalciteAccordionItemElement; } } - const childNodes = Array.from(this.el.childNodes); - const position = childNodes.indexOf(addedAccordionItem); - this.updateAccordionItems(addedAccordionItem, position); + this.updateAccordionItems(addedAccordionItem); }); /** list of `accordion-item`s */ - accordionItems: { - accordionItem: HTMLCalciteAccordionItemElement; - position: number; - }[] = []; - - /** keep track of whether the items have been sorted so we don't re-sort */ - private sorted = false; + accordionItems: HTMLCalciteAccordionItemElement[] = []; /** keep track of the requested item for multi mode */ private requestedAccordionItem: HTMLCalciteAccordionItemElement; @@ -168,23 +153,19 @@ export class Accordion { // //-------------------------------------------------------------------------- - private updateAccordionItems = ( - addedAccordionItem?: HTMLCalciteAccordionItemElement, - position?: number - ): void => { + private updateAccordionItems = (addedAccordionItem?: HTMLCalciteAccordionItemElement): void => { if (addedAccordionItem) { - this.accordionItems.push({ accordionItem: addedAccordionItem, position: position }); + this.accordionItems.push(addedAccordionItem); } else { const accordionItemsQueryArray = Array.from( this.el.querySelectorAll("calcite-accordion-item") ); - this.accordionItems = accordionItemsQueryArray.map((_item, index) => { - return { accordionItem: accordionItemsQueryArray[index], position: null }; - }); + this.accordionItems = accordionItemsQueryArray; } - this.accordionItems.forEach((object) => { - const accordionItem = object.accordionItem; + this.accordionItems.forEach((item) => { + const accordionItem = item; + accordionItem.iconPosition = this.iconPosition; accordionItem.iconType = this.iconType; accordionItem.selectionMode = this.selectionMode; @@ -192,8 +173,4 @@ export class Accordion { accordionItem.accordionParent = this.el; }); }; - - private sortItems = ( - accordionItems: { accordionItem: HTMLCalciteAccordionItemElement; position: number }[] - ): any[] => accordionItems.sort((a, b) => a.position - b.position).map((a) => a.accordionItem); } From bd1f09d6661fed14762ac5c26f65b4139140d89a Mon Sep 17 00:00:00 2001 From: eliza Date: Mon, 26 Jun 2023 13:32:09 -0700 Subject: [PATCH 10/13] clean up logic that relates to tracking addedItem as no longer necessary --- .../src/components/accordion/accordion.tsx | 24 +++---------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/packages/calcite-components/src/components/accordion/accordion.tsx b/packages/calcite-components/src/components/accordion/accordion.tsx index 6fa3a0ac370..08ab1c5c6bb 100644 --- a/packages/calcite-components/src/components/accordion/accordion.tsx +++ b/packages/calcite-components/src/components/accordion/accordion.tsx @@ -128,18 +128,7 @@ export class Accordion { // //-------------------------------------------------------------------------- - mutationObserver = createObserver("mutation", (mutationList) => { - let addedAccordionItem: HTMLCalciteAccordionItemElement; - for (const mutation of mutationList) { - if (mutation.type === "childList") { - const addedNodes = Array.from(mutation.addedNodes); - addedAccordionItem = addedNodes.find( - (node) => node instanceof HTMLCalciteAccordionItemElement - ) as HTMLCalciteAccordionItemElement; - } - } - this.updateAccordionItems(addedAccordionItem); - }); + mutationObserver = createObserver("mutation", () => this.updateAccordionItems()); /** list of `accordion-item`s */ accordionItems: HTMLCalciteAccordionItemElement[] = []; @@ -153,15 +142,8 @@ export class Accordion { // //-------------------------------------------------------------------------- - private updateAccordionItems = (addedAccordionItem?: HTMLCalciteAccordionItemElement): void => { - if (addedAccordionItem) { - this.accordionItems.push(addedAccordionItem); - } else { - const accordionItemsQueryArray = Array.from( - this.el.querySelectorAll("calcite-accordion-item") - ); - this.accordionItems = accordionItemsQueryArray; - } + private updateAccordionItems = (): void => { + this.accordionItems = Array.from(this.el.querySelectorAll("calcite-accordion-item")); this.accordionItems.forEach((item) => { const accordionItem = item; From e04142918f03f71236fa783f7b933d7a7e50faa7 Mon Sep 17 00:00:00 2001 From: eliza Date: Fri, 7 Jul 2023 16:12:43 -0700 Subject: [PATCH 11/13] refactors and cleanup --- .../src/components/accordion/accordion.e2e.ts | 52 ++++++++++++------- .../src/components/accordion/accordion.tsx | 7 +-- 2 files changed, 34 insertions(+), 25 deletions(-) diff --git a/packages/calcite-components/src/components/accordion/accordion.e2e.ts b/packages/calcite-components/src/components/accordion/accordion.e2e.ts index e4fc3f2d818..11f92c0d198 100644 --- 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, renders } from "../../tests/commonTests"; import { html } from "../../../support/formatting"; import { CSS } from "../accordion-item/resources"; @@ -34,33 +34,45 @@ describe("calcite-accordion", () => { accessible(`${accordionContent}`); }); - it("renders default props when none are provided", async () => { - const page = await newE2EPage(); - await page.setContent(` - - ${accordionContent} - `); - const element = await page.find("calcite-accordion"); - - expect(await element.getProperty("appearance")).toBe("solid"); - expect(await element.getProperty("iconPosition")).toBe("end"); - expect(await element.getProperty("scale")).toBe("m"); - expect(await element.getProperty("selectionMode")).toBe("multiple"); - expect(await element.getProperty("iconType")).toBe("chevron"); + 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" + } + ]); }); - it("renders inheritable props: `iconPosition`, `iconType`, `selectionMode`, and `scale` to match those set on parent (non-default)", async () => { + it("inheritable props: `iconPosition`, `iconType`, `selectionMode`, and `scale` modified on the parent get passed into items", async () => { const page = await newE2EPage(); await page.setContent(` ${accordionContentInheritablePropsNonDefault} `); - const element = await page.find("calcite-accordion"); + const accordionItems = await page.findAll("calcite-accordion-items"); - expect(await element.getProperty("iconPosition")).toBe("start"); - expect(await element.getProperty("iconType")).toBe("plus-minus"); - expect(await element.getProperty("selectionMode")).toBe("single-persist"); - expect(await element.getProperty("scale")).toBe("l"); + 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("accordion properties can be set as an attribute as well as a prop", async () => { diff --git a/packages/calcite-components/src/components/accordion/accordion.tsx b/packages/calcite-components/src/components/accordion/accordion.tsx index 08ab1c5c6bb..e417640a52a 100644 --- a/packages/calcite-components/src/components/accordion/accordion.tsx +++ b/packages/calcite-components/src/components/accordion/accordion.tsx @@ -130,8 +130,7 @@ export class Accordion { mutationObserver = createObserver("mutation", () => this.updateAccordionItems()); - /** list of `accordion-item`s */ - accordionItems: HTMLCalciteAccordionItemElement[] = []; + private accordionItems: HTMLCalciteAccordionItemElement[] = []; /** keep track of the requested item for multi mode */ private requestedAccordionItem: HTMLCalciteAccordionItemElement; @@ -145,9 +144,7 @@ export class Accordion { private updateAccordionItems = (): void => { this.accordionItems = Array.from(this.el.querySelectorAll("calcite-accordion-item")); - this.accordionItems.forEach((item) => { - const accordionItem = item; - + this.accordionItems.forEach((accordionItem) => { accordionItem.iconPosition = this.iconPosition; accordionItem.iconType = this.iconType; accordionItem.selectionMode = this.selectionMode; From 3390921426af0fa5c993fbf014b8cd9ad15d5e63 Mon Sep 17 00:00:00 2001 From: eliza Date: Fri, 7 Jul 2023 16:57:46 -0700 Subject: [PATCH 12/13] simplify updateAccordionItems to remove the use of the private accordionItems --- .../src/components/accordion/accordion.tsx | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/calcite-components/src/components/accordion/accordion.tsx b/packages/calcite-components/src/components/accordion/accordion.tsx index e417640a52a..6eb9b62336c 100644 --- a/packages/calcite-components/src/components/accordion/accordion.tsx +++ b/packages/calcite-components/src/components/accordion/accordion.tsx @@ -130,8 +130,6 @@ export class Accordion { mutationObserver = createObserver("mutation", () => this.updateAccordionItems()); - private accordionItems: HTMLCalciteAccordionItemElement[] = []; - /** keep track of the requested item for multi mode */ private requestedAccordionItem: HTMLCalciteAccordionItemElement; @@ -141,15 +139,13 @@ export class Accordion { // //-------------------------------------------------------------------------- - private updateAccordionItems = (): void => { - this.accordionItems = Array.from(this.el.querySelectorAll("calcite-accordion-item")); - - this.accordionItems.forEach((accordionItem) => { - accordionItem.iconPosition = this.iconPosition; - accordionItem.iconType = this.iconType; - accordionItem.selectionMode = this.selectionMode; - accordionItem.scale = this.scale; - accordionItem.accordionParent = this.el; + 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; }); - }; + } } From 025912e4269ae1c50dd9350d5d5dd867a59c2950 Mon Sep 17 00:00:00 2001 From: eliza Date: Fri, 7 Jul 2023 18:32:08 -0700 Subject: [PATCH 13/13] test for prop reflects --- .../src/components/accordion/accordion.e2e.ts | 55 +++++++++---------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/packages/calcite-components/src/components/accordion/accordion.e2e.ts b/packages/calcite-components/src/components/accordion/accordion.e2e.ts index 11f92c0d198..8066c71b847 100644 --- 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, defaults, hidden, renders } from "../../tests/commonTests"; +import { accessible, defaults, hidden, reflects, renders } from "../../tests/commonTests"; import { html } from "../../../support/formatting"; import { CSS } from "../accordion-item/resources"; @@ -59,6 +59,31 @@ describe("calcite-accordion", () => { ]); }); + 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(` @@ -75,34 +100,6 @@ describe("calcite-accordion", () => { }); }); - it("accordion properties can be set as an attribute as well as a prop", async () => { - const page = await newE2EPage(); - await page.setContent(` - - ${accordionContentInheritablePropsNonDefault} - `); - const element = await page.find("calcite-accordion"); - expect(await element.getProperty("iconPosition")).toBe("start"); - - element.setProperty("iconPosition", "end"); - await page.waitForChanges(); - expect(await element.getProperty("iconPosition")).toBe("end"); - - element.setAttribute("icon-position", "start"); - await page.waitForChanges(); - expect(await element.getProperty("iconPosition")).toBe("start"); - - expect(await element.getProperty("selectionMode")).toBe("single-persist"); - - element.setProperty("selectionMode", "single"); - await page.waitForChanges(); - expect(await element.getProperty("selectionMode")).toBe("single"); - - element.setAttribute("selection-mode", "multiple"); - await page.waitForChanges(); - expect(await element.getAttribute("selection-mode")).toBe("multiple"); - }); - it("renders requested props when valid props are provided", async () => { const page = await newE2EPage(); await page.setContent(`