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.

Sign up for GitHub

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

feat(list-item): deprecate open/close props in favor of expanded/collapsed #11003

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from
+98 −45
Draft

feat(list-item): deprecate open/close props in favor of expanded/collapsed #11003

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f313a7
feat(list-item): deprecate open/close props in favor of expanded/coll…
Elijbet Dec 6, 2024
baff4a7
update open, closed, and closable props when expanded, collapsed, and…
Elijbet Dec 6, 2024
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
30 changes: 25 additions & 5 deletions packages/calcite-components/src/components/list-item/list-item.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,26 @@ describe("calcite-list-item", () => {
propertyName: "open",
defaultValue: false,
},
{
propertyName: "expanded",
defaultValue: false,
},
{
propertyName: "closed",
defaultValue: false,
},
{
propertyName: "collapsed",
defaultValue: false,
},
{
propertyName: "closable",
defaultValue: false,
},
{
propertyName: "collapsible",
defaultValue: false,
},
{
propertyName: "dragHandle",
defaultValue: false,
Expand Down Expand Up @@ -341,13 +361,13 @@ describe("calcite-list-item", () => {
it("should fire close event when closed", async () => {
const page = await newE2EPage({ html: "<calcite-list-item closable>test</calcite-list-item>" });

const calciteListItemClose = await page.spyOnEvent("calciteListItemClose", "window");
const calciteListItemCollapsed = await page.spyOnEvent("calciteListItemCollapsed", "window");

const closeButton = await page.find(`calcite-list-item >>> .${CSS.actionsEnd} calcite-action`);

await closeButton.click();

expect(calciteListItemClose).toHaveReceivedEventTimes(1);
expect(calciteListItemCollapsed).toHaveReceivedEventTimes(1);
});

it("should fire calciteListItemToggle event when opened and closed", async () => {
Expand All @@ -362,7 +382,7 @@ describe("calcite-list-item", () => {

expect(await listItem.getProperty("open")).toBe(false);

const openButton = await page.find(`calcite-list-item >>> .${CSS.openContainer}`);
const openButton = await page.find(`calcite-list-item >>> .${CSS.expandedContainer}`);

await openButton.click();
expect(await listItem.getProperty("open")).toBe(true);
Expand All @@ -383,7 +403,7 @@ describe("calcite-list-item", () => {

expect(await listItem.getProperty("open")).toBe(false);

const openButton = await page.find(`calcite-list-item >>> .${CSS.openContainer}`);
const openButton = await page.find(`calcite-list-item >>> .${CSS.expandedContainer}`);

expect(openButton.getAttribute("title")).toBe(null);

Expand All @@ -403,7 +423,7 @@ describe("calcite-list-item", () => {
></calcite-list-item>`,
});

const openButton = await page.find(`calcite-list-item >>> .${CSS.openContainer}`);
const openButton = await page.find(`calcite-list-item >>> .${CSS.expandedContainer}`);

expect(openButton).toBe(null);
});
Expand Down
103 changes: 68 additions & 35 deletions packages/calcite-components/src/components/list-item/list-item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export class ListItem

@state() level: number = null;

@state() openable = false;
@state() expandable = false;

@state() parentListEl: List["el"];

Expand All @@ -111,12 +111,26 @@ export class ListItem
*/
@property() bordered = false;

/** When `true`, a close button is added to the component. */
/**
* When `true`, a close button is added to the component.
*
* @deprecated Use `collapsible` prop instead.
*/
@property({ reflect: true }) closable = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be collapsible. It isn't collapsed, it isn't shown at all.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think closed/closable needs to remain as is.


/** When `true`, hides the component. */
/** When `true`, a close button is added to the component. */
@property({ reflect: true }) collapsible = false;

/**
* When `true`, hides the component.
*
* @deprecated Use `collapsed` prop instead.
*/
@property({ reflect: true }) closed = false;

/** When `true`, hides the component. */
@property({ reflect: true }) collapsed = false;

/** A description for the component. Displays below the label text. */
@property() description: string;

Expand Down Expand Up @@ -177,9 +191,16 @@ export class ListItem
*/
@property() moveToItems: MoveTo[] = [];

/** When `true`, the item is open to show child components. */
/**
* When `true`, the item is open to show child components.
*
* @deprecated Use `expanded` prop instead.
*/
@property({ reflect: true }) open = false;

/** When `true`, the item is expanded to show child components. */
@property({ reflect: true }) expanded = false;

/**
* Specifies the size of the component.
*
Expand Down Expand Up @@ -300,7 +321,7 @@ export class ListItem
calciteInternalListItemToggle = createEvent({ cancelable: false });

/** Fires when the close button is clicked. */
calciteListItemClose = createEvent({ cancelable: false });
calciteListItemCollapsed = createEvent({ cancelable: false });

/** Fires when the component is selected. */
calciteListItemSelect = createEvent({ cancelable: false });
Expand Down Expand Up @@ -357,20 +378,32 @@ export class ListItem
To account for this semantics change, the checks for (this.hasUpdated || value != defaultValue) was added in this method
Please refactor your code to reduce the need for this check.
Docs: https://qawebgis.esri.com/arcgis-components/?path=/docs/lumina-transition-from-stencil--docs#watching-for-property-changes */
if (changes.has("expanded")) {
this.open = this.expanded;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is correct. expanded should set expanded, open should set open.

I think what needs to happen is that open and expanded each need a custom setter to set the other one when one is set.

}

if (changes.has("collapsible")) {
this.closable = this.collapsible;
}

if (changes.has("collapsed")) {
this.closed = this.collapsed;
}

if (changes.has("active") && (this.hasUpdated || this.active !== false)) {
this.activeHandler(this.active);
}

if (changes.has("closed") && (this.hasUpdated || this.closed !== false)) {
this.handleClosedChange();
this.handleCollapsedChange();
}

if (changes.has("disabled") && (this.hasUpdated || this.disabled !== false)) {
this.handleDisabledChange();
}

if (changes.has("open") && (this.hasUpdated || this.open !== false)) {
this.handleOpenChange();
this.handleExpandedChange();
}

if (changes.has("selected") && (this.hasUpdated || this.selected !== false)) {
Expand All @@ -382,7 +415,7 @@ export class ListItem
}

if (changes.has("displayMode") && this.hasUpdated) {
this.handleOpenableChange(this.defaultSlotEl.value);
this.handleExpandableChange(this.defaultSlotEl.value);
}
}

Expand All @@ -404,15 +437,15 @@ export class ListItem
}
}

private handleClosedChange(): void {
private handleCollapsedChange(): void {
this.emitCalciteInternalListItemChange();
}

private handleDisabledChange(): void {
this.emitCalciteInternalListItemChange();
}

private handleOpenChange(): void {
private handleExpandedChange(): void {
this.emitCalciteInternalListItemToggle();
}

Expand All @@ -431,7 +464,7 @@ export class ListItem

private handleCalciteInternalListDefaultSlotChanges(event: CustomEvent<void>): void {
event.stopPropagation();
this.handleOpenableChange(this.defaultSlotEl.value);
this.handleExpandableChange(this.defaultSlotEl.value);
}

private setSortHandleEl(el: SortHandle["el"]): void {
Expand Down Expand Up @@ -489,9 +522,9 @@ export class ListItem
this.calciteInternalListItemChange.emit();
}

private handleCloseClick(): void {
private handleCollapseClick(): void {
this.closed = true;
this.calciteListItemClose.emit();
this.calciteListItemCollapsed.emit();
}

private handleContentSlotChange(event: Event): void {
Expand Down Expand Up @@ -534,16 +567,16 @@ export class ListItem
}
}

private handleOpenableChange(slotEl: HTMLSlotElement): void {
private handleExpandableChange(slotEl: HTMLSlotElement): void {
if (!slotEl) {
return;
}

this.openable = this.displayMode === "nested" && hasListItemChildren(slotEl);
this.expandable = this.displayMode === "nested" && hasListItemChildren(slotEl);
}

private handleDefaultSlotChange(event: Event): void {
this.handleOpenableChange(event.target as HTMLSlotElement);
this.handleExpandableChange(event.target as HTMLSlotElement);
}

private handleToggleClick(): void {
Expand Down Expand Up @@ -603,7 +636,7 @@ export class ListItem
actionsStartEl: { value: actionsStartEl },
actionsEndEl: { value: actionsEndEl },
open,
openable,
expandable,
} = this;

const cells = this.getGridCells();
Expand All @@ -620,7 +653,7 @@ export class ListItem
event.preventDefault();
const nextIndex = currentIndex + 1;
if (currentIndex === -1) {
if (!open && openable) {
if (!open && expandable) {
this.toggle(true);
this.focusCell(null);
} else if (cells[0]) {
Expand All @@ -634,7 +667,7 @@ export class ListItem
const prevIndex = currentIndex - 1;
if (currentIndex === -1) {
this.focusCell(null);
if (open && openable) {
if (open && expandable) {
this.toggle(false);
} else {
this.calciteInternalFocusPreviousItem.emit();
Expand Down Expand Up @@ -759,34 +792,34 @@ export class ListItem
) : null;
}

private renderOpen(): JsxNode {
const { el, open, openable, messages, displayMode, scale } = this;
private renderExpanded(): JsxNode {
const { el, open, expandable, messages, displayMode, scale } = this;

if (displayMode !== "nested") {
return null;
}

const dir = getElementDir(el);

const icon = openable
const icon = expandable
? open
? ICONS.open
: dir === "rtl"
? ICONS.closedRTL
: ICONS.closedLTR
? ICONS.collapsedRTL
: ICONS.collapsedLTR
: ICONS.blank;

const iconScale = getIconScale(scale);

const tooltip = openable ? (open ? messages.collapse : messages.expand) : undefined;
const tooltip = expandable ? (open ? messages.collapse : messages.expand) : undefined;

const openClickHandler = openable ? this.handleToggleClick : undefined;
const expandedClickHandler = expandable ? this.handleToggleClick : undefined;

return (
<div
class={CSS.openContainer}
key="open-container"
onClick={openClickHandler}
class={CSS.expandedContainer}
key="expanded-container"
onClick={expandedClickHandler}
title={tooltip}
>
<calcite-icon icon={icon} key={icon} scale={iconScale} />
Expand Down Expand Up @@ -827,11 +860,11 @@ export class ListItem
{closable ? (
<calcite-action
appearance="transparent"
class={CSS.close}
class={CSS.collapse}
icon={ICONS.close}
key="close-action"
label={messages.close}
onClick={this.handleCloseClick}
onClick={this.handleCollapseClick}
scale={this.scale}
text={messages.close}
/>
Expand Down Expand Up @@ -881,7 +914,7 @@ export class ListItem
<div
class={{
[CSS.nestedContainer]: true,
[CSS.nestedContainerOpen]: this.openable && this.open,
[CSS.nestedContainerExpanded]: this.expandable && this.open,
}}
>
<slot onSlotChange={this.handleDefaultSlotChange} ref={this.defaultSlotEl} />
Expand Down Expand Up @@ -941,7 +974,7 @@ export class ListItem

override render(): JsxNode {
const {
openable,
expandable,
open,
level,
active,
Expand Down Expand Up @@ -974,7 +1007,7 @@ export class ListItem
<InteractiveContainer disabled={disabled}>
<div class={{ [CSS.wrapper]: true, [CSS.wrapperBordered]: wrapperBordered }}>
<div
ariaExpanded={openable ? open : null}
ariaExpanded={expandable ? open : null}
ariaLabel={label}
ariaLevel={level}
ariaSelected={selected}
Expand All @@ -996,7 +1029,7 @@ export class ListItem
>
{this.renderDragHandle()}
{this.renderSelected()}
{this.renderOpen()}
{this.renderExpanded()}
{this.renderActionsStart()}
<div
class={{
Expand Down
10 changes: 5 additions & 5 deletions packages/calcite-components/src/components/list-item/resources.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export const CSS = {
contentContainerSelectable: "content-container--selectable",
contentContainerHasCenterContent: "content-container--has-center-content",
nestedContainer: "nested-container",
nestedContainerOpen: "nested-container--open",
nestedContainerExpanded: "nested-container--expanded",
content: "content",
row: "row",
gridCell: "grid-cell",
Expand All @@ -27,9 +27,9 @@ export const CSS = {
actionsEnd: "actions-end",
selectionContainer: "selection-container",
selectionContainerSingle: "selection-container--single",
openContainer: "open-container",
expandedContainer: "expanded-container",
dragContainer: "drag-container",
close: "close",
collapse: "collapse",
};

export const SLOTS = {
Expand All @@ -49,8 +49,8 @@ export const ICONS = {
selectedSingle: "circle-inset-large",
unselectedMultiple: "square",
unselectedSingle: "circle",
closedLTR: "chevron-right",
closedRTL: "chevron-left",
collapsedLTR: "chevron-right",
collapsedRTL: "chevron-left",
open: "chevron-down",
blank: "blank",
close: "x",
Expand Down
Loading