From 5abab937dee3b36c5cf133989080d3e8a2244f93 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Thu, 9 May 2024 15:52:24 +0300 Subject: [PATCH 01/20] feat(ui5-cb-group-item): introduce nested grouping --- packages/main/src/ComboBoxGroupItem.ts | 13 +++++++++++++ packages/main/src/ComboBoxPopover.hbs | 6 +++++- packages/main/test/pages/ComboBox.html | 26 +++++++++++++++----------- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/packages/main/src/ComboBoxGroupItem.ts b/packages/main/src/ComboBoxGroupItem.ts index b571098f0c42..dc38b35db150 100644 --- a/packages/main/src/ComboBoxGroupItem.ts +++ b/packages/main/src/ComboBoxGroupItem.ts @@ -1,7 +1,9 @@ import customElement from "@ui5/webcomponents-base/dist/decorators/customElement.js"; import property from "@ui5/webcomponents-base/dist/decorators/property.js"; +import slot from "@ui5/webcomponents-base/dist/decorators/slot.js"; import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js"; import type { IComboBoxItem } from "./ComboBox.js"; +import ComboBoxItem from "./ComboBoxItem.js"; /** * @class @@ -31,6 +33,17 @@ class ComboBoxGroupItem extends UI5Element implements IComboBoxItem { @property({ type: Boolean }) focused!: boolean; + /** + * Defines the items of the ui5-li-group. + * @public + */ + @slot({ + "default": true, + invalidateOnChildChange: true, + type: HTMLElement, + }) + items!: Array; + /** * Used to avoid tag name checks * @protected diff --git a/packages/main/src/ComboBoxPopover.hbs b/packages/main/src/ComboBoxPopover.hbs index de90747e0444..d056a3587b5c 100644 --- a/packages/main/src/ComboBoxPopover.hbs +++ b/packages/main/src/ComboBoxPopover.hbs @@ -70,7 +70,11 @@ > {{#each _filteredItems}} {{#if isGroupItem}} - {{ this.text }} + + {{#each this.items}} + {{> listItem}} + {{/each}} + {{else}} {{> listItem}} {{/if}} diff --git a/packages/main/test/pages/ComboBox.html b/packages/main/test/pages/ComboBox.html index 599010e454a3..3d7f489baf05 100644 --- a/packages/main/test/pages/ComboBox.html +++ b/packages/main/test/pages/ComboBox.html @@ -87,17 +87,21 @@ Items with grouping and value state: - - - - - - - - - - - + + + + + + + + + + + + + + + From 6977ed11120b43e3595760328eb43d02f9c2852a Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Fri, 10 May 2024 19:01:36 +0300 Subject: [PATCH 02/20] feat(ui5-cb-group-item): introduce nested grouping (draft) implement item filtering --- packages/main/src/ComboBox.ts | 49 ++++++++++++++++---------- packages/main/src/ComboBoxGroupItem.ts | 5 ++- packages/main/src/ComboBoxItem.ts | 7 ++++ packages/main/src/ComboBoxPopover.hbs | 8 +++-- packages/main/test/pages/ComboBox.html | 2 ++ 5 files changed, 47 insertions(+), 24 deletions(-) diff --git a/packages/main/src/ComboBox.ts b/packages/main/src/ComboBox.ts index ae20e94989de..e2c4b290f37f 100644 --- a/packages/main/src/ComboBox.ts +++ b/packages/main/src/ComboBox.ts @@ -96,6 +96,8 @@ interface IComboBoxItem extends UI5Element { selected?: boolean, additionalText?: string, stableDomRef: string, + _isVisible?: boolean, + items?: Array } type ValueStateAnnouncement = Record, string>; @@ -601,6 +603,19 @@ class ComboBox extends UI5Element { const { value } = e.target as HTMLInputElement; const shouldAutocomplete = this.shouldAutocomplete(e); + // Reset item filtration + this.items.forEach(item => { + if (item.isGroupItem) { + item.items?.forEach(i => { + i._isVisible = false; + }); + + return; + } + + item._isVisible = false; + }); + if (e.target === this.inner) { // stop the native event, as the semantic "input" would be fired. e.stopImmediatePropagation(); @@ -939,28 +954,26 @@ class ComboBox extends UI5Element { } _filterItems(str: string) { - const itemsToFilter = this.items.filter(item => !item.isGroupItem); - const filteredItems = (Filters[this.filter] || Filters.StartsWithPerTerm)(str, itemsToFilter, "text"); - - // Return the filtered items and their group items - return this.items.filter((item, idx, allItems) => ComboBox._groupItemFilter(item, ++idx, allItems, filteredItems) || filteredItems.indexOf(item) !== -1); - } + const itemsToFilter: Array = []; + const itemGroups: Array = []; - /** - * Returns true if the group header should be shown (if there is a filtered suggestion item for this group item) - * @private - */ - static _groupItemFilter(item: IComboBoxItem, idx: number, allItems: Array, filteredItems: Array) { - if (item.isGroupItem) { - let groupHasFilteredItems; + this.items.forEach(item => { + if (item.items?.length) { + const filteredItems = (Filters[this.filter] || Filters.StartsWithPerTerm)(str, item.items, "text"); + filteredItems.forEach(i => { + i._isVisible = true; + }); - while (allItems[idx] && !allItems[idx].isGroupItem && !groupHasFilteredItems) { - groupHasFilteredItems = filteredItems.indexOf(allItems[idx]) !== -1; - idx++; + if (filteredItems.length) { + itemGroups.push(item); + } + return; } - return groupHasFilteredItems; - } + itemsToFilter.push(item); + }); + + return itemGroups; } _getFirstMatchingItem(current: string): ComboBoxItem | undefined { diff --git a/packages/main/src/ComboBoxGroupItem.ts b/packages/main/src/ComboBoxGroupItem.ts index dc38b35db150..b4926aee04a3 100644 --- a/packages/main/src/ComboBoxGroupItem.ts +++ b/packages/main/src/ComboBoxGroupItem.ts @@ -3,7 +3,6 @@ import property from "@ui5/webcomponents-base/dist/decorators/property.js"; import slot from "@ui5/webcomponents-base/dist/decorators/slot.js"; import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js"; import type { IComboBoxItem } from "./ComboBox.js"; -import ComboBoxItem from "./ComboBoxItem.js"; /** * @class @@ -27,7 +26,7 @@ class ComboBoxGroupItem extends UI5Element implements IComboBoxItem { text!: string; /** - * Indicates whether the item is focssed + * Indicates whether the item is focused * @protected */ @property({ type: Boolean }) @@ -42,7 +41,7 @@ class ComboBoxGroupItem extends UI5Element implements IComboBoxItem { invalidateOnChildChange: true, type: HTMLElement, }) - items!: Array; + items!: Array; /** * Used to avoid tag name checks diff --git a/packages/main/src/ComboBoxItem.ts b/packages/main/src/ComboBoxItem.ts index 22ecf6aeccce..b011d5f514db 100644 --- a/packages/main/src/ComboBoxItem.ts +++ b/packages/main/src/ComboBoxItem.ts @@ -31,6 +31,13 @@ class ComboBoxItem extends UI5Element implements IComboBoxItem { @property() additionalText!: string + /** + * Indicates whether the item is filtered + * @private + */ + @property({ type: Boolean, noAttribute: true }) + _isVisible!: boolean; + /** * Indicates whether the item is focssed * @protected diff --git a/packages/main/src/ComboBoxPopover.hbs b/packages/main/src/ComboBoxPopover.hbs index d056a3587b5c..ff60639c1151 100644 --- a/packages/main/src/ComboBoxPopover.hbs +++ b/packages/main/src/ComboBoxPopover.hbs @@ -71,9 +71,11 @@ {{#each _filteredItems}} {{#if isGroupItem}} - {{#each this.items}} - {{> listItem}} - {{/each}} + {{#each this.items}} + {{#if _isVisible}} + {{> listItem}} + {{/if}} + {{/each}} {{else}} {{> listItem}} diff --git a/packages/main/test/pages/ComboBox.html b/packages/main/test/pages/ComboBox.html index 3d7f489baf05..54f23e740b8e 100644 --- a/packages/main/test/pages/ComboBox.html +++ b/packages/main/test/pages/ComboBox.html @@ -91,6 +91,8 @@ + + From 970e138fec8932fefbff326b5fb118679534ff08 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Mon, 13 May 2024 18:44:22 +0300 Subject: [PATCH 03/20] feat(ui5-cb-group-item): introduce nested grouping (draft) fix filtering and selection --- packages/main/src/ComboBox.ts | 39 +++++++++++++------ ...boBoxGroupItem.ts => ComboBoxItemGroup.ts} | 2 +- packages/main/test/pages/ComboBox.html | 20 +++++----- 3 files changed, 38 insertions(+), 23 deletions(-) rename packages/main/src/{ComboBoxGroupItem.ts => ComboBoxItemGroup.ts} (97%) diff --git a/packages/main/src/ComboBox.ts b/packages/main/src/ComboBox.ts index e2c4b290f37f..35f584c7a42c 100644 --- a/packages/main/src/ComboBox.ts +++ b/packages/main/src/ComboBox.ts @@ -75,7 +75,7 @@ import type { ListItemClickEventDetail } from "./List.js"; import BusyIndicator from "./BusyIndicator.js"; import Button from "./Button.js"; import StandardListItem from "./StandardListItem.js"; -import ComboBoxGroupItem from "./ComboBoxGroupItem.js"; +import ComboBoxGroupItem from "./ComboBoxItemGroup.js"; import ListItemGroupHeader from "./ListItemGroupHeader.js"; import ComboBoxFilter from "./types/ComboBoxFilter.js"; import type FormSupportT from "./features/InputElementsFormSupport.js"; @@ -954,26 +954,30 @@ class ComboBox extends UI5Element { } _filterItems(str: string) { - const itemsToFilter: Array = []; - const itemGroups: Array = []; + let filteredItem:IComboBoxItem; + let filteredGroupItems: Array = []; + const filteredItems: Array = []; + const filteredItemGroups: Array = []; this.items.forEach(item => { if (item.items?.length) { - const filteredItems = (Filters[this.filter] || Filters.StartsWithPerTerm)(str, item.items, "text"); - filteredItems.forEach(i => { + filteredGroupItems = (Filters[this.filter] || Filters.StartsWithPerTerm)(str, item.items, "text"); + filteredGroupItems.forEach(i => { i._isVisible = true; }); - if (filteredItems.length) { - itemGroups.push(item); + if (filteredGroupItems.length) { + filteredItemGroups.push(item); } + return; } - itemsToFilter.push(item); + [filteredItem] = (Filters[this.filter] || Filters.StartsWithPerTerm)(str, [item], "text"); + filteredItem && filteredItems.push(filteredItem); }); - return itemGroups; + return [...filteredItemGroups, ...filteredItems]; } _getFirstMatchingItem(current: string): ComboBoxItem | undefined { @@ -984,7 +988,7 @@ class ComboBox extends UI5Element { return; } - const matchingItems: Array = (this._startsWithMatchingItems(current).filter(item => !item.isGroupItem) as Array); + const matchingItems: Array = (this._startsWithMatchingItems(current).map(item => (item?.items?.length && item.items[0]) || item) as Array); if (matchingItems.length) { return matchingItems[0]; @@ -1006,11 +1010,22 @@ class ComboBox extends UI5Element { const shouldSelectionBeCleared = currentlyFocusedItem && currentlyFocusedItem.isGroupItem; const itemToBeSelected = this._filteredItems.find(item => { - return !item.isGroupItem && (item.text === this.value) && !shouldSelectionBeCleared; + return ((!item.isGroupItem && (item.text === this.value)) || (item.items?.length && (item.items[0].text === this.value))) && !shouldSelectionBeCleared; }); + if (!itemToBeSelected) { + return; + } + this._filteredItems = this._filteredItems.map(item => { - item.selected = item === itemToBeSelected; + if (!item.items) { + item.selected = item === itemToBeSelected; + } + + if (item.items && !!itemToBeSelected?.items?.length) { + item.items[0].selected = itemToBeSelected.items[0] === item.items[0]; + } + return item; }); } diff --git a/packages/main/src/ComboBoxGroupItem.ts b/packages/main/src/ComboBoxItemGroup.ts similarity index 97% rename from packages/main/src/ComboBoxGroupItem.ts rename to packages/main/src/ComboBoxItemGroup.ts index b4926aee04a3..7332d00099d9 100644 --- a/packages/main/src/ComboBoxGroupItem.ts +++ b/packages/main/src/ComboBoxItemGroup.ts @@ -15,7 +15,7 @@ import type { IComboBoxItem } from "./ComboBox.js"; * @implements {IComboBoxItem} * @since 1.0.0-rc.15 */ -@customElement("ui5-cb-group-item") +@customElement("ui5-cb-item-group") class ComboBoxGroupItem extends UI5Element implements IComboBoxItem { /** * Defines the text of the component. diff --git a/packages/main/test/pages/ComboBox.html b/packages/main/test/pages/ComboBox.html index 54f23e740b8e..cd5bceab1370 100644 --- a/packages/main/test/pages/ComboBox.html +++ b/packages/main/test/pages/ComboBox.html @@ -87,23 +87,23 @@ Items with grouping and value state: - + - - + + - + - + - + @@ -111,23 +111,23 @@
Items with grouping: - + - + - + - + From dd6e4114e03890b6aed3f0fe9f97916e827da958 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Tue, 14 May 2024 06:48:24 +0300 Subject: [PATCH 04/20] feat(ui5-cb-group-item): introduce nested grouping fix item selection bugs implement partial navigation focus handling --- packages/main/src/ComboBox.ts | 88 +++++++++++++------ packages/main/src/ComboBoxPopover.hbs | 2 +- packages/main/test/pages/ComboBox.html | 17 ++-- packages/main/test/specs/ComboBox.spec.js | 101 +++++++++++----------- 4 files changed, 122 insertions(+), 86 deletions(-) diff --git a/packages/main/src/ComboBox.ts b/packages/main/src/ComboBox.ts index 35f584c7a42c..fdd20d370d46 100644 --- a/packages/main/src/ComboBox.ts +++ b/packages/main/src/ComboBox.ts @@ -676,36 +676,66 @@ class ComboBox extends UI5Element { } _startsWithMatchingItems(str: string): Array { - return Filters.StartsWith(str, this._filteredItems, "text"); + const filteredItems: Array = []; + + this._filteredItems.forEach(item => { + if (item.items?.length) { + filteredItems.push(...item.items); + return; + } + + filteredItems.push(item); + }); + + return Filters.StartsWith(str, filteredItems, "text"); } _clearFocus() { - this._filteredItems.map(item => { + const allItems = this._getAllItems(); + + allItems.map(item => { item.focused = false; return item; }); } + _getAllItems() { + const allItems: Array = []; + this._filteredItems.forEach(item => { + if (item.items && item.items.length) { + item.items.forEach(groupedItem => { + allItems.push(groupedItem); + }); + return; + } + + allItems.push(item); + }); + + return allItems; + } + handleNavKeyPress(e: KeyboardEvent) { + const allItems = this._getAllItems(); + if (this.focused && (isHome(e) || isEnd(e)) && this.value) { return; } const isOpen = this.open; - const currentItem = this._filteredItems.find(item => { + const currentItem = allItems.find(item => { return isOpen ? item.focused : item.selected; }); - const indexOfItem = currentItem ? this._filteredItems.indexOf(currentItem) : -1; - + const indexOfItem = currentItem ? allItems.indexOf(currentItem) : -1; e.preventDefault(); if (this.focused && isOpen && (isUp(e) || isPageUp(e) || isPageDown(e))) { return; } - if (this._filteredItems.length - 1 === indexOfItem && isDown(e)) { + if (allItems.length - 1 === indexOfItem && isDown(e)) { return; } @@ -724,12 +754,15 @@ class ComboBox extends UI5Element { } _handleItemNavigation(e: KeyboardEvent, indexOfItem: number, isForward: boolean) { + const allItems = this._getAllItems(); + // const suggPopover = await this._getPicker(); + const isOpen = this.open; - const currentItem = this._filteredItems[indexOfItem]; - const nextItem = isForward ? this._filteredItems[indexOfItem + 1] : this._filteredItems[indexOfItem - 1]; - const isGroupItem = currentItem && currentItem.isGroupItem; + const currentItem: IComboBoxItem = allItems[indexOfItem]; + + const nextItem = isForward ? allItems[indexOfItem + 1] : allItems[indexOfItem - 1]; - if ((!isOpen) && ((isGroupItem && !nextItem) || (!isGroupItem && !currentItem))) { + if ((!isOpen) && (!nextItem || !currentItem)) { return; } @@ -737,12 +770,13 @@ class ComboBox extends UI5Element { if (isOpen) { this._itemFocused = true; - this.value = isGroupItem ? "" : currentItem.text; + this.value = currentItem.text; this.focused = false; + currentItem.focused = true; } else { this.focused = true; - this.value = isGroupItem ? nextItem.text : currentItem.text; + this.value = currentItem.text; currentItem.focused = false; } @@ -751,10 +785,6 @@ class ComboBox extends UI5Element { this._announceSelectedItem(indexOfItem); this._scrollToItem(indexOfItem, isForward); - if (isGroupItem && isOpen) { - return; - } - // autocomplete const item = this._getFirstMatchingItem(this.value); item && this._applyAtomicValueAndSelection(item, (this.open ? this._userTypedValue : ""), true); @@ -873,12 +903,16 @@ class ComboBox extends UI5Element { if (isEnter(e)) { const focusedItem = this._filteredItems.find(item => { + if (item?.items?.length) { + return item.items.find(groupItem => groupItem.focused); + } + return item.focused; }); this._fireChangeEvent(); - if (picker?.open && !focusedItem?.isGroupItem) { + if (picker?.open && focusedItem && !focusedItem?.isGroupItem) { this._closeRespPopover(); this.focused = true; this.inner.setSelectionRange(this.value.length, this.value.length); @@ -1008,23 +1042,23 @@ class ComboBox extends UI5Element { _selectMatchingItem() { const currentlyFocusedItem = this.items.find(item => item.focused); const shouldSelectionBeCleared = currentlyFocusedItem && currentlyFocusedItem.isGroupItem; + let itemToBeSelected: IComboBoxItem | undefined; - const itemToBeSelected = this._filteredItems.find(item => { - return ((!item.isGroupItem && (item.text === this.value)) || (item.items?.length && (item.items[0].text === this.value))) && !shouldSelectionBeCleared; + this._filteredItems.forEach(item => { + if (!shouldSelectionBeCleared && !itemToBeSelected) { + itemToBeSelected = ((!item.isGroupItem && (item.text === this.value)) ? item : item?.items?.find(i => i.text === this.value)); + } }); - if (!itemToBeSelected) { - return; - } - this._filteredItems = this._filteredItems.map(item => { if (!item.items) { item.selected = item === itemToBeSelected; + return item; } - if (item.items && !!itemToBeSelected?.items?.length) { - item.items[0].selected = itemToBeSelected.items[0] === item.items[0]; - } + item.items.forEach(groupItem => { + groupItem.selected = itemToBeSelected === groupItem; + }); return item; }); @@ -1086,7 +1120,7 @@ class ComboBox extends UI5Element { _announceSelectedItem(indexOfItem: number) { const currentItem = this._filteredItems[indexOfItem]; const nonGroupItems = this._filteredItems.filter(item => !item.isGroupItem); - const currentItemAdditionalText = currentItem.additionalText || ""; + const currentItemAdditionalText = currentItem?.additionalText || ""; const isGroupItem = currentItem?.isGroupItem; const itemPositionText = ComboBox.i18nBundle.getText(LIST_ITEM_POSITION, nonGroupItems.indexOf(currentItem) + 1, nonGroupItems.length); const groupHeaderText = ComboBox.i18nBundle.getText(LIST_ITEM_GROUP_HEADER); diff --git a/packages/main/src/ComboBoxPopover.hbs b/packages/main/src/ComboBoxPopover.hbs index ff60639c1151..db2aae0912d4 100644 --- a/packages/main/src/ComboBoxPopover.hbs +++ b/packages/main/src/ComboBoxPopover.hbs @@ -70,7 +70,7 @@ > {{#each _filteredItems}} {{#if isGroupItem}} - + {{#each this.items}} {{#if _isVisible}} {{> listItem}} diff --git a/packages/main/test/pages/ComboBox.html b/packages/main/test/pages/ComboBox.html index cd5bceab1370..a2a4dbe8e460 100644 --- a/packages/main/test/pages/ComboBox.html +++ b/packages/main/test/pages/ComboBox.html @@ -92,8 +92,8 @@ - + @@ -111,26 +111,27 @@
Items with grouping: - + - - + + - - + + - - + + +
diff --git a/packages/main/test/specs/ComboBox.spec.js b/packages/main/test/specs/ComboBox.spec.js index 6b8c4b457459..2706674539ad 100644 --- a/packages/main/test/specs/ComboBox.spec.js +++ b/packages/main/test/specs/ComboBox.spec.js @@ -648,68 +648,69 @@ describe("General interaction", () => { describe("Grouping", () => { - it ("Tests group filtering", async () => { - await browser.url(`test/pages/ComboBox.html`); +it ("Tests group filtering", async () => { + await browser.url(`test/pages/ComboBox.html`); - const combo = await browser.$("#combo-grouping"); - const input = await combo.shadow$("#ui5-combobox-input"); - const arrow = await combo.shadow$("[input-icon]"); - let popover = await combo.shadow$("ui5-responsive-popover"); - let groupItems = await popover.$("ui5-list").$$("ui5-li-group-header"); - let listItems = await popover.$("ui5-list").$$("ui5-li"); + const combo = await browser.$("#combo-grouping"); + const input = await combo.shadow$("#ui5-combobox-input"); + const arrow = await combo.shadow$("[input-icon]"); + let popover = await combo.shadow$("ui5-responsive-popover"); + let groupItems = await popover.$("ui5-list").$$("ui5-li-group"); + let listItems; - await arrow.click(); - assert.strictEqual(groupItems.length, 4, "Group items should be 4"); - assert.strictEqual(listItems.length, 13, "Items should be 13"); + await arrow.click(); + listItems = await popover.$("ui5-list").$$("ui5-li-group ui5-li"); - await input.keys("c"); + assert.strictEqual(groupItems.length, 4, "Group items should be 4"); + assert.strictEqual(listItems.length, 13, "Items should be 13"); - popover = await combo.shadow$("ui5-responsive-popover"); - groupItems = await popover.$("ui5-list").$$("ui5-li-group-header"); - listItems = await popover.$("ui5-list").$$("ui5-li"); + await input.keys("c"); - assert.strictEqual(groupItems.length, 1, "Filtered group items should be 1"); - assert.strictEqual(listItems.length, 2, "Filtered items should be 2"); - }); + popover = await combo.shadow$("ui5-responsive-popover"); + groupItems = await popover.$("ui5-list").$$("ui5-li-group"); + listItems = await popover.$("ui5-list").$$("ui5-li"); - it ("Tests group item focusability", async () => { - await browser.url(`test/pages/ComboBox.html`); + assert.strictEqual(groupItems.length, 1, "Filtered group items should be 1"); + assert.strictEqual(listItems.length, 2, "Filtered items should be 2"); +}); - const combo = await browser.$("#combo-grouping"); - const input = await combo.shadow$("#ui5-combobox-input"); - const arrow = await combo.shadow$("[input-icon]"); - const popover = await combo.shadow$("ui5-responsive-popover"); - let groupItem; +// it ("Tests group item focusability", async () => { +// await browser.url(`test/pages/ComboBox.html`); - await arrow.click(); - await input.keys("ArrowDown"); +// const combo = await browser.$("#combo-grouping"); +// const input = await combo.shadow$("#ui5-combobox-input"); +// const arrow = await combo.shadow$("[input-icon]"); +// const popover = await combo.shadow$("ui5-responsive-popover"); +// let groupItem; - groupItem = await popover.$("ui5-list").$$("ui5-li-group-header")[0]; +// await arrow.click(); +// await input.keys("ArrowDown"); - assert.ok(await groupItem.getProperty("focused"), "The first group header should be focused"); - }); +// groupItem = await popover.$("ui5-list").$$("ui5-li-group")[0].shadow$("ui5-li-group-header"); - it ("Tests input value while group item is focused", async () => { - const combo = await browser.$("#combo-grouping"); - const input = await combo.shadow$("#ui5-combobox-input"); - const arrow = await combo.shadow$("[input-icon]"); - const popover = await combo.shadow$("ui5-responsive-popover"); - let groupItem; +// assert.ok(await groupItem.getAttribute("focused"), "The first group header should be focused"); +// }); - await input.keys("a"); - await input.keys("ArrowDown"); - await input.keys("ArrowDown"); - await input.keys("ArrowDown"); - await input.keys("ArrowDown"); - await input.keys("ArrowDown"); - await input.keys("ArrowDown"); +// it ("Tests input value while group item is focused", async () => { +// const combo = await browser.$("#combo-grouping"); +// const input = await combo.shadow$("#ui5-combobox-input"); +// const popover = await combo.shadow$("ui5-responsive-popover"); +// let groupItem; - groupItem = await popover.$("ui5-list").$$("ui5-li-group-header")[1]; +// await input.keys("a"); +// await input.keys("ArrowDown"); +// await input.keys("ArrowDown"); +// await input.keys("ArrowDown"); +// await input.keys("ArrowDown"); +// await input.keys("ArrowDown"); +// await input.keys("ArrowDown"); - assert.ok(await groupItem.getProperty("focused"), "The second group header should be focused"); - assert.strictEqual(await combo.getProperty("filterValue"), "a", "Filter value should be the initial one"); - assert.strictEqual(await combo.getProperty("value"), "", "Temp value should be reset to the initial filter value - no autocomplete"); - }); +// groupItem = await popover.$("ui5-list").$$("ui5-li-group")[1].shadow$("ui5-li-group-header"); + +// assert.ok(await groupItem.getAttribute("focused"), "The second group header should be focused"); +// assert.strictEqual(await combo.getProperty("filterValue"), "a", "Filter value should be the initial one"); +// assert.strictEqual(await combo.getProperty("value"), "", "Temp value should be reset to the initial filter value - no autocomplete"); +// }); it ("Pressing enter on a group item should not close the picker", async () => { await browser.url(`test/pages/ComboBox.html`); @@ -814,14 +815,14 @@ describe("Accessibility", async () => { await input.keys("ArrowDown"); - assert.strictEqual(await invisibleMessageSpan.getHTML(false), itemAnnouncement3, "First list item is announced") + // assert.strictEqual(await invisibleMessageSpan.getHTML(false), itemAnnouncement3, "First list item is announced") await input.keys("ArrowDown"); await input.keys("ArrowDown"); await input.keys("ArrowDown"); await input.keys("ArrowDown"); - assert.strictEqual(await invisibleMessageSpan.getHTML(false), itemAnnouncement2, "Second group header is announced") + // assert.strictEqual(await invisibleMessageSpan.getHTML(false), itemAnnouncement2, "Second group header is announced") }); it ("Tests setting value programmatically", async () => { From 03f88c3e3789babe9a5aefc0b8a12a8764259ef3 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Thu, 16 May 2024 14:08:21 +0300 Subject: [PATCH 05/20] feat(ui5-cb-group-item): introduce nested grouping improve focus handling --- packages/main/src/ComboBox.ts | 36 ++++++++++++++++----------- packages/main/src/ComboBoxPopover.hbs | 2 +- packages/main/src/ListItemGroup.hbs | 2 +- packages/main/src/ListItemGroup.ts | 7 ++++++ 4 files changed, 30 insertions(+), 17 deletions(-) diff --git a/packages/main/src/ComboBox.ts b/packages/main/src/ComboBox.ts index fdd20d370d46..0206c7da08c8 100644 --- a/packages/main/src/ComboBox.ts +++ b/packages/main/src/ComboBox.ts @@ -691,7 +691,7 @@ class ComboBox extends UI5Element { } _clearFocus() { - const allItems = this._getAllItems(); + const allItems = this._getItems(); allItems.map(item => { item.focused = false; @@ -700,13 +700,14 @@ class ComboBox extends UI5Element { }); } - _getAllItems() { + _getItems() { const allItems: Array = []; + this._filteredItems.forEach(item => { - if (item.items && item.items.length) { - item.items.forEach(groupedItem => { - allItems.push(groupedItem); - }); + if (item.isGroupItem && item.items?.length) { + const groupedItems = [item, ...item.items]; + allItems.push(...groupedItems); + return; } @@ -717,7 +718,7 @@ class ComboBox extends UI5Element { } handleNavKeyPress(e: KeyboardEvent) { - const allItems = this._getAllItems(); + const allItems = this._getItems(); if (this.focused && (isHome(e) || isEnd(e)) && this.value) { return; @@ -754,7 +755,7 @@ class ComboBox extends UI5Element { } _handleItemNavigation(e: KeyboardEvent, indexOfItem: number, isForward: boolean) { - const allItems = this._getAllItems(); + const allItems = this._getItems(); // const suggPopover = await this._getPicker(); const isOpen = this.open; @@ -859,11 +860,12 @@ class ComboBox extends UI5Element { } _handlePageDown(e: KeyboardEvent, indexOfItem: number) { - const itemsLength = this._filteredItems.length; + const allItems = this._getItems(); + const itemsLength = allItems.length; const isProposedIndexValid = indexOfItem + SKIP_ITEMS_SIZE < itemsLength; indexOfItem = isProposedIndexValid ? indexOfItem + SKIP_ITEMS_SIZE : itemsLength - 1; - const shouldMoveForward = this._filteredItems[indexOfItem].isGroupItem && !this.open; + const shouldMoveForward = allItems[indexOfItem].isGroupItem && !this.open; this._handleItemNavigation(e, indexOfItem, shouldMoveForward); } @@ -883,7 +885,7 @@ class ComboBox extends UI5Element { } _handleEnd(e: KeyboardEvent) { - this._handleItemNavigation(e, this._filteredItems.length - 1, true /* isForward */); + this._handleItemNavigation(e, this._getItems().length - 1, true /* isForward */); } _keyup() { @@ -902,12 +904,16 @@ class ComboBox extends UI5Element { } if (isEnter(e)) { - const focusedItem = this._filteredItems.find(item => { - if (item?.items?.length) { - return item.items.find(groupItem => groupItem.focused); + let focusedItem: IComboBoxItem | undefined; + + this._filteredItems.forEach(item => { + if (item?.items?.length && !focusedItem) { + focusedItem = item.items.find(groupItem => groupItem.focused); } - return item.focused; + if (item.focused) { + focusedItem = item; + } }); this._fireChangeEvent(); diff --git a/packages/main/src/ComboBoxPopover.hbs b/packages/main/src/ComboBoxPopover.hbs index db2aae0912d4..87f45607e009 100644 --- a/packages/main/src/ComboBoxPopover.hbs +++ b/packages/main/src/ComboBoxPopover.hbs @@ -70,7 +70,7 @@ > {{#each _filteredItems}} {{#if isGroupItem}} - + {{#each this.items}} {{#if _isVisible}} {{> listItem}} diff --git a/packages/main/src/ListItemGroup.hbs b/packages/main/src/ListItemGroup.hbs index ddc04efa436c..f22ee34c9f1b 100644 --- a/packages/main/src/ListItemGroup.hbs +++ b/packages/main/src/ListItemGroup.hbs @@ -1,6 +1,6 @@
    {{#if hasHeader}} - + {{#if hasFormattedHeader}} {{else}} diff --git a/packages/main/src/ListItemGroup.ts b/packages/main/src/ListItemGroup.ts index bfbfb56e76ed..36b125f0929d 100644 --- a/packages/main/src/ListItemGroup.ts +++ b/packages/main/src/ListItemGroup.ts @@ -53,6 +53,13 @@ class ListItemGroup extends UI5Element { @property({ type: String }) headerAccessibleName!: string; + /** + * Indicates if the element is on focus + * @private + */ + @property({ type: Boolean }) + focused!: boolean; + /** * Defines the items of the ui5-li-group. * @public From c675a15c67838984172fd2d2516293a5a279790e Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Fri, 17 May 2024 13:57:08 +0300 Subject: [PATCH 06/20] feat(ui5-cb-group-item): introduce nested grouping fix tests --- packages/main/src/ComboBox.ts | 19 ++-- packages/main/src/ListItemGroup.hbs | 2 +- packages/main/src/ListItemGroup.ts | 7 -- packages/main/test/specs/ComboBox.spec.js | 110 +++++++++++----------- 4 files changed, 67 insertions(+), 71 deletions(-) diff --git a/packages/main/src/ComboBox.ts b/packages/main/src/ComboBox.ts index 0206c7da08c8..bed898d3789b 100644 --- a/packages/main/src/ComboBox.ts +++ b/packages/main/src/ComboBox.ts @@ -756,14 +756,13 @@ class ComboBox extends UI5Element { _handleItemNavigation(e: KeyboardEvent, indexOfItem: number, isForward: boolean) { const allItems = this._getItems(); - // const suggPopover = await this._getPicker(); const isOpen = this.open; const currentItem: IComboBoxItem = allItems[indexOfItem]; - + const isGroupItem = currentItem && currentItem.isGroupItem; const nextItem = isForward ? allItems[indexOfItem + 1] : allItems[indexOfItem - 1]; - if ((!isOpen) && (!nextItem || !currentItem)) { + if ((!isOpen) && ((isGroupItem && !nextItem) || (!isGroupItem && !currentItem))) { return; } @@ -771,13 +770,13 @@ class ComboBox extends UI5Element { if (isOpen) { this._itemFocused = true; - this.value = currentItem.text; + this.value = isGroupItem ? "" : currentItem.text; this.focused = false; currentItem.focused = true; } else { this.focused = true; - this.value = currentItem.text; + this.value = isGroupItem ? "" : currentItem.text; currentItem.focused = false; } @@ -786,6 +785,9 @@ class ComboBox extends UI5Element { this._announceSelectedItem(indexOfItem); this._scrollToItem(indexOfItem, isForward); + if (isGroupItem && isOpen) { + return; + } // autocomplete const item = this._getFirstMatchingItem(this.value); item && this._applyAtomicValueAndSelection(item, (this.open ? this._userTypedValue : ""), true); @@ -918,7 +920,7 @@ class ComboBox extends UI5Element { this._fireChangeEvent(); - if (picker?.open && focusedItem && !focusedItem?.isGroupItem) { + if (picker?.open && !focusedItem?.isGroupItem) { this._closeRespPopover(); this.focused = true; this.inner.setSelectionRange(this.value.length, this.value.length); @@ -1124,8 +1126,9 @@ class ComboBox extends UI5Element { } _announceSelectedItem(indexOfItem: number) { - const currentItem = this._filteredItems[indexOfItem]; - const nonGroupItems = this._filteredItems.filter(item => !item.isGroupItem); + const allItems = this._getItems(); + const currentItem = allItems[indexOfItem]; + const nonGroupItems = allItems.filter(item => !item.isGroupItem); const currentItemAdditionalText = currentItem?.additionalText || ""; const isGroupItem = currentItem?.isGroupItem; const itemPositionText = ComboBox.i18nBundle.getText(LIST_ITEM_POSITION, nonGroupItems.indexOf(currentItem) + 1, nonGroupItems.length); diff --git a/packages/main/src/ListItemGroup.hbs b/packages/main/src/ListItemGroup.hbs index f22ee34c9f1b..ddc04efa436c 100644 --- a/packages/main/src/ListItemGroup.hbs +++ b/packages/main/src/ListItemGroup.hbs @@ -1,6 +1,6 @@
      {{#if hasHeader}} - + {{#if hasFormattedHeader}} {{else}} diff --git a/packages/main/src/ListItemGroup.ts b/packages/main/src/ListItemGroup.ts index 36b125f0929d..bfbfb56e76ed 100644 --- a/packages/main/src/ListItemGroup.ts +++ b/packages/main/src/ListItemGroup.ts @@ -53,13 +53,6 @@ class ListItemGroup extends UI5Element { @property({ type: String }) headerAccessibleName!: string; - /** - * Indicates if the element is on focus - * @private - */ - @property({ type: Boolean }) - focused!: boolean; - /** * Defines the items of the ui5-li-group. * @public diff --git a/packages/main/test/specs/ComboBox.spec.js b/packages/main/test/specs/ComboBox.spec.js index 2706674539ad..c0d4c56e2380 100644 --- a/packages/main/test/specs/ComboBox.spec.js +++ b/packages/main/test/specs/ComboBox.spec.js @@ -648,69 +648,69 @@ describe("General interaction", () => { describe("Grouping", () => { -it ("Tests group filtering", async () => { - await browser.url(`test/pages/ComboBox.html`); + it ("Tests group filtering", async () => { + await browser.url(`test/pages/ComboBox.html`); - const combo = await browser.$("#combo-grouping"); - const input = await combo.shadow$("#ui5-combobox-input"); - const arrow = await combo.shadow$("[input-icon]"); - let popover = await combo.shadow$("ui5-responsive-popover"); - let groupItems = await popover.$("ui5-list").$$("ui5-li-group"); - let listItems; + const combo = await browser.$("#combo-grouping"); + const input = await combo.shadow$("#ui5-combobox-input"); + const arrow = await combo.shadow$("[input-icon]"); + let popover = await combo.shadow$("ui5-responsive-popover"); + let groupItems = await popover.$("ui5-list").$$("ui5-li-group"); + let listItems; - await arrow.click(); - listItems = await popover.$("ui5-list").$$("ui5-li-group ui5-li"); + await arrow.click(); + listItems = await popover.$("ui5-list").$$("ui5-li-group ui5-li"); - assert.strictEqual(groupItems.length, 4, "Group items should be 4"); - assert.strictEqual(listItems.length, 13, "Items should be 13"); + assert.strictEqual(groupItems.length, 4, "Group items should be 4"); + assert.strictEqual(listItems.length, 13, "Items should be 13"); - await input.keys("c"); + await input.keys("c"); - popover = await combo.shadow$("ui5-responsive-popover"); - groupItems = await popover.$("ui5-list").$$("ui5-li-group"); - listItems = await popover.$("ui5-list").$$("ui5-li"); + popover = await combo.shadow$("ui5-responsive-popover"); + groupItems = await popover.$("ui5-list").$$("ui5-li-group"); + listItems = await popover.$("ui5-list").$$("ui5-li"); - assert.strictEqual(groupItems.length, 1, "Filtered group items should be 1"); - assert.strictEqual(listItems.length, 2, "Filtered items should be 2"); -}); + assert.strictEqual(groupItems.length, 1, "Filtered group items should be 1"); + assert.strictEqual(listItems.length, 2, "Filtered items should be 2"); + }); -// it ("Tests group item focusability", async () => { -// await browser.url(`test/pages/ComboBox.html`); + it ("Tests group item focusability", async () => { + await browser.url(`test/pages/ComboBox.html`); -// const combo = await browser.$("#combo-grouping"); -// const input = await combo.shadow$("#ui5-combobox-input"); -// const arrow = await combo.shadow$("[input-icon]"); -// const popover = await combo.shadow$("ui5-responsive-popover"); -// let groupItem; + const combo = await browser.$("#combo-grouping"); + const input = await combo.shadow$("#ui5-combobox-input"); + const arrow = await combo.shadow$("[input-icon]"); + const popover = await combo.shadow$("ui5-responsive-popover"); + let groupItem; -// await arrow.click(); -// await input.keys("ArrowDown"); + await arrow.click(); + await input.keys("ArrowDown"); -// groupItem = await popover.$("ui5-list").$$("ui5-li-group")[0].shadow$("ui5-li-group-header"); + groupItem = await popover.$("ui5-list").$("ui5-li-group").shadow$("ui5-li-group-header"); -// assert.ok(await groupItem.getAttribute("focused"), "The first group header should be focused"); -// }); + assert.ok(await groupItem.getProperty("focused"), "The first group header should be focused"); + }); -// it ("Tests input value while group item is focused", async () => { -// const combo = await browser.$("#combo-grouping"); -// const input = await combo.shadow$("#ui5-combobox-input"); -// const popover = await combo.shadow$("ui5-responsive-popover"); -// let groupItem; + it ("Tests input value while group item is focused", async () => { + const combo = await browser.$("#combo-grouping"); + const input = await combo.shadow$("#ui5-combobox-input"); + const popover = await combo.shadow$("ui5-responsive-popover"); + let groupItem; -// await input.keys("a"); -// await input.keys("ArrowDown"); -// await input.keys("ArrowDown"); -// await input.keys("ArrowDown"); -// await input.keys("ArrowDown"); -// await input.keys("ArrowDown"); -// await input.keys("ArrowDown"); + await input.keys("a"); + await input.keys("ArrowDown"); + await input.keys("ArrowDown"); + await input.keys("ArrowDown"); + await input.keys("ArrowDown"); + await input.keys("ArrowDown"); + await input.keys("ArrowDown"); -// groupItem = await popover.$("ui5-list").$$("ui5-li-group")[1].shadow$("ui5-li-group-header"); + groupItem = await popover.$("ui5-list").$$("ui5-li-group")[1]; -// assert.ok(await groupItem.getAttribute("focused"), "The second group header should be focused"); -// assert.strictEqual(await combo.getProperty("filterValue"), "a", "Filter value should be the initial one"); -// assert.strictEqual(await combo.getProperty("value"), "", "Temp value should be reset to the initial filter value - no autocomplete"); -// }); + assert.ok(await groupItem.getProperty("focused"), "The second group header should be focused"); + assert.strictEqual(await combo.getProperty("filterValue"), "a", "Filter value should be the initial one"); + assert.strictEqual(await combo.getProperty("value"), "", "Temp value should be reset to the initial filter value - no autocomplete"); + }); it ("Pressing enter on a group item should not close the picker", async () => { await browser.url(`test/pages/ComboBox.html`); @@ -815,14 +815,14 @@ describe("Accessibility", async () => { await input.keys("ArrowDown"); - // assert.strictEqual(await invisibleMessageSpan.getHTML(false), itemAnnouncement3, "First list item is announced") + assert.strictEqual(await invisibleMessageSpan.getHTML(false), itemAnnouncement3, "First list item is announced") await input.keys("ArrowDown"); await input.keys("ArrowDown"); await input.keys("ArrowDown"); await input.keys("ArrowDown"); - // assert.strictEqual(await invisibleMessageSpan.getHTML(false), itemAnnouncement2, "Second group header is announced") + assert.strictEqual(await invisibleMessageSpan.getHTML(false), itemAnnouncement2, "Second group header is announced") }); it ("Tests setting value programmatically", async () => { @@ -923,7 +923,7 @@ describe("Keyboard navigation", async () => { await arrow.click(); await input.keys("ArrowDown"); - groupItem = await popover.$("ui5-list").$$("ui5-li-group-header")[0]; + groupItem = await popover.$("ui5-list").$$("ui5-li-group")[0]; assert.strictEqual(await groupItem.getProperty("focused"), true, "The first group header should be focused"); @@ -933,7 +933,7 @@ describe("Keyboard navigation", async () => { await input.keys("ArrowDown"); await input.keys("ArrowDown"); - listItem = await popover.$("ui5-list").$$("ui5-li")[0]; + listItem = await popover.$("ui5-list").$("ui5-li-group ui5-li"); assert.strictEqual(await listItem.getProperty("focused"), true, "The first list item after the group header should be focused"); @@ -968,7 +968,7 @@ describe("Keyboard navigation", async () => { await input.keys("ArrowDown"); await input.keys("ArrowDown"); - groupItem = await popover.$("ui5-list").$$("ui5-li-group-header")[0]; + groupItem = await popover.$("ui5-list").$("ui5-li-group"); assert.strictEqual(await groupItem.getProperty("focused"), true, "The first group header should be focused"); }); @@ -1181,7 +1181,7 @@ describe("Keyboard navigation", async () => { let isInVisibleArea = await browser.executeAsync(async done => { const combobox = document.getElementById("combo-grouping"); const picker = await combobox._getPicker(); - const listItem = picker.querySelector(".ui5-combobox-items-list ui5-li:last-child"); + const listItem = picker.querySelector(".ui5-combobox-items-list ui5-li-group:last-child ui5-li:last-child"); const scrollableRect = picker.shadowRoot.querySelector(".ui5-popup-content").getBoundingClientRect(); const elementRect = listItem.getBoundingClientRect(); @@ -1210,7 +1210,7 @@ describe("Keyboard navigation", async () => { isInVisibleArea = await browser.executeAsync(async done => { const combobox = document.getElementById("combo-grouping"); const picker = await combobox._getPicker(); - const listItem = picker.querySelector(".ui5-combobox-items-list ui5-li:last-child"); + const listItem = picker.querySelector(".ui5-combobox-items-list ui5-li-group:last-child ui5-li:last-child"); const scrollableRect = picker.shadowRoot.querySelector(".ui5-popup-content").getBoundingClientRect(); const elementRect = listItem.getBoundingClientRect(); From 6b181bed17488d0edec0751815175a60aeda8316 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Mon, 20 May 2024 12:27:50 +0300 Subject: [PATCH 07/20] feat(ui5-combobox): introduce nested grouping add accessibility roles and tests --- packages/main/src/ComboBoxItem.ts | 4 ++-- packages/main/src/ComboBoxPopover.hbs | 2 ++ packages/main/test/specs/ComboBox.spec.js | 16 ++++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/main/src/ComboBoxItem.ts b/packages/main/src/ComboBoxItem.ts index b011d5f514db..7684c79e52f1 100644 --- a/packages/main/src/ComboBoxItem.ts +++ b/packages/main/src/ComboBoxItem.ts @@ -35,8 +35,8 @@ class ComboBoxItem extends UI5Element implements IComboBoxItem { * Indicates whether the item is filtered * @private */ - @property({ type: Boolean, noAttribute: true }) - _isVisible!: boolean; + @property({ type: Boolean, noAttribute: true }) + _isVisible!: boolean; /** * Indicates whether the item is focssed diff --git a/packages/main/src/ComboBoxPopover.hbs b/packages/main/src/ComboBoxPopover.hbs index af68ef0ba99a..87521b210287 100644 --- a/packages/main/src/ComboBoxPopover.hbs +++ b/packages/main/src/ComboBoxPopover.hbs @@ -62,6 +62,7 @@ {{/unless}} { assert.ok(await popover.getProperty("open"), "Popover remains open"); }); + + it ("Pressing enter on a group item should not close the picker", async () => { + await browser.url(`test/pages/ComboBox.html`); + + const combo = await browser.$("#combo-grouping"); + const input = await combo.shadow$("#ui5-combobox-input"); + const popover = await combo.shadow$("ui5-responsive-popover"); + const listItem = await popover.$("ui5-list").$$("ui5-li")[0]; + const hiddenItem = await popover.$("ui5-list").$$("ui5-li")[5]; + + await input.click(); + await input.keys("a"); + + assert.ok(await listItem.getProperty("_isVisible"), "The filtered item is shown"); + assert.notOk(await hiddenItem.getProperty("_isVisible"), "The filtered-out item is not rendered"); + }); }); describe("Accessibility", async () => { From a42ae331ef89ba72e8b12699fc5a29a2b8e0f979 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Mon, 20 May 2024 13:20:00 +0300 Subject: [PATCH 08/20] feat(ui5-cb-group-item): introduce nested grouping (draft) fix test --- packages/main/test/specs/ComboBox.spec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/main/test/specs/ComboBox.spec.js b/packages/main/test/specs/ComboBox.spec.js index 45dbcf4c21d1..8b270b444388 100644 --- a/packages/main/test/specs/ComboBox.spec.js +++ b/packages/main/test/specs/ComboBox.spec.js @@ -733,14 +733,14 @@ describe("Grouping", () => { const combo = await browser.$("#combo-grouping"); const input = await combo.shadow$("#ui5-combobox-input"); const popover = await combo.shadow$("ui5-responsive-popover"); - const listItem = await popover.$("ui5-list").$$("ui5-li")[0]; - const hiddenItem = await popover.$("ui5-list").$$("ui5-li")[5]; + const list = await popover.$("ui5-list"); + const listItem = await list.$("ui5-li"); await input.click(); await input.keys("a"); assert.ok(await listItem.getProperty("_isVisible"), "The filtered item is shown"); - assert.notOk(await hiddenItem.getProperty("_isVisible"), "The filtered-out item is not rendered"); + assert.strictEqual(await list.$$("ui5-li").length, 5, "Group items are filtered correctly"); }); }); From e888c7229b945fdaa9e6f23c8d53599b2b9c78df Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Mon, 20 May 2024 16:27:32 +0300 Subject: [PATCH 09/20] feat(ui5-cb-group-item): introduce nested grouping (draft) add tests --- packages/main/src/ComboBox.ts | 2 ++ packages/main/test/specs/ComboBox.spec.js | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/main/src/ComboBox.ts b/packages/main/src/ComboBox.ts index cf3ea8dbb698..431d18d62626 100644 --- a/packages/main/src/ComboBox.ts +++ b/packages/main/src/ComboBox.ts @@ -78,6 +78,7 @@ import BusyIndicator from "./BusyIndicator.js"; import Button from "./Button.js"; import StandardListItem from "./StandardListItem.js"; import ComboBoxGroupItem from "./ComboBoxItemGroup.js"; +import ListItemGroup from "./ListItemGroup.js"; import ListItemGroupHeader from "./ListItemGroupHeader.js"; import ComboBoxFilter from "./types/ComboBoxFilter.js"; import PopoverHorizontalAlign from "./types/PopoverHorizontalAlign.js"; @@ -178,6 +179,7 @@ type ComboBoxSelectionChangeEventDetail = { BusyIndicator, Button, StandardListItem, + ListItemGroup, ListItemGroupHeader, Popover, ComboBoxGroupItem, diff --git a/packages/main/test/specs/ComboBox.spec.js b/packages/main/test/specs/ComboBox.spec.js index 8b270b444388..935162093478 100644 --- a/packages/main/test/specs/ComboBox.spec.js +++ b/packages/main/test/specs/ComboBox.spec.js @@ -739,7 +739,9 @@ describe("Grouping", () => { await input.click(); await input.keys("a"); - assert.ok(await listItem.getProperty("_isVisible"), "The filtered item is shown"); + assert.ok(await listItem, "The filtered item is shown"); + assert.strictEqual(await list.getAttribute("accessible-role"), "ListBox", "The list item has the correct role attribute"); + assert.strictEqual(await listItem.getAttribute("accessible-role"), "Option", "The list item has the correct role attribute"); assert.strictEqual(await list.$$("ui5-li").length, 5, "Group items are filtered correctly"); }); }); From ed34062ce228036b3ee8ee7febecd46cbd96336d Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Mon, 20 May 2024 16:49:59 +0300 Subject: [PATCH 10/20] feat(ui5-cb-group-item): introduce nested grouping fix mobile tests --- packages/main/test/specs/ComboBox.mobile.spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/main/test/specs/ComboBox.mobile.spec.js b/packages/main/test/specs/ComboBox.mobile.spec.js index fcea70fb51dd..37e93bcfa569 100644 --- a/packages/main/test/specs/ComboBox.mobile.spec.js +++ b/packages/main/test/specs/ComboBox.mobile.spec.js @@ -1,4 +1,4 @@ -import { assert } from "chai"; +import { assert } from "chai"; are shown describe("Basic mobile picker rendering and interaction", () => { before(async () => { @@ -243,7 +243,7 @@ describe("Picker filtering", () => { const dialogInput = await combo.shadow$("ui5-responsive-popover").$("[ui5-input]"); const dialogList = await combo.shadow$("ui5-responsive-popover").$('ui5-list') - assert.strictEqual(await dialogList.$$('ui5-li').length, 8, "All of the items are shown (8)"); + assert.strictEqual(await dialogList.$$('ui5-li').length, 9, "All of the items are shown (8)"); await dialogInput.keys("B"); assert.strictEqual(await dialogList.$$('ui5-li').length, 3, "There are 3 filtered items"); }); @@ -259,9 +259,9 @@ describe("Picker filtering", () => { const dialogInput = await combo.shadow$("ui5-responsive-popover").$("[ui5-input]"); const dialogList = await combo.shadow$("ui5-responsive-popover").$('ui5-list') - assert.strictEqual(await dialogList.$$('ui5-li-group-header').length, 3, "All of the group header list items are shown (3)"); + assert.strictEqual(await dialogList.$$('ui5-li-group').length, 3, "All of the group header list items are shown (3)"); await dialogInput.keys("B"); - assert.strictEqual(await dialogList.$$('ui5-li-group-header').length, 1, "There is only 1 visible group header"); + assert.strictEqual(await dialogList.$$('ui5-li-group').length, 1, "There is only 1 visible group"); }); }); From dd5dc0585715c479f1c95e82404a1af586b07e11 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Mon, 20 May 2024 17:22:37 +0300 Subject: [PATCH 11/20] Update ComboBox.mobile.spec.js --- packages/main/test/specs/ComboBox.mobile.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/main/test/specs/ComboBox.mobile.spec.js b/packages/main/test/specs/ComboBox.mobile.spec.js index 37e93bcfa569..ab51fd06d0ee 100644 --- a/packages/main/test/specs/ComboBox.mobile.spec.js +++ b/packages/main/test/specs/ComboBox.mobile.spec.js @@ -1,4 +1,4 @@ -import { assert } from "chai"; are shown +import { assert } from "chai"; describe("Basic mobile picker rendering and interaction", () => { before(async () => { From 313f34ce357b5cb9796ab4acbac62e01323f9b32 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Tue, 21 May 2024 16:37:14 +0300 Subject: [PATCH 12/20] feat(ui5-cb-group-item): introduce nested grouping fix item filtration on mobile --- packages/main/src/ComboBox.ts | 48 +++++++++++++------ packages/main/src/ComboBoxItemGroup.ts | 6 ++- packages/main/src/ComboBoxPopover.hbs | 16 ++++--- .../main/test/specs/ComboBox.mobile.spec.js | 4 +- packages/main/test/specs/ComboBox.spec.js | 6 ++- 5 files changed, 53 insertions(+), 27 deletions(-) diff --git a/packages/main/src/ComboBox.ts b/packages/main/src/ComboBox.ts index 431d18d62626..004df577becb 100644 --- a/packages/main/src/ComboBox.ts +++ b/packages/main/src/ComboBox.ts @@ -457,7 +457,21 @@ class ComboBox extends UI5Element implements IFormInputElement { if (this.open && !this._isKeyNavigation) { const items = this._filterItems(this.filterValue); - this._filteredItems = items.length ? items : this.items; + this._filteredItems = (items.length && items) || []; + } + + if (((!this._filteredItems.length && !isPhone) || !this._filteredItems.some(i => i._isVisible)) && this.value) { + this.items.forEach(item => { + if (item?.items?.length) { + item.items.forEach(i => { + i._isVisible = true; + }); + } else { + item._isVisible = true; + } + }); + + this._filteredItems = this.items; } if (!this._initialRendering && document.activeElement === this && !this._filteredItems.length && popover) { @@ -639,19 +653,6 @@ class ComboBox extends UI5Element implements IFormInputElement { const { value } = e.target as HTMLInputElement; const shouldAutocomplete = this.shouldAutocomplete(e); - // Reset item filtration - this.items.forEach(item => { - if (item.isGroupItem) { - item.items?.forEach(i => { - i._isVisible = false; - }); - - return; - } - - item._isVisible = false; - }); - if (e.target === this.inner) { // stop the native event, as the semantic "input" would be fired. e.stopImmediatePropagation(); @@ -1042,6 +1043,19 @@ class ComboBox extends UI5Element implements IFormInputElement { const filteredItems: Array = []; const filteredItemGroups: Array = []; + // Reset item filtration + this.items.forEach(item => { + if (item.isGroupItem) { + item.items?.forEach(i => { + i._isVisible = false; + }); + return; + } + + item._isVisible = false; + }); + + // Filter items this.items.forEach(item => { if (item.items?.length) { filteredGroupItems = (Filters[this.filter] || Filters.StartsWithPerTerm)(str, item.items, "text"); @@ -1057,7 +1071,11 @@ class ComboBox extends UI5Element implements IFormInputElement { } [filteredItem] = (Filters[this.filter] || Filters.StartsWithPerTerm)(str, [item], "text"); - filteredItem && filteredItems.push(filteredItem); + + if (filteredItem) { + filteredItem._isVisible = true; + filteredItems.push(filteredItem); + } }); return [...filteredItemGroups, ...filteredItems]; diff --git a/packages/main/src/ComboBoxItemGroup.ts b/packages/main/src/ComboBoxItemGroup.ts index 7332d00099d9..0f6178ba66ba 100644 --- a/packages/main/src/ComboBoxItemGroup.ts +++ b/packages/main/src/ComboBoxItemGroup.ts @@ -33,7 +33,7 @@ class ComboBoxGroupItem extends UI5Element implements IComboBoxItem { focused!: boolean; /** - * Defines the items of the ui5-li-group. + * Defines the items of the ui5-cb-item-group. * @public */ @slot({ @@ -54,6 +54,10 @@ class ComboBoxGroupItem extends UI5Element implements IComboBoxItem { get stableDomRef() { return this.getAttribute("stable-dom-ref") || `${this._id}-stable-dom-ref`; } + + get _isVisible() { + return this.items.some(item => item._isVisible); + } } ComboBoxGroupItem.define(); diff --git a/packages/main/src/ComboBoxPopover.hbs b/packages/main/src/ComboBoxPopover.hbs index 87521b210287..098be08e7c6d 100644 --- a/packages/main/src/ComboBoxPopover.hbs +++ b/packages/main/src/ComboBoxPopover.hbs @@ -72,13 +72,15 @@ > {{#each _filteredItems}} {{#if isGroupItem}} - - {{#each this.items}} - {{#if _isVisible}} - {{> listItem}} - {{/if}} - {{/each}} - + {{#if _isVisible}} + + {{#each this.items}} + {{#if _isVisible}} + {{> listItem}} + {{/if}} + {{/each}} + + {{/if}} {{else}} {{> listItem}} {{/if}} diff --git a/packages/main/test/specs/ComboBox.mobile.spec.js b/packages/main/test/specs/ComboBox.mobile.spec.js index ab51fd06d0ee..79b515848015 100644 --- a/packages/main/test/specs/ComboBox.mobile.spec.js +++ b/packages/main/test/specs/ComboBox.mobile.spec.js @@ -245,7 +245,7 @@ describe("Picker filtering", () => { assert.strictEqual(await dialogList.$$('ui5-li').length, 9, "All of the items are shown (8)"); await dialogInput.keys("B"); - assert.strictEqual(await dialogList.$$('ui5-li').length, 3, "There are 3 filtered items"); + assert.strictEqual(await dialogList.$$('ui5-li').length, 4, "There are 4 filtered items"); }); it("Should filter group header list items", async () => { @@ -261,7 +261,7 @@ describe("Picker filtering", () => { assert.strictEqual(await dialogList.$$('ui5-li-group').length, 3, "All of the group header list items are shown (3)"); await dialogInput.keys("B"); - assert.strictEqual(await dialogList.$$('ui5-li-group').length, 1, "There is only 1 visible group"); + assert.strictEqual(await dialogList.$$('ui5-li-group').length, 2, "There is only 1 visible group"); }); }); diff --git a/packages/main/test/specs/ComboBox.spec.js b/packages/main/test/specs/ComboBox.spec.js index 935162093478..8b17904fd119 100644 --- a/packages/main/test/specs/ComboBox.spec.js +++ b/packages/main/test/specs/ComboBox.spec.js @@ -145,7 +145,7 @@ describe("General interaction", () => { listItems = await popover.$("ui5-list").$$("ui5-li"); // assert - assert.strictEqual(listItems.length, 0, "Items should be 0"); + assert.notOk(listItems.some(item => item._isVisible), "Rendered items should be 0"); assert.notOk(await popover.getProperty("open"), "Popover should close"); }); @@ -655,10 +655,12 @@ describe("Grouping", () => { const input = await combo.shadow$("#ui5-combobox-input"); const arrow = await combo.shadow$(".inputIcon"); let popover = await combo.shadow$("ui5-responsive-popover"); - let groupItems = await popover.$("ui5-list").$$("ui5-li-group"); let listItems; + let groupItems; await arrow.click(); + + groupItems = await popover.$("ui5-list").$$("ui5-li-group"); listItems = await popover.$("ui5-list").$$("ui5-li-group ui5-li"); assert.strictEqual(groupItems.length, 4, "Group items should be 4"); From 1eb42fe42967e7036f66ee788aea8c3aa280c973 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Wed, 22 May 2024 14:01:35 +0300 Subject: [PATCH 13/20] feat(ui5-combobox): introduce nested grouping of items minor refactoring and fixes --- packages/main/src/ComboBox.ts | 61 ++++++++++++----------- packages/main/src/ComboBoxItemGroup.ts | 6 +-- packages/main/test/specs/ComboBox.spec.js | 2 +- 3 files changed, 37 insertions(+), 32 deletions(-) diff --git a/packages/main/src/ComboBox.ts b/packages/main/src/ComboBox.ts index 004df577becb..1e80d6d8b447 100644 --- a/packages/main/src/ComboBox.ts +++ b/packages/main/src/ComboBox.ts @@ -77,7 +77,7 @@ import type { ListItemClickEventDetail } from "./List.js"; import BusyIndicator from "./BusyIndicator.js"; import Button from "./Button.js"; import StandardListItem from "./StandardListItem.js"; -import ComboBoxGroupItem from "./ComboBoxItemGroup.js"; +import ComboBoxItemGroup from "./ComboBoxItemGroup.js"; import ListItemGroup from "./ListItemGroup.js"; import ListItemGroupHeader from "./ListItemGroupHeader.js"; import ComboBoxFilter from "./types/ComboBoxFilter.js"; @@ -182,7 +182,7 @@ type ComboBoxSelectionChangeEventDetail = { ListItemGroup, ListItemGroupHeader, Popover, - ComboBoxGroupItem, + ComboBoxItemGroup, Input, SuggestionItem, ], @@ -456,21 +456,14 @@ class ComboBox extends UI5Element implements IFormInputElement { if (this.open && !this._isKeyNavigation) { const items = this._filterItems(this.filterValue); - this._filteredItems = (items.length && items) || []; } - if (((!this._filteredItems.length && !isPhone) || !this._filteredItems.some(i => i._isVisible)) && this.value) { - this.items.forEach(item => { - if (item?.items?.length) { - item.items.forEach(i => { - i._isVisible = true; - }); - } else { - item._isVisible = true; - } - }); + const hasNoVisibleItems = !this._filteredItems.length || !this._filteredItems.some(i => i._isVisible); + // If there is no filtered items matching the value, show all items when the arrow is pressed + if (((hasNoVisibleItems && !isPhone) && this.value)) { + this.items.forEach(this._makeAllVisible.bind(this)); this._filteredItems = this.items; } @@ -631,6 +624,19 @@ class ComboBox extends UI5Element implements IFormInputElement { this._selectMatchingItem(); } + _resetItemVisibility() { + this.items.forEach(item => { + if (item.isGroupItem) { + item.items?.forEach(i => { + i._isVisible = false; + }); + return; + } + + item._isVisible = false; + }); + } + _arrowClick() { this.inner.focus(); this._resetFilter(); @@ -737,14 +743,14 @@ class ComboBox extends UI5Element implements IFormInputElement { }); } + // Get groups and items as a flat array for filtering _getItems() { const allItems: Array = []; this._filteredItems.forEach(item => { - if (item.isGroupItem && item.items?.length) { + if (item.items?.length) { const groupedItems = [item, ...item.items]; allItems.push(...groupedItems); - return; } @@ -1043,19 +1049,7 @@ class ComboBox extends UI5Element implements IFormInputElement { const filteredItems: Array = []; const filteredItemGroups: Array = []; - // Reset item filtration - this.items.forEach(item => { - if (item.isGroupItem) { - item.items?.forEach(i => { - i._isVisible = false; - }); - return; - } - - item._isVisible = false; - }); - - // Filter items + this._resetItemVisibility(); this.items.forEach(item => { if (item.items?.length) { filteredGroupItems = (Filters[this.filter] || Filters.StartsWithPerTerm)(str, item.items, "text"); @@ -1218,6 +1212,17 @@ class ComboBox extends UI5Element implements IFormInputElement { } } + _makeAllVisible(item: IComboBoxItem) { + if (item?.items?.length) { + item.items.forEach(groupItem => { + groupItem._isVisible = true; + }); + return; + } + + item._isVisible = true; + } + async _scrollToItem(indexOfItem: number, forward: boolean) { const picker = await this._getPicker(); const list = picker.querySelector(".ui5-combobox-items-list") as List; diff --git a/packages/main/src/ComboBoxItemGroup.ts b/packages/main/src/ComboBoxItemGroup.ts index 0f6178ba66ba..657b039c87c9 100644 --- a/packages/main/src/ComboBoxItemGroup.ts +++ b/packages/main/src/ComboBoxItemGroup.ts @@ -16,7 +16,7 @@ import type { IComboBoxItem } from "./ComboBox.js"; * @since 1.0.0-rc.15 */ @customElement("ui5-cb-item-group") -class ComboBoxGroupItem extends UI5Element implements IComboBoxItem { +class ComboBoxItemGroup extends UI5Element implements IComboBoxItem { /** * Defines the text of the component. * @default "" @@ -60,6 +60,6 @@ class ComboBoxGroupItem extends UI5Element implements IComboBoxItem { } } -ComboBoxGroupItem.define(); +ComboBoxItemGroup.define(); -export default ComboBoxGroupItem; +export default ComboBoxItemGroup; diff --git a/packages/main/test/specs/ComboBox.spec.js b/packages/main/test/specs/ComboBox.spec.js index 8b17904fd119..e09e21998723 100644 --- a/packages/main/test/specs/ComboBox.spec.js +++ b/packages/main/test/specs/ComboBox.spec.js @@ -729,7 +729,7 @@ describe("Grouping", () => { assert.ok(await popover.getProperty("open"), "Popover remains open"); }); - it ("Pressing enter on a group item should not close the picker", async () => { + it ("Grouped items should be filtered and with the correct role attributes", async () => { await browser.url(`test/pages/ComboBox.html`); const combo = await browser.$("#combo-grouping"); From 981427ad970a6f7f32876bc4b938b6009c7d221c Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Wed, 22 May 2024 14:44:52 +0300 Subject: [PATCH 14/20] feat(ui5-combobox): introduce nested grouping of items refactor group items checks --- packages/main/src/ComboBox.ts | 24 ++++++++++++------------ packages/main/src/ComboBoxItemGroup.ts | 5 +++++ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/packages/main/src/ComboBox.ts b/packages/main/src/ComboBox.ts index 1e80d6d8b447..a569031a0218 100644 --- a/packages/main/src/ComboBox.ts +++ b/packages/main/src/ComboBox.ts @@ -77,7 +77,7 @@ import type { ListItemClickEventDetail } from "./List.js"; import BusyIndicator from "./BusyIndicator.js"; import Button from "./Button.js"; import StandardListItem from "./StandardListItem.js"; -import ComboBoxItemGroup from "./ComboBoxItemGroup.js"; +import ComboBoxItemGroup, { isInstanceOfComboBoxItemGroup } from "./ComboBoxItemGroup.js"; import ListItemGroup from "./ListItemGroup.js"; import ListItemGroupHeader from "./ListItemGroupHeader.js"; import ComboBoxFilter from "./types/ComboBoxFilter.js"; @@ -626,7 +626,7 @@ class ComboBox extends UI5Element implements IFormInputElement { _resetItemVisibility() { this.items.forEach(item => { - if (item.isGroupItem) { + if (isInstanceOfComboBoxItemGroup(item)) { item.items?.forEach(i => { i._isVisible = false; }); @@ -722,7 +722,7 @@ class ComboBox extends UI5Element implements IFormInputElement { const filteredItems: Array = []; this._filteredItems.forEach(item => { - if (item.items?.length) { + if (isInstanceOfComboBoxItemGroup(item)) { filteredItems.push(...item.items); return; } @@ -748,7 +748,7 @@ class ComboBox extends UI5Element implements IFormInputElement { const allItems: Array = []; this._filteredItems.forEach(item => { - if (item.items?.length) { + if (isInstanceOfComboBoxItemGroup(item)) { const groupedItems = [item, ...item.items]; allItems.push(...groupedItems); return; @@ -952,7 +952,7 @@ class ComboBox extends UI5Element implements IFormInputElement { let focusedItem: IComboBoxItem | undefined; this._filteredItems.forEach(item => { - if (item?.items?.length && !focusedItem) { + if (isInstanceOfComboBoxItemGroup(item) && !focusedItem) { focusedItem = item.items.find(groupItem => groupItem.focused); } @@ -1051,7 +1051,7 @@ class ComboBox extends UI5Element implements IFormInputElement { this._resetItemVisibility(); this.items.forEach(item => { - if (item.items?.length) { + if (isInstanceOfComboBoxItemGroup(item)) { filteredGroupItems = (Filters[this.filter] || Filters.StartsWithPerTerm)(str, item.items, "text"); filteredGroupItems.forEach(i => { i._isVisible = true; @@ -1075,7 +1075,7 @@ class ComboBox extends UI5Element implements IFormInputElement { return [...filteredItemGroups, ...filteredItems]; } - _getFirstMatchingItem(current: string): ComboBoxItem | undefined { + _getFirstMatchingItem(current: string): IComboBoxItem | void { const currentlyFocusedItem = this.items.find(item => item.focused === true); if (currentlyFocusedItem?.isGroupItem) { @@ -1083,14 +1083,14 @@ class ComboBox extends UI5Element implements IFormInputElement { return; } - const matchingItems: Array = (this._startsWithMatchingItems(current).map(item => (item?.items?.length && item.items[0]) || item) as Array); + const matchingItems: Array = (this._startsWithMatchingItems(current).map(item => (isInstanceOfComboBoxItemGroup(item) && item.items[0]) || item)); if (matchingItems.length) { return matchingItems[0]; } } - _applyAtomicValueAndSelection(item: ComboBoxItem, filterValue: string, highlightValue: boolean) { + _applyAtomicValueAndSelection(item: IComboBoxItem, filterValue: string, highlightValue: boolean) { const value = (item && item.text) || ""; this.inner.value = value; @@ -1112,12 +1112,12 @@ class ComboBox extends UI5Element implements IFormInputElement { }); this._filteredItems = this._filteredItems.map(item => { - if (!item.items) { + if (!isInstanceOfComboBoxItemGroup(item)) { item.selected = item === itemToBeSelected; return item; } - item.items.forEach(groupItem => { + item.items?.forEach(groupItem => { groupItem.selected = itemToBeSelected === groupItem; }); @@ -1213,7 +1213,7 @@ class ComboBox extends UI5Element implements IFormInputElement { } _makeAllVisible(item: IComboBoxItem) { - if (item?.items?.length) { + if (isInstanceOfComboBoxItemGroup(item)) { item.items.forEach(groupItem => { groupItem._isVisible = true; }); diff --git a/packages/main/src/ComboBoxItemGroup.ts b/packages/main/src/ComboBoxItemGroup.ts index 657b039c87c9..3b97e032a8c9 100644 --- a/packages/main/src/ComboBoxItemGroup.ts +++ b/packages/main/src/ComboBoxItemGroup.ts @@ -62,4 +62,9 @@ class ComboBoxItemGroup extends UI5Element implements IComboBoxItem { ComboBoxItemGroup.define(); +const isInstanceOfComboBoxItemGroup = (object: any): object is ComboBoxItemGroup => { + return "isGroupItem" in object; +}; + +export { isInstanceOfComboBoxItemGroup }; export default ComboBoxItemGroup; From fd8c9529d639e4069c78b41885fb4b2cdabd28c0 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Wed, 22 May 2024 17:07:23 +0300 Subject: [PATCH 15/20] feat(ui5-combobox): introduce nested grouping of items fix condition bug --- packages/main/src/ComboBox.ts | 4 ++-- packages/main/src/ComboBoxItemGroup.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/main/src/ComboBox.ts b/packages/main/src/ComboBox.ts index a569031a0218..92665c3dfe06 100644 --- a/packages/main/src/ComboBox.ts +++ b/packages/main/src/ComboBox.ts @@ -117,7 +117,7 @@ enum ValueStateIconMapping { } type ComboBoxSelectionChangeEventDetail = { - item: ComboBoxItem, + item: IComboBoxItem, }; /** @@ -462,7 +462,7 @@ class ComboBox extends UI5Element implements IFormInputElement { const hasNoVisibleItems = !this._filteredItems.length || !this._filteredItems.some(i => i._isVisible); // If there is no filtered items matching the value, show all items when the arrow is pressed - if (((hasNoVisibleItems && !isPhone) && this.value)) { + if (((hasNoVisibleItems && !isPhone()) && this.value)) { this.items.forEach(this._makeAllVisible.bind(this)); this._filteredItems = this.items; } diff --git a/packages/main/src/ComboBoxItemGroup.ts b/packages/main/src/ComboBoxItemGroup.ts index 3b97e032a8c9..01bc36e123b7 100644 --- a/packages/main/src/ComboBoxItemGroup.ts +++ b/packages/main/src/ComboBoxItemGroup.ts @@ -63,7 +63,7 @@ class ComboBoxItemGroup extends UI5Element implements IComboBoxItem { ComboBoxItemGroup.define(); const isInstanceOfComboBoxItemGroup = (object: any): object is ComboBoxItemGroup => { - return "isGroupItem" in object; + return object.isGroupItem === true; }; export { isInstanceOfComboBoxItemGroup }; From 9f18ba411ff4ee65d2546306073a09251a07f675 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Thu, 23 May 2024 09:36:14 +0300 Subject: [PATCH 16/20] feat(ui5-combobox): introduce nested grouping of items revert selectioon-change parameter type --- packages/main/src/ComboBox.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/main/src/ComboBox.ts b/packages/main/src/ComboBox.ts index 92665c3dfe06..cd27518152b1 100644 --- a/packages/main/src/ComboBox.ts +++ b/packages/main/src/ComboBox.ts @@ -377,7 +377,7 @@ class ComboBox extends UI5Element implements IFormInputElement { * @public */ @slot({ type: HTMLElement, "default": true, invalidateOnChildChange: true }) - items!: Array; + items!: Array; /** * Defines the value state message that will be displayed as pop up under the component. @@ -1195,7 +1195,7 @@ class ComboBox extends UI5Element implements IFormInputElement { } _clear() { - const selectedItem = this.items.find(item => item.selected) as (ComboBoxItem | undefined); + const selectedItem = this.items.find(item => item.selected); if (selectedItem?.text === this.value) { this.fireEvent("change"); From f576e7c05ce871f461be14898cafb69341a9537c Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Thu, 23 May 2024 10:54:56 +0300 Subject: [PATCH 17/20] feat(ui5-combobox): introduce nested grouping of items fix item types --- packages/main/src/ComboBox.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/main/src/ComboBox.ts b/packages/main/src/ComboBox.ts index cd27518152b1..13c0d0cb64a8 100644 --- a/packages/main/src/ComboBox.ts +++ b/packages/main/src/ComboBox.ts @@ -117,7 +117,7 @@ enum ValueStateIconMapping { } type ComboBoxSelectionChangeEventDetail = { - item: IComboBoxItem, + item: ComboBoxItem, }; /** @@ -377,7 +377,7 @@ class ComboBox extends UI5Element implements IFormInputElement { * @public */ @slot({ type: HTMLElement, "default": true, invalidateOnChildChange: true }) - items!: Array; + items!: Array; /** * Defines the value state message that will be displayed as pop up under the component. @@ -680,7 +680,7 @@ class ComboBox extends UI5Element implements IFormInputElement { if (value !== "" && (item && !item.selected && !item.isGroupItem)) { this.fireEvent("selection-change", { - item, + item: item as ComboBoxItem, }); } } @@ -837,7 +837,7 @@ class ComboBox extends UI5Element implements IFormInputElement { if ((item && !item.selected)) { this.fireEvent("selection-change", { - item, + item: item as ComboBoxItem, }); } From f190ec37498115251bb0d3d11cdf62d964c3f8af Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Fri, 24 May 2024 15:15:48 +0300 Subject: [PATCH 18/20] feat(ui5-combobox): introduce nested grouping of items fix code review bugs --- packages/main/src/ComboBox.ts | 29 ++++++++------------- packages/main/test/pages/ComboBox.html | 36 +++++++++++++------------- 2 files changed, 29 insertions(+), 36 deletions(-) diff --git a/packages/main/src/ComboBox.ts b/packages/main/src/ComboBox.ts index 13c0d0cb64a8..4a6fb4872b5d 100644 --- a/packages/main/src/ComboBox.ts +++ b/packages/main/src/ComboBox.ts @@ -719,18 +719,8 @@ class ComboBox extends UI5Element implements IFormInputElement { } _startsWithMatchingItems(str: string): Array { - const filteredItems: Array = []; - - this._filteredItems.forEach(item => { - if (isInstanceOfComboBoxItemGroup(item)) { - filteredItems.push(...item.items); - return; - } - - filteredItems.push(item); - }); - - return Filters.StartsWith(str, filteredItems, "text"); + const allItems:Array = this._getItems(); + return Filters.StartsWith(str, allItems, "text"); } _clearFocus() { @@ -819,7 +809,7 @@ class ComboBox extends UI5Element implements IFormInputElement { currentItem.focused = true; } else { this.focused = true; - this.value = isGroupItem ? "" : currentItem.text; + this.value = isGroupItem ? nextItem.text : currentItem.text; currentItem.focused = false; } @@ -889,9 +879,10 @@ class ComboBox extends UI5Element implements IFormInputElement { } _handlePageUp(e: KeyboardEvent, indexOfItem: number) { + const allItems = this._getItems(); const isProposedIndexValid = indexOfItem - SKIP_ITEMS_SIZE > -1; indexOfItem = isProposedIndexValid ? indexOfItem - SKIP_ITEMS_SIZE : 0; - const shouldMoveForward = this._filteredItems[indexOfItem].isGroupItem && !this.open; + const shouldMoveForward = allItems[indexOfItem].isGroupItem && !this.open; if (!isProposedIndexValid && this.hasValueStateText && this.open) { this._clearFocus(); @@ -940,6 +931,7 @@ class ComboBox extends UI5Element implements IFormInputElement { _keydown(e: KeyboardEvent) { const isNavKey = isDown(e) || isUp(e) || isPageUp(e) || isPageDown(e) || isHome(e) || isEnd(e); const picker = this.responsivePopover; + const allItems: Array = this._getItems(); this._autocomplete = !(isBackSpace(e) || isDelete(e)); this._isKeyNavigation = false; @@ -988,7 +980,7 @@ class ComboBox extends UI5Element implements IFormInputElement { this._resetFilter(); this._toggleRespPopover(); - const selectedItem = this._filteredItems.find(item => { + const selectedItem = allItems.find(item => { return item.selected; }); @@ -1076,14 +1068,15 @@ class ComboBox extends UI5Element implements IFormInputElement { } _getFirstMatchingItem(current: string): IComboBoxItem | void { - const currentlyFocusedItem = this.items.find(item => item.focused === true); + const allItems = this._getItems(); + const currentlyFocusedItem = allItems.find(item => item.focused === true); if (currentlyFocusedItem?.isGroupItem) { this.value = this.filterValue; return; } - const matchingItems: Array = (this._startsWithMatchingItems(current).map(item => (isInstanceOfComboBoxItemGroup(item) && item.items[0]) || item)); + const matchingItems: Array = (this._startsWithMatchingItems(current).filter(item => !isInstanceOfComboBoxItemGroup(item))); if (matchingItems.length) { return matchingItems[0]; @@ -1226,7 +1219,7 @@ class ComboBox extends UI5Element implements IFormInputElement { async _scrollToItem(indexOfItem: number, forward: boolean) { const picker = await this._getPicker(); const list = picker.querySelector(".ui5-combobox-items-list") as List; - const listItem = list?.items[indexOfItem]; + const listItem = list?.listItems[indexOfItem]; if (listItem) { const pickerRect = picker.getBoundingClientRect(); diff --git a/packages/main/test/pages/ComboBox.html b/packages/main/test/pages/ComboBox.html index a2a4dbe8e460..a85c65505dde 100644 --- a/packages/main/test/pages/ComboBox.html +++ b/packages/main/test/pages/ComboBox.html @@ -91,7 +91,7 @@ - + @@ -112,26 +112,26 @@ Items with grouping: - - - - - + + + + + - - - - - + + + + + - - - + + + - - - - + + + +
From a32848bc3049db7d34aa3e2e68951cfc1d9d6a8d Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Sat, 25 May 2024 06:15:38 +0300 Subject: [PATCH 19/20] feat(ui5-combobox): introduce nested grouping of items remove isGroupItem check from ComboBoxItem --- packages/main/src/ComboBox.ts | 8 ++++---- packages/main/src/ComboBoxItem.ts | 8 -------- packages/main/src/ComboBoxItemGroup.ts | 2 +- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/packages/main/src/ComboBox.ts b/packages/main/src/ComboBox.ts index 4a6fb4872b5d..3011f0175a2c 100644 --- a/packages/main/src/ComboBox.ts +++ b/packages/main/src/ComboBox.ts @@ -94,7 +94,7 @@ const SKIP_ITEMS_SIZE = 10; interface IComboBoxItem extends UI5Element { text: string, focused: boolean, - isGroupItem: boolean, + isGroupItem?: boolean, selected?: boolean, additionalText?: string, stableDomRef: string, @@ -882,7 +882,7 @@ class ComboBox extends UI5Element implements IFormInputElement { const allItems = this._getItems(); const isProposedIndexValid = indexOfItem - SKIP_ITEMS_SIZE > -1; indexOfItem = isProposedIndexValid ? indexOfItem - SKIP_ITEMS_SIZE : 0; - const shouldMoveForward = allItems[indexOfItem].isGroupItem && !this.open; + const shouldMoveForward = isInstanceOfComboBoxItemGroup(allItems[indexOfItem]) && !this.open; if (!isProposedIndexValid && this.hasValueStateText && this.open) { this._clearFocus(); @@ -901,13 +901,13 @@ class ComboBox extends UI5Element implements IFormInputElement { const isProposedIndexValid = indexOfItem + SKIP_ITEMS_SIZE < itemsLength; indexOfItem = isProposedIndexValid ? indexOfItem + SKIP_ITEMS_SIZE : itemsLength - 1; - const shouldMoveForward = allItems[indexOfItem].isGroupItem && !this.open; + const shouldMoveForward = isInstanceOfComboBoxItemGroup(allItems[indexOfItem]) && !this.open; this._handleItemNavigation(e, indexOfItem, shouldMoveForward); } _handleHome(e: KeyboardEvent) { - const shouldMoveForward = this._filteredItems[0].isGroupItem && !this.open; + const shouldMoveForward = isInstanceOfComboBoxItemGroup(this._filteredItems[0]) && !this.open; if (this.hasValueStateText && this.open) { this._clearFocus(); diff --git a/packages/main/src/ComboBoxItem.ts b/packages/main/src/ComboBoxItem.ts index 7684c79e52f1..3b923b1a0379 100644 --- a/packages/main/src/ComboBoxItem.ts +++ b/packages/main/src/ComboBoxItem.ts @@ -52,14 +52,6 @@ class ComboBoxItem extends UI5Element implements IComboBoxItem { @property({ type: Boolean }) selected!: boolean; - /** - * Used to avoid tag name checks - * @protected - */ - get isGroupItem(): boolean { - return false; - } - get stableDomRef() { return this.getAttribute("stable-dom-ref") || `${this._id}-stable-dom-ref`; } diff --git a/packages/main/src/ComboBoxItemGroup.ts b/packages/main/src/ComboBoxItemGroup.ts index 01bc36e123b7..3b97e032a8c9 100644 --- a/packages/main/src/ComboBoxItemGroup.ts +++ b/packages/main/src/ComboBoxItemGroup.ts @@ -63,7 +63,7 @@ class ComboBoxItemGroup extends UI5Element implements IComboBoxItem { ComboBoxItemGroup.define(); const isInstanceOfComboBoxItemGroup = (object: any): object is ComboBoxItemGroup => { - return object.isGroupItem === true; + return "isGroupItem" in object; }; export { isInstanceOfComboBoxItemGroup }; From 9b8ba3d0e0a496fc17fd8e6655a39ffda3e37c40 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Tue, 28 May 2024 06:59:06 +0300 Subject: [PATCH 20/20] feat(ui5-combobox): introduce nested grouping of items add more tests for navigation and selection behavior --- packages/main/test/specs/ComboBox.spec.js | 110 +++++++++++++++++++++- 1 file changed, 107 insertions(+), 3 deletions(-) diff --git a/packages/main/test/specs/ComboBox.spec.js b/packages/main/test/specs/ComboBox.spec.js index e09e21998723..2e7c1fddc82c 100644 --- a/packages/main/test/specs/ComboBox.spec.js +++ b/packages/main/test/specs/ComboBox.spec.js @@ -1040,7 +1040,7 @@ describe("Keyboard navigation", async () => { const input = await combo.shadow$("#ui5-combobox-input"); const arrow = await combo.shadow$(".inputIcon"); const popover = await combo.shadow$("ui5-responsive-popover"); - let listItem, prevListItem; + let prevListItem; await input.click(); await input.keys("ArrowDown"); @@ -1070,6 +1070,28 @@ describe("Keyboard navigation", async () => { assert.strictEqual(await prevListItem.getProperty("focused"), false, "The previously focused item is no longer focused"); }); + it ("Navigates back and forward through items in multiple groups", async () => { + await browser.url(`test/pages/ComboBox.html`); + + const combo = await browser.$("#value-state-grouping"); + const input = await combo.shadow$("#ui5-combobox-input"); + const arrow = await combo.shadow$(".inputIcon"); + + await input.click(); + + for (let i = 0; i < 5; i++) { + await input.keys("ArrowDown"); + } + + assert.equal(await combo.getProperty("value"), "Bahrain", "The value is updated with the first suggestion item of the second group"); + + for (let i = 0; i < 4; i++) { + await input.keys("ArrowUp"); + } + + assert.strictEqual(await combo.getProperty("value"), "Argentina", "The value is updated with the first suggestion item of the first group"); + }); + it ("Should focus the next/previous focusable element on TAB/SHIFT+TAB", async () => { await browser.url(`test/pages/ComboBox.html`); @@ -1143,6 +1165,36 @@ describe("Keyboard navigation", async () => { assert.strictEqual(await input.getProperty("value"), "Chile", "The +10 item should be selected on PAGEDOWN"); }); + it ("Should select previous item when the last suggestion is selected and the picker is closed", async () => { + await browser.url(`test/pages/ComboBox.html`); + + const combo = await browser.$("#combo-grouping"); + + await combo.click(); + await combo.keys("PageDown"); + await combo.keys("PageDown"); + await combo.keys("ArrowUp"); + + assert.strictEqual(await combo.getProperty("value"), "Albania", "The value is updated correctly"); + }); + + it ("Should keep the value from the selected items after closing the picker", async () => { + await browser.url(`test/pages/ComboBox.html`); + + const combo = await browser.$("#combo-grouping"); + const arrow = await combo.shadow$(".inputIcon"); + + await arrow.click(); + await combo.keys("ArrowDown"); + await combo.keys("ArrowDown"); + + assert.strictEqual(await combo.getProperty("value"), "Algeria", "The value is updated correctly"); + + await combo.keys("F4"); + + assert.strictEqual(await combo.getProperty("value"), "Algeria", "The value remains in the input field after picker is closed"); + }); + it ("Should select first matching item", async () => { await browser.url(`test/pages/ComboBox.html`); @@ -1216,7 +1268,7 @@ describe("Keyboard navigation", async () => { !isElementBelowViewport && !isElementLeftOfViewport && !isElementRightOfViewport - ); + ); done(isListItemInVisibleArea); }); @@ -1245,12 +1297,64 @@ describe("Keyboard navigation", async () => { !isElementBelowViewport && !isElementLeftOfViewport && !isElementRightOfViewport - ); + ); done(isListItemInVisibleArea); }); assert.ok(isInVisibleArea, "Item should be displayed in the viewport"); + + await input.keys("Home"); + + let isFirstItemInVisibleArea = await browser.executeAsync(async done => { + const combobox = document.getElementById("combo-grouping"); + const picker = await combobox._getPicker(); + const firstListItem = picker.querySelector(".ui5-combobox-items-list ui5-li-group:first-child ui5-li:first-child"); + const scrollableRect = picker.shadowRoot.querySelector(".ui5-popup-content").getBoundingClientRect(); + const firstItemBoundingClientRect = firstListItem.getBoundingClientRect(); + + // Check if the element is within the visible area + const isFirstItemAboveViewport = firstItemBoundingClientRect.bottom < scrollableRect.top; + const isFirstItemBelowViewport = firstItemBoundingClientRect.top > scrollableRect.bottom; + const isFirstItemLeftOfViewport = firstItemBoundingClientRect.right < scrollableRect.left; + const isFirstItemRightOfViewport = firstItemBoundingClientRect.left > scrollableRect.right; + + const isFirstItemInVisibleArea = ( + !isFirstItemAboveViewport && + !isFirstItemBelowViewport && + !isFirstItemLeftOfViewport && + !isFirstItemRightOfViewport + ); + + done(isFirstItemInVisibleArea); + }); + + assert.ok(isFirstItemInVisibleArea, "The first item should be displayed in the viewport"); + + let isLastItemInVisibleArea = await browser.executeAsync(async done => { + const combobox = document.getElementById("combo-grouping"); + const picker = await combobox._getPicker(); + const lastListItem = picker.querySelector(".ui5-combobox-items-list ui5-li-group:first-child ui5-li:first-child"); + const scrollableRect = picker.shadowRoot.querySelector(".ui5-popup-content").getBoundingClientRect(); + const lastItemBoundingClientRect = lastListItem.getBoundingClientRect(); + + // Check if the element is within the visible area + const isLastItemAboveViewport = lastItemBoundingClientRect.bottom < scrollableRect.top; + const isLastItemBelowViewport = lastItemBoundingClientRect.top > scrollableRect.bottom; + const isLastItemLeftOfViewport = lastItemBoundingClientRect.right < scrollableRect.left; + const isLastItemRightOfViewport = lastItemBoundingClientRect.left > scrollableRect.right; + + const isLastItemInVisibleArea = ( + !isLastItemAboveViewport && + !isLastItemBelowViewport && + !isLastItemLeftOfViewport && + !isLastItemRightOfViewport + ); + + done(isLastItemInVisibleArea); + }); + + assert.ok(isLastItemInVisibleArea, "The first item should be displayed in the viewport"); }); it ("Should get the physical DOM reference for the cb item", async () => {