Skip to content

Commit 2dcef25

Browse files
jcfrancobenelan
authored andcommitted
fix(dropdown-group): fix error caused by early removal (#11612)
**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.
1 parent c9a30b4 commit 2dcef25

File tree

3 files changed

+32
-19
lines changed

3 files changed

+32
-19
lines changed

packages/calcite-components/src/components/dropdown-group/dropdown-group.e2e.ts

+17
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,23 @@ describe("calcite-dropdown-group", () => {
7676
}
7777
});
7878

79+
it("does not throw if removed right after append", async () => {
80+
async function runTest(): Promise<void> {
81+
const page = await newE2EPage();
82+
// group needs to load early for error to occur
83+
await page.setContent(html`<calcite-dropdown-group></calcite-dropdown-group>`);
84+
85+
await page.evaluate(() => {
86+
const dropdownGroup = document.createElement("calcite-dropdown-group");
87+
document.body.append(dropdownGroup);
88+
dropdownGroup.remove();
89+
});
90+
await page.waitForChanges();
91+
}
92+
93+
await expect(runTest()).resolves.toBeUndefined();
94+
});
95+
7996
describe("theme", () => {
8097
const tokens: ComponentTestTokens = {
8198
"--calcite-dropdown-group-border-color": [

packages/calcite-components/src/components/dropdown-group/dropdown-group.tsx

+8-15
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,6 @@ export class DropdownGroup extends LitElement {
2727

2828
// #region Private Properties
2929

30-
/** position of group within a dropdown */
31-
private groupPosition: number;
32-
3330
private mutationObserver = createObserver("mutation", () => this.updateItems());
3431

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

45+
/**
46+
* The position of the group in the dropdown menu.
47+
*
48+
* @internal
49+
*/
50+
@property() position: number = -1;
51+
4852
/**
4953
* Specifies the size of the component inherited from the parent `calcite-dropdown`, defaults to `m`.
5054
*
@@ -87,10 +91,6 @@ export class DropdownGroup extends LitElement {
8791
this.mutationObserver?.observe(this.el, { childList: true });
8892
}
8993

90-
load(): void {
91-
this.groupPosition = this.getGroupPosition();
92-
}
93-
9494
override willUpdate(changes: PropertyValues<this>): void {
9595
/* 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.
9696
To account for this semantics change, the checks for (this.hasUpdated || value != defaultValue) was added in this method
@@ -123,13 +123,6 @@ export class DropdownGroup extends LitElement {
123123
);
124124
}
125125

126-
private getGroupPosition(): number {
127-
return Array.prototype.indexOf.call(
128-
this.el.parentElement.querySelectorAll("calcite-dropdown-group"),
129-
this.el,
130-
);
131-
}
132-
133126
// #endregion
134127

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

144137
const dropdownSeparator =
145-
this.groupPosition > 0 ? <div class={CSS.separator} role="separator" /> : null;
138+
this.position > 0 ? <div class={CSS.separator} role="separator" /> : null;
146139
/* 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 "??=" */
147140
this.el.ariaLabel = this.groupTitle;
148141
/* 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 "??=" */

packages/calcite-components/src/components/dropdown/dropdown.tsx

+7-4
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ export class Dropdown
311311

312312
private handlePropsChange(): void {
313313
this.updateItems();
314-
this.updateGroupScale();
314+
this.updateGroupProps();
315315
}
316316

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

433433
this.updateItems();
434-
this.updateGroupScale();
434+
this.updateGroupProps();
435435
}
436436

437-
private updateGroupScale(): void {
438-
this.groups?.forEach((group) => (group.scale = this.scale));
437+
private updateGroupProps(): void {
438+
this.groups.forEach((group, index) => {
439+
group.scale = this.scale;
440+
group.position = index;
441+
});
439442
}
440443

441444
private resizeObserverCallback(entries: ResizeObserverEntry[]): void {

0 commit comments

Comments
 (0)