From 5e1e2a757e8a179f0632804ba8bd3b191a4c0aa3 Mon Sep 17 00:00:00 2001 From: Radoslav Karaivanov Date: Mon, 6 Jan 2025 12:31:35 +0200 Subject: [PATCH 1/5] refactor(combo): Use key-bindings controller This commit migrates the combo component to use the general key-bindings controller from the library. Aside from unifying the implementation across components this also improves several behaviours of the combo: * Keyboard events inside the combo are no longer "swallowed". * Tab navigation is more natural, i.e. there are no longer 2 tab stops per combo component. Updated the combo test suite to accomodate for the changes. --- src/components/combo/combo.spec.ts | 262 +++++------- src/components/combo/combo.ts | 86 ++-- .../combo/controllers/navigation.ts | 395 ++++++++---------- .../common/controllers/key-bindings.ts | 13 +- stories/avatar.stories.ts | 5 - 5 files changed, 342 insertions(+), 419 deletions(-) diff --git a/src/components/combo/combo.spec.ts b/src/components/combo/combo.spec.ts index 536ef030c..a3e4eb809 100644 --- a/src/components/combo/combo.spec.ts +++ b/src/components/combo/combo.spec.ts @@ -1,12 +1,23 @@ import { elementUpdated, expect, fixture, html } from '@open-wc/testing'; import { spy } from 'sinon'; +import { + altKey, + arrowDown, + arrowUp, + endKey, + enterKey, + homeKey, + spaceBar, +} from '../common/controllers/key-bindings.js'; import { defineComponents } from '../common/definitions/defineComponents.js'; import { first } from '../common/util.js'; import { type ValidationContainerTestsParams, createFormAssociatedTestBed, runValidationContainerTests, + simulateClick, + simulateKeyboard, } from '../common/utils.spec.js'; import type IgcInputComponent from '../input/input.js'; import type IgcComboHeaderComponent from './combo-header.js'; @@ -15,12 +26,12 @@ import type IgcComboListComponent from './combo-list.js'; import IgcComboComponent from './combo.js'; describe('Combo', () => { - interface City { + type City = { id: string; name: string; country: string; zip: string; - } + }; let input: IgcInputComponent; let searchInput: IgcInputComponent; @@ -262,8 +273,8 @@ describe('Combo', () => { it('should open the menu upon clicking on the input', async () => { const eventSpy = spy(combo, 'emitEvent'); - input.click(); + simulateClick(input); await elementUpdated(combo); expect(eventSpy).calledWith('igcOpening'); @@ -275,7 +286,7 @@ describe('Combo', () => { const eventSpy = spy(combo, 'emitEvent'); await combo.show(); - input.click(); + simulateClick(input); await elementUpdated(combo); expect(eventSpy).calledWith('igcClosing'); @@ -289,7 +300,8 @@ describe('Combo', () => { event.preventDefault(); }); const eventSpy = spy(combo, 'emitEvent'); - input.click(); + + simulateClick(input); await elementUpdated(combo); expect(eventSpy).calledOnceWithExactly('igcOpening', { @@ -304,7 +316,8 @@ describe('Combo', () => { event.preventDefault(); }); const eventSpy = spy(combo, 'emitEvent'); - input.click(); + + simulateClick(input); await elementUpdated(combo); expect(eventSpy).calledOnceWithExactly('igcClosing', { @@ -347,7 +360,7 @@ describe('Combo', () => { it('should configure the filtering options by attribute', async () => { combo.setAttribute( 'filtering-options', - '{"filterKey": "zip", "caseSensitive": true}' + JSON.stringify({ filterKey: 'zip', caseSensitive: true }) ); await elementUpdated(combo); @@ -356,7 +369,10 @@ describe('Combo', () => { }); it('should correctly merge partially provided filtering options', async () => { - combo.setAttribute('filtering-options', '{"caseSensitive": true }'); + combo.setAttribute( + 'filtering-options', + JSON.stringify({ caseSensitive: true }) + ); await elementUpdated(combo); expect(combo.filteringOptions.filterKey).not.to.be.undefined; @@ -364,14 +380,12 @@ describe('Combo', () => { }); it('should select/deselect an item by value key', async () => { - const item = cities[0]; + const item = first(cities); combo.open = true; combo.select([item[combo.valueKey!]]); await elementUpdated(combo); - await new Promise((resolve) => { - setTimeout(resolve, 200); - }); + await list.layoutComplete; const selected = items(combo).find((item) => item.selected); expect(selected?.innerText).to.equal(item[combo.displayKey!]); @@ -393,40 +407,30 @@ describe('Combo', () => { combo.open = true; await elementUpdated(combo); - const item = cities[0]; + const item = first(cities); combo.select([item]); await elementUpdated(combo); - await new Promise((resolve) => { - setTimeout(resolve, 100); - }); + await list.layoutComplete; - const selected = items(combo).find((item) => item.selected); - expect(selected?.innerText).to.equal(item[combo.displayKey!]); + const selected = items(combo).find((item) => item.selected)!; + expect(selected.innerText).to.equal(item[combo.displayKey!]); combo.deselect([item]); - await elementUpdated(combo); - - items(combo).forEach((item) => { - expect(item.selected).to.be.false; - }); + expect(items(combo).every((item) => !item.selected)).to.be.true; }); it('should select/deselect all items', async () => { combo.select(); await elementUpdated(combo); - items(combo).forEach((item) => { - expect(item.selected).to.be.true; - }); + expect(items(combo).every((item) => item.selected)).to.be.true; combo.deselect(); await elementUpdated(combo); - items(combo).forEach((item) => { - expect(item.selected).to.be.false; - }); + expect(items(combo).every((item) => !item.selected)).to.be.true; }); it('should clear the selection by pressing on the clear button', async () => { @@ -439,10 +443,7 @@ describe('Combo', () => { (button! as HTMLSpanElement).click(); await elementUpdated(combo); - - items(combo).forEach((item) => { - expect(item.selected).to.be.false; - }); + expect(items(combo).every((item) => !item.selected)).to.be.true; }); it('should toggle case sensitivity by pressing on the case sensitive icon', async () => { @@ -462,7 +463,7 @@ describe('Combo', () => { }); it('should not fire igcChange event on selection/deselection via methods calls', async () => { - const item = cities[0]; + const item = first(cities); combo.select([item[combo.valueKey!]]); combo.addEventListener('igcChange', (event: CustomEvent) => @@ -479,7 +480,7 @@ describe('Combo', () => { cancelable: true, detail: { newValue: ['BG01'], - items: [cities[0]], + items: [first(cities)], type: 'selection', }, }; @@ -488,8 +489,8 @@ describe('Combo', () => { await elementUpdated(combo); await list.layoutComplete; - items(combo)[0].click(); - expect(combo.value).to.deep.equal(['BG01']); + first(items(combo)).click(); + expect(combo.value).to.eql(['BG01']); expect(eventSpy).calledWithExactly('igcChange', args); }); @@ -499,7 +500,7 @@ describe('Combo', () => { cancelable: true, detail: { newValue: ['BG02', 'BG03'], - items: [cities[0]], + items: [first(cities)], type: 'deselection', }, }; @@ -509,11 +510,11 @@ describe('Combo', () => { await elementUpdated(combo); await list.layoutComplete; - expect(combo.value).to.deep.equal(['BG01', 'BG02', 'BG03']); + expect(combo.value).to.eql(['BG01', 'BG02', 'BG03']); - items(combo)[0].click(); + first(items(combo)).click(); await elementUpdated(combo); - expect(combo.value).to.deep.equal(['BG02', 'BG03']); + expect(combo.value).to.eql(['BG02', 'BG03']); expect(eventSpy).calledWithExactly('igcChange', args); }); @@ -528,11 +529,11 @@ describe('Combo', () => { await elementUpdated(combo); await list.layoutComplete; - items(combo)[0].click(); + first(items(combo)).click(); await elementUpdated(combo); expect(eventSpy).calledWith('igcChange'); - expect(combo.value.length).to.equal(0); + expect(combo.value).to.be.empty; }); it('should be able to cancel the deselection event', async () => { @@ -546,11 +547,11 @@ describe('Combo', () => { await elementUpdated(combo); await list.layoutComplete; - items(combo)[0].click(); + first(items(combo)).click(); await elementUpdated(combo); expect(eventSpy).calledWith('igcChange'); - expect(combo.value.length).to.equal(2); + expect(combo.value).lengthOf(2); }); it('should not stringify values in event', async () => { @@ -575,7 +576,7 @@ describe('Combo', () => { 'igcChange', ({ detail }) => { expect(detail.newValue).to.eql([data[0].id]); - expect(detail.items).to.deep.equal([data[0]]); + expect(detail.items).to.eql([data[0]]); }, { once: true } ); @@ -583,7 +584,7 @@ describe('Combo', () => { await combo.show(); await list.layoutComplete; - items(combo)[0].click(); + first(items(combo)).click(); await elementUpdated(combo); expect(combo.value).to.eql([0]); @@ -619,11 +620,12 @@ describe('Combo', () => { it('opens the list of options when Down or Alt+Down keys are pressed', async () => { combo.open = false; - pressKey(input, 'ArrowDown', 1, { altKey: false }); + + simulateKeyboard(input, arrowDown); expect(combo.open).to.be.true; combo.open = false; - pressKey(input, 'ArrowDown', 1, { altKey: true }); + simulateKeyboard(input, [altKey, arrowDown]); expect(combo.open).to.be.true; }); @@ -631,7 +633,7 @@ describe('Combo', () => { await combo.show(); expect(combo.open).to.be.true; - pressKey(searchInput, 'ArrowUp', 1, { altKey: false }); + simulateKeyboard(searchInput, arrowUp); expect(combo.open).to.be.false; }); @@ -640,7 +642,7 @@ describe('Combo', () => { await list.layoutComplete; expect(items(combo)[0].active).to.be.false; - pressKey(searchInput, 'ArrowDown', 1, { altKey: false }); + simulateKeyboard(searchInput, arrowDown); await elementUpdated(combo); @@ -655,12 +657,12 @@ describe('Combo', () => { await list.layoutComplete; expect(items(combo)[0].active).to.be.false; - pressKey(list, 'ArrowDown', 2, { altKey: false }); + simulateKeyboard(list, arrowDown, 2); await elementUpdated(combo); expect(items(combo)[1].active).to.be.true; - pressKey(options, 'ArrowUp', 1, { altKey: false }); + simulateKeyboard(options, arrowUp); await elementUpdated(combo); @@ -674,8 +676,7 @@ describe('Combo', () => { await combo.show(); await list.layoutComplete; - pressKey(options, 'Home', 1, { altKey: false }); - + simulateKeyboard(options, homeKey); await elementUpdated(combo); expect(items(combo)[0].active).to.be.true; @@ -688,8 +689,7 @@ describe('Combo', () => { await combo.show(); await list.layoutComplete; - pressKey(options, 'End', 1, { altKey: false }); - + simulateKeyboard(options, endKey); await elementUpdated(combo); const itms = items(combo); @@ -703,9 +703,8 @@ describe('Combo', () => { await combo.show(); await list.layoutComplete; - pressKey(options, 'ArrowDown', 2, { altKey: false }); - pressKey(options, ' ', 1, { altKey: false }); - + simulateKeyboard(options, arrowDown, 2); + simulateKeyboard(options, spaceBar); await elementUpdated(combo); const itms = items(combo); @@ -721,9 +720,8 @@ describe('Combo', () => { await combo.show(); await list.layoutComplete; - pressKey(options, 'ArrowDown', 1, { altKey: false }); - pressKey(options, 'Enter', 1, { altKey: false }); - + simulateKeyboard(options, arrowDown); + simulateKeyboard(options, enterKey); await elementUpdated(combo); expect(combo.value).to.eql(['BG01']); @@ -741,9 +739,8 @@ describe('Combo', () => { await combo.show(); await list.layoutComplete; - pressKey(options, 'ArrowDown', 1, { altKey: false }); - pressKey(options, 'Enter', 1, { altKey: false }); - + simulateKeyboard(options, arrowDown); + simulateKeyboard(options, enterKey); await elementUpdated(combo); expect(combo.value).to.eql(['BG01']); @@ -787,28 +784,28 @@ describe('Combo', () => { await list.layoutComplete; await filterCombo('Sao'); - expect(items(combo).length).to.equal(0); + expect(items(combo)).to.be.empty; await filterCombo('São'); - expect(items(combo).length).to.equal(1); + expect(items(combo)).lengthOf(1); }); it('should use the main input for filtering in single selection mode', async () => { - const filter = combo.shadowRoot!.querySelector('[part="filter-input"]'); + const filter = combo.shadowRoot!.querySelector('[part="filter-input"]')!; combo.singleSelect = true; await elementUpdated(combo); await combo.show(); await list.layoutComplete; - expect(filter!.getAttribute('hidden')).to.exist; + expect(filter.getAttribute('hidden')).to.exist; expect(input.getAttribute('readonly')).to.not.exist; - expect(items(combo).length).to.equal(cities.length); + expect(items(combo)).lengthOf(cities.length); await filterCombo('sof'); - expect(items(combo).length).to.equal(1); - expect(items(combo)[0].innerText).to.equal('Sofia'); + expect(items(combo)).lengthOf(1); + expect(first(items(combo)).innerText).to.equal('Sofia'); }); it('should select the first matched item upon pressing enter after search', async () => { @@ -820,12 +817,12 @@ describe('Combo', () => { await filterCombo('sof'); - expect(items(combo)[0].active).to.be.true; - - pressKey(input, 'Enter'); + expect(first(items(combo)).active).to.be.true; + simulateKeyboard(input, enterKey); await elementUpdated(combo); - expect(combo.value[0]).to.equal('BG01'); + + expect(first(combo.value)).to.equal('BG01'); }); it('should select only one item at a time in single selection mode', async () => { @@ -836,23 +833,23 @@ describe('Combo', () => { await list.layoutComplete; input.dispatchEvent(new CustomEvent('igcInput', { detail: 'v' })); - pressKey(input, 'ArrowDown'); + simulateKeyboard(input, arrowDown); await elementUpdated(combo); await list.layoutComplete; - expect(items(combo)[0].active).to.be.true; - expect(items(combo)[0].selected).to.be.false; + expect(first(items(combo)).active).to.be.true; + expect(first(items(combo)).selected).to.be.false; - pressKey(options, ' '); + simulateKeyboard(options, spaceBar); await elementUpdated(combo); await list.layoutComplete; expect(items(combo)[1].selected).to.be.true; - pressKey(options, 'ArrowDown', 2); - pressKey(options, ' '); + simulateKeyboard(options, arrowDown, 2); + simulateKeyboard(options, spaceBar); await elementUpdated(combo); await list.layoutComplete; @@ -873,11 +870,11 @@ describe('Combo', () => { input.dispatchEvent(new CustomEvent('igcInput', { detail: 'sof' })); await elementUpdated(combo); - pressKey(input, 'Enter'); + simulateKeyboard(input, enterKey); await elementUpdated(combo); expect(input.value).to.equal('Sofia'); - expect(combo.value).to.deep.equal(['BG01']); + expect(combo.value).to.eql(['BG01']); }); it('should clear selection upon changing the search term via input', async () => { @@ -888,26 +885,23 @@ describe('Combo', () => { await list.layoutComplete; input.dispatchEvent(new CustomEvent('igcInput', { detail: 'v' })); - pressKey(input, 'ArrowDown'); + simulateKeyboard(input, arrowDown); await elementUpdated(combo); await list.layoutComplete; - pressKey(options, ' '); + simulateKeyboard(options, spaceBar); await elementUpdated(combo); await list.layoutComplete; expect(items(combo)[1].selected).to.be.true; - expect(combo.value).to.deep.equal(['BG02']); + expect(combo.value).to.eql(['BG02']); await filterCombo('sof'); - items(combo).forEach((i) => { - expect(i.selected).to.be.false; - }); - - expect(combo.value).to.deep.equal([]); + expect(items(combo).every((item) => !item.selected)).to.be.true; + expect(combo.value).to.be.empty; }); it('Selection API should select nothing in single selection mode if nothing is passed', async () => { @@ -920,11 +914,8 @@ describe('Combo', () => { combo.select(); await elementUpdated(combo); - items(combo).forEach((i) => { - expect(i.selected).to.be.false; - }); - - expect(combo.value.length).to.equal(0); + expect(items(combo).every((item) => !item.selected)).to.be.true; + expect(combo.value).to.be.empty; }); it('Selection API should deselect everything in single selection mode if nothing is passed', async () => { @@ -941,7 +932,7 @@ describe('Combo', () => { combo.deselect(); await elementUpdated(combo); - expect(combo.value).to.eql([]); + expect(combo.value).to.be.empty; }); it('Selection API should not deselect current value in single selection mode with wrong valueKey passed', async () => { @@ -970,13 +961,13 @@ describe('Combo', () => { await elementUpdated(combo); - const match = cities.find((i) => i.id === selection); - expect(combo.value[0]).to.equal(selection); + const match = cities.find((i) => i.id === selection)!; + expect(first(combo.value)).to.equal(selection); const selected = items(combo).filter((i) => i.selected); - expect(selected.length).to.equal(1); - expect(selected[0].innerText).to.equal(match?.name); + expect(selected).lengthOf(1); + expect(first(selected).innerText).to.equal(match.name); }); it('should deselect a single item using valueKey as argument with the Selection API', async () => { @@ -990,12 +981,12 @@ describe('Combo', () => { await elementUpdated(combo); - expect(combo.value[0]).to.equal(selection); + expect(first(combo.value)).to.equal(selection); combo.deselect(selection); await elementUpdated(combo); - expect(combo.value.length).to.equal(0); + expect(combo.value).to.be.empty; items(combo).forEach((i) => { expect(i.selected).to.be.false; @@ -1008,17 +999,17 @@ describe('Combo', () => { await combo.show(); await list.layoutComplete; - const item = cities[0]; + const item = first(cities); combo.select(item); await elementUpdated(combo); - expect(combo.value[0]).to.equal(item); + expect(first(combo.value)).to.equal(item); const selected = items(combo).filter((i) => i.selected); - expect(selected.length).to.equal(1); - expect(selected[0].innerText).to.equal(item?.name); + expect(selected).lengthOf(1); + expect(first(selected).innerText).to.equal(item.name); }); it('should deselect the item passed as argument with the Selection API', async () => { @@ -1028,21 +1019,18 @@ describe('Combo', () => { await combo.show(); await list.layoutComplete; - const item = cities[0]; + const item = first(cities); combo.select(item); await elementUpdated(combo); - expect(combo.value[0]).to.equal(item); + expect(first(combo.value)).to.equal(item); combo.deselect(item); await elementUpdated(combo); - expect(combo.value.length).to.equal(0); - - items(combo).forEach((i) => { - expect(i.selected).to.be.false; - }); + expect(combo.value).to.be.empty; + expect(items(combo).every((item) => !item.selected)).to.be.true; }); it('should select item(s) even if the list of items has been filtered', async () => { @@ -1056,8 +1044,8 @@ describe('Combo', () => { await list.layoutComplete; // Verify we can only see one item in the list - expect(items(combo).length).to.equal(1); - expect(items(combo)[0].innerText).to.equal('Sofia'); + expect(items(combo)).lengthOf(1); + expect(first(items(combo)).innerText).to.equal('Sofia'); // Select an item not visible in the list using the API const selection = 'US01'; @@ -1065,7 +1053,7 @@ describe('Combo', () => { await elementUpdated(combo); // The combo value should've updated - expect(combo.value[0]).to.equal(selection); + expect(first(combo.value)).to.equal(selection); // Let's verify the list of items has been updated searchInput.dispatchEvent(new CustomEvent('igcInput', { detail: '' })); @@ -1077,10 +1065,10 @@ describe('Combo', () => { const selected = items(combo).filter((item) => item.selected); // We should only see one item as selected - expect(selected.length).to.equal(1); + expect(selected).lengthOf(1); // It should match the one selected via the API - expect(selected[0].innerText).to.equal('New York'); + expect(first(selected).innerText).to.equal('New York'); }); it('should deselect item(s) even if the list of items has been filtered', async () => { @@ -1095,11 +1083,11 @@ describe('Combo', () => { let selected = items(combo).filter((item) => item.selected); // We should only see one item as selected - expect(selected.length).to.equal(1); + expect(selected).lengthOf(1); // It should match the one selected via the API - expect(selected[0].innerText).to.equal('New York'); - expect(combo.value[0]).to.equal(selection); + expect(first(selected).innerText).to.equal('New York'); + expect(first(combo.value)).to.equal(selection); // Filter the list of items searchInput.dispatchEvent(new CustomEvent('igcInput', { detail: 'sof' })); @@ -1108,15 +1096,15 @@ describe('Combo', () => { await list.layoutComplete; // Verify we can only see one item in the list - expect(items(combo).length).to.equal(1); - expect(items(combo)[0].innerText).to.equal('Sofia'); + expect(items(combo)).lengthOf(1); + expect(first(items(combo)).innerText).to.equal('Sofia'); // Deselect the previously selected item while the list is filtered combo.deselect(selection); await elementUpdated(combo); // The value should be updated - expect(combo.value.length).to.equal(0); + expect(combo.value).to.be.empty; // Verify the list of items has been updated searchInput.dispatchEvent(new CustomEvent('igcInput', { detail: '' })); @@ -1128,7 +1116,7 @@ describe('Combo', () => { selected = items(combo).filter((item) => item.selected); // No items should be selected - expect(selected.length).to.equal(0); + expect(selected).to.be.empty; }); it('should display primitive values correctly', async () => { @@ -1474,21 +1462,3 @@ describe('Combo', () => { }); }); }); - -const pressKey = ( - target: HTMLElement, - key: string, - times = 1, - options?: object -) => { - for (let i = 0; i < times; i++) { - target.dispatchEvent( - new KeyboardEvent('keydown', { - key: key, - bubbles: true, - composed: true, - ...options, - }) - ); - } -}; diff --git a/src/components/combo/combo.ts b/src/components/combo/combo.ts index a50cf192d..778f7bdec 100644 --- a/src/components/combo/combo.ts +++ b/src/components/combo/combo.ts @@ -1,12 +1,8 @@ import { LitElement, type TemplateResult, html, nothing } from 'lit'; -import { - property, - query, - queryAssignedElements, - state, -} from 'lit/decorators.js'; +import { property, queryAssignedElements, state } from 'lit/decorators.js'; import { ifDefined } from 'lit/directives/if-defined.js'; import { live } from 'lit/directives/live.js'; +import { createRef, ref } from 'lit/directives/ref.js'; import { themes } from '../../theming/theming-decorator.js'; import { addRootClickHandler } from '../common/controllers/root-click.js'; @@ -36,7 +32,7 @@ import IgcComboHeaderComponent from './combo-header.js'; import IgcComboItemComponent from './combo-item.js'; import IgcComboListComponent from './combo-list.js'; import { DataController } from './controllers/data.js'; -import { NavigationController } from './controllers/navigation.js'; +import { ComboNavigationController } from './controllers/navigation.js'; import { SelectionController } from './controllers/selection.js'; import { styles } from './themes/combo.base.css.js'; import { styles as shared } from './themes/shared/combo.common.css.js'; @@ -68,6 +64,7 @@ import { comboValidators } from './validators.js'; * @slot suffix - Renders content after the input of the combo. * @slot header - Renders a container before the list of options of the combo. * @slot footer - Renders a container after the list of options of the combo. + * @slot empty - Renders content when the combo dropdown list has no items/data. * @slot helper-text - Renders content below the input of the combo. * @slot toggle-icon - Renders content inside the suffix container of the combo. * @slot clear-icon - Renders content inside the suffix container of the combo. @@ -135,6 +132,15 @@ export default class IgcComboComponent< return comboValidators; } + /** The primary input of the combo component. */ + private _inputRef = createRef(); + + /** The search input of the combo component. */ + private _searchRef = createRef(); + + /** The combo virtualized dropdown list. */ + private _listRef = createRef(); + protected override _formValue: FormValue[]>; private _data: T[] = []; @@ -160,7 +166,11 @@ export default class IgcComboComponent< protected _state = new DataController(this); protected _selection = new SelectionController(this, this._state); - protected _navigation = new NavigationController(this, this._state); + protected _navigation = new ComboNavigationController(this, this._state, { + input: this._inputRef, + search: this._searchRef, + list: this._listRef, + }); @queryAssignedElements({ slot: 'suffix' }) protected inputSuffix!: Array; @@ -168,15 +178,6 @@ export default class IgcComboComponent< @queryAssignedElements({ slot: 'prefix' }) protected inputPrefix!: Array; - @query('[part="search-input"]') - protected _searchInput!: IgcInputComponent; - - @query('#target', true) - private _input!: IgcInputComponent; - - @query(IgcComboListComponent.tagName, true) - private _list!: IgcComboListComponent; - /** The data source used to generate the list of options. */ /* treatAsRef */ @property({ attribute: false }) @@ -458,11 +459,6 @@ export default class IgcComboComponent< }); this.addEventListener('blur', this._handleBlur); - - this.addEventListener( - 'keydown', - this._navigation.navigateHost.bind(this._navigation) - ); } protected override async firstUpdated() { @@ -524,20 +520,20 @@ export default class IgcComboComponent< if (!initial) { this._validate(); - this._list.requestUpdate(); + this._listRef.value!.requestUpdate(); } } /* alternateName: focusComponent */ /** Sets focus on the component. */ public override focus(options?: FocusOptions) { - this._input.focus(options); + this._inputRef.value!.focus(options); } /* alternateName: blurComponent */ /** Removes focus from the component. */ public override blur() { - this._input.blur(); + this._inputRef.value!.blur(); } /** @@ -612,7 +608,7 @@ export default class IgcComboComponent< this._navigation.active = detail ? matchIndex : -1; // update the list after changing the active item - this._list.requestUpdate(); + this._listRef.value!.requestUpdate(); // clear the selection upon typing this.clearSingleSelection(); @@ -651,11 +647,11 @@ export default class IgcComboComponent< } if (!this.singleSelect) { - this._list.focus(); + this._listRef.value!.focus(); } if (!this.autofocusList) { - this._searchInput.focus(); + this._searchRef.value!.focus(); } return true; @@ -743,17 +739,6 @@ export default class IgcComboComponent< `; }; - protected listKeydownHandler(event: KeyboardEvent) { - const target = findElementFromEventPath( - IgcComboListComponent.tagName, - event - ); - - if (target) { - this._navigation.navigateList(event, target); - } - } - protected itemClickHandler(event: PointerEvent) { const target = findElementFromEventPath( IgcComboItemComponent.tagName, @@ -767,10 +752,10 @@ export default class IgcComboComponent< this.toggleSelect(target.index); if (this.singleSelect) { - this._input.focus(); + this._inputRef.value!.focus(); this._hide(); } else { - this._searchInput.focus(); + this._searchRef.value!.focus(); } } @@ -815,14 +800,6 @@ export default class IgcComboComponent< this._navigation.active = -1; } - protected handleMainInputKeydown(e: KeyboardEvent) { - this._navigation.navigateMainInput(e, this._list); - } - - protected handleSearchInputKeydown(e: KeyboardEvent) { - this._navigation.navigateSearchInput(e, this._list); - } - protected toggleCaseSensitivity() { this.filteringOptions = { caseSensitive: !this.filteringOptions.caseSensitive, @@ -875,6 +852,7 @@ export default class IgcComboComponent< private renderMainInput() { return html` +
${this.renderSearchInput()}
= -1; - -enum DIRECTION { - Up = -1, - Down = 1, -} - -export class NavigationController - implements ReactiveController -{ - protected hostHandlers = new Map( - Object.entries({ - Escape: this.escape, - }) - ); - - protected mainInputHandlers = new Map( - Object.entries({ - Escape: this.escape, - ArrowUp: this.hide, - ArrowDown: this.mainInputArrowDown, - Tab: this.tab, - Enter: this.enter, - }) - ); - - protected searchInputHandlers = new Map( - Object.entries({ - Escape: this.escape, - ArrowUp: this.escape, - ArrowDown: this.inputArrowDown, - Tab: this.inputArrowDown, - }) - ); - - protected listHandlers = new Map( - Object.entries({ - ArrowDown: this.arrowDown, - ArrowUp: this.arrowUp, - ' ': this.space, - Enter: this.enter, - Escape: this.escape, - Tab: this.tab, - Home: this.home, - End: this.end, - }) - ); - - protected _active = START_INDEX; - - public get input() { - // @ts-expect-error protected access - return this.host.singleSelect ? this.host._input : this.host._searchInput; - } - - public get dataState() { - return this.state.dataState; - } - - public show() { - // @ts-expect-error protected access - this.host._show(true); - } +type ComboNavigationConfig = { + /** The primary input of the combo component. */ + input: Ref; + /** The search input of the combo component. */ + search: Ref; + /** The combo virtualized dropdown list. */ + list: Ref; +}; - public hide() { - // @ts-expect-error protected access - this.host._hide(true); - } +export class ComboNavigationController { + private _active = -1; + private _config: ComboNavigationConfig; - public toggleSelect(index: number) { - // @ts-expect-error protected access - this.host.toggleSelect(index); + public get active(): number { + return this._active; } - public select(index: number) { - // @ts-expect-error protected access - this.host.selectByIndex(index); + public set active(value: number) { + this._active = value; + this.combo.requestUpdate(); } - protected get currentItem() { - const item = this.active; - return item === START_INDEX ? START_INDEX : item; + public get input(): IgcInputComponent { + return this._config.input.value!; } - protected get firstItem() { - return this.dataState.findIndex((i: ComboRecord) => i.header !== true); + public get searchInput(): IgcInputComponent { + return this._config.search.value!; } - protected get lastItem() { - return this.dataState.length - 1; + public get list(): IgcComboListComponent { + return this._config.list.value!; } - protected scrollToActive( - container: IgcComboListComponent, - behavior: ScrollBehavior = 'auto' - ) { - container.element(this.active)?.scrollIntoView({ - block: 'center', - behavior, - }); - - container.requestUpdate(); + protected get firstItem(): number { + return this.state.dataState.findIndex((rec) => !rec.header); } - public get active() { - return this._active; + protected get lastItem(): number { + return this.state.dataState.length - 1; } - public set active(node: number) { - this._active = node; - this.host.requestUpdate(); + protected async hide(): Promise { + // @ts-expect-error: protected access + return await this.combo._hide(true); } - constructor( - protected host: ComboHost, - protected state: DataController - ) { - this.host.addController(this); + protected async show(): Promise { + // @ts-expect-error: protected access + return await this.combo._show(true); } - protected home(container: IgcComboListComponent) { - this.active = this.firstItem; - this.scrollToActive(container, 'smooth'); + public toggleSelect(index: number): void { + // @ts-expect-error protected access + this.combo.toggleSelect(index); } - protected end(container: IgcComboListComponent) { - this.active = this.lastItem; - this.scrollToActive(container, 'smooth'); + protected select(index: number): void { + // @ts-expect-error: protected access + this.combo.selectByIndex(index); } - protected space() { - if (this.active === START_INDEX) { + private onSpace = (): void => { + if (this._active === -1) { return; } - const item = this.dataState[this.active]; - + const item = this.state.dataState[this._active]; if (!item.header) { - this.toggleSelect(this.active); + this.toggleSelect(this._active); } - } - - protected escape() { - this.hide(); - this.host.focus(); - } + }; - protected enter() { - if (this.active === START_INDEX) { + private onEnter = async (): Promise => { + if (this._active === -1) { return; } - const item = this.dataState[this.active]; + const item = this.state.dataState[this._active]; - if (!item.header && this.host.singleSelect) { + if (!item.header && this.combo.singleSelect) { this.select(this.active); } - this.hide(); - requestAnimationFrame(() => this.input.select()); - this.host.focus(); - } + if (await this.hide()) { + this.input.select(); + this.combo.focus(); + } + }; - protected inputArrowDown(container: IgcComboListComponent) { - container.focus(); - this.arrowDown(container); - } + private onTab = async (): Promise => { + if (this.combo.open) { + await this.hide(); + } + }; - protected async mainInputArrowDown(container: IgcComboListComponent) { - this.show(); - await container.updateComplete; + private onEscape = async (): Promise => { + if (await this.hide()) { + this.input.focus(); + } + }; - if (this.host.singleSelect) { - container.focus(); - this.arrowDown(container); + private onMainInputArrowDown = async (): Promise => { + if (!this.combo.open && !(await this.show())) { + return; } - } - protected tab() { - this.hide(); - this.host.blur(); - } + if (this.combo.singleSelect) { + this.onSearchArrowDown(); + } + }; - protected arrowDown(container: IgcComboListComponent) { - this.getNextItem(DIRECTION.Down); - this.scrollToActive(container); - } + private onSearchArrowDown = (): void => { + this.list.focus(); + this.onArrowDown(); + }; - protected arrowUp(container: IgcComboListComponent) { - this.getNextItem(DIRECTION.Up); - this.scrollToActive(container); - } + private onHome = (): void => { + this.active = this.firstItem; + this.scrollToActive(); + }; - protected getNextItem(direction: DIRECTION) { - const next = this.getNearestItem(this.currentItem, direction); + private onEnd = (): void => { + this.active = this.lastItem; + this.scrollToActive(); + }; - if (next === -1) { - if (this.active === this.firstItem) { - this.input.focus(); - this.active = START_INDEX; - } - return; - } + private onArrowUp = (): void => { + this.getNextItem(-1); + this.scrollToActive(); + }; - this.active = next; - } + private onArrowDown = (): void => { + this.getNextItem(1); + this.scrollToActive(); + }; - protected getNearestItem(startIndex: number, direction: number) { - let index = startIndex; - const items = this.dataState; + private scrollToActive(behavior?: ScrollBehavior): void { + this.list.element(this.active)?.scrollIntoView({ + block: 'center', + behavior: behavior ?? 'auto', + }); - while (items[index + direction]?.header) { - index += direction; - } + this.list.requestUpdate(); + } - index += direction; + private getNearestItem(start: number, delta: -1 | 1): number { + let index = start; + const items = this.state.dataState; - if (index >= 0 && index < items.length) { - return index; + while (items[index + delta]?.header) { + index += delta; } - return -1; - } - - public hostConnected() {} - public hostDisconnected() { - this.active = START_INDEX; - } + index += delta; - public navigateTo(item: T, container: IgcComboListComponent) { - this.active = this.dataState.findIndex((i) => i === item); - this.scrollToActive(container, 'smooth'); + return index >= 0 && index < items.length ? index : -1; } - public navigateHost(event: KeyboardEvent) { - if (this.hostHandlers.has(event.key)) { - event.preventDefault(); - this.hostHandlers.get(event.key)!.call(this); + private getNextItem(delta: -1 | 1): void { + const next = this.getNearestItem(this._active, delta); + if (next === -1) { + if (this.active === this.firstItem) { + (this.combo.singleSelect ? this.input : this.searchInput).focus(); + this.active = -1; + } + return; } - } - - public navigateMainInput( - event: KeyboardEvent, - container: IgcComboListComponent - ) { - event.stopPropagation(); - if (this.mainInputHandlers.has(event.key)) { - event.preventDefault(); - this.mainInputHandlers.get(event.key)!.call(this, container); - } + this.active = next; } - public navigateSearchInput( - event: KeyboardEvent, - container: IgcComboListComponent + constructor( + protected combo: ComboHost, + protected state: DataController, + config: ComboNavigationConfig ) { - event.stopPropagation(); - - if (this.searchInputHandlers.has(event.key)) { - event.preventDefault(); - this.searchInputHandlers.get(event.key)!.call(this, container); - } + this.combo.addController(this as ReactiveController); + this._config = config; + + const bindingDefaults = { + preventDefault: true, + triggers: ['keydownRepeat'], + } as KeyBindingOptions; + + const skip = (): boolean => this.combo.disabled; + + // Combo + addKeybindings(this.combo, { skip, bindingDefaults }) + .set(tabKey, this.onTab, { preventDefault: false }) + .set([shiftKey, tabKey], this.onTab, { preventDefault: false }) + .set(escapeKey, this.onEscape); + + // Main input + addKeybindings(this.combo, { + skip, + ref: this._config.input, + bindingDefaults, + }) + .set(arrowUp, async () => await this.hide()) + .set([altKey, arrowDown], this.onMainInputArrowDown) + .set(arrowDown, this.onMainInputArrowDown) + .set(enterKey, this.onEnter); + + // Search input + addKeybindings(this.combo, { + skip, + ref: this._config.search, + bindingDefaults, + }) + .set(arrowUp, this.onEscape) + .set(arrowDown, this.onSearchArrowDown); + + // List + addKeybindings(this.combo, { + skip, + ref: this._config.list, + bindingDefaults, + }) + .set(arrowUp, this.onArrowUp) + .set(arrowDown, this.onArrowDown) + .set(homeKey, this.onHome) + .set(endKey, this.onEnd) + .set(spaceBar, this.onSpace) + .set(enterKey, this.onEnter); } - public navigateList(event: KeyboardEvent, container: IgcComboListComponent) { - event.stopPropagation(); - - if (this.listHandlers.has(event.key)) { - event.preventDefault(); - this.listHandlers.get(event.key)!.call(this, container); - } + public hostDisconnected(): void { + this._active = -1; } } diff --git a/src/components/common/controllers/key-bindings.ts b/src/components/common/controllers/key-bindings.ts index 9427a6a19..b5ee6dc53 100644 --- a/src/components/common/controllers/key-bindings.ts +++ b/src/components/common/controllers/key-bindings.ts @@ -179,6 +179,7 @@ class KeyBindingController implements ReactiveController { protected _observedElement?: Element; protected _options?: KeyBindingControllerOptions; private bindings = new Map(); + private allowedKeys = new Set(); private pressedKeys = new Set(); protected get _element() { @@ -249,6 +250,10 @@ class KeyBindingController implements ReactiveController { private shouldSkip(event: KeyboardEvent) { const skip = this._options?.skip; + if (!this.allowedKeys.has(event.key.toLowerCase())) { + return true; + } + if (!findElementFromEventPath((e) => e === this._element, event)) { return true; } @@ -268,7 +273,9 @@ class KeyBindingController implements ReactiveController { } const key = event.key.toLowerCase(); - Modifiers.has(key) ? this.pressedKeys.clear() : this.pressedKeys.add(key); + if (!Modifiers.has(key)) { + this.pressedKeys.add(key); + } const pendingKeys = Array.from(this.pressedKeys); const modifiers = Array.from(Modifiers.values()).filter( @@ -305,6 +312,10 @@ class KeyBindingController implements ReactiveController { const combination = createCombinationKey(keys, modifiers); const _options = { ...this._options?.bindingDefaults, ...options }; + for (const key of [...keys, ...modifiers]) { + this.allowedKeys.add(key); + } + this.bindings.set(combination, { keys, handler, diff --git a/stories/avatar.stories.ts b/stories/avatar.stories.ts index b164e4d66..eeea737db 100644 --- a/stories/avatar.stories.ts +++ b/stories/avatar.stories.ts @@ -63,11 +63,6 @@ interface IgcAvatarArgs { } type Story = StoryObj; -registerIcon( - 'home', - 'https://unpkg.com/material-design-icons@3.0.1/action/svg/production/ic_home_24px.svg' -); - // endregion export const Image: Story = { From 7245ba23430a6ce57bd9db9e3bbf0bdb2d558344 Mon Sep 17 00:00:00 2001 From: Radoslav Karaivanov Date: Mon, 25 Aug 2025 10:18:46 +0300 Subject: [PATCH 2/5] fix: Post merge fixes --- src/components/combo/combo.spec.ts | 6 +- .../combo/controllers/navigation.ts | 104 +++++++++--------- 2 files changed, 55 insertions(+), 55 deletions(-) diff --git a/src/components/combo/combo.spec.ts b/src/components/combo/combo.spec.ts index 218b9cf92..e398818e5 100644 --- a/src/components/combo/combo.spec.ts +++ b/src/components/combo/combo.spec.ts @@ -12,11 +12,13 @@ import { } from '../common/controllers/key-bindings.js'; import { defineComponents } from '../common/definitions/defineComponents.js'; import { first } from '../common/util.js'; -import { createFormAssociatedTestBed } from '../common/utils.spec.js'; import { - runValidationContainerTests, + createFormAssociatedTestBed, simulateClick, simulateKeyboard, +} from '../common/utils.spec.js'; +import { + runValidationContainerTests, type ValidationContainerTestsParams, ValidityHelpers, } from '../common/validity-helpers.spec.js'; diff --git a/src/components/combo/controllers/navigation.ts b/src/components/combo/controllers/navigation.ts index d3fd5d1a5..5e5dd4ec3 100644 --- a/src/components/combo/controllers/navigation.ts +++ b/src/components/combo/controllers/navigation.ts @@ -1,8 +1,6 @@ import type { ReactiveController } from 'lit'; import type { Ref } from 'lit/directives/ref.js'; - import { - type KeyBindingOptions, addKeybindings, altKey, arrowDown, @@ -11,6 +9,7 @@ import { enterKey, escapeKey, homeKey, + type KeyBindingOptions, shiftKey, spaceBar, tabKey, @@ -54,20 +53,20 @@ export class ComboNavigationController { return this._config.list.value!; } - protected get firstItem(): number { + protected get _firstItem(): number { return this.state.dataState.findIndex((rec) => !rec.header); } - protected get lastItem(): number { + protected get _lastItem(): number { return this.state.dataState.length - 1; } - protected async hide(): Promise { + protected async _hide(): Promise { // @ts-expect-error: protected access return await this.combo._hide(true); } - protected async show(): Promise { + protected async _show(): Promise { // @ts-expect-error: protected access return await this.combo._show(true); } @@ -77,12 +76,12 @@ export class ComboNavigationController { this.combo.toggleSelect(index); } - protected select(index: number): void { + protected _select(index: number): void { // @ts-expect-error: protected access this.combo.selectByIndex(index); } - private onSpace = (): void => { + private _onSpace = (): void => { if (this._active === -1) { return; } @@ -93,7 +92,7 @@ export class ComboNavigationController { } }; - private onEnter = async (): Promise => { + private _onEnter = async (): Promise => { if (this._active === -1) { return; } @@ -101,63 +100,63 @@ export class ComboNavigationController { const item = this.state.dataState[this._active]; if (!item.header && this.combo.singleSelect) { - this.select(this.active); + this._select(this.active); } - if (await this.hide()) { + if (await this._hide()) { this.input.select(); this.combo.focus(); } }; - private onTab = async (): Promise => { + private _onTab = async (): Promise => { if (this.combo.open) { - await this.hide(); + await this._hide(); } }; - private onEscape = async (): Promise => { - if (await this.hide()) { + private _onEscape = async (): Promise => { + if (await this._hide()) { this.input.focus(); } }; - private onMainInputArrowDown = async (): Promise => { - if (!this.combo.open && !(await this.show())) { + private _onMainInputArrowDown = async (): Promise => { + if (!this.combo.open && !(await this._show())) { return; } if (this.combo.singleSelect) { - this.onSearchArrowDown(); + this._onSearchArrowDown(); } }; - private onSearchArrowDown = (): void => { + private _onSearchArrowDown = (): void => { this.list.focus(); - this.onArrowDown(); + this._onArrowDown(); }; - private onHome = (): void => { - this.active = this.firstItem; - this.scrollToActive(); + private _onHome = (): void => { + this.active = this._firstItem; + this._scrollToActive(); }; - private onEnd = (): void => { - this.active = this.lastItem; - this.scrollToActive(); + private _onEnd = (): void => { + this.active = this._lastItem; + this._scrollToActive(); }; - private onArrowUp = (): void => { - this.getNextItem(-1); - this.scrollToActive(); + private _onArrowUp = (): void => { + this._getNextItem(-1); + this._scrollToActive(); }; - private onArrowDown = (): void => { - this.getNextItem(1); - this.scrollToActive(); + private _onArrowDown = (): void => { + this._getNextItem(1); + this._scrollToActive(); }; - private scrollToActive(behavior?: ScrollBehavior): void { + private _scrollToActive(behavior?: ScrollBehavior): void { this.list.element(this.active)?.scrollIntoView({ block: 'center', behavior: behavior ?? 'auto', @@ -166,7 +165,7 @@ export class ComboNavigationController { this.list.requestUpdate(); } - private getNearestItem(start: number, delta: -1 | 1): number { + private _getNearestItem(start: number, delta: -1 | 1): number { let index = start; const items = this.state.dataState; @@ -179,10 +178,10 @@ export class ComboNavigationController { return index >= 0 && index < items.length ? index : -1; } - private getNextItem(delta: -1 | 1): void { - const next = this.getNearestItem(this._active, delta); + private _getNextItem(delta: -1 | 1): void { + const next = this._getNearestItem(this._active, delta); if (next === -1) { - if (this.active === this.firstItem) { + if (this.active === this._firstItem) { (this.combo.singleSelect ? this.input : this.searchInput).focus(); this.active = -1; } @@ -201,7 +200,6 @@ export class ComboNavigationController { this._config = config; const bindingDefaults = { - preventDefault: true, triggers: ['keydownRepeat'], } as KeyBindingOptions; @@ -209,9 +207,9 @@ export class ComboNavigationController { // Combo addKeybindings(this.combo, { skip, bindingDefaults }) - .set(tabKey, this.onTab, { preventDefault: false }) - .set([shiftKey, tabKey], this.onTab, { preventDefault: false }) - .set(escapeKey, this.onEscape); + .set(tabKey, this._onTab, { preventDefault: false }) + .set([shiftKey, tabKey], this._onTab, { preventDefault: false }) + .set(escapeKey, this._onEscape); // Main input addKeybindings(this.combo, { @@ -219,10 +217,10 @@ export class ComboNavigationController { ref: this._config.input, bindingDefaults, }) - .set(arrowUp, async () => await this.hide()) - .set([altKey, arrowDown], this.onMainInputArrowDown) - .set(arrowDown, this.onMainInputArrowDown) - .set(enterKey, this.onEnter); + .set(arrowUp, async () => await this._hide()) + .set([altKey, arrowDown], this._onMainInputArrowDown) + .set(arrowDown, this._onMainInputArrowDown) + .set(enterKey, this._onEnter); // Search input addKeybindings(this.combo, { @@ -230,8 +228,8 @@ export class ComboNavigationController { ref: this._config.search, bindingDefaults, }) - .set(arrowUp, this.onEscape) - .set(arrowDown, this.onSearchArrowDown); + .set(arrowUp, this._onEscape) + .set(arrowDown, this._onSearchArrowDown); // List addKeybindings(this.combo, { @@ -239,12 +237,12 @@ export class ComboNavigationController { ref: this._config.list, bindingDefaults, }) - .set(arrowUp, this.onArrowUp) - .set(arrowDown, this.onArrowDown) - .set(homeKey, this.onHome) - .set(endKey, this.onEnd) - .set(spaceBar, this.onSpace) - .set(enterKey, this.onEnter); + .set(arrowUp, this._onArrowUp) + .set(arrowDown, this._onArrowDown) + .set(homeKey, this._onHome) + .set(endKey, this._onEnd) + .set(spaceBar, this._onSpace) + .set(enterKey, this._onEnter); } public hostDisconnected(): void { From 580ef963e6a62520ee025bcf9e395cf9a4f50668 Mon Sep 17 00:00:00 2001 From: MonikaKirkova Date: Thu, 2 Oct 2025 16:44:27 +0300 Subject: [PATCH 3/5] feat(combo): update Escape key behavior --- src/components/combo/combo.spec.ts | 81 +++++++++++++++++++ src/components/combo/combo.ts | 21 ++--- .../combo/controllers/navigation.ts | 14 +++- 3 files changed, 104 insertions(+), 12 deletions(-) diff --git a/src/components/combo/combo.spec.ts b/src/components/combo/combo.spec.ts index e398818e5..2701db330 100644 --- a/src/components/combo/combo.spec.ts +++ b/src/components/combo/combo.spec.ts @@ -7,8 +7,10 @@ import { arrowUp, endKey, enterKey, + escapeKey, homeKey, spaceBar, + tabKey, } from '../common/controllers/key-bindings.js'; import { defineComponents } from '../common/definitions/defineComponents.js'; import { first } from '../common/util.js'; @@ -716,6 +718,48 @@ describe('Combo', () => { expect(combo.open).to.be.true; }); + it('should close the menu by pressing the Tab key', async () => { + await combo.show(); + await list.layoutComplete; + + simulateKeyboard(options, tabKey, 1); + await elementUpdated(combo); + expect(combo.open).to.be.false; + + await combo.show(); + simulateKeyboard(searchInput, tabKey, 1); + await elementUpdated(combo); + + expect(combo.open).to.be.false; + + await combo.show(); + simulateKeyboard(input, tabKey, 1); + await elementUpdated(combo); + + expect(combo.open).to.be.false; + }); + + it('should clear the selection by pressing the Escape key when the combo is closed', async () => { + combo.autofocusList = true; + combo.select(['BG01', 'BG02']); + await elementUpdated(combo); + + await combo.show(); + await list.layoutComplete; + + simulateKeyboard(options, escapeKey, 1); + await elementUpdated(combo); + + expect(combo.open).to.be.false; + expect(combo.value.length).to.equal(2); + + simulateKeyboard(input, escapeKey, 1); + await elementUpdated(combo); + + expect(combo.open).to.be.false; + expect(combo.value.length).to.equal(0); + }); + it('should select the active item and close the menu by pressing Enter in single selection', async () => { combo.singleSelect = true; await elementUpdated(combo); @@ -750,6 +794,43 @@ describe('Combo', () => { expect(combo.open).to.be.false; }); + it('should close the menu by pressing the Tab key in single selection', async () => { + await combo.show(); + await list.layoutComplete; + + simulateKeyboard(options, tabKey, 1); + await elementUpdated(combo); + + expect(combo.open).to.be.false; + + await combo.show(); + simulateKeyboard(input, tabKey, 1); + await elementUpdated(combo); + + expect(combo.open).to.be.false; + }); + + it('should clear the selection by pressing the Escape key in single selection', async () => { + combo.singleSelect = true; + combo.select('BG01'); + await elementUpdated(combo); + + await combo.show(); + await list.layoutComplete; + + simulateKeyboard(options, escapeKey, 1); + await elementUpdated(combo); + + expect(combo.open).to.be.false; + expect(combo.value.length).to.equal(1); + + simulateKeyboard(input, escapeKey, 1); + await elementUpdated(combo); + + expect(combo.open).to.be.false; + expect(combo.value.length).to.equal(0); + }); + it('should support a single selection variant', async () => { combo.singleSelect = true; await elementUpdated(combo); diff --git a/src/components/combo/combo.ts b/src/components/combo/combo.ts index 7eca4d3a8..546b549b0 100644 --- a/src/components/combo/combo.ts +++ b/src/components/combo/combo.ts @@ -799,6 +799,17 @@ export default class IgcComboComponent< this.updateValue(); } + /** @hidden @internal */ + public clearSelection() { + if (this.singleSelect) { + this.resetSearchTerm(); + this.clearSingleSelection(); + } else { + this._selection.deselect([], true); + } + this.updateValue(); + } + protected clearSingleSelection() { const _selection = this._selection.asArray; const selection = first(_selection); @@ -812,15 +823,7 @@ export default class IgcComboComponent< protected handleClearIconClick(e: PointerEvent) { e.stopPropagation(); - - if (this.singleSelect) { - this.resetSearchTerm(); - this.clearSingleSelection(); - } else { - this._selection.deselect([], true); - } - - this.updateValue(); + this.clearSelection(); this._navigation.active = -1; } diff --git a/src/components/combo/controllers/navigation.ts b/src/components/combo/controllers/navigation.ts index 5e5dd4ec3..5a7f973ed 100644 --- a/src/components/combo/controllers/navigation.ts +++ b/src/components/combo/controllers/navigation.ts @@ -109,13 +109,19 @@ export class ComboNavigationController { } }; - private _onTab = async (): Promise => { + private _onTab = async (shiftKey?: boolean): Promise => { if (this.combo.open) { + if (shiftKey) { + this.combo.focus(); + } await this._hide(); } }; private _onEscape = async (): Promise => { + if (!this.combo.open) { + this.combo.clearSelection(); + } if (await this._hide()) { this.input.focus(); } @@ -207,8 +213,10 @@ export class ComboNavigationController { // Combo addKeybindings(this.combo, { skip, bindingDefaults }) - .set(tabKey, this._onTab, { preventDefault: false }) - .set([shiftKey, tabKey], this._onTab, { preventDefault: false }) + .set(tabKey, () => this._onTab(), { preventDefault: false }) + .set([shiftKey, tabKey], () => this._onTab(true), { + preventDefault: false, + }) .set(escapeKey, this._onEscape); // Main input From c155cd317dd8eb5ef7df00deea7fa76ba1cda683 Mon Sep 17 00:00:00 2001 From: Radoslav Karaivanov Date: Thu, 2 Oct 2025 18:39:11 +0300 Subject: [PATCH 4/5] refactor: Some minor code clean-ups --- src/components/combo/controllers/navigation.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/components/combo/controllers/navigation.ts b/src/components/combo/controllers/navigation.ts index 5a7f973ed..f9b0d5b15 100644 --- a/src/components/combo/controllers/navigation.ts +++ b/src/components/combo/controllers/navigation.ts @@ -71,7 +71,7 @@ export class ComboNavigationController { return await this.combo._show(true); } - public toggleSelect(index: number): void { + protected _toggleSelection(index: number): void { // @ts-expect-error protected access this.combo.toggleSelect(index); } @@ -88,7 +88,7 @@ export class ComboNavigationController { const item = this.state.dataState[this._active]; if (!item.header) { - this.toggleSelect(this._active); + this._toggleSelection(this._active); } }; @@ -109,9 +109,11 @@ export class ComboNavigationController { } }; - private _onTab = async (shiftKey?: boolean): Promise => { + private _onTab = async ({ shiftKey }: KeyboardEvent): Promise => { if (this.combo.open) { if (shiftKey) { + // Move focus to the main input of the combo + // before the Shift+Tab behavior kicks in. this.combo.focus(); } await this._hide(); @@ -122,6 +124,7 @@ export class ComboNavigationController { if (!this.combo.open) { this.combo.clearSelection(); } + if (await this._hide()) { this.input.focus(); } @@ -213,8 +216,8 @@ export class ComboNavigationController { // Combo addKeybindings(this.combo, { skip, bindingDefaults }) - .set(tabKey, () => this._onTab(), { preventDefault: false }) - .set([shiftKey, tabKey], () => this._onTab(true), { + .set(tabKey, this._onTab, { preventDefault: false }) + .set([shiftKey, tabKey], this._onTab, { preventDefault: false, }) .set(escapeKey, this._onEscape); From c5ce90048507fd22057a7b858dd94cffc6f65d8f Mon Sep 17 00:00:00 2001 From: Radoslav Karaivanov Date: Thu, 9 Oct 2025 12:50:30 +0300 Subject: [PATCH 5/5] fix: Arrow Up behavior in both single-select and disable-filtering states --- src/components/combo/combo.spec.ts | 81 ++++++++++++++++--- .../combo/controllers/navigation.ts | 11 ++- 2 files changed, 76 insertions(+), 16 deletions(-) diff --git a/src/components/combo/combo.spec.ts b/src/components/combo/combo.spec.ts index 2701db330..380a539f7 100644 --- a/src/components/combo/combo.spec.ts +++ b/src/components/combo/combo.spec.ts @@ -16,6 +16,7 @@ import { defineComponents } from '../common/definitions/defineComponents.js'; import { first } from '../common/util.js'; import { createFormAssociatedTestBed, + isFocused, simulateClick, simulateKeyboard, } from '../common/utils.spec.js'; @@ -187,16 +188,16 @@ describe('Combo', () => { >` ); - options = combo.shadowRoot!.querySelector( + options = combo.renderRoot.querySelector( '[part="list"]' ) as IgcComboListComponent; - input = combo.shadowRoot!.querySelector( + input = combo.renderRoot.querySelector( 'igc-input#target' ) as IgcInputComponent; - searchInput = combo.shadowRoot!.querySelector( + searchInput = combo.renderRoot.querySelector( '[part="search-input"]' ) as IgcInputComponent; - list = combo.shadowRoot!.querySelector( + list = combo.renderRoot.querySelector( 'igc-combo-list' ) as IgcComboListComponent; }); @@ -697,8 +698,8 @@ describe('Combo', () => { simulateKeyboard(options, endKey); await elementUpdated(combo); - const itms = items(combo); - expect(itms[itms.length - 1].active).to.be.true; + const _items = items(combo); + expect(_items[_items.length - 1].active).to.be.true; }); it('should select the active item by pressing the Space key', async () => { @@ -712,12 +713,72 @@ describe('Combo', () => { simulateKeyboard(options, spaceBar); await elementUpdated(combo); - const itms = items(combo); - expect(itms[1].active).to.be.true; - expect(itms[1].selected).to.be.true; + const _items = items(combo); + expect(_items[1].active).to.be.true; + expect(_items[1].selected).to.be.true; expect(combo.open).to.be.true; }); + it('should move focus to the filter input and the close the dropdown on subsequent Arrow Up keypress', async () => { + await combo.show(); + await list.layoutComplete; + + // Move active state to first item and focus to the dropdown + simulateKeyboard(searchInput, arrowDown); + await elementUpdated(combo); + + expect(isFocused(list)).to.be.true; + expect(isFocused(searchInput)).to.be.false; + + // Move focus to the search input + simulateKeyboard(list, arrowUp); + await elementUpdated(combo); + + expect(isFocused(list)).to.be.false; + expect(isFocused(searchInput)).to.be.true; + + simulateKeyboard(searchInput, arrowUp); + await elementUpdated(combo); + + expect(combo.open).to.be.false; + }); + + it('should close the dropdown on Arrow Up when disable-filtering is set', async () => { + combo.disableFiltering = true; + await elementUpdated(combo); + + await combo.show(); + await list.layoutComplete; + + // Activate first item + simulateKeyboard(list, arrowDown); + await elementUpdated(combo); + + expect(isFocused(list)).to.be.true; + + simulateKeyboard(list, arrowUp); + await elementUpdated(combo); + + expect(combo.open).to.be.false; + }); + + it('should close the dropdown on Arrow Up in single-select mode', async () => { + combo.singleSelect = true; + await elementUpdated(combo); + + await combo.show(); + await list.layoutComplete; + + // Activate first item + simulateKeyboard(list, arrowDown); + await elementUpdated(combo); + + simulateKeyboard(list, arrowUp); + await elementUpdated(combo); + + expect(combo.open).to.be.false; + }); + it('should close the menu by pressing the Tab key', async () => { await combo.show(); await list.layoutComplete; @@ -1221,7 +1282,7 @@ describe('Combo', () => { }); }); - it('should be able to get the currently selected items by calling the `selectoin` getter', async () => { + it('should be able to get the currently selected items by calling the `selection` getter', async () => { combo.select([cities[0].id, cities[1].id, cities[2].id]); await elementUpdated(combo); diff --git a/src/components/combo/controllers/navigation.ts b/src/components/combo/controllers/navigation.ts index f9b0d5b15..0072d9852 100644 --- a/src/components/combo/controllers/navigation.ts +++ b/src/components/combo/controllers/navigation.ts @@ -189,12 +189,11 @@ export class ComboNavigationController { private _getNextItem(delta: -1 | 1): void { const next = this._getNearestItem(this._active, delta); - if (next === -1) { - if (this.active === this._firstItem) { - (this.combo.singleSelect ? this.input : this.searchInput).focus(); - this.active = -1; - } - return; + + if (next === -1 && this.active === this._firstItem) { + this.searchInput.checkVisibility() // Non single-select or disable-filtering combo configuration + ? this.searchInput.focus() // Delegate to search input handlers + : this._onEscape(); // Close dropdown and move focus back to main input } this.active = next;