Skip to content

Commit 7c7641c

Browse files
authored
refactor(combo): overhaul selection, data pipeline, and component architecture (#2164)
* refactor(combo): overhaul selection, data pipeline, and component architecture Major refactoring of the `IgcComboComponent` and its supporting infrastructure to improve maintainability, correctness, and adherence to project conventions. - `combo.ts` - Inherits from `IgcBaseComboBoxComponent` instead of bare `LitElement`, sharing open/close lifecycle (`_show`, `_hide`) with picker components. - Removed the `SelectionController` — selection state (`_selected: Set<T>`) is now managed directly inside the component for a simpler, single-source-of-truth model. - Replaced `@watch('open')` decorator with `willUpdate()` lifecycle hook. - Replaced `@queryAssignedElements` with `addSlotController` / `setSlots` for slot detection (no more `inputPrefix` / `inputSuffix` public properties). - Form value serialization moved into the `setFormValue` transformer on `createFormValueState` — removes the separate `_setFormValue()` override. - `_setDefaultValue` now handles invalid JSON gracefully instead of throwing. - `firstUpdated` removed; initialization is handled by `willUpdate` / `willUpdate` data-sync guard for the delayed-data scenario. - Added `_syncSelectionFromValue()` (value → Set) and `_syncValueFromSelection()` (Set → form value) for explicit one-way data flow. - Added `_resolveItems()`, `_resolveItemValue()`, `_getValues()` private helpers. - All internal render helpers and event handlers renamed to `_camelCase` convention (e.g., `renderMainInput` → `_renderMainInput`, `handleMainInput` → `_handleMainInput`). - `groupKey` / `groupSorting` setters removed — now simple `@property` declarations; pipeline invalidation driven by `willUpdate`. - `filteringOptions` and `disableFiltering` setters simplified. - `label` and `placeholder` changed from `!` non-null asserted to optional (`?`). - `placeholderSearch` setter typed as `string | undefined`. - Added `Iterator.from` usage for lazy mapping/filtering of selections. - `controllers/data.ts` — `DataController` → `DataState` - Introduced a **dirty flag** pattern: `invalidate()` marks state as dirty and `hostUpdate()` runs the pipeline lazily before each render, batching multiple property changes into a single pipeline execution. - Locale-aware `Intl.Collator` initialized from host locale; updated via `updateLocale(locale)`. - `dataState` is now a read-only accessor. - Added `invalidate()` and `updateLocale()` as explicit public API. - Internal pipeline methods prefixed with `_`. - `controllers/navigation.ts` - Removed all `// @ts-expect-error: protected access` hacks. - Added `interactions` config object to decouple `hide`, `show`, `toggleSelection`, `select`, and `clearSelection` callbacks — the navigation controller no longer reaches into the combo's protected methods directly. - `_active` field is now public (`active`) so it can be read without casting. - `_getNearestItem` rewritten with a `for` loop (was `while` with off-by-one risk). - Navigation mutations now call `requestUpdate('_activeIndex', previous)` with the old value so Lit schedules targeted updates. - Keybinding setup moved to the constructor body (was at the end). - `controllers/selection.ts` - **Deleted.** All selection logic inlined into `combo.ts`. - `combo-item.ts` - `renderCheckbox` renamed to `_renderCheckbox`. - Switched from `nothing` conditional to Lit's `cache()` directive to preserve the checkbox DOM subtree when `hideCheckbox` toggles. - `connectedCallback` marked `@internal`. - Missing `@attr` / `@default` JSDoc tags added. - `combo-list.ts` / `combo-header.ts` - Added explicit `void` return type to `register()` methods. - `common/mixins/combo-box.ts` - `IgcBaseComboBoxLikeComponent` renamed to `IgcBaseComboBoxComponent` (core open/ close behaviour only: `open`, `_show`, `_hide`, `_handleAnchorClick`, events). - `IgcComboBoxBaseLikeComponent` extracted as a new subclass that adds `keepOpenOnSelect` and `keepOpenOnOutsideClick` — used by Dropdown, Select, DatePicker, and DateRangePicker. - Protected event helpers renamed: `emitClosing` → `_emitClosing`, etc. - `handleAnchorClick` → `_handleAnchorClick`. - Utility functions (`getItems`, `getActiveItems`, etc.) given explicit return types. - `setInitialSelectionState` simplified using `last()` and direct assignment. - `common/util.ts` - `findElementFromEventPath` renamed to `getElementFromPath` with full JSDoc. - New `stopPropagation(event)` utility function extracted (used in combo template). - `asArray` now uses `value == null` instead of `isDefined(value)`. - Cross-component rename: `findElementFromEventPath` → `getElementFromPath` Updated all callers: `button-group`, `calendar`, `carousel`, `drag`, `key-bindings`, `root-click`, `date-picker`, `date-range-picker`, `dropdown`, `resize-controller`, `select`, `stepper`, `tabs`, `tile`, `tree-item`. --- - Added test: *"should handle invalid JSON in value attribute gracefully"*. - Removed `await list.layoutComplete` from keyboard-selection test (no longer needed).
1 parent f74b953 commit 7c7641c

29 files changed

Lines changed: 970 additions & 942 deletions

File tree

src/components/button-group/button-group.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { watch } from '../common/decorators/watch.js';
1010
import { registerComponent } from '../common/definitions/register.js';
1111
import type { Constructor } from '../common/mixins/constructor.js';
1212
import { EventEmitterMixin } from '../common/mixins/event-emitter.js';
13-
import { findElementFromEventPath, last } from '../common/util.js';
13+
import { getElementFromPath, last } from '../common/util.js';
1414
import type { ButtonGroupSelection, ContentOrientation } from '../types.js';
1515
import { styles } from './themes/group.base.css.js';
1616
import { all } from './themes/group.js';
@@ -167,10 +167,7 @@ export default class IgcButtonGroupComponent extends EventEmitterMixin<
167167
}
168168

169169
private handleClick(event: MouseEvent) {
170-
const button = findElementFromEventPath(
171-
IgcToggleButtonComponent.tagName,
172-
event
173-
);
170+
const button = getElementFromPath(IgcToggleButtonComponent.tagName, event);
174171

175172
if (button) {
176173
this.isMultiple

src/components/calendar/calendar.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ import { EventEmitterMixin } from '../common/mixins/event-emitter.js';
2222
import { partMap } from '../common/part-map.js';
2323
import {
2424
clamp,
25-
findElementFromEventPath,
2625
first,
2726
formatString,
27+
getElementFromPath,
2828
last,
2929
} from '../common/util.js';
3030
import IgcIconComponent from '../icon/icon.js';
@@ -267,7 +267,7 @@ export default class IgcCalendarComponent extends EventEmitterMixin<
267267
//#region Keyboard event handlers
268268

269269
private _shouldSkipKeyboardEvent(_: Element, event: KeyboardEvent): boolean {
270-
return !findElementFromEventPath(
270+
return !getElementFromPath(
271271
`${IgcDaysViewComponent.tagName}, ${IgcMonthsViewComponent.tagName}, ${IgcYearsViewComponent.tagName}`,
272272
event
273273
);

src/components/calendar/helpers.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import {
22
asNumber,
3-
findElementFromEventPath,
43
first,
4+
getElementFromPath,
55
isString,
66
last,
77
modulo,
@@ -154,7 +154,7 @@ export function convertToDates(
154154
* Returns the value of the selected/activated element (day/month/year) in the calendar view.
155155
*/
156156
export function getViewElement(event: Event): number {
157-
const element = findElementFromEventPath<HTMLElement>('[data-value]', event);
157+
const element = getElementFromPath<HTMLElement>('[data-value]', event);
158158
return element ? asNumber(element.dataset.value, -1) : -1;
159159
}
160160

src/components/carousel/carousel.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,9 @@ import { partMap } from '../common/part-map.js';
4242
import {
4343
addSafeEventListener,
4444
asNumber,
45-
findElementFromEventPath,
4645
first,
4746
formatString,
47+
getElementFromPath,
4848
isEmpty,
4949
isLTR,
5050
last,
@@ -501,7 +501,7 @@ export default class IgcCarouselComponent extends EventEmitterMixin<
501501
}
502502

503503
private async _handleIndicatorClick(event: PointerEvent): Promise<void> {
504-
const indicator = findElementFromEventPath(
504+
const indicator = getElementFromPath(
505505
IgcCarouselIndicatorComponent.tagName,
506506
event
507507
)!;

src/components/combo/combo-header.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export default class IgcComboHeaderComponent extends LitElement {
1212
public static override styles = [styles, shared];
1313

1414
/* blazorSuppress */
15-
public static register() {
15+
public static register(): void {
1616
registerComponent(IgcComboHeaderComponent);
1717
}
1818

src/components/combo/combo-item.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { html, LitElement, nothing, type PropertyValues } from 'lit';
22
import { property } from 'lit/decorators.js';
3+
import { cache } from 'lit/directives/cache.js';
34
import { addThemingController } from '../../theming/theming-controller.js';
45
import IgcCheckboxComponent from '../checkbox/checkbox.js';
56
import { addInternalsController } from '../common/controllers/internals.js';
@@ -38,13 +39,16 @@ export default class IgcComboItemComponent extends LitElement {
3839

3940
/**
4041
* Determines whether the item is active.
42+
* @attr active
43+
* @default false
4144
*/
4245
@property({ type: Boolean, reflect: true })
4346
public active = false;
4447

4548
/**
4649
* Determines whether the item is active.
4750
* @attr hide-checkbox
51+
* @default false
4852
*/
4953
@property({ type: Boolean, attribute: 'hide-checkbox' })
5054
public hideCheckbox = false;
@@ -54,18 +58,19 @@ export default class IgcComboItemComponent extends LitElement {
5458
addThemingController(this, all);
5559
}
5660

57-
public override connectedCallback() {
61+
/** @internal */
62+
public override connectedCallback(): void {
5863
super.connectedCallback();
5964
this.role = 'option';
6065
}
6166

62-
protected override willUpdate(changedProperties: PropertyValues<this>): void {
63-
if (changedProperties.has('selected')) {
67+
protected override willUpdate(props: PropertyValues<this>): void {
68+
if (props.has('selected')) {
6469
this._internals.setARIA({ ariaSelected: this.selected.toString() });
6570
}
6671
}
6772

68-
private renderCheckbox() {
73+
private _renderCheckbox() {
6974
return html`
7075
<section part="prefix">
7176
<igc-checkbox
@@ -79,7 +84,7 @@ export default class IgcComboItemComponent extends LitElement {
7984

8085
protected override render() {
8186
return html`
82-
${!this.hideCheckbox ? this.renderCheckbox() : nothing}
87+
${cache(this.hideCheckbox ? nothing : this._renderCheckbox())}
8388
<section id="content" part="content">
8489
<slot></slot>
8590
</section>

src/components/combo/combo-list.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { LitVirtualizer } from '@lit-labs/virtualizer/LitVirtualizer.js';
2-
32
import { registerComponent } from '../common/definitions/register.js';
43

54
/* blazorSuppress */
@@ -9,7 +8,7 @@ export default class IgcComboListComponent extends LitVirtualizer {
98
public override scroller = true;
109

1110
/* blazorSuppress */
12-
public static register() {
11+
public static register(): void {
1312
registerComponent(IgcComboListComponent);
1413
}
1514
}

src/components/combo/combo.spec.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1028,7 +1028,6 @@ describe('Combo', () => {
10281028
simulateKeyboard(options, spaceBar);
10291029

10301030
await elementUpdated(combo);
1031-
await list.layoutComplete;
10321031

10331032
expect(items(combo)[1].selected).to.be.false;
10341033
expect(items(combo)[2].selected).to.be.true;
@@ -1518,6 +1517,26 @@ describe('Combo', () => {
15181517
spec.assertSubmitHasValue('US01');
15191518
});
15201519

1520+
it('should handle invalid JSON in value attribute gracefully', () => {
1521+
const defaultValue = spec.element.defaultValue;
1522+
1523+
spec.setAttributes({ value: 'invalid' });
1524+
expect(spec.element.value).to.be.empty;
1525+
expect(spec.element.defaultValue).to.equal(defaultValue);
1526+
});
1527+
1528+
it('should handle empty string in value attribute gracefully', () => {
1529+
spec.setAttributes({ value: '' });
1530+
expect(spec.element.value).to.be.empty;
1531+
expect(spec.element.defaultValue).to.be.empty;
1532+
});
1533+
1534+
it('should handle null in value attribute gracefully', () => {
1535+
spec.setAttributes({ value: null as any });
1536+
expect(spec.element.value).to.be.empty;
1537+
expect(spec.element.defaultValue).to.be.empty;
1538+
});
1539+
15211540
it('reflects disabled ancestor state', () => {
15221541
spec.setAncestorDisabledState(true);
15231542
expect(spec.element.disabled).to.be.true;

0 commit comments

Comments
 (0)