From 58c5e32d1e4fd5fbe325d1c0ee827d99eb77bc2b Mon Sep 17 00:00:00 2001 From: Aidan Timson Date: Mon, 16 Mar 2026 10:20:51 +0000 Subject: [PATCH] Fix no items in combobox unintentionally adding no items value --- src/components/ha-picker-combo-box.ts | 148 ++++++++++++-------- test/components/ha-picker-combo-box.test.ts | 64 +++++++++ 2 files changed, 157 insertions(+), 55 deletions(-) create mode 100644 test/components/ha-picker-combo-box.test.ts diff --git a/src/components/ha-picker-combo-box.ts b/src/components/ha-picker-combo-box.ts index 0e7bb52b87..a60297ab7f 100644 --- a/src/components/ha-picker-combo-box.ts +++ b/src/components/ha-picker-combo-box.ts @@ -67,6 +67,32 @@ export interface PickerComboBoxIndexSelectedDetail { export const NO_ITEMS_AVAILABLE_ID = "___no_items_available___"; const PADDING_ID = "___padding___"; +const isSelectablePickerComboBoxItem = ( + item: string | PickerComboBoxItem | undefined +): item is PickerComboBoxItem => + !!item && + typeof item !== "string" && + item.id !== NO_ITEMS_AVAILABLE_ID && + item.id !== PADDING_ID; + +const findSelectablePickerComboBoxIndex = ( + items: (string | PickerComboBoxItem)[], + startIndex: number, + step: 1 | -1 +) => { + for ( + let index = startIndex; + index >= 0 && index < items.length; + index += step + ) { + if (isSelectablePickerComboBoxItem(items[index])) { + return index; + } + } + + return -1; +}; + export const DEFAULT_ROW_RENDERER_CONTENT = (item: PickerComboBoxItem) => html` ${item.icon ? html`` @@ -567,7 +593,7 @@ export class HaPickerComboBox extends ScrollableFadeMixin(LitElement) { /** * Initialize keyboard selection to the currently selected value, - * or fall back to the first item when searching (skipping section titles). + * or fall back to the first selectable item when searching. */ private _initializeSelectedIndex(): void { if (!this.virtualizerElement?.items?.length) { @@ -578,13 +604,12 @@ export class HaPickerComboBox extends ScrollableFadeMixin(LitElement) { if (initialIndex === 0 && !this._search) { return; } - let index = initialIndex; - // Skip section titles (strings) - if (typeof this.virtualizerElement.items[index] === "string") { - index += 1; - } - // Bounds check: ensure index is valid after skipping section title - if (index >= this.virtualizerElement.items.length) { + const index = findSelectablePickerComboBoxIndex( + this.virtualizerElement.items as (string | PickerComboBoxItem)[], + initialIndex, + 1 + ); + if (index === -1) { return; } this._selectedItemIndex = index; @@ -600,7 +625,10 @@ export class HaPickerComboBox extends ScrollableFadeMixin(LitElement) { this._searchFieldElement?.focus(); - const items = this.virtualizerElement.items as PickerComboBoxItem[]; + const items = this.virtualizerElement.items as ( + | string + | PickerComboBoxItem + )[]; const maxItems = items.length - 1; @@ -617,24 +645,17 @@ export class HaPickerComboBox extends ScrollableFadeMixin(LitElement) { } } - const nextIndex = - maxItems === this._selectedItemIndex - ? this._selectedItemIndex - : this._selectedItemIndex + 1; + const nextIndex = findSelectablePickerComboBoxIndex( + items, + Math.min(this._selectedItemIndex + 1, maxItems), + 1 + ); - if (!items[nextIndex]) { + if (nextIndex === -1) { return; } - if (typeof items[nextIndex] === "string") { - // Skip titles, padding and empty search - if (nextIndex === maxItems) { - return; - } - this._selectedItemIndex = nextIndex + 1; - } else { - this._selectedItemIndex = nextIndex; - } + this._selectedItemIndex = nextIndex; this._scrollToSelectedItem(); }; @@ -647,23 +668,22 @@ export class HaPickerComboBox extends ScrollableFadeMixin(LitElement) { } if (this._selectedItemIndex > 0) { - const nextIndex = this._selectedItemIndex - 1; + const items = this.virtualizerElement.items as ( + | string + | PickerComboBoxItem + )[]; - const items = this.virtualizerElement.items as PickerComboBoxItem[]; + const nextIndex = findSelectablePickerComboBoxIndex( + items, + this._selectedItemIndex - 1, + -1 + ); - if (!items[nextIndex]) { + if (nextIndex === -1) { return; } - if (typeof items[nextIndex] === "string") { - // Skip titles, padding and empty search - if (nextIndex === 0) { - return; - } - this._selectedItemIndex = nextIndex - 1; - } else { - this._selectedItemIndex = nextIndex; - } + this._selectedItemIndex = nextIndex; this._scrollToSelectedItem(); } @@ -675,14 +695,18 @@ export class HaPickerComboBox extends ScrollableFadeMixin(LitElement) { return; } - const nextIndex = 0; + const nextIndex = findSelectablePickerComboBoxIndex( + this.virtualizerElement.items as (string | PickerComboBoxItem)[], + 0, + 1 + ); - if (typeof this.virtualizerElement.items[nextIndex] === "string") { - this._selectedItemIndex = nextIndex + 1; - } else { - this._selectedItemIndex = nextIndex; + if (nextIndex === -1) { + return; } + this._selectedItemIndex = nextIndex; + this._scrollToSelectedItem(); }; @@ -692,14 +716,18 @@ export class HaPickerComboBox extends ScrollableFadeMixin(LitElement) { return; } - const nextIndex = this.virtualizerElement.items.length - 1; + const nextIndex = findSelectablePickerComboBoxIndex( + this.virtualizerElement.items as (string | PickerComboBoxItem)[], + this.virtualizerElement.items.length - 1, + -1 + ); - if (typeof this.virtualizerElement.items[nextIndex] === "string") { - this._selectedItemIndex = nextIndex - 1; - } else { - this._selectedItemIndex = nextIndex; + if (nextIndex === -1) { + return; } + this._selectedItemIndex = nextIndex; + this._scrollToSelectedItem(); }; @@ -729,19 +757,29 @@ export class HaPickerComboBox extends ScrollableFadeMixin(LitElement) { private _pickItem = (ev: KeyboardEvent, newTab: boolean) => { ev.stopPropagation(); + const selectableItems = ( + (this.virtualizerElement?.items as (PickerComboBoxItem | string)[]) || [] + ).reduce<{ item: PickerComboBoxItem; index: number }[]>( + (result, item, index) => { + if (isSelectablePickerComboBoxItem(item)) { + result.push({ item, index }); + } + + return result; + }, + [] + ); + if ( this.virtualizerElement?.items?.length !== undefined && this.virtualizerElement.items.length < 4 && // it still can have a section title and a padding item - this.virtualizerElement.items.filter((item) => typeof item !== "string") - .length === 1 + selectableItems.length === 1 ) { - ( - this.virtualizerElement?.items as (PickerComboBoxItem | string)[] - ).forEach((item, index) => { - if (typeof item !== "string") { - this._fireSelectedEvents(item.id, index, newTab); - } - }); + this._fireSelectedEvents( + selectableItems[0].item.id, + selectableItems[0].index, + newTab + ); return; } @@ -758,7 +796,7 @@ export class HaPickerComboBox extends ScrollableFadeMixin(LitElement) { const item = this.virtualizerElement?.items[ this._selectedItemIndex ] as PickerComboBoxItem; - if (item) { + if (isSelectablePickerComboBoxItem(item)) { this._fireSelectedEvents(item.id, this._selectedItemIndex, newTab); } }; diff --git a/test/components/ha-picker-combo-box.test.ts b/test/components/ha-picker-combo-box.test.ts new file mode 100644 index 0000000000..03fc71949d --- /dev/null +++ b/test/components/ha-picker-combo-box.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it, vi } from "vitest"; +import { + HaPickerComboBox, + NO_ITEMS_AVAILABLE_ID, +} from "../../src/components/ha-picker-combo-box"; + +describe("ha-picker-combo-box", () => { + const setVirtualizerItems = ( + combo: HaPickerComboBox, + items: { id: string; primary: string }[] + ) => { + Object.defineProperty(combo, "virtualizerElement", { + configurable: true, + value: { items }, + }); + }; + + it("prefers the real selectable item over the no-items placeholder", () => { + const combo = new HaPickerComboBox(); + const fireSelectedEvents = vi.fn(); + const stopPropagation = vi.fn(); + const preventDefault = vi.fn(); + + setVirtualizerItems(combo, [ + { id: NO_ITEMS_AVAILABLE_ID, primary: "" }, + { id: "___ADD_NEW___buglabel", primary: "Add new label 'buglabel'" }, + ]); + (combo as any)._fireSelectedEvents = fireSelectedEvents; + + (combo as any)._pickItem( + { + stopPropagation, + preventDefault, + }, + false + ); + + expect(fireSelectedEvents).toHaveBeenCalledWith( + "___ADD_NEW___buglabel", + 1, + false + ); + }); + + it("does not select the no-items placeholder by itself", () => { + const combo = new HaPickerComboBox(); + const fireSelectedEvents = vi.fn(); + const stopPropagation = vi.fn(); + const preventDefault = vi.fn(); + + setVirtualizerItems(combo, [{ id: NO_ITEMS_AVAILABLE_ID, primary: "" }]); + (combo as any)._fireSelectedEvents = fireSelectedEvents; + + (combo as any)._pickItem( + { + stopPropagation, + preventDefault, + }, + false + ); + + expect(fireSelectedEvents).not.toHaveBeenCalled(); + }); +});