Skip to content

Commit

Permalink
fix(accordion, accordion-item): icon-position, icon-type, `select…
Browse files Browse the repository at this point in the history
…ion-mode` and `scale` can now be set as props or attributes (#7191)

**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.
  • Loading branch information
Elijbet authored Jul 8, 2023
1 parent 743ed34 commit 2b09aba
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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";

Expand All @@ -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" });
});
Expand All @@ -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 () => {
Expand Down
66 changes: 40 additions & 26 deletions packages/calcite-components/src/components/accordion/accordion.tsx
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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";

Expand All @@ -45,6 +59,14 @@ export class Accordion {
SelectionMode
> = "multiple";

@Watch("iconPosition")
@Watch("iconType")
@Watch("scale")
@Watch("selectionMode")
handlePropsChange(): void {
this.updateAccordionItems();
}

//--------------------------------------------------------------------------
//
// Events
Expand All @@ -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 {
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
});
}
}

0 comments on commit 2b09aba

Please sign in to comment.