From d72375fea872b3a1cd0bd5c8356a87d315864387 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Fri, 24 Jan 2025 16:44:33 -0800 Subject: [PATCH] fix(combobox): improve prop update times (#11383) **Related Issue:** #10731 ## Summary Fixes prop update delays introduced in #10310 by moving event emitting to Lit's [pre-update phase](https://lit.dev/docs/components/lifecycle/#reactive-update-cycle). --- .../combobox-item/combobox-item.tsx | 39 +++++++++++++------ .../src/components/combobox/combobox.e2e.ts | 20 ++++++++++ .../src/components/combobox/combobox.tsx | 22 +++++++---- 3 files changed, 63 insertions(+), 18 deletions(-) diff --git a/packages/calcite-components/src/components/combobox-item/combobox-item.tsx b/packages/calcite-components/src/components/combobox-item/combobox-item.tsx index ffccf8128e9..6291e933e87 100644 --- a/packages/calcite-components/src/components/combobox-item/combobox-item.tsx +++ b/packages/calcite-components/src/components/combobox-item/combobox-item.tsx @@ -41,6 +41,12 @@ export class ComboboxItem extends LitElement implements InteractiveComponent { // #endregion + // #region Private Properties + + private _selected = false; + + // #endregion + // #region Public Properties /** When `true`, the component is active. */ @@ -91,7 +97,18 @@ export class ComboboxItem extends LitElement implements InteractiveComponent { @property() scale: Scale = "m"; /** When `true`, the component is selected. */ - @property({ reflect: true }) selected: boolean = false; + @property({ reflect: true }) + get selected(): boolean { + return this._selected; + } + set selected(value: boolean) { + const oldValue = this._selected; + if (value !== oldValue) { + this._selected = value; + // we emit directly to avoid delays updating the parent combobox + this.emitItemChange(); + } + } /** * Specifies the selection mode of the component, where: @@ -168,18 +185,14 @@ export class ComboboxItem extends LitElement implements InteractiveComponent { } override willUpdate(changes: PropertyValues): 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 - 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("disabled") && this.hasUpdated) || - (changes.has("selected") && this.hasUpdated) || - (changes.has("textLabel") && this.hasUpdated) || - (changes.has("heading") && this.hasUpdated) || - (changes.has("label") && this.hasUpdated) + this.hasUpdated && + (changes.has("disabled") || + changes.has("heading") || + changes.has("label") || + changes.has("textLabel")) ) { - this.calciteInternalComboboxItemChange.emit(); + this.emitItemChange(); } } @@ -191,6 +204,10 @@ export class ComboboxItem extends LitElement implements InteractiveComponent { // #region Private Methods + private emitItemChange(): void { + this.calciteInternalComboboxItemChange.emit(); + } + private handleDefaultSlotChange(event: Event): void { this.hasContent = slotChangeHasContent(event); } diff --git a/packages/calcite-components/src/components/combobox/combobox.e2e.ts b/packages/calcite-components/src/components/combobox/combobox.e2e.ts index 2b38fd91481..80c18f80327 100644 --- a/packages/calcite-components/src/components/combobox/combobox.e2e.ts +++ b/packages/calcite-components/src/components/combobox/combobox.e2e.ts @@ -1363,6 +1363,26 @@ describe("calcite-combobox", () => { expect(await item2.getProperty("selected")).toBe(true); expect(eventSpy).toHaveReceivedEventTimes(1); }); + + it("updates the value immediately after selecting an item programmatically", async () => { + const page = await newE2EPage(); + await page.setContent(html` + + + + + + `); + + const immediateValueAfterSelected = await page.evaluate(async () => { + const combobox = document.querySelector("calcite-combobox"); + const firstItem = document.querySelector("calcite-combobox-item"); + firstItem.selected = true; + return combobox.value; + }); + + expect(immediateValueAfterSelected).toBe("1"); + }); }); describe("clearing values", () => { diff --git a/packages/calcite-components/src/components/combobox/combobox.tsx b/packages/calcite-components/src/components/combobox/combobox.tsx index 3c32e3a3004..0f074d38c3d 100644 --- a/packages/calcite-components/src/components/combobox/combobox.tsx +++ b/packages/calcite-components/src/components/combobox/combobox.tsx @@ -252,6 +252,8 @@ export class Combobox private selectedIndicatorChipEl: Chip["el"]; + private _selectedItems: HTMLCalciteComboboxItemElement["el"][] = []; + private get showingInlineIcon(): boolean { const { placeholderIcon, selectionMode, selectedItems, open } = this; const selectedItem = selectedItems[0]; @@ -397,7 +399,17 @@ export class Combobox * * @readonly */ - @property() selectedItems: HTMLCalciteComboboxItemElement["el"][] = []; + @property() get selectedItems(): HTMLCalciteComboboxItemElement["el"][] { + return this._selectedItems; + } + + set selectedItems(selectedItems: HTMLCalciteComboboxItemElement["el"][]) { + const oldSelectedItems = this._selectedItems; + if (selectedItems !== oldSelectedItems) { + this._selectedItems = selectedItems; + this.selectedItemsHandler(); + } + } /** * When `selectionMode` is `"ancestors"` or `"multiple"`, specifies the display of multiple `calcite-combobox-item` selections, where: @@ -594,10 +606,6 @@ export class Combobox if (changes.has("flipPlacements")) { this.flipPlacementsHandler(); } - - if (changes.has("selectedItems") && (this.hasUpdated || this.selectedItems?.length > 0)) { - this.selectedItemsHandler(); - } } override updated(): void { @@ -741,8 +749,8 @@ export class Combobox } private getValue(): string | string[] { - const items = this.selectedItems.map((item) => item?.value?.toString()); - return items?.length ? (items.length > 1 ? items : items[0]) : ""; + const items = this.selectedItems.map((item) => item.value?.toString()); + return items.length ? (items.length > 1 ? items : items[0]) : ""; } private comboboxInViewport(): boolean {