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

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

Merged
merged 22 commits into from
Jul 8, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
d23b4ee
refactor(accordion, accordion-item): as an outdated pattern
Elijbet Jun 16, 2023
e47234f
Merge branch 'master' into elijbet/6038-refactor-getElementProp-accor…
Elijbet Jun 16, 2023
2bb02a3
Merge branch 'master' into elijbet/6038-refactor-getElementProp-accor…
Elijbet Jun 17, 2023
f0b2834
refactoring and renaming
Elijbet Jun 17, 2023
5751aec
fix failing tests
Elijbet Jun 20, 2023
9af21d2
for stability, refactor test to check for prop instead of the attribu…
Elijbet Jun 20, 2023
4eea4b5
refactor redundant items prop, owning accordion sets itself as parent…
Elijbet Jun 21, 2023
bb4f0e0
Merge branch 'master' into elijbet/6038-refactor-getElementProp-accor…
Elijbet Jun 21, 2023
516057a
Merge branch 'master' into elijbet/6038-refactor-getElementProp-accor…
Elijbet Jun 23, 2023
cd18c9b
cleanup
Elijbet Jun 23, 2023
cad8ffa
Merge branch 'master' into elijbet/6038-refactor-getElementProp-accor…
Elijbet Jun 23, 2023
4644b25
expand mutationObserver function to retrieve accordion item being add…
Elijbet Jun 24, 2023
b90f041
Merge branch 'master' into elijbet/6038-refactor-getElementProp-accor…
Elijbet Jun 26, 2023
5548bff
accordion properties can be set as an attribute as well as a prop tes…
Elijbet Jun 26, 2023
03ff0bc
simplify by updating accordion items by their order in the DOM, remov…
Elijbet Jun 26, 2023
bd1f09d
clean up logic that relates to tracking addedItem as no longer necessary
Elijbet Jun 26, 2023
52c5b1b
Merge branch 'main' into elijbet/6038-refactor-getElementProp-accordi…
Elijbet Jul 7, 2023
e041429
refactors and cleanup
Elijbet Jul 7, 2023
3390921
simplify updateAccordionItems to remove the use of the private accord…
Elijbet Jul 7, 2023
827c6dd
Merge branch 'main' into elijbet/6038-refactor-getElementProp-accordi…
Elijbet Jul 7, 2023
025912e
test for prop reflects
Elijbet Jul 8, 2023
27638db
Merge branch 'main' into elijbet/6038-refactor-getElementProp-accordi…
Elijbet Jul 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Expand Up @@ -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 @@ -33,11 +41,26 @@ describe("calcite-accordion", () => {
${accordionContent}
</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");

expect(await element.getProperty("appearance")).toBe("solid");
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
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 () => {
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
const page = await newE2EPage();
await page.setContent(`
<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(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 () => {
Expand Down
100 changes: 81 additions & 19 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,13 +84,22 @@ export class Accordion {
//
//--------------------------------------------------------------------------

connectedCallback(): void {
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;
}
}

disconnectedCallback(): void {
this.mutationObserver?.disconnect();
}

render(): VNode {
const transparent = this.appearance === "transparent";
return (
Expand All @@ -89,19 +120,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,8 +135,26 @@ export class Accordion {
//
//--------------------------------------------------------------------------

/** created list of Accordion items */
private items = [];
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);
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
this.updateAccordionItems(addedAccordionItem, position);
});

/** list of `accordion-item`s */
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
accordionItems: {
accordionItem: HTMLCalciteAccordionItemElement;
position: number;
}[] = [];

/** keep track of whether the items have been sorted so we don't re-sort */
private sorted = false;
Expand All @@ -132,6 +168,32 @@ export class Accordion {
//
//--------------------------------------------------------------------------

private sortItems = (items: any[]): any[] =>
items.sort((a, b) => a.position - b.position).map((a) => a.item);
private updateAccordionItems = (
addedAccordionItem?: HTMLCalciteAccordionItemElement,
position?: number
): void => {
if (addedAccordionItem) {
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
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;
accordionItem.iconPosition = this.iconPosition;
accordionItem.iconType = this.iconType;
accordionItem.selectionMode = this.selectionMode;
accordionItem.scale = this.scale;
accordionItem.accordionParent = this.el;
});
};

private sortItems = (
accordionItems: { accordionItem: HTMLCalciteAccordionItemElement; position: number }[]
): any[] => accordionItems.sort((a, b) => a.position - b.position).map((a) => a.accordionItem);
}