From df9f3bac25c148f5f0dae4b3603c4d6105fcd722 Mon Sep 17 00:00:00 2001 From: Radoslav Karaivanov Date: Tue, 25 Aug 2026 10:29:29 +0300 Subject: [PATCH] Fix core grid bugs and rendering performance Filtering: OR-only expression trees matched every record because the empty AND set was vacuously true; nullish cell values crashed string conditions. Fix the match logic and coerce nullish operands once in normalizeCase. Events: cancelable sorting/filtering fired after state was already mutated. Hand listeners a candidate copy and commit it onto the state-held object only when the event passes, preserving both object identity and the listener-can-modify contract. Emit sorted/filtered after the data view updates. Pipeline: concurrent async runs could resolve out of order. Sequence them with an epoch token; stale results are discarded and a rejected hook keeps the previous data state. Expose the documented but missing public columns setter and stop columnReducer from dropping sibling columns inside containers. Navigation: state was module-level and shared across grid instances; hidden columns were reachable and a nullish active node produced NaN row indices. Move state per instance and clamp over visible columns. Performance: key row rendering by column field so cells are reused, share a single Intl.Collator, memoize dot-path segments, and drive the scrollbar offset from a ResizeObserver coalesced into rAF instead of per-render measurement. Tests: 178 -> 227, coverage 98%. --- src/components/cell.ts | 2 +- src/components/column.ts | 3 +- src/components/filter-row.ts | 33 +++-- src/components/grid.ts | 86 +++++++++++-- src/components/header-row.ts | 8 +- src/components/header.ts | 2 +- src/components/row.ts | 2 +- src/controllers/dom.ts | 82 ++++++++++-- src/controllers/filter.ts | 57 +++++---- src/controllers/navigation.ts | 51 ++++---- src/controllers/resize.ts | 10 +- src/controllers/sort.ts | 39 +++--- src/internal/constants.ts | 6 +- src/internal/normalize-case.ts | 9 +- src/internal/types.ts | 1 - src/internal/utils.ts | 19 ++- src/operations/base.ts | 3 +- src/operations/filter.ts | 7 ++ src/operations/sort.ts | 5 +- test/column-config.test.ts | 18 +++ test/data-operation/filter-ui.test.ts | 110 +++++++++++++++- test/data-operation/filter.test.ts | 175 ++++++++++++++++++++++++++ test/data-operation/pipeline.test.ts | 79 ++++++++++++ test/data-operation/sort-ui.test.ts | 50 +++++++- test/dom.test.ts | 69 ++++++++++ test/grid.properties.test.ts | 43 +++++++ test/navigation.test.ts | 162 ++++++++++++++++++++++++ test/nested-field.test.ts | 16 +++ test/resize.test.ts | 17 +++ 29 files changed, 1040 insertions(+), 124 deletions(-) create mode 100644 test/data-operation/pipeline.test.ts create mode 100644 test/dom.test.ts create mode 100644 test/navigation.test.ts diff --git a/src/components/cell.ts b/src/components/cell.ts index 49209d4..9ed3634 100644 --- a/src/components/cell.ts +++ b/src/components/cell.ts @@ -77,7 +77,7 @@ export default class IgcGridLiteCell extends LitElement { } protected override update(props: PropertyValues): void { - if (props.has('adoptRootStyles')) { + if (props.has('adoptRootStyles') || props.has('cellTemplate')) { this._adoptedStylesController.shouldAdoptStyles(this._shouldAdoptStyles); } diff --git a/src/components/column.ts b/src/components/column.ts index 8ac68c8..a7ae2f8 100644 --- a/src/components/column.ts +++ b/src/components/column.ts @@ -7,6 +7,7 @@ import { GRID_COLUMN_TAG } from '../internal/tags.js'; import type { BaseColumnConfiguration, ColumnSortConfiguration, + DataType, IgcCellContext, IgcHeaderContext, Keys, @@ -44,7 +45,7 @@ export class IgcGridLiteColumn /** The data type of the column's values. */ @property({ attribute: 'data-type' }) - public dataType?: 'number' | 'string' | 'boolean' = 'string'; + public dataType?: DataType = 'string'; /** The header text of the column. */ @property() diff --git a/src/components/filter-row.ts b/src/components/filter-row.ts index 3c685b7..6365277 100644 --- a/src/components/filter-row.ts +++ b/src/components/filter-row.ts @@ -23,6 +23,9 @@ import type { FilterExpression, FilterOperation, OperandKeys } from '../operatio import { styles } from '../styles/filter-row/filter-row.css.js'; import { all } from '../styles/themes/filtering-row-themes.js'; +/** Number of filter expressions shown as chips before collapsing into a single counted chip. */ +const MAX_PREVIEW_CHIPS = 3; + type ExpressionChipProps = { expression: FilterExpression; selected: boolean; @@ -135,12 +138,19 @@ export default class IgcFilterRow extends LitElement { const key = event.detail.value as OperandKeys>; // XXX: Types - this.expression.condition = (getFilterOperandsFor(this.column) as any)[key] as FilterOperation< + const condition = (getFilterOperandsFor(this.column) as any)[key] as FilterOperation< PropertyType >; - if (this.input.value || this.expression.condition.unary) { - this.filterController.filterWithEvent(this.expression, 'modify'); + if (this.input.value || condition.unary) { + this.filterController.filterWithEvent( + { ...this.expression, condition }, + 'modify', + this.expression + ); + } else { + // Nothing to filter by yet - the condition is only staged in the UI. + this.expression.condition = condition; } this.requestUpdate(); @@ -156,9 +166,11 @@ export default class IgcFilterRow extends LitElement { : 'add'; if (shouldUpdate) { - this.expression.searchTerm = value as any; - - this.filterController.filterWithEvent(this.expression, type); + this.filterController.filterWithEvent( + { ...this.expression, searchTerm: value as any }, + type, + this.expression + ); } else { this.#removeExpression(this.expression); } @@ -206,8 +218,9 @@ export default class IgcFilterRow extends LitElement { return async (e: Event) => { e.stopPropagation(); - expression.criteria = expression.criteria === 'and' ? 'or' : 'and'; - this.filterController.filterWithEvent(expression, 'modify'); + const criteria = expression.criteria === 'and' ? 'or' : 'and'; + + this.filterController.filterWithEvent({ ...expression, criteria }, 'modify', expression); this.requestUpdate(); }; } @@ -385,8 +398,8 @@ export default class IgcFilterRow extends LitElement { protected renderFilterState(column: ColumnConfiguration) { const state = this.filterController.get(column.field); - const partial = state && state.length < 3; - const hidden = state && state.length >= 3; + const partial = state && state.length < MAX_PREVIEW_CHIPS; + const hidden = state && state.length >= MAX_PREVIEW_CHIPS; const open = () => { this.column = column; diff --git a/src/components/grid.ts b/src/components/grid.ts index f7365ee..427d6a6 100644 --- a/src/components/grid.ts +++ b/src/components/grid.ts @@ -40,11 +40,16 @@ import IgcGridLiteHeaderRow from './header-row.js'; import IgcGridLiteRow from './row.js'; import IgcVirtualizer from './virtualizer.js'; -/** Column reducer matching either direct column element or one nested in container */ +/** Column reducer matching either a direct column element or every one nested in a container */ function columnReducer(acc: T[], el: T): T[] { const tag = IgcGridLiteColumn.tagName; - const column = el.matches(tag) ? el : el.querySelector(tag); - if (column) acc.push(column as unknown as T); + + if (el.matches(tag)) { + acc.push(el); + return acc; + } + + acc.push(...(Array.from(el.querySelectorAll(tag)) as unknown as T[])); return acc; } @@ -193,6 +198,23 @@ export class IgcGridLite extends EventEmitterBase = Promise.resolve(); + + /** + * Resolves when the most recently started pipeline run has settled and its result + * is rendered. Consumed by the sort/filter controllers so that `sorted`/`filtered` + * are emitted against an up to date {@link IgcGridLite.dataView}. + * + * @internal + */ + public get _pipelineComplete(): Promise { + // The `updateComplete` hop lets a just-requested PIPELINE update install its task first. + return this.updateComplete.then(() => this._pipelineTask); + } + @property({ type: Boolean, reflect: true, attribute: 'adopt-root-styles' }) public adoptRootStyles = false; @@ -258,7 +280,7 @@ export class IgcGridLite extends EventEmitterBase[] { - return Array.from(this._stateController.sorting.state.values()); + return Array.from(this._stateController.sorting.state.values(), (expr) => ({ ...expr })); } /** @@ -278,9 +300,25 @@ export class IgcGridLite extends EventEmitterBase[] { - return this._stateController.filtering.state.values.flatMap((each) => each.all); + return this._stateController.filtering.state.values.flatMap((each) => + each.all.map((expr) => ({ ...expr })) + ); + } + + /** + * Sets the column configuration for the grid. + * + * @remarks + * Passing an empty collection resets the columns, which - with `autoGenerate` - + * lets the next data source binding re-generate them. + */ + public set columns(configuration: ColumnConfiguration[]) { + this._stateController.setColumnConfiguration(asArray(configuration)); } + /** + * Returns the column configuration of the grid. + */ public get columns(): ColumnConfiguration[] { return this._stateController.columns.map((col) => ({ ...col })); } @@ -323,9 +361,35 @@ export class IgcGridLite extends EventEmitterBase`) fire on every requested update. + * Renaming the method silently disables the pipeline. + */ @watch(PIPELINE) - protected async pipeline() { - this._dataState = await this._dataController.apply([...this.data], this._stateController); + protected pipeline(): void { + this._pipelineTask = this._runPipeline(); + } + + /** Runs the data operations, discarding a result superseded by a newer run. */ + private async _runPipeline(): Promise { + const epoch = ++this._pipelineEpoch; + + try { + const state = await this._dataController.apply([...this.data], this._stateController); + + if (epoch !== this._pipelineEpoch) { + return; + } + + this._dataState = state; + } catch (e) { + // A failing hook must not blank the grid - keep the previous data state and report. + // biome-ignore lint/suspicious/noConsole: the pipeline hooks are user code; swallowing their errors hides bugs + console.error(e); + } + + await this.updateComplete; } constructor() { @@ -371,10 +435,10 @@ export class IgcGridLite extends EventEmitterBase | FilterExpression[]): void { - const expressions = asArray(config).filter((expr) => { - const column = this.getColumn(expr.key); - return column !== undefined; - }); + // Copies - the caller's expression objects must not be rewritten. + const expressions = asArray(config) + .filter((expr) => this.getColumn(expr.key) !== undefined) + .map((expr) => ({ ...expr })); for (const expr of expressions) { if (!isString(expr.condition)) { diff --git a/src/components/header-row.ts b/src/components/header-row.ts index e44e034..8ed3f5f 100644 --- a/src/components/header-row.ts +++ b/src/components/header-row.ts @@ -47,12 +47,14 @@ export default class IgcGridLiteHeaderRow extends LitElement { this._state?.filtering.setActiveColumn(header?.column); } - protected override shouldUpdate(props: PropertyValues): boolean { + protected override willUpdate(props: PropertyValues): void { + // Column configuration is passed by reference, so the headers have to be + // refreshed explicitly whenever the row itself updates. for (const header of this.headers) { header.requestUpdate(); } - return super.shouldUpdate(props); + super.willUpdate(props); } protected override render() { @@ -62,7 +64,7 @@ export default class IgcGridLiteHeaderRow extends LitElement { return html` ${repeat( columns, - (column) => column, + (column) => column.field, (column) => html` extends LitElement { } protected override update(props: PropertyValues): void { - if (props.has('adoptRootStyles')) { + if (props.has('adoptRootStyles') || props.has('column')) { this._adoptedStylesController.shouldAdoptStyles( this.adoptRootStyles && this.column.headerTemplate != null ); diff --git a/src/components/row.ts b/src/components/row.ts index f0440c3..2faf8e3 100644 --- a/src/components/row.ts +++ b/src/components/row.ts @@ -51,7 +51,7 @@ export default class IgcGridLiteRow extends LitElement { return html` ${repeat( columns, - (column) => column, + (column) => column.field, (column) => html` implements ReactiveController { protected readonly _host: GridHost; protected readonly _state: StateController; + /** The virtualizer currently being tracked for scrollbar changes. */ + #observed?: IgcVirtualizer; + #resizeObserver?: ResizeObserver; + + /** Last written offset in pixels. -1 marks "never measured". */ + #scrollOffset = -1; + #pendingFrame?: number; + + #onScrollbarChange = (): void => { + if (this.#pendingFrame !== undefined) { + return; + } + + this.#pendingFrame = requestAnimationFrame(() => { + this.#pendingFrame = undefined; + this.#applyScrollOffset(); + }); + }; + constructor(host: GridHost, state: StateController) { this._host = host; this._state = state; @@ -21,27 +44,60 @@ class GridDOMController implements ReactiveController { registerGridIcons(); this.setGridColumnSizes(); + // The virtualizer is part of the host template - wait for it before observing. this._host.updateComplete.then(() => { - this._state.virtualizer?.addEventListener( - 'visibilityChanged', - () => { - this.setScrollOffset(); - }, - { once: true } - ); + this.#observeVirtualizer(); }); } + public hostDisconnected(): void { + if (this.#pendingFrame !== undefined) { + cancelAnimationFrame(this.#pendingFrame); + this.#pendingFrame = undefined; + } + + this.#resizeObserver?.disconnect(); + this.#resizeObserver = undefined; + + this.#observed?.removeEventListener(VISIBILITY_CHANGED, this.#onScrollbarChange); + this.#observed = undefined; + } + public hostUpdate(): void { - this.setScrollOffset(); this.setGridColumnSizes(); } - public setScrollOffset(): void { - const size = this._state.virtualizer - ? this._state.virtualizer.offsetWidth - this._state.virtualizer.clientWidth - : 0; - this._host.style.setProperty('--scrollbar-offset', `${size}px`); + /** + * Watches the virtualizer for anything which can toggle its scrollbar: a new + * visible range, or a content box resize - the scrollbar itself shrinks the + * content box. Keeping the measurement here avoids a forced layout on every + * host update. + */ + #observeVirtualizer(): void { + const virtualizer = this._state.virtualizer; + + if (!virtualizer || virtualizer === this.#observed) { + return; + } + + this.#observed = virtualizer; + virtualizer.addEventListener(VISIBILITY_CHANGED, this.#onScrollbarChange); + + this.#resizeObserver = new ResizeObserver(this.#onScrollbarChange); + this.#resizeObserver.observe(virtualizer); + } + + /** Writes the scrollbar offset CSS variable, but only when the measurement changed. */ + #applyScrollOffset(): void { + const virtualizer = this.#observed; + const offset = virtualizer ? virtualizer.offsetWidth - virtualizer.clientWidth : 0; + + if (offset === this.#scrollOffset) { + return; + } + + this.#scrollOffset = offset; + this._host.style.setProperty(SCROLLBAR_OFFSET_VAR, `${offset}px`); } protected setGridColumnSizes(): void { diff --git a/src/controllers/filter.ts b/src/controllers/filter.ts index 33c0661..5d742e4 100644 --- a/src/controllers/filter.ts +++ b/src/controllers/filter.ts @@ -8,6 +8,9 @@ import { FilterState } from '../operations/filter/state.js'; import type { FilterExpression } from '../operations/filter/types.js'; import type { StateController } from './state.js'; +/** The kind of modification a `filtering` event describes. */ +export type FilterEventType = 'add' | 'modify' | 'remove'; + export class FilterController implements ReactiveController { private readonly _stateController: StateController; @@ -30,13 +33,9 @@ export class FilterController implements ReactiveController { return this._stateController.virtualizer; } - #emitFilteringEvent(expression: FilterExpression, type: 'add' | 'modify' | 'remove') { + #emitFilteringEvent(key: Keys, expressions: FilterExpression[], type: FilterEventType) { return this.host.emitEvent('filtering', { - detail: { - key: expression.key, - expressions: [expression], - type, - }, + detail: { key, expressions, type }, cancelable: true, }); } @@ -67,7 +66,7 @@ export class FilterController implements ReactiveController { } public reset(key?: Keys) { - key ? this.state.delete(key) : this.state.clear(); + key !== undefined ? this.state.delete(key) : this.state.clear(); } public setActiveColumn(column?: ColumnConfiguration) { @@ -94,30 +93,21 @@ export class FilterController implements ReactiveController { public async removeAllExpressions(key: Keys) { const state = this.get(key)?.all ?? []; - if ( - !this.host.emitEvent('filtering', { - detail: { - key, - expressions: state, - type: 'remove', - }, - cancelable: true, - }) - ) { + if (!this.#emitFilteringEvent(key, state, 'remove')) { return; } this.reset(key); this.#filter([]); - await this.host.updateComplete; + await this.host._pipelineComplete; this.#emitFilteredEvent({ key, state: this.get(key)?.all ?? [] }); } public async removeExpression(expression: FilterExpression) { const state = this.get(expression.key); - if (!this.#emitFilteringEvent(expression, 'remove')) { + if (!this.#emitFilteringEvent(expression.key, [expression], 'remove')) { return; } @@ -129,19 +119,36 @@ export class FilterController implements ReactiveController { this.#filter([]); - await this.host.updateComplete; + await this.host._pipelineComplete; this.#emitFilteredEvent({ key: expression.key, state: state?.all ?? [] }); } - public async filterWithEvent(expression: FilterExpression, type: 'add' | 'modify' | 'remove') { - if (!this.#emitFilteringEvent(expression, type)) { + /** + * Emits `filtering` for `expression` and, if it passes, applies it. + * + * @remarks + * `target` is the state-held expression the change belongs to. Callers pass a candidate + * copy as `expression` so that a canceled event leaves the stored state untouched; on + * commit the candidate is folded back into `target`, preserving the object identity the + * expression tree and the chip selection rely on. + */ + public async filterWithEvent( + expression: FilterExpression, + type: FilterEventType, + target: FilterExpression = expression + ) { + if (!this.#emitFilteringEvent(expression.key, [expression], type)) { return; } - this.#filter(expression); + if (target !== expression) { + Object.assign(target, expression); + } + + this.#filter(target); - await this.host.updateComplete; - this.#emitFilteredEvent({ key: expression.key, state: this.get(expression.key)?.all ?? [] }); + await this.host._pipelineComplete; + this.#emitFilteredEvent({ key: target.key, state: this.get(target.key)?.all ?? [] }); } public filter(expression: FilterExpression | FilterExpression[]) { diff --git a/src/controllers/navigation.ts b/src/controllers/navigation.ts index 4af79b7..0d3fe22 100644 --- a/src/controllers/navigation.ts +++ b/src/controllers/navigation.ts @@ -1,6 +1,6 @@ import type { ReactiveController } from 'lit'; import type IgcGridLiteRow from '../components/row.js'; -import { NAVIGATION_STATE, SENTINEL_NODE } from '../internal/constants.js'; +import { SENTINEL_NODE } from '../internal/constants.js'; import { GRID_ROW_TAG } from '../internal/tags.js'; import type { ActiveNode, Keys } from '../internal/types.js'; import type { StateController } from './state.js'; @@ -21,36 +21,37 @@ export class NavigationController implements ReactiveControlle return this._state.virtualizer; } - protected _navigationState = NAVIGATION_STATE; - protected _active = SENTINEL_NODE; + protected _active = SENTINEL_NODE as ActiveNode; protected get nextNode() { - const node = this._navigationState.get('current')!; - return node === SENTINEL_NODE - ? { column: this._firstColumn, row: 0 } - : ({ ...node } as ActiveNode); + return this._active === SENTINEL_NODE + ? ({ column: this._firstColumn, row: 0 } as ActiveNode) + : ({ ...this._active } as ActiveNode); } + /** Rows render only the visible columns, so navigation walks the same sequence. */ protected get _columns() { - return this._state.columns; + return this._state.columns.filter((column) => !column.hidden); } protected get _firstColumn() { - return this._state.host.getColumn(0)!.field ?? ''; + return this._columns.at(0)?.field ?? SENTINEL_NODE.column; } + // Both lookups clamp at the ends; a hidden or missing key (index -1) lands on the + // first visible column. protected getPreviousColumn(key: Keys) { - return this._columns[Math.max(this._columns.indexOf(this._state.host.getColumn(key)!) - 1, 0)] - .field; + const columns = this._columns; + const index = columns.findIndex((column) => column.field === key); + + return columns[Math.max(index - 1, 0)].field; } protected getNextColumn(key: Keys) { - return this._columns[ - Math.min( - this._columns.indexOf(this._state.host.getColumn(key)!) + 1, - this._columns.length - 1 - ) - ].field; + const columns = this._columns; + const index = columns.findIndex((column) => column.field === key); + + return columns[Math.min(index + 1, columns.length - 1)].field; } protected queryRowByIndex(index: number) { @@ -70,13 +71,11 @@ export class NavigationController implements ReactiveControlle } public get active(): ActiveNode { - return this._active as ActiveNode; + return this._active; } public set active(node: ActiveNode) { this._active = node ?? SENTINEL_NODE; - this._navigationState.set('previous', this._active); - this._navigationState.set('current', node); this._state.host.requestUpdate(); } @@ -123,14 +122,18 @@ export class NavigationController implements ReactiveControlle public hostDisconnected() { this.active = SENTINEL_NODE as ActiveNode; - this._navigationState = NAVIGATION_STATE; } public navigate(event: KeyboardEvent) { - if (this.handlers.has(event.key)) { - event.preventDefault(); - this.handlers.get(event.key)!.call(this); + const handler = this.handlers.get(event.key); + + // Nothing to move to without a visible column or a data row. + if (!handler || this._columns.length === 0 || this._state.host.totalItems === 0) { + return; } + + event.preventDefault(); + handler.call(this); } public async navigateTo(row: number, column?: Keys, activate = false) { diff --git a/src/controllers/resize.ts b/src/controllers/resize.ts index da19202..2aced8b 100644 --- a/src/controllers/resize.ts +++ b/src/controllers/resize.ts @@ -13,11 +13,13 @@ export class ResizeController implements ReactiveController { public indicatorOffset = 0; #maxSize(key: Keys, headerWidth: number) { - const max = this.host.rows - .map((row) => row.cells.find((cell) => cell.column.field === key)!) - .reduce((prev, current) => (current.offsetWidth > prev ? current.offsetWidth : prev), 0); + // A rendered row may not carry a cell for the column (e.g. it was just hidden). + const max = this.host.rows.reduce((prev, row) => { + const cell = row.cells.find((cell) => cell.column.field === key); + return cell && cell.offsetWidth > prev ? cell.offsetWidth : prev; + }, 0); - return Math.max(...[MIN_COL_RESIZE_WIDTH, max, headerWidth]); + return Math.max(MIN_COL_RESIZE_WIDTH, max, headerWidth); } /** diff --git a/src/controllers/sort.ts b/src/controllers/sort.ts index 853f9fa..3dadb29 100644 --- a/src/controllers/sort.ts +++ b/src/controllers/sort.ts @@ -15,10 +15,6 @@ export class SortController implements ReactiveController { return this.host.sortingOptions.mode === 'multiple'; } - get #isTriStateSort() { - return true; - } - #resolveSortOptions(column?: ColumnConfiguration) { const expr: Pick, 'caseSensitive' | 'comparer'> = { caseSensitive: false, @@ -45,16 +41,17 @@ export class SortController implements ReactiveController { } as SortingExpression; } + /** Tri-state cycle: ascending -> descending -> none. */ #orderBy(dir?: SortingDirection): SortingDirection { - return this.#isTriStateSort - ? dir === 'ascending' - ? 'descending' - : dir === 'descending' - ? 'none' - : 'ascending' - : dir === 'ascending' - ? 'descending' - : 'ascending'; + if (dir === 'ascending') { + return 'descending'; + } + + if (dir === 'descending') { + return 'none'; + } + + return 'ascending'; } #emitSortingEvent(detail: SortingExpression) { @@ -84,18 +81,26 @@ export class SortController implements ReactiveController { this._sort(expression); - await this.host.updateComplete; + await this.host._pipelineComplete; this.#emitSortedEvent(expression); } + /** + * Returns the expression the next sort operation would apply for `column`. + * + * @remarks + * The result is a candidate copy - the stored state is only updated once the + * operation is committed through {@link SortController._sort}. + */ public prepareExpression(column: ColumnConfiguration): SortingExpression { if (this.state.has(column.field)) { const expr = this.state.get(column.field)!; - return Object.assign(expr, { + return { + ...expr, direction: this.#orderBy(expr.direction), ...this.#resolveSortOptions(column), - }); + }; } // Initial state @@ -103,7 +108,7 @@ export class SortController implements ReactiveController { } public reset(key?: Keys) { - key ? this.state.delete(key) : this.state.clear(); + key !== undefined ? this.state.delete(key) : this.state.clear(); } protected _sort(expressions: SortingExpression | SortingExpression[]) { diff --git a/src/internal/constants.ts b/src/internal/constants.ts index 7adee17..1494e8c 100644 --- a/src/internal/constants.ts +++ b/src/internal/constants.ts @@ -1,4 +1,4 @@ -import type { ActiveNode, ColumnConfiguration, NavigationState } from './types.js'; +import type { ActiveNode, ColumnConfiguration } from './types.js'; const columnKey = Symbol(); const NON_EXISTING_COLUMN = Symbol(); @@ -21,7 +21,3 @@ export const DEFAULT_COLUMN_CONFIG = Object.freeze>({ sortable: false, filterable: false, }); -export const NAVIGATION_STATE: Map> = new Map([ - ['previous', SENTINEL_NODE], - ['current', SENTINEL_NODE], -]); diff --git a/src/internal/normalize-case.ts b/src/internal/normalize-case.ts index 4314caa..4b229db 100644 --- a/src/internal/normalize-case.ts +++ b/src/internal/normalize-case.ts @@ -1,3 +1,8 @@ -export function normalizeCase(string: string, caseSensitive?: boolean) { - return caseSensitive ? string : string.toLowerCase(); +/** + * Single coercion point for string comparisons: a missing record value is treated + * as an empty string so operands never dereference null/undefined. + */ +export function normalizeCase(string: string | null | undefined, caseSensitive?: boolean) { + const value = string ?? ''; + return caseSensitive ? value : value.toLowerCase(); } diff --git a/src/internal/types.ts b/src/internal/types.ts index 1fedfdd..0c31bc8 100644 --- a/src/internal/types.ts +++ b/src/internal/types.ts @@ -6,7 +6,6 @@ import type IgcGridLiteHeader from '../components/header.js'; import type IgcGridLiteRow from '../components/row.js'; import type { SortComparer } from '../operations/sort/types.js'; -export type NavigationState = 'previous' | 'current'; export type GridHost = ReactiveControllerHost & IgcGridLite; export type Themes = { diff --git a/src/internal/utils.ts b/src/internal/utils.ts index ce0a484..3b913d5 100644 --- a/src/internal/utils.ts +++ b/src/internal/utils.ts @@ -8,6 +8,23 @@ function _isObject(entity: unknown): entity is Record { return entity != null && typeof entity === 'object'; } +/** + * Dot-path -> segments cache. Filtering and sorting resolve the same handful of paths + * once per record, so the split is done once per path instead of per lookup. + */ +const pathSegments = new Map(); + +function getPathSegments(path: string): string[] { + let segments = pathSegments.get(path); + + if (!segments) { + segments = path.split('.'); + pathSegments.set(path, segments); + } + + return segments; +} + /** * Resolves a value from an object using a path string. * Supports nested properties using dot notation (e.g., 'prop.nestedProp'). @@ -18,7 +35,7 @@ function _isObject(entity: unknown): entity is Record { */ export function resolveFieldValue(obj: T, path: Keys): PropertyType { if (typeof path === 'string' && path.includes('.')) { - return path.split('.').reduce((current, key) => { + return getPathSegments(path).reduce((current, key) => { return _isObject(current) && key in current ? current[key] : undefined; }, obj) as PropertyType; } diff --git a/src/operations/base.ts b/src/operations/base.ts index 3be1641..3d9c137 100644 --- a/src/operations/base.ts +++ b/src/operations/base.ts @@ -1,3 +1,4 @@ +import { normalizeCase } from '../internal/normalize-case.js'; import type { Keys, PropertyType } from '../internal/types.js'; import { resolveFieldValue } from '../internal/utils.js'; @@ -7,7 +8,7 @@ export default abstract class DataOperation = Keys> { } protected resolveCase>(value: U, caseSensitive?: boolean) { - return typeof value === 'string' && !caseSensitive ? (value.toLowerCase() as U) : value; + return typeof value === 'string' ? (normalizeCase(value, caseSensitive) as U) : value; } public abstract apply(...args: unknown[]): T[]; diff --git a/src/operations/filter.ts b/src/operations/filter.ts index 10cc5dc..8609eba 100644 --- a/src/operations/filter.ts +++ b/src/operations/filter.ts @@ -17,6 +17,13 @@ export default class FilterDataOperation extends DataOperation if (ors.length > 0 && ors.some((expr) => this.resolveFilter(record, expr))) { return true; } + + // With no ANDs the ORs are the whole answer - `every` on an empty set is + // vacuously true and would otherwise let every record through. + if (ands.length === 0) { + return ors.length === 0; + } + return ands.every((expr) => this.resolveFilter(record, expr)); } diff --git a/src/operations/sort.ts b/src/operations/sort.ts index b29f283..1d31913 100644 --- a/src/operations/sort.ts +++ b/src/operations/sort.ts @@ -1,6 +1,9 @@ import DataOperation from './base.js'; import type { SortState } from './sort/types.js'; +// A single collator is reused across comparisons - `localeCompare` builds one per call. +const collator = new Intl.Collator(); + export default class SortDataOperation extends DataOperation { protected orderBy = new Map( Object.entries({ @@ -11,7 +14,7 @@ export default class SortDataOperation extends DataOperation { protected compareValues(first: U, second: U) { if (typeof first === 'string' && typeof second === 'string') { - return first.localeCompare(second); + return collator.compare(first, second); } return first > second ? 1 : first < second ? -1 : 0; } diff --git a/test/column-config.test.ts b/test/column-config.test.ts index 6948f78..3525369 100644 --- a/test/column-config.test.ts +++ b/test/column-config.test.ts @@ -47,6 +47,24 @@ describe('Column configuration', () => { } }); + it('Multiple columns inside a single slotted container', async () => { + const newKeys: Array> = ['id', 'name']; + const container = document.createElement('div'); + + container.append( + ...newKeys.map((key) => { + const col = document.createElement(GRID_COLUMN_TAG) as IgcGridLiteColumn; + col.field = key; + return col; + }) + ); + + TDD.grid.replaceChildren(container); + await elementUpdated(TDD.grid); + + expect(TDD.grid.columns.map((column) => column.field)).to.eql(newKeys); + }); + it('Updating configuration', async () => { await TDD.updateColumns([ { field: 'id', header: 'Primary' }, diff --git a/test/data-operation/filter-ui.test.ts b/test/data-operation/filter-ui.test.ts index 9cba372..cd6e278 100644 --- a/test/data-operation/filter-ui.test.ts +++ b/test/data-operation/filter-ui.test.ts @@ -3,10 +3,18 @@ import sinon from 'sinon'; import type { Keys } from '../../src/internal/types.js'; import { NumberOperands } from '../../src/operations/filter/operands/number.js'; import { StringOperands } from '../../src/operations/filter/operands/string.js'; -import type { FilterExpression, OperandKeys } from '../../src/operations/filter/types.js'; +import type { + FilterExpression, + FilterOperation, + OperandKeys, +} from '../../src/operations/filter/types.js'; import GridTestFixture from '../utils/grid-fixture.js'; import data, { type TestData } from '../utils/test-data.js'; +function conditionName(expression: FilterExpression): string { + return (expression.condition as FilterOperation).name; +} + class FilterFixture extends GridTestFixture { public override updateConfig(): void { this.columnConfig = this.columnConfig.map((config) => ({ @@ -348,6 +356,106 @@ describe('Grid UI filter', () => { expect(TDD.grid.totalItems).to.equal(8); }); + it('`filtered` is emitted after the data view is updated', async () => { + // Stands in for a remote filter - the result lands after the update which starts it. + TDD.grid.dataPipelineConfiguration = { + filter: async ({ data }) => { + await Promise.resolve(); + return data.filter((record) => record.name === 'a'); + }, + }; + + let totalOnFiltered = -1; + TDD.grid.addEventListener( + 'filtered', + () => { + totalOnFiltered = TDD.grid.totalItems; + }, + { once: true } + ); + + await TDD.activateFilterRow('name'); + await TDD.filterByInput('a'); + + expect(totalOnFiltered).to.equal(1); + }); + + it('Canceling `filtering` on input leaves the expression untouched', async () => { + await TDD.activateFilterRow('name'); + await TDD.filterByInput('a'); + + let stateDuringDispatch: FilterExpression[] = []; + TDD.grid.addEventListener( + 'filtering', + (e) => { + stateDuringDispatch = TDD.grid.filterExpressions; + e.preventDefault(); + }, + { once: true } + ); + + await TDD.filterByInput('b'); + + expect(stateDuringDispatch).lengthOf(1); + expect(stateDuringDispatch[0].searchTerm).to.equal('a'); + + expect(TDD.grid.filterExpressions[0].searchTerm).to.equal('a'); + expect(TDD.activeChips[0].textContent?.trim()).to.equal('a'); + expect(TDD.grid.totalItems).to.equal(2); + }); + + it('Canceling `filtering` on condition change leaves the expression untouched', async () => { + await TDD.activateFilterRow('importance'); + await TDD.filterByInput('l'); + + expect(TDD.grid.totalItems).to.equal(3); + + let stateDuringDispatch: FilterExpression[] = []; + TDD.grid.addEventListener( + 'filtering', + (e) => { + stateDuringDispatch = TDD.grid.filterExpressions; + e.preventDefault(); + }, + { once: true } + ); + + await TDD.changeFilterCondition('endsWith'); + + expect(conditionName(stateDuringDispatch[0])).to.equal('contains'); + expect(conditionName(TDD.grid.filterExpressions[0])).to.equal('contains'); + expect(TDD.grid.totalItems).to.equal(3); + }); + + it('Canceling `filtering` on criteria toggle leaves the expression untouched', async () => { + await TDD.activateFilterRow('importance'); + await TDD.changeFilterCondition('startsWith'); + + await TDD.filterByInput('l'); + await TDD.commitInput(); + await TDD.filterByInput('h'); + await TDD.commitInput(); + + expect(TDD.grid.totalItems).to.equal(0); + + let stateDuringDispatch: FilterExpression[] = []; + TDD.grid.addEventListener( + 'filtering', + (e) => { + stateDuringDispatch = TDD.grid.filterExpressions; + e.preventDefault(); + }, + { once: true } + ); + + await TDD.toggleCriteria(TDD.filterRow.activeCriteriaButtons[0]); + + expect(stateDuringDispatch[1].criteria).to.equal('and'); + expect(TDD.grid.filterExpressions[1].criteria).to.equal('and'); + expect(TDD.filterRow.activeCriteriaButtons[0].textContent?.trim()).to.equal('and'); + expect(TDD.grid.totalItems).to.equal(0); + }); + it('Modify event arguments mid-flight', async () => { const spy = sinon.spy(TDD.grid, 'emitEvent'); diff --git a/test/data-operation/filter.test.ts b/test/data-operation/filter.test.ts index 93f140b..fc690bc 100644 --- a/test/data-operation/filter.test.ts +++ b/test/data-operation/filter.test.ts @@ -56,11 +56,29 @@ class TDDFilterState { } } +interface NullishData { + name: string; + count: number; +} + +/** + * Records with null / undefined / missing values for both a string and a number column. + * The first record is fully populated so operand resolution picks the right data type. + */ +const nullishData = [ + { name: 'Alpha', count: 1 }, + { name: null, count: null }, + { name: undefined, count: undefined }, + {}, +] as unknown as NullishData[]; + const TDD = new TDDFilterState(data); +const NullishTDD = new TDDFilterState(nullishData); describe('Filter operations', () => { beforeEach(() => { TDD.clearState(); + NullishTDD.clearState(); }); describe('String operands', () => { @@ -277,6 +295,163 @@ describe('Filter operations', () => { }); }); + describe('Criteria resolution', () => { + it('Only ANDs -> every expression must match', () => { + TDD.addCondition('id', 'greaterThan', { searchTerm: 3 }) + .addCondition('id', 'lessThan', { searchTerm: 5 }) + .run(); + + expect(TDD.result).lengthOf(1); + expect(TDD.first.id).to.equal(4); + }); + + it('Only ORs -> non-matching records are filtered out', () => { + TDD.addCondition('id', 'equals', { searchTerm: 1, criteria: 'or' }) + .addCondition('id', 'equals', { searchTerm: 2, criteria: 'or' }) + .run(); + + expect(TDD.result).lengthOf(2); + expect(TDD.first.id).to.equal(1); + expect(TDD.last.id).to.equal(2); + }); + + it('Only ORs -> no match leaves an empty result', () => { + TDD.addCondition('name', 'equals', { searchTerm: 'nope', criteria: 'or' }) + .addCondition('name', 'equals', { searchTerm: 'nada', criteria: 'or' }) + .run(); + + expect(TDD.result).empty; + }); + + it('Mixed -> ORs pass through, ANDs still apply', () => { + TDD.addCondition('id', 'greaterThan', { searchTerm: 3 }) + .addCondition('id', 'lessThan', { searchTerm: 5 }) + .addCondition('id', 'greaterThanOrEqual', { searchTerm: 6, criteria: 'or' }) + .run(); + + expect(TDD.result).lengthOf(4); + expect(TDD.first.id).to.equal(4); + expect(TDD.last.id).to.equal(8); + }); + }); + + describe('Nullish values [string operands]', () => { + it('`contains` [case insensitive]', () => { + NullishTDD.addCondition('name', 'contains', { searchTerm: 'a' }).run(); + expect(NullishTDD.result).lengthOf(1); + }); + + it('`contains` [case sensitive]', () => { + NullishTDD.addCondition('name', 'contains', { searchTerm: 'A', caseSensitive: true }).run(); + expect(NullishTDD.result).lengthOf(1); + }); + + it('`doesNotContain` [case insensitive]', () => { + NullishTDD.addCondition('name', 'doesNotContain', { searchTerm: 'a' }).run(); + expect(NullishTDD.result).lengthOf(3); + }); + + it('`doesNotContain` [case sensitive]', () => { + NullishTDD.addCondition('name', 'doesNotContain', { + searchTerm: 'A', + caseSensitive: true, + }).run(); + expect(NullishTDD.result).lengthOf(3); + }); + + it('`startsWith` [case insensitive]', () => { + NullishTDD.addCondition('name', 'startsWith', { searchTerm: 'al' }).run(); + expect(NullishTDD.result).lengthOf(1); + }); + + it('`startsWith` [case sensitive]', () => { + NullishTDD.addCondition('name', 'startsWith', { + searchTerm: 'Al', + caseSensitive: true, + }).run(); + expect(NullishTDD.result).lengthOf(1); + }); + + it('`endsWith` [case insensitive]', () => { + NullishTDD.addCondition('name', 'endsWith', { searchTerm: 'HA' }).run(); + expect(NullishTDD.result).lengthOf(1); + }); + + it('`endsWith` [case sensitive]', () => { + NullishTDD.addCondition('name', 'endsWith', { searchTerm: 'ha', caseSensitive: true }).run(); + expect(NullishTDD.result).lengthOf(1); + }); + + it('`equals` [case insensitive]', () => { + NullishTDD.addCondition('name', 'equals', { searchTerm: 'alpha' }).run(); + expect(NullishTDD.result).lengthOf(1); + }); + + it('`equals` [case sensitive]', () => { + NullishTDD.addCondition('name', 'equals', { searchTerm: 'Alpha', caseSensitive: true }).run(); + expect(NullishTDD.result).lengthOf(1); + }); + + it('`equals` empty string matches nullish values', () => { + NullishTDD.addCondition('name', 'equals', { searchTerm: '' }).run(); + expect(NullishTDD.result).lengthOf(3); + }); + + it('`doesNotEqual` [case insensitive]', () => { + NullishTDD.addCondition('name', 'doesNotEqual', { searchTerm: 'alpha' }).run(); + expect(NullishTDD.result).lengthOf(3); + }); + + it('`doesNotEqual` [case sensitive]', () => { + NullishTDD.addCondition('name', 'doesNotEqual', { + searchTerm: 'Alpha', + caseSensitive: true, + }).run(); + expect(NullishTDD.result).lengthOf(3); + }); + + it('`empty` / `notEmpty`', () => { + NullishTDD.addCondition('name', 'empty').run(); + expect(NullishTDD.result).lengthOf(3); + + NullishTDD.clearState(); + NullishTDD.addCondition('name', 'notEmpty').run(); + expect(NullishTDD.result).lengthOf(1); + }); + }); + + describe('Nullish values [number operands]', () => { + it('`equals` does not match nullish', () => { + NullishTDD.addCondition('count', 'equals', { searchTerm: 1 }).run(); + expect(NullishTDD.result).lengthOf(1); + }); + + it('`doesNotEqual` matches nullish', () => { + NullishTDD.addCondition('count', 'doesNotEqual', { searchTerm: 1 }).run(); + expect(NullishTDD.result).lengthOf(3); + }); + + it('`greaterThan` does not match nullish', () => { + NullishTDD.addCondition('count', 'greaterThan', { searchTerm: 0 }).run(); + expect(NullishTDD.result).lengthOf(1); + }); + + it('`lessThan` does not match undefined', () => { + // `null` coerces to 0 in relational comparisons, `undefined` yields NaN. + NullishTDD.addCondition('count', 'lessThan', { searchTerm: 5 }).run(); + expect(NullishTDD.result).lengthOf(2); + }); + + it('`empty` / `notEmpty`', () => { + NullishTDD.addCondition('count', 'empty').run(); + expect(NullishTDD.result).lengthOf(3); + + NullishTDD.clearState(); + NullishTDD.addCondition('count', 'notEmpty').run(); + expect(NullishTDD.result).lengthOf(1); + }); + }); + describe('Nested field operands', () => { it('`contains` on nested path [case insensitive]', () => { TDD.addCondition('address.city', 'contains', { searchTerm: 'new' }).run(); diff --git a/test/data-operation/pipeline.test.ts b/test/data-operation/pipeline.test.ts new file mode 100644 index 0000000..6446606 --- /dev/null +++ b/test/data-operation/pipeline.test.ts @@ -0,0 +1,79 @@ +import { elementUpdated, expect } from '@open-wc/testing'; +import sinon from 'sinon'; +import type { DataPipelineHook } from '../../src/internal/types.js'; +import GridTestFixture from '../utils/grid-fixture.js'; +import data, { type TestData } from '../utils/test-data.js'; + +type PendingCall = { + data: TestData[]; + resolve: (value: TestData[]) => void; +}; + +/** Sort hook which defers every call so the test controls the response order. */ +function deferredSort(calls: PendingCall[]): DataPipelineHook { + return ({ data }) => new Promise((resolve) => calls.push({ data, resolve })); +} + +function byId(records: TestData[], direction: 'ascending' | 'descending'): TestData[] { + const sorted = [...records].sort((a, b) => a.id - b.id); + return direction === 'ascending' ? sorted : sorted.reverse(); +} + +class PipelineFixture extends GridTestFixture { + public settle() { + return this.waitForUpdate(); + } +} + +const TDD = new PipelineFixture(data); + +describe('Grid data pipeline', () => { + beforeEach(async () => { + await TDD.setUp(); + }); + + afterEach(() => { + TDD.tearDown(); + }); + + it('Applies the latest result when responses arrive out of order', async () => { + const calls: PendingCall[] = []; + TDD.grid.dataPipelineConfiguration = { sort: deferredSort(calls) }; + + TDD.grid.sort({ key: 'id', direction: 'ascending' }); + await elementUpdated(TDD.grid); + + TDD.grid.sort({ key: 'id', direction: 'descending' }); + await elementUpdated(TDD.grid); + + expect(calls).lengthOf(2); + + // The newer request settles first, the stale one right after it. + calls[1].resolve(byId(calls[1].data, 'descending')); + calls[0].resolve(byId(calls[0].data, 'ascending')); + + await TDD.settle(); + + expect(TDD.grid.dataView[0].id).to.equal(8); + }); + + it('A rejected hook keeps the previous data state', async () => { + // The grid surfaces pipeline failures through console.error - capture them. + const stub = sinon.stub(console, 'error'); + + try { + TDD.grid.dataPipelineConfiguration = { + sort: () => Promise.reject(new Error('pipeline failure')), + }; + + await TDD.sort({ key: 'id', direction: 'descending' }); + + expect(TDD.grid.dataView).lengthOf(data.length); + expect(TDD.grid.dataView[0].id).to.equal(data[0].id); + expect(stub.firstCall?.firstArg).to.be.instanceOf(Error); + expect(stub.firstCall.firstArg.message).to.equal('pipeline failure'); + } finally { + stub.restore(); + } + }); +}); diff --git a/test/data-operation/sort-ui.test.ts b/test/data-operation/sort-ui.test.ts index 331b636..0659d5e 100644 --- a/test/data-operation/sort-ui.test.ts +++ b/test/data-operation/sort-ui.test.ts @@ -2,8 +2,9 @@ import { expect } from '@open-wc/testing'; import sinon from 'sinon'; import { SORT_ICON_ASCENDING, SORT_ICON_DESCENDING } from '../../src/internal/constants.js'; import type { Keys } from '../../src/internal/types.js'; +import type { SortingExpression } from '../../src/operations/sort/types.js'; import GridTestFixture from '../utils/grid-fixture.js'; -import data, { importanceComparer } from '../utils/test-data.js'; +import data, { importanceComparer, type TestData } from '../utils/test-data.js'; class SortFixture extends GridTestFixture { public override updateConfig(): void { @@ -222,6 +223,53 @@ describe('Grid UI sort', () => { expect(TDD.rows.first.data.id).to.equal(1); }); + it('`sorted` is emitted after the data view is updated', async () => { + // Stands in for a remote sort - the result lands after the update which starts it. + TDD.grid.dataPipelineConfiguration = { + sort: async ({ data }) => { + await Promise.resolve(); + return [...data].reverse(); + }, + }; + + let idOnSorted = -1; + TDD.grid.addEventListener( + 'sorted', + () => { + idOnSorted = TDD.grid.dataView[0].id; + }, + { once: true } + ); + + await TDD.sortHeader('id'); + + expect(idOnSorted).to.equal(8); + }); + + it('Canceling `sorting` leaves the sort state untouched', async () => { + await TDD.sortHeader('id'); + + let stateDuringDispatch: SortingExpression[] = []; + TDD.grid.addEventListener( + 'sorting', + (e) => { + stateDuringDispatch = TDD.grid.sortingExpressions; + e.preventDefault(); + }, + { once: true } + ); + + await TDD.sortHeader('id'); + + // The listener must not observe the pending descending direction. + expect(stateDuringDispatch).lengthOf(1); + expect(stateDuringDispatch[0].direction).to.equal('ascending'); + + expect(TDD.grid.sortingExpressions[0].direction).to.equal('ascending'); + expect(TDD.rows.first.data.id).to.equal(1); + TDD.indicatorIsAscending('id'); + }); + it('Modify event arguments mid-flight', async () => { const spy = sinon.spy(TDD.grid, 'emitEvent'); TDD.grid.addEventListener( diff --git a/test/dom.test.ts b/test/dom.test.ts new file mode 100644 index 0000000..9d6f3f5 --- /dev/null +++ b/test/dom.test.ts @@ -0,0 +1,69 @@ +import { expect, nextFrame } from '@open-wc/testing'; +import GridTestFixture from './utils/grid-fixture.js'; +import data, { type TestData } from './utils/test-data.js'; + +const SCROLLBAR_OFFSET_VAR = '--scrollbar-offset'; + +/** Stand-in for a scrollbar - headless Chromium renders overlay ones with zero width. */ +const FAKE_SCROLLBAR_WIDTH = 20; + +class DomFixture extends GridTestFixture { + public get scrollbarOffset(): string { + return this.grid.style.getPropertyValue(SCROLLBAR_OFFSET_VAR); + } + + /** + * Widens the virtualizer border so that `offsetWidth - clientWidth` reports a + * non-zero value, the way a classic scrollbar would. + */ + public async addFakeScrollbar(): Promise { + const style = document.createElement('style'); + style.textContent = `igc-virtualizer { border-inline-end: ${FAKE_SCROLLBAR_WIDTH}px solid transparent; }`; + this.grid.renderRoot.appendChild(style); + + // Resize delivery + the deferred CSS variable write. + await nextFrame(); + await nextFrame(); + await nextFrame(); + } +} + +describe('Grid DOM reuse', () => { + const TDD = new DomFixture(data); + + beforeEach(async () => await TDD.setUp()); + afterEach(() => TDD.tearDown()); + + it('keeps header and cell elements on a column configuration update', async () => { + const header = TDD.headers.get('name').element; + const cell = TDD.rows.first.cells.get('name').element; + + await TDD.updateColumns({ field: 'name', header: 'Updated' }); + + expect(TDD.headers.get('name').text).to.equal('Updated'); + expect(TDD.headers.get('name').element === header).to.be.true; + expect(TDD.rows.first.cells.get('name').element === cell).to.be.true; + }); +}); + +describe('Grid scrollbar offset', () => { + const TDD = new DomFixture(data, { height: '100px' }); + + beforeEach(async () => await TDD.setUp()); + afterEach(() => TDD.tearDown()); + + it('tracks the virtualizer scrollbar without a host update', async () => { + expect(TDD.scrollbarOffset).to.equal('0px'); + + await TDD.addFakeScrollbar(); + + expect(TDD.scrollbarOffset).to.equal(`${FAKE_SCROLLBAR_WIDTH}px`); + }); + + it('keeps the offset up to date when the data source changes', async () => { + await TDD.addFakeScrollbar(); + await TDD.updateProperty('data', [data[0]]); + + expect(TDD.scrollbarOffset).to.equal(`${FAKE_SCROLLBAR_WIDTH}px`); + }); +}); diff --git a/test/grid.properties.test.ts b/test/grid.properties.test.ts index c7b97fb..b678e65 100644 --- a/test/grid.properties.test.ts +++ b/test/grid.properties.test.ts @@ -106,6 +106,17 @@ describe('Grid auto-generate column configuration', () => { expect(autoGenerateTDD.grid.rows).lengthOf(testData.length); }); + + it('Resetting the columns collection re-runs auto generation', async () => { + expect(autoGenerateTDD.grid.columns).lengthOf(keys.size); + + autoGenerateTDD.grid.columns = []; + expect(autoGenerateTDD.grid.columns).to.be.empty; + + await autoGenerateTDD.updateProperty('data', [{ alpha: 1, beta: 'x' }] as any); + + expect(autoGenerateTDD.grid.columns.map((column) => column.field)).to.eql(['alpha', 'beta']); + }); }); describe('Grid properties (initial bindings)', () => { @@ -211,6 +222,38 @@ describe('Grid properties', () => { expect(TDD.grid.filterExpressions).lengthOf(3); }); + + it('Sort expressions (get) returns copies', async () => { + await TDD.sort({ key: 'id', direction: 'ascending' }); + + const [expression] = TDD.grid.sortingExpressions; + expression.direction = 'descending'; + + expect(TDD.grid.sortingExpressions[0].direction).to.equal('ascending'); + }); + + it('Filter expressions (get) returns copies', async () => { + await TDD.filter({ key: 'name', condition: 'contains', searchTerm: 'a' }); + + const [expression] = TDD.grid.filterExpressions; + expression.searchTerm = 'b'; + + expect(TDD.grid.filterExpressions[0].searchTerm).to.equal('a'); + expect(TDD.grid.totalItems).to.equal(2); + }); + + it('filter() does not mutate the passed expressions', async () => { + const expression: FilterExpression = { + key: 'name', + condition: 'contains', + searchTerm: 'a', + }; + + await TDD.filter(expression); + + expect(expression.condition).to.equal('contains'); + expect(expression.criteria).to.be.undefined; + }); }); describe('Grid properties (late column slotting)', () => { diff --git a/test/navigation.test.ts b/test/navigation.test.ts new file mode 100644 index 0000000..09e9aef --- /dev/null +++ b/test/navigation.test.ts @@ -0,0 +1,162 @@ +import { setupIgnoreWindowResizeObserverLoopErrors } from '@lit-labs/virtualizer/support/resize-observer-errors.js'; +import { elementUpdated, expect, fixture, fixtureCleanup, html, nextFrame } from '@open-wc/testing'; +import type { TemplateResult } from 'lit'; +import { IgcGridLite } from '../src/components/grid.js'; +import IgcVirtualizer from '../src/components/virtualizer.js'; +import { GRID_TAG } from '../src/internal/tags.js'; + +interface Item { + id: number; + name: string; + city: string; +} + +const FIELDS = ['id', 'name', 'city'] as const; + +const data: Item[] = [ + { id: 1, name: 'Anna', city: 'Sofia' }, + { id: 2, name: 'Boris', city: 'Varna' }, + { id: 3, name: 'Clara', city: 'Burgas' }, + { id: 4, name: 'Doris', city: 'Ruse' }, +]; + +function gridTemplate(hidden: string[] = [], items: Item[] = data): TemplateResult { + return html` + + ${FIELDS.map( + (field) => + html`` + )} + + `; +} + +function bodyOf(grid: IgcGridLite): IgcVirtualizer { + return grid.renderRoot.querySelector(IgcVirtualizer.tagName)!; +} + +async function setup(...templates: TemplateResult[]): Promise[]> { + IgcGridLite.register(); + + const wrapper = await fixture( + html`
${templates}
` + ); + const grids = Array.from(wrapper.querySelectorAll>(GRID_TAG)); + + // An empty grid never runs a virtualizer layout, so wait on the host update instead. + await Promise.all( + grids.map((grid) => (grid.data.length > 0 ? bodyOf(grid).layoutComplete : elementUpdated(grid))) + ); + + return grids; +} + +async function press(grid: IgcGridLite, key: string): Promise { + bodyOf(grid).dispatchEvent(new KeyboardEvent('keydown', { key, bubbles: true, composed: true })); + await Promise.all([elementUpdated(grid), nextFrame()]); + await nextFrame(); +} + +/** The active cell of a grid as a plain node, or undefined when nothing is active. */ +function activeCell(grid: IgcGridLite) { + for (const row of grid.rows) { + const cell = row.cells.find((each) => each.active); + + if (cell) { + return { row: row.index, column: cell.column.field }; + } + } + + return undefined; +} + +describe('Grid navigation', () => { + setupIgnoreWindowResizeObserverLoopErrors(beforeEach, afterEach); + afterEach(() => fixtureCleanup()); + + describe('Per-instance state', () => { + it('Keyboard navigation in one grid is not affected by another', async () => { + const [first, second] = await setup(gridTemplate(), gridTemplate()); + + await first.navigateTo(0, 'id', true); + await second.navigateTo(2, 'city', true); + + await press(first, 'ArrowDown'); + + expect(activeCell(first)).to.eql({ row: 1, column: 'id' }); + expect(activeCell(second)).to.eql({ row: 2, column: 'city' }); + }); + + it('Disconnecting a grid does not reset another', async () => { + const [first, second] = await setup(gridTemplate(), gridTemplate()); + + await first.navigateTo(0, 'id', true); + await second.navigateTo(1, 'name', true); + + first.remove(); + await nextFrame(); + + await press(second, 'ArrowDown'); + expect(activeCell(second)).to.eql({ row: 2, column: 'name' }); + }); + + it('Nullish active node falls back to the initial position', async () => { + const [grid] = await setup(gridTemplate()); + + await grid.navigateTo(2, 'city', true); + + // @ts-expect-error - private member access + grid._stateController.active = null; + await press(grid, 'ArrowDown'); + + expect(activeCell(grid)).to.eql({ row: 1, column: 'id' }); + }); + }); + + describe('Hidden columns', () => { + it('ArrowRight skips a hidden column', async () => { + const [grid] = await setup(gridTemplate(['name'])); + + await grid.navigateTo(0, 'id', true); + await press(grid, 'ArrowRight'); + + expect(activeCell(grid)).to.eql({ row: 0, column: 'city' }); + }); + + it('ArrowLeft skips a hidden column', async () => { + const [grid] = await setup(gridTemplate(['name'])); + + await grid.navigateTo(0, 'city', true); + await press(grid, 'ArrowLeft'); + + expect(activeCell(grid)).to.eql({ row: 0, column: 'id' }); + }); + + it('Initial position is the first visible column', async () => { + const [grid] = await setup(gridTemplate(['id'])); + + await press(grid, 'ArrowDown'); + + expect(activeCell(grid)).to.eql({ row: 1, column: 'name' }); + }); + }); + + describe('Empty grid', () => { + it('Navigation is a no-op without columns and rows', async () => { + const [grid] = await setup(html``); + + const navigate = () => { + // @ts-expect-error - private member access + grid._stateController.navigation.navigate( + new KeyboardEvent('keydown', { key: 'ArrowDown' }) + ); + }; + + expect(navigate).to.not.throw(); + expect(activeCell(grid)).to.be.undefined; + }); + }); +}); diff --git a/test/nested-field.test.ts b/test/nested-field.test.ts index 0b52377..0917712 100644 --- a/test/nested-field.test.ts +++ b/test/nested-field.test.ts @@ -63,4 +63,20 @@ describe('resolveFieldValue utility', () => { const partialObj = { id: 1, user: null } as unknown as NestedTestData; expect(resolveFieldValue(partialObj, 'user.name')).to.be.undefined; }); + + it('should resolve the same path across different objects', () => { + for (const record of nestedData) { + expect(resolveFieldValue(record, 'user.address.city')).to.equal(record.user.address.city); + } + + // Repeated resolution must stay stable - path segments are cached between calls. + expect(resolveFieldValue(nestedData[0], 'user.address.city')).to.equal('New York'); + expect(resolveFieldValue(nestedData[1], 'user.address.city')).to.equal('Los Angeles'); + }); + + it('should not confuse a dotted path with a same-prefixed one', () => { + expect(resolveFieldValue(testObj, 'user.name')).to.equal('Alice'); + expect(resolveFieldValue(testObj, 'user.address.code')).to.equal(10001); + expect(resolveFieldValue(testObj, 'user.age')).to.equal(30); + }); }); diff --git a/test/resize.test.ts b/test/resize.test.ts index 13329ce..04bb742 100644 --- a/test/resize.test.ts +++ b/test/resize.test.ts @@ -1,4 +1,5 @@ import { expect, html } from '@open-wc/testing'; +import type { ResizeController } from '../src/controllers/resize.js'; import { MIN_COL_RESIZE_WIDTH } from '../src/internal/constants.js'; import type { Keys } from '../src/internal/types.js'; import GridTestFixture from './utils/grid-fixture.js'; @@ -23,6 +24,11 @@ class ResizeFixture extends GridTestFixture { expect(this.rows.first.cells.get(name).element.offsetWidth).to.equal(initial + delta); } + public get resizeController() { + // @ts-expect-error - Protected member access + return this.grid._stateController.resizing as ResizeController; + } + public getTranslate(element: HTMLElement) { const matrix = new DOMMatrixReadOnly(getComputedStyle(element).getPropertyValue('transform')); return { @@ -114,5 +120,16 @@ describe('Column resizing', () => { expect(header.offsetWidth).greaterThan(600); expect(header.offsetWidth).to.equal(cell.offsetWidth); }); + + it('Auto size (column without rendered cells)', async () => { + const header = TDD.headers.get('name').element; + const column = TDD.grid.getColumn('name')!; + + // Hiding the column drops its cells from the rendered rows. + await TDD.updateColumns({ field: 'name', hidden: true }); + + await TDD.resizeController.autosize(column, header); + expect(column.width).to.equal(`${MIN_COL_RESIZE_WIDTH}px`); + }); }); });