Skip to content

Commit

Permalink
fix(dropdown-group): fix error caused by early removal (#11612)
Browse files Browse the repository at this point in the history
**Related Issue:** #10028

## Summary

This fixes an error caused by `dropdown-group`s looking up their parent
element. It updates `dropdown` to set the position directly on groups,
avoiding parent lookups.
  • Loading branch information
jcfranco authored Feb 21, 2025
1 parent dbe284b commit c4002e5
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,23 @@ describe("calcite-dropdown-group", () => {
}
});

it("does not throw if removed right after append", async () => {
async function runTest(): Promise<void> {
const page = await newE2EPage();
// group needs to load early for error to occur
await page.setContent(html`<calcite-dropdown-group></calcite-dropdown-group>`);

await page.evaluate(() => {
const dropdownGroup = document.createElement("calcite-dropdown-group");
document.body.append(dropdownGroup);
dropdownGroup.remove();
});
await page.waitForChanges();
}

await expect(runTest()).resolves.toBeUndefined();
});

describe("theme", () => {
const tokens: ComponentTestTokens = {
"--calcite-dropdown-group-border-color": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ export class DropdownGroup extends LitElement {

// #region Private Properties

/** position of group within a dropdown */
private groupPosition: number;

private mutationObserver = createObserver("mutation", () => this.updateItems());

/** the requested group */
Expand All @@ -45,6 +42,13 @@ export class DropdownGroup extends LitElement {
/** Specifies and displays a group title. */
@property({ reflect: true }) groupTitle: string;

/**
* The position of the group in the dropdown menu.
*
* @internal
*/
@property() position: number = -1;

/**
* Specifies the size of the component inherited from the parent `calcite-dropdown`, defaults to `m`.
*
Expand Down Expand Up @@ -87,10 +91,6 @@ export class DropdownGroup extends LitElement {
this.mutationObserver?.observe(this.el, { childList: true });
}

load(): void {
this.groupPosition = this.getGroupPosition();
}

override willUpdate(changes: PropertyValues<this>): void {
/* TODO: [MIGRATION] First time Lit calls willUpdate(), changes will include not just properties provided by the user, but also any default values your component set.
To account for this semantics change, the checks for (this.hasUpdated || value != defaultValue) was added in this method
Expand Down Expand Up @@ -123,13 +123,6 @@ export class DropdownGroup extends LitElement {
);
}

private getGroupPosition(): number {
return Array.prototype.indexOf.call(
this.el.parentElement.querySelectorAll("calcite-dropdown-group"),
this.el,
);
}

// #endregion

// #region Rendering
Expand All @@ -142,7 +135,7 @@ export class DropdownGroup extends LitElement {
) : null;

const dropdownSeparator =
this.groupPosition > 0 ? <div class={CSS.separator} role="separator" /> : null;
this.position > 0 ? <div class={CSS.separator} role="separator" /> : null;
/* TODO: [MIGRATION] This used <Host> before. In Stencil, <Host> props overwrite user-provided props. If you don't wish to overwrite user-values, replace "=" here with "??=" */
this.el.ariaLabel = this.groupTitle;
/* TODO: [MIGRATION] This used <Host> before. In Stencil, <Host> props overwrite user-provided props. If you don't wish to overwrite user-values, replace "=" here with "??=" */
Expand Down
11 changes: 7 additions & 4 deletions packages/calcite-components/src/components/dropdown/dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ export class Dropdown

private handlePropsChange(): void {
this.updateItems();
this.updateGroupScale();
this.updateGroupProps();
}

private closeCalciteDropdownOnClick(event: MouseEvent): void {
Expand Down Expand Up @@ -431,11 +431,14 @@ export class Dropdown
this.groups = groups;

this.updateItems();
this.updateGroupScale();
this.updateGroupProps();
}

private updateGroupScale(): void {
this.groups?.forEach((group) => (group.scale = this.scale));
private updateGroupProps(): void {
this.groups.forEach((group, index) => {
group.scale = this.scale;
group.position = index;
});
}

private resizeObserverCallback(entries: ResizeObserverEntry[]): void {
Expand Down

0 comments on commit c4002e5

Please sign in to comment.