From 7f37e8ddc31153e03599f8e7eec101b32c095473 Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Thu, 2 Feb 2023 17:04:10 -0600
Subject: [PATCH 01/23] partial
---
.../src/table-column/base/index.ts | 5 +-
.../text/tests/table-column-text.spec.ts | 2 +-
packages/nimble-components/src/table/index.ts | 44 +-
.../src/table/models/table-validator.spec.ts | 453 +++++++++---------
.../src/table/models/table-validator.ts | 36 +-
.../nimble-components/src/table/template.ts | 25 +-
.../src/table/tests/table.stories.ts | 8 +-
packages/nimble-components/src/table/types.ts | 2 +
8 files changed, 306 insertions(+), 269 deletions(-)
diff --git a/packages/nimble-components/src/table-column/base/index.ts b/packages/nimble-components/src/table-column/base/index.ts
index 414961d441..6696c5d699 100644
--- a/packages/nimble-components/src/table-column/base/index.ts
+++ b/packages/nimble-components/src/table-column/base/index.ts
@@ -1,4 +1,4 @@
-import type { ElementStyles, ViewTemplate } from '@microsoft/fast-element';
+import { attr, ElementStyles, ViewTemplate } from '@microsoft/fast-element';
import { FoundationElement } from '@microsoft/fast-foundation';
import type {
TableCellRecord,
@@ -13,6 +13,9 @@ export abstract class TableColumn<
TCellRecord extends TableCellRecord = TableCellRecord,
TColumnConfig = unknown
> extends FoundationElement {
+ @attr({ attribute: 'column-id' })
+ public columnId?: string | null;
+
/**
* The template to use to render the cell content for the column
*/
diff --git a/packages/nimble-components/src/table-column/text/tests/table-column-text.spec.ts b/packages/nimble-components/src/table-column/text/tests/table-column-text.spec.ts
index 25e32c6154..56480fc4c6 100644
--- a/packages/nimble-components/src/table-column/text/tests/table-column-text.spec.ts
+++ b/packages/nimble-components/src/table-column/text/tests/table-column-text.spec.ts
@@ -62,7 +62,7 @@ describe('TableColumnText', () => {
});
}
- it('changing fieldName updates display', async () => {
+ fit('changing fieldName updates display', async () => {
element.setData([{ field: 'foo', anotherField: 'bar' }]);
await connect();
await waitForUpdatesAsync();
diff --git a/packages/nimble-components/src/table/index.ts b/packages/nimble-components/src/table/index.ts
index ee06b7ce6a..2e301cb9f5 100644
--- a/packages/nimble-components/src/table/index.ts
+++ b/packages/nimble-components/src/table/index.ts
@@ -9,7 +9,7 @@ import {
getCoreRowModel as tanStackGetCoreRowModel,
TableOptionsResolved as TanStackTableOptionsResolved
} from '@tanstack/table-core';
-import type { TableColumn } from '../table-column/base';
+import { TableColumn } from '../table-column/base';
import { TableValidator } from './models/table-validator';
import { styles } from './styles';
import { template } from './template';
@@ -38,7 +38,7 @@ export class Table<
public tableData: TableRowState<TData>[] = [];
@observable
- public readonly columns: TableColumn[] = [];
+ public columns: TableColumn[] = [];
/**
* @internal
@@ -55,6 +55,9 @@ export class Table<
*/
public readonly viewport!: HTMLElement;
+ @observable
+ public childItems: Element[] = [];
+
/**
* @internal
*/
@@ -84,18 +87,12 @@ export class Table<
this.setTableData(newData);
}
- public idFieldNameChanged(
- _prev: string | undefined,
- _next: string | undefined
- ): void {
- // Force TanStack to detect a data update because a row's ID is only
- // generated when creating a new row model.
- this.setTableData(this.table.options.data);
- }
-
public override connectedCallback(): void {
super.connectedCallback();
this.virtualizer.connectedCallback();
+
+ this.tableValidator.validateColumnIds(this.columns.map(x => x.columnId));
+ this.canRenderRows = this.checkValidity();
}
public override disconnectedCallback(): void {
@@ -106,6 +103,31 @@ export class Table<
return this.tableValidator.isValid();
}
+ private idFieldNameChanged(
+ _prev: string | undefined,
+ _next: string | undefined
+ ): void {
+ // Force TanStack to detect a data update because a row's ID is only
+ // generated when creating a new row model.
+ this.setTableData(this.table.options.data);
+ }
+
+ private columnsChanged(
+ _prev: TableColumn[] | undefined,
+ _next: TableColumn[]
+ ): void {
+ if (!this.$fastController.isConnected) {
+ return;
+ }
+
+ this.tableValidator.validateColumnIds(this.columns.map(x => x.columnId));
+ this.canRenderRows = this.checkValidity();
+ }
+
+ private childItemsChanged(_prev: undefined | Element[], next: Element[]): void {
+ this.columns = next.filter(x => x instanceof TableColumn).map(x => x as TableColumn);
+ }
+
private setTableData(newData: readonly TData[]): void {
const data = newData.map(record => {
return { ...record };
diff --git a/packages/nimble-components/src/table/models/table-validator.spec.ts b/packages/nimble-components/src/table/models/table-validator.spec.ts
index b7f41cf68d..74b1b285ef 100644
--- a/packages/nimble-components/src/table/models/table-validator.spec.ts
+++ b/packages/nimble-components/src/table/models/table-validator.spec.ts
@@ -1,250 +1,229 @@
+import type { TableRecord, TableValidity } from '../types';
import { TableValidator } from './table-validator';
-describe('TableValidator', () => {
- it('setting valid field for ID is valid', () => {
- const data = [
- { stringField: 'value-1', numberField: 10 },
- { stringField: 'value-2', numberField: 11 }
- ];
- const validator = new TableValidator();
-
- const isValid = validator.validateRecordIds(data, 'stringField');
- expect(isValid).toBeTrue();
- expect(validator.isValid()).toBeTrue();
- const validity = validator.getValidity();
- expect(validity.duplicateRecordId).toBeFalse();
- expect(validity.invalidRecordId).toBeFalse();
- expect(validity.missingRecordId).toBeFalse();
- });
-
- it('setting `undefined` field for ID is valid', () => {
- const data = [
- { stringField: 'value-1', numberField: 10 },
- { stringField: 'value-2', numberField: 11 }
- ];
- const validator = new TableValidator();
-
- const isValid = validator.validateRecordIds(data, undefined);
- expect(isValid).toBeTrue();
- expect(validator.isValid()).toBeTrue();
- const validity = validator.getValidity();
- expect(validity.duplicateRecordId).toBeFalse();
- expect(validity.invalidRecordId).toBeFalse();
- expect(validity.missingRecordId).toBeFalse();
- });
-
- it('setting `null` field for ID is valid', () => {
- const data = [
- { stringField: 'value-1', numberField: 10 },
- { stringField: 'value-2', numberField: 11 }
- ];
- const validator = new TableValidator();
-
- const isValid = validator.validateRecordIds(data, null);
- expect(isValid).toBeTrue();
- expect(validator.isValid()).toBeTrue();
- const validity = validator.getValidity();
- expect(validity.duplicateRecordId).toBeFalse();
- expect(validity.invalidRecordId).toBeFalse();
- expect(validity.missingRecordId).toBeFalse();
- });
-
- it('setting data with duplicate IDs is invalid', () => {
- const data = [
- { stringField: 'value-1', numberField: 10 },
- { stringField: 'value-1', numberField: 11 }
- ];
- const validator = new TableValidator();
-
- const isValid = validator.validateRecordIds(data, 'stringField');
- expect(isValid).toBeFalse();
- expect(validator.isValid()).toBeFalse();
- const validity = validator.getValidity();
- expect(validity.duplicateRecordId).toBeTrue();
- expect(validity.invalidRecordId).toBeFalse();
- expect(validity.missingRecordId).toBeFalse();
- });
-
- it('setting data with invalid ID value type is invalid', () => {
- const data = [
- { stringField: 'value-1', numberField: 10 },
- { stringField: 'value-2', numberField: 11 }
- ];
- const validator = new TableValidator();
-
- const isValid = validator.validateRecordIds(data, 'numberField');
- expect(isValid).toBeFalse();
- expect(validator.isValid()).toBeFalse();
- const validity = validator.getValidity();
- expect(validity.duplicateRecordId).toBeFalse();
- expect(validity.invalidRecordId).toBeTrue();
- expect(validity.missingRecordId).toBeFalse();
- });
-
- it('setting data with empty ID value is valid', () => {
- const data = [
- { stringField: 'value-1', numberField: 10 },
- { stringField: '', numberField: 11 }
- ];
- const validator = new TableValidator();
-
- const isValid = validator.validateRecordIds(data, 'stringField');
- expect(isValid).toBeTrue();
- expect(validator.isValid()).toBeTrue();
- const validity = validator.getValidity();
- expect(validity.duplicateRecordId).toBeFalse();
- expect(validity.invalidRecordId).toBeFalse();
- expect(validity.missingRecordId).toBeFalse();
- });
-
- it('setting data with undefined ID value is invalid', () => {
- const data = [
- { stringField: 'value-1', numberField: 10 },
- { stringField: undefined, numberField: 11 }
- ];
- const validator = new TableValidator();
-
- const isValid = validator.validateRecordIds(data, 'stringField');
- expect(isValid).toBeFalse();
- expect(validator.isValid()).toBeFalse();
- const validity = validator.getValidity();
- expect(validity.duplicateRecordId).toBeFalse();
- expect(validity.invalidRecordId).toBeTrue();
- expect(validity.missingRecordId).toBeFalse();
- });
-
- it('setting data with null ID value is invalid', () => {
- const data = [
- { stringField: 'value-1', numberField: 10 },
- { stringField: undefined, numberField: 11 }
- ];
- const validator = new TableValidator();
-
- const isValid = validator.validateRecordIds(data, 'stringField');
- expect(isValid).toBeFalse();
- expect(validator.isValid()).toBeFalse();
- const validity = validator.getValidity();
- expect(validity.duplicateRecordId).toBeFalse();
- expect(validity.invalidRecordId).toBeTrue();
- expect(validity.missingRecordId).toBeFalse();
- });
-
- it('setting data with missing IDs is invalid', () => {
- const data = [
- { stringField: 'value-1', numberField: 10 },
- { stringField: 'value-2', numberField: 11 }
- ];
- const validator = new TableValidator();
-
- const isValid = validator.validateRecordIds(data, 'missingField');
- expect(isValid).toBeFalse();
- expect(validator.isValid()).toBeFalse();
- const validity = validator.getValidity();
- expect(validity.duplicateRecordId).toBeFalse();
- expect(validity.invalidRecordId).toBeFalse();
- expect(validity.missingRecordId).toBeTrue();
- });
-
- it('multiple errors are reported during validation', () => {
- const data = [
- { stringField: 'value-1', numberField: 10 },
- { stringField: 'value-2', numberField: 11 },
- { stringField: 'value-1', numberField: 12 },
- { numberField: 12 },
- { stringField: true, numberField: 12 }
- ];
- const validator = new TableValidator();
-
- const isValid = validator.validateRecordIds(data, 'stringField');
- expect(isValid).toBeFalse();
- expect(validator.isValid()).toBeFalse();
- const validity = validator.getValidity();
- expect(validity.duplicateRecordId).toBeTrue();
- expect(validity.invalidRecordId).toBeTrue();
- expect(validity.missingRecordId).toBeTrue();
- });
-
- it('setting ID field name to undefined makes configuration valid', () => {
- const data = [
- { stringField: 'value-1', numberField: 10 },
- { stringField: 'value-2', numberField: 11 }
- ];
- const validator = new TableValidator();
-
- let isValid = validator.validateRecordIds(data, 'missingField');
- expect(isValid).toBeFalse();
- expect(validator.isValid()).toBeFalse();
-
- isValid = validator.validateRecordIds(data, undefined);
- expect(isValid).toBeTrue();
- expect(validator.isValid()).toBeTrue();
- });
-
- it('setting a valid ID field name makes an invalid configuration valid', () => {
- const data = [
- { stringField: 'value-1', numberField: 10 },
- { stringField: 'value-2', numberField: 11 }
- ];
- const validator = new TableValidator();
+fdescribe('TableValidator', () => {
+ let validator: TableValidator<TableRecord>;
- let isValid = validator.validateRecordIds(data, 'missingField');
- expect(isValid).toBeFalse();
- expect(validator.isValid()).toBeFalse();
-
- isValid = validator.validateRecordIds(data, 'stringField');
- expect(isValid).toBeTrue();
- expect(validator.isValid()).toBeTrue();
- });
-
- it('setting invalid ID field name makes a valid configuration invalid', () => {
- const data = [
- { stringField: 'value-1', numberField: 10 },
- { stringField: 'value-2', numberField: 11 }
- ];
- const validator = new TableValidator();
-
- let isValid = validator.validateRecordIds(data, 'stringField');
- expect(isValid).toBeTrue();
- expect(validator.isValid()).toBeTrue();
-
- isValid = validator.validateRecordIds(data, 'missingField');
- expect(isValid).toBeFalse();
- expect(validator.isValid()).toBeFalse();
+ beforeEach(() => {
+ validator = new TableValidator();
});
- it('ID field name can be an empty string', () => {
- const data = [
- // eslint-disable-next-line @typescript-eslint/naming-convention
- { stringField: 'value-1', numberField: 10, '': 'empty-1' },
- // eslint-disable-next-line @typescript-eslint/naming-convention
- { stringField: 'value-2', numberField: 11, '': 'empty-2' }
- ];
- const validator = new TableValidator();
+ function validateValidity(invalidKeys: (keyof TableValidity)[]): void {
+ expect(validator.isValid()).toEqual(invalidKeys.length === 0);
- const isValid = validator.validateRecordIds(data, '');
- expect(isValid).toBeTrue();
- expect(validator.isValid()).toBeTrue();
const validity = validator.getValidity();
- expect(validity.duplicateRecordId).toBeFalse();
- expect(validity.invalidRecordId).toBeFalse();
- expect(validity.missingRecordId).toBeFalse();
+ expect(validity.duplicateRecordId).toBe(invalidKeys.some(x => x === 'duplicateRecordId'));
+ expect(validity.missingRecordId).toBe(invalidKeys.some(x => x === 'missingRecordId'));
+ expect(validity.invalidRecordId).toBe(invalidKeys.some(x => x === 'invalidRecordId'));
+ expect(validity.duplicateColumnId).toBe(invalidKeys.some(x => x === 'duplicateColumnId'));
+ expect(validity.missingColumnId).toBe(invalidKeys.some(x => x === 'missingColumnId'));
+ }
+
+ describe('record ID validation', () => {
+ it('setting valid field for ID is valid', () => {
+ const data = [
+ { stringField: 'value-1', numberField: 10 },
+ { stringField: 'value-2', numberField: 11 }
+ ];
+
+ const isValid = validator.validateRecordIds(data, 'stringField');
+ expect(isValid).toBeTrue();
+ validateValidity([]);
+ });
+
+ it('setting `undefined` field for ID is valid', () => {
+ const data = [
+ { stringField: 'value-1', numberField: 10 },
+ { stringField: 'value-2', numberField: 11 }
+ ];
+
+ const isValid = validator.validateRecordIds(data, undefined);
+ expect(isValid).toBeTrue();
+ validateValidity([]);
+ });
+
+ it('setting `null` field for ID is valid', () => {
+ const data = [
+ { stringField: 'value-1', numberField: 10 },
+ { stringField: 'value-2', numberField: 11 }
+ ];
+
+ const isValid = validator.validateRecordIds(data, null);
+ expect(isValid).toBeTrue();
+ validateValidity([]);
+ });
+
+ it('setting data with duplicate IDs is invalid', () => {
+ const data = [
+ { stringField: 'value-1', numberField: 10 },
+ { stringField: 'value-1', numberField: 11 }
+ ];
+
+ const isValid = validator.validateRecordIds(data, 'stringField');
+ expect(isValid).toBeFalse();
+ validateValidity(['duplicateRecordId']);
+ });
+
+ it('setting data with invalid ID value type is invalid', () => {
+ const data = [
+ { stringField: 'value-1', numberField: 10 },
+ { stringField: 'value-2', numberField: 11 }
+ ];
+
+ const isValid = validator.validateRecordIds(data, 'numberField');
+ expect(isValid).toBeFalse();
+ validateValidity(['invalidRecordId']);
+ });
+
+ it('setting data with empty ID value is valid', () => {
+ const data = [
+ { stringField: 'value-1', numberField: 10 },
+ { stringField: '', numberField: 11 }
+ ];
+
+ const isValid = validator.validateRecordIds(data, 'stringField');
+ expect(isValid).toBeTrue();
+ validateValidity([]);
+ });
+
+ it('setting data with undefined ID value is invalid', () => {
+ const data = [
+ { stringField: 'value-1', numberField: 10 },
+ { stringField: undefined, numberField: 11 }
+ ];
+
+ const isValid = validator.validateRecordIds(data, 'stringField');
+ expect(isValid).toBeFalse();
+ validateValidity(['invalidRecordId']);
+ });
+
+ it('setting data with null ID value is invalid', () => {
+ const data = [
+ { stringField: 'value-1', numberField: 10 },
+ { stringField: undefined, numberField: 11 }
+ ];
+
+ const isValid = validator.validateRecordIds(data, 'stringField');
+ expect(isValid).toBeFalse();
+ validateValidity(['invalidRecordId']);
+ });
+
+ it('setting data with missing IDs is invalid', () => {
+ const data = [
+ { stringField: 'value-1', numberField: 10 },
+ { stringField: 'value-2', numberField: 11 }
+ ];
+
+ const isValid = validator.validateRecordIds(data, 'missingField');
+ expect(isValid).toBeFalse();
+ validateValidity(['missingRecordId']);
+ });
+
+ it('multiple errors are reported during validation', () => {
+ const data = [
+ { stringField: 'value-1', numberField: 10 },
+ { stringField: 'value-2', numberField: 11 },
+ { stringField: 'value-1', numberField: 12 },
+ { numberField: 12 },
+ { stringField: true, numberField: 12 }
+ ];
+
+ const isValid = validator.validateRecordIds(data, 'stringField');
+ expect(isValid).toBeFalse();
+ validateValidity(['missingRecordId', 'duplicateRecordId', 'invalidRecordId']);
+ });
+
+ it('setting ID field name to undefined makes configuration valid', () => {
+ const data = [
+ { stringField: 'value-1', numberField: 10 },
+ { stringField: 'value-2', numberField: 11 }
+ ];
+
+ let isValid = validator.validateRecordIds(data, 'missingField');
+ expect(isValid).toBeFalse();
+ expect(validator.isValid()).toBeFalse();
+
+ isValid = validator.validateRecordIds(data, undefined);
+ expect(isValid).toBeTrue();
+ expect(validator.isValid()).toBeTrue();
+ });
+
+ it('setting a valid ID field name makes an invalid configuration valid', () => {
+ const data = [
+ { stringField: 'value-1', numberField: 10 },
+ { stringField: 'value-2', numberField: 11 }
+ ];
+
+ let isValid = validator.validateRecordIds(data, 'missingField');
+ expect(isValid).toBeFalse();
+ expect(validator.isValid()).toBeFalse();
+
+ isValid = validator.validateRecordIds(data, 'stringField');
+ expect(isValid).toBeTrue();
+ expect(validator.isValid()).toBeTrue();
+ });
+
+ it('setting invalid ID field name makes a valid configuration invalid', () => {
+ const data = [
+ { stringField: 'value-1', numberField: 10 },
+ { stringField: 'value-2', numberField: 11 }
+ ];
+
+ let isValid = validator.validateRecordIds(data, 'stringField');
+ expect(isValid).toBeTrue();
+ expect(validator.isValid()).toBeTrue();
+
+ isValid = validator.validateRecordIds(data, 'missingField');
+ expect(isValid).toBeFalse();
+ expect(validator.isValid()).toBeFalse();
+ });
+
+ it('ID field name can be an empty string', () => {
+ const data = [
+ // eslint-disable-next-line @typescript-eslint/naming-convention
+ { stringField: 'value-1', numberField: 10, '': 'empty-1' },
+ // eslint-disable-next-line @typescript-eslint/naming-convention
+ { stringField: 'value-2', numberField: 11, '': 'empty-2' }
+ ];
+
+ const isValid = validator.validateRecordIds(data, '');
+ expect(isValid).toBeTrue();
+ validateValidity([]);
+ });
+
+ it('validation occurs when ID field name is an empty string', () => {
+ const data = [
+ // eslint-disable-next-line @typescript-eslint/naming-convention
+ { stringField: 'value-1', numberField: 10, '': 'empty-1' },
+ // eslint-disable-next-line @typescript-eslint/naming-convention
+ { stringField: 'value-2', numberField: 11, '': 'empty-1' }
+ ];
+
+ const isValid = validator.validateRecordIds(data, '');
+ expect(isValid).toBeFalse();
+ validateValidity(['duplicateRecordId']);
+ });
});
- it('validation occurs when ID field name is an empty string', () => {
- const data = [
- // eslint-disable-next-line @typescript-eslint/naming-convention
- { stringField: 'value-1', numberField: 10, '': 'empty-1' },
- // eslint-disable-next-line @typescript-eslint/naming-convention
- { stringField: 'value-2', numberField: 11, '': 'empty-1' }
+ describe('column ID validation', () => {
+ const columnConfigurations: { columnIds: (string | undefined | null)[], invalidKeys: (keyof TableValidity)[], testDescription: string }[] = [
+ { columnIds: [null, null, undefined, undefined], invalidKeys: [], testDescription: 'does not require column IDs' },
+ { columnIds: [null, 'my-id', undefined, undefined], invalidKeys: ['missingColumnId'], testDescription: 'requires column IDs for all columns if a column ID is defined for at least one' },
+ { columnIds: ['my-id-1', 'my-id-2', 'my-id-3'], invalidKeys: [], testDescription: 'unique defined IDs are valid' },
+ { columnIds: ['my-id-1', 'my-id-2', 'my-id-2'], invalidKeys: ['duplicateColumnId'], testDescription: 'duplicate column IDs is invalid' },
+ { columnIds: ['my-id-1', 'my-id-2', 'my-id-2', undefined], invalidKeys: ['missingColumnId', 'duplicateColumnId'], testDescription: 'reports multiple column ID validation errors' },
+ { columnIds: ['my-id-1', 'my-id-2', ''], invalidKeys: [], testDescription: 'empty string is a valid column ID' },
+ { columnIds: ['my-id-1', '', ''], invalidKeys: ['duplicateColumnId'], testDescription: 'detects duplicate empty string column IDs' },
+ { columnIds: [null, undefined, ''], invalidKeys: ['missingColumnId'], testDescription: 'requires column IDs for all columns if one is set to empty string' },
];
- const validator = new TableValidator();
- const isValid = validator.validateRecordIds(data, '');
- expect(isValid).toBeFalse();
- expect(validator.isValid()).toBeFalse();
- const validity = validator.getValidity();
- expect(validity.duplicateRecordId).toBeTrue();
- expect(validity.invalidRecordId).toBeFalse();
- expect(validity.missingRecordId).toBeFalse();
+ for (const columnConfiguration of columnConfigurations) {
+ // eslint-disable-next-line @typescript-eslint/no-loop-func
+ it(columnConfiguration.testDescription, () => {
+ const isValid = validator.validateColumnIds(columnConfiguration.columnIds);
+ expect(isValid).toBe(columnConfiguration.invalidKeys.length === 0);
+ validateValidity(columnConfiguration.invalidKeys);
+ });
+ }
});
});
diff --git a/packages/nimble-components/src/table/models/table-validator.ts b/packages/nimble-components/src/table/models/table-validator.ts
index f7bfebdffa..57ce5e3ab4 100644
--- a/packages/nimble-components/src/table/models/table-validator.ts
+++ b/packages/nimble-components/src/table/models/table-validator.ts
@@ -1,3 +1,4 @@
+import type { TableColumn } from '../../table-column/base';
import type { TableRecord, TableValidity } from '../types';
/**
@@ -8,12 +9,16 @@ export class TableValidator<TData extends TableRecord> {
private duplicateRecordId = false;
private missingRecordId = false;
private invalidRecordId = false;
+ private duplicateColumnId = false;
+ private missingColumnId = false;
public getValidity(): TableValidity {
return {
duplicateRecordId: this.duplicateRecordId,
missingRecordId: this.missingRecordId,
- invalidRecordId: this.invalidRecordId
+ invalidRecordId: this.invalidRecordId,
+ duplicateColumnId: this.duplicateColumnId,
+ missingColumnId: this.missingColumnId
};
}
@@ -59,4 +64,33 @@ export class TableValidator<TData extends TableRecord> {
&& !this.duplicateRecordId
);
}
+
+ public validateColumnIds(columnIds: (string | null | undefined)[]): boolean {
+ this.missingColumnId = false;
+ this.duplicateColumnId = false;
+
+ const anyColumnsHaveIds = columnIds.some(columnId => columnId !== undefined && columnId !== null);
+
+ if (!anyColumnsHaveIds) {
+ return true;
+ }
+
+ const idSet = new Set<string>();
+ for (const columnId of columnIds) {
+ if (typeof columnId !== 'string') {
+ this.missingColumnId = true;
+ continue;
+ }
+
+ if (idSet.has(columnId)) {
+ this.duplicateColumnId = true;
+ }
+ idSet.add(columnId);
+ }
+
+ return (
+ !this.missingColumnId
+ && !this.duplicateColumnId
+ );
+ }
}
diff --git a/packages/nimble-components/src/table/template.ts b/packages/nimble-components/src/table/template.ts
index 9cd81a35bf..251a8d5319 100644
--- a/packages/nimble-components/src/table/template.ts
+++ b/packages/nimble-components/src/table/template.ts
@@ -1,4 +1,5 @@
import {
+ children,
ElementsFilter,
html,
ref,
@@ -11,22 +12,19 @@ import type { VirtualItem } from '@tanstack/virtual-core';
import type { Table } from '.';
import { TableHeader } from './components/header';
import { TableRow } from './components/row';
-import { TableColumn } from '../table-column/base';
-
-const isTableColumn = (): ElementsFilter => {
- const filter: ElementsFilter = (
- value: Node,
- _: number,
- __: Node[]
- ): boolean => {
- return value instanceof TableColumn;
- };
- return filter;
-};
+import type { TableColumn } from '../table-column/base';
// prettier-ignore
export const template = html<Table>`
- <template role="table">
+ <template
+ role="table"
+ ${children({
+ property: 'childItems',
+ attributeFilter: ['column-id', 'field-name'],
+ subtree: true,
+ selector: '*'
+ })}
+ >
<div class="table-container">
<div role="rowgroup" class="header-container" style="margin-right: ${x => x.virtualizer.headerContainerMarginRight}px;">
<div class="header-row" role="row">
@@ -55,6 +53,5 @@ export const template = html<Table>`
</div>
</div>
</div>
- <slot ${slotted({ property: 'columns', filter: isTableColumn() })}></slot>
</template>
`;
diff --git a/packages/nimble-components/src/table/tests/table.stories.ts b/packages/nimble-components/src/table/tests/table.stories.ts
index ce816a45d0..cd23dd5fdc 100644
--- a/packages/nimble-components/src/table/tests/table.stories.ts
+++ b/packages/nimble-components/src/table/tests/table.stories.ts
@@ -125,10 +125,10 @@ const metadata: Meta<TableArgs> = {
id-field-name="${x => dataSetIdFieldNames[x.data]}"
data-unused="${x => x.updateData(x)}"
>
- <nimble-table-column-text field-name="firstName" placeholder="no value">First Name</nimble-table-column-text>
- <nimble-table-column-text field-name="lastName" placeholder="no value">Last Name</nimble-table-column-text>
- <nimble-table-column-text field-name="favoriteColor" placeholder="no value">Favorite Color</nimble-table-column-text>
- <nimble-table-column-text field-name="quote" placeholder="no value">Quote</nimble-table-column-text>
+ <nimble-table-column-text field-name="firstName" placeholder="no value" column-id="foo">First Name</nimble-table-column-text>
+ <nimble-table-column-text field-name="lastName" placeholder="no value" column-id="food">Last Name</nimble-table-column-text>
+ <nimble-table-column-text field-name="favoriteColor" placeholder="no value" column-id="bar">Favorite Color</nimble-table-column-text>
+ <nimble-table-column-text field-name="quote" placeholder="no value" column-id="baz">Quote</nimble-table-column-text>
</nimble-table>
<style class="code-hide">
#usage-warning {
diff --git a/packages/nimble-components/src/table/types.ts b/packages/nimble-components/src/table/types.ts
index d55547d5bb..115f028b6d 100644
--- a/packages/nimble-components/src/table/types.ts
+++ b/packages/nimble-components/src/table/types.ts
@@ -46,6 +46,8 @@ export interface TableValidity {
readonly duplicateRecordId: boolean;
readonly missingRecordId: boolean;
readonly invalidRecordId: boolean;
+ readonly duplicateColumnId: boolean;
+ readonly missingColumnId: boolean;
}
export interface TableRowState<TData extends TableRecord = TableRecord> {
From 1c9a69b6644594aec4f652c74f01fabda3bdf1e0 Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Fri, 3 Feb 2023 10:11:10 -0600
Subject: [PATCH 02/23] more tests
---
.../text/tests/table-column-text.spec.ts | 2 +-
.../src/table/models/table-validator.spec.ts | 32 +++++++-
.../src/table/tests/table.spec.ts | 81 +++++++++++++++++--
3 files changed, 108 insertions(+), 7 deletions(-)
diff --git a/packages/nimble-components/src/table-column/text/tests/table-column-text.spec.ts b/packages/nimble-components/src/table-column/text/tests/table-column-text.spec.ts
index 56480fc4c6..25e32c6154 100644
--- a/packages/nimble-components/src/table-column/text/tests/table-column-text.spec.ts
+++ b/packages/nimble-components/src/table-column/text/tests/table-column-text.spec.ts
@@ -62,7 +62,7 @@ describe('TableColumnText', () => {
});
}
- fit('changing fieldName updates display', async () => {
+ it('changing fieldName updates display', async () => {
element.setData([{ field: 'foo', anotherField: 'bar' }]);
await connect();
await waitForUpdatesAsync();
diff --git a/packages/nimble-components/src/table/models/table-validator.spec.ts b/packages/nimble-components/src/table/models/table-validator.spec.ts
index 74b1b285ef..965e348288 100644
--- a/packages/nimble-components/src/table/models/table-validator.spec.ts
+++ b/packages/nimble-components/src/table/models/table-validator.spec.ts
@@ -1,7 +1,7 @@
import type { TableRecord, TableValidity } from '../types';
import { TableValidator } from './table-validator';
-fdescribe('TableValidator', () => {
+describe('TableValidator', () => {
let validator: TableValidator<TableRecord>;
beforeEach(() => {
@@ -226,4 +226,34 @@ fdescribe('TableValidator', () => {
});
}
});
+
+ describe('validation checks do not reset unrelated state', () => {
+ it('invalid record IDs stay invalid when validating column IDs', () => {
+ const data = [
+ { stringField: 'value-1', numberField: 10 },
+ { stringField: 'value-2', numberField: 11 },
+ { stringField: 'value-1', numberField: 12 },
+ { numberField: 12 },
+ { stringField: true, numberField: 12 }
+ ];
+
+ const recordIdsAreValid = validator.validateRecordIds(data, 'stringField');
+ expect(recordIdsAreValid).toBeFalse();
+ validateValidity(['missingRecordId', 'duplicateRecordId', 'invalidRecordId']);
+
+ const columnIdsAreValid = validator.validateColumnIds(['id-1', 'id-2', 'id-3']);
+ expect(columnIdsAreValid).toBeTrue();
+ validateValidity(['missingRecordId', 'duplicateRecordId', 'invalidRecordId']);
+ });
+
+ it('invalid column IDs stay invalid when validating record IDs', () => {
+ const columnIdsAreValid = validator.validateColumnIds(['id-1', 'id-1', undefined]);
+ expect(columnIdsAreValid).toBeFalse();
+ validateValidity(['missingColumnId', 'duplicateColumnId']);
+
+ const recordIdsAreValid = validator.validateRecordIds([], undefined);
+ expect(recordIdsAreValid).toBeTrue();
+ validateValidity(['missingColumnId', 'duplicateColumnId']);
+ });
+ });
});
diff --git a/packages/nimble-components/src/table/tests/table.spec.ts b/packages/nimble-components/src/table/tests/table.spec.ts
index cdc892a2ee..41a8e4edc8 100644
--- a/packages/nimble-components/src/table/tests/table.spec.ts
+++ b/packages/nimble-components/src/table/tests/table.spec.ts
@@ -1,6 +1,7 @@
import { html } from '@microsoft/fast-element';
import { DesignSystem } from '@microsoft/fast-foundation';
import { Table } from '..';
+import type { TableColumn } from '../../table-column/base';
import { TableColumnText } from '../../table-column/text';
import { waitForUpdatesAsync } from '../../testing/async-helpers';
import { controlHeight } from '../../theme-provider/design-tokens';
@@ -40,14 +41,10 @@ const largeTableData = Array.from(Array(500), (_, i) => {
};
});
-const tableColumnText = DesignSystem.tagFor(TableColumnText);
-
// prettier-ignore
async function setup(): Promise<Fixture<Table<SimpleTableRecord>>> {
return fixture<Table<SimpleTableRecord>>(
- html`<nimble-table>
- <${tableColumnText} field-name="stringData">stringData</${tableColumnText}>
- </nimble-table>`
+ html`<nimble-table></nimble-table>`
);
}
@@ -56,6 +53,8 @@ describe('Table', () => {
let connect: () => Promise<void>;
let disconnect: () => Promise<void>;
let pageObject: TablePageObject<SimpleTableRecord>;
+ let column1: TableColumn;
+ let column2: TableColumn;
// The assumption being made here is that the values in the data are equal to their
// rendered representation (no formatting).
@@ -108,6 +107,17 @@ describe('Table', () => {
beforeEach(async () => {
({ element, connect, disconnect } = await setup());
pageObject = new TablePageObject<SimpleTableRecord>(element);
+
+ const tableColumnTextTag = DesignSystem.tagFor(TableColumnText);
+ column1 = document.createElement(tableColumnTextTag) as TableColumn;
+ column1.textContent = 'stringData';
+ (column1 as TableColumnText).fieldName = 'stringData';
+ column2 = document.createElement(tableColumnTextTag) as TableColumn;
+ column2.textContent = 'moreStringData';
+ (column2 as TableColumnText).fieldName = 'stringData';
+
+ element.appendChild(column1);
+ element.appendChild(column2);
});
afterEach(async () => {
@@ -414,6 +424,67 @@ describe('Table', () => {
});
});
+ describe('column IDs', () => {
+ it('duplicate column IDs marks the table as invalid and rows are not rendered', async () => {
+ element.setData(simpleTableData);
+ column1.columnId = 'my-column-id';
+ column2.columnId = 'my-column-id';
+ await connect();
+ await waitForUpdatesAsync();
+
+ expect(pageObject.getRenderedRowCount()).toBe(0);
+ expect(element.checkValidity()).toBeFalse();
+ expect(element.validity.duplicateColumnId).toBeTrue();
+ expect(element.validity.missingColumnId).toBeFalse();
+ });
+
+ it('missing column IDs marks the table as invalid and rows are not rendered', async () => {
+ element.setData(simpleTableData);
+ column1.columnId = 'my-column-id';
+ column2.columnId = undefined;
+ await connect();
+ await waitForUpdatesAsync();
+
+ expect(pageObject.getRenderedRowCount()).toBe(0);
+ expect(element.checkValidity()).toBeFalse();
+ expect(element.validity.duplicateColumnId).toBeFalse();
+ expect(element.validity.missingColumnId).toBeTrue();
+ });
+
+ it('table validity updates if column IDs become valid', async () => {
+ element.setData(simpleTableData);
+ column1.columnId = 'my-column-id';
+ column2.columnId = undefined;
+ await connect();
+ await waitForUpdatesAsync();
+
+ expect(pageObject.getRenderedRowCount()).toBe(0);
+ expect(element.checkValidity()).toBeFalse();
+
+ column2.setAttribute('column-id', 'my-other-column-id');
+ await waitForUpdatesAsync();
+
+ verifyRenderedData(simpleTableData);
+ expect(element.checkValidity()).toBeTrue();
+ });
+
+ it('table validity updates if column IDs become invalid', async () => {
+ element.setData(simpleTableData);
+ await connect();
+ await waitForUpdatesAsync();
+
+ verifyRenderedData(simpleTableData);
+ expect(element.checkValidity()).toBeTrue();
+
+ column1.setAttribute('column-id', 'my-column-id');
+ column2.setAttribute('column-id', 'my-column-id');
+ await waitForUpdatesAsync();
+
+ expect(pageObject.getRenderedRowCount()).toBe(0);
+ expect(element.checkValidity()).toBeFalse();
+ });
+ });
+
describe('uses virtualization', () => {
it('to render fewer rows (based on viewport size)', async () => {
await connect();
From 6de11a49af1642d67abdead5272f3bc9cd47081d Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Fri, 3 Feb 2023 10:13:45 -0600
Subject: [PATCH 03/23] format
---
.../src/table/models/table-validator.ts | 1 -
packages/nimble-components/src/table/template.ts | 12 +++++-------
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/packages/nimble-components/src/table/models/table-validator.ts b/packages/nimble-components/src/table/models/table-validator.ts
index 57ce5e3ab4..76e1d71b9c 100644
--- a/packages/nimble-components/src/table/models/table-validator.ts
+++ b/packages/nimble-components/src/table/models/table-validator.ts
@@ -1,4 +1,3 @@
-import type { TableColumn } from '../../table-column/base';
import type { TableRecord, TableValidity } from '../types';
/**
diff --git a/packages/nimble-components/src/table/template.ts b/packages/nimble-components/src/table/template.ts
index 251a8d5319..22ff9d9542 100644
--- a/packages/nimble-components/src/table/template.ts
+++ b/packages/nimble-components/src/table/template.ts
@@ -1,10 +1,8 @@
import {
children,
- ElementsFilter,
html,
ref,
repeat,
- slotted,
when
} from '@microsoft/fast-element';
import { DesignSystem } from '@microsoft/fast-foundation';
@@ -19,11 +17,11 @@ export const template = html<Table>`
<template
role="table"
${children({
- property: 'childItems',
- attributeFilter: ['column-id', 'field-name'],
- subtree: true,
- selector: '*'
- })}
+ property: 'childItems',
+ attributeFilter: ['column-id'],
+ subtree: true,
+ selector: '*'
+ })}
>
<div class="table-container">
<div role="rowgroup" class="header-container" style="margin-right: ${x => x.virtualizer.headerContainerMarginRight}px;">
From 037ac7e617ca00b98cc45865ee2486fe1c0affe3 Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Fri, 3 Feb 2023 10:30:09 -0600
Subject: [PATCH 04/23] Update TableValidity interface in Blazor
---
packages/nimble-blazor/NimbleBlazor/TableValidity.cs | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/packages/nimble-blazor/NimbleBlazor/TableValidity.cs b/packages/nimble-blazor/NimbleBlazor/TableValidity.cs
index de35c8e6d6..67ab1f58da 100644
--- a/packages/nimble-blazor/NimbleBlazor/TableValidity.cs
+++ b/packages/nimble-blazor/NimbleBlazor/TableValidity.cs
@@ -9,6 +9,10 @@ public interface ITableValidity
public bool MissingRecordId { get; }
public bool InvalidRecordId { get; }
+
+ public bool DuplicateColumnId { get; }
+
+ public bool MissingColumnId { get; }
}
internal class TableValidity : ITableValidity
@@ -21,4 +25,10 @@ internal class TableValidity : ITableValidity
[JsonPropertyName("invalidRecordId")]
public bool InvalidRecordId { get; set; }
+
+ [JsonPropertyName("duplicateColumnId")]
+ public bool DuplicateColumnId { get; set; }
+
+ [JsonPropertyName("missingColumnId")]
+ public bool MissingColumnId { get; set; }
}
From 6082e59cfc2115ea14aa2d5e1c2c7a03c564aae0 Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Fri, 3 Feb 2023 10:45:22 -0600
Subject: [PATCH 05/23] column id support in blazor
---
.../NimbleBlazor/Components/NimbleTableColumnText.razor | 2 +-
.../NimbleBlazor/Components/NimbleTableColumnText.razor.cs | 6 ++++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/packages/nimble-blazor/NimbleBlazor/Components/NimbleTableColumnText.razor b/packages/nimble-blazor/NimbleBlazor/Components/NimbleTableColumnText.razor
index 26a2dc5a54..2ed123db5f 100644
--- a/packages/nimble-blazor/NimbleBlazor/Components/NimbleTableColumnText.razor
+++ b/packages/nimble-blazor/NimbleBlazor/Components/NimbleTableColumnText.razor
@@ -1,6 +1,6 @@
@namespace NimbleBlazor
-<nimble-table-column-text field-name="@FieldName" placeholder="@Placeholder">
+<nimble-table-column-text column-id="@ColumnId" field-name="@FieldName" placeholder="@Placeholder">
@ChildContent
</nimble-table-column-text>
diff --git a/packages/nimble-blazor/NimbleBlazor/Components/NimbleTableColumnText.razor.cs b/packages/nimble-blazor/NimbleBlazor/Components/NimbleTableColumnText.razor.cs
index 16947dd289..a7aef0a30c 100644
--- a/packages/nimble-blazor/NimbleBlazor/Components/NimbleTableColumnText.razor.cs
+++ b/packages/nimble-blazor/NimbleBlazor/Components/NimbleTableColumnText.razor.cs
@@ -5,6 +5,12 @@ namespace NimbleBlazor;
public partial class NimbleTableColumnText : ComponentBase
{
+ /// <summary>
+ /// The ID of the column of a <see cref="NimbleTable{TData}"/>
+ /// </summary>
+ [Parameter]
+ public string? ColumnId { get; set; }
+
/// <summary>
/// Gets or sets the field in the element representing a row of data in a <see cref="NimbleTable{TData}"/>to display
/// </summary>
From 03e0a8fe3224e0a705b4401fd53aef8151a54191 Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Fri, 3 Feb 2023 11:02:57 -0600
Subject: [PATCH 06/23] Angular
---
.../nimble-table-column-text.directive.ts | 10 ++++
...nimble-table-column-text.directive.spec.ts | 51 ++++++++++++++++---
.../tests/nimble-table.directive.spec.ts | 4 +-
3 files changed, 58 insertions(+), 7 deletions(-)
diff --git a/angular-workspace/projects/ni/nimble-angular/src/directives/table-column/text/nimble-table-column-text.directive.ts b/angular-workspace/projects/ni/nimble-angular/src/directives/table-column/text/nimble-table-column-text.directive.ts
index 4a14931539..66416fa0bf 100644
--- a/angular-workspace/projects/ni/nimble-angular/src/directives/table-column/text/nimble-table-column-text.directive.ts
+++ b/angular-workspace/projects/ni/nimble-angular/src/directives/table-column/text/nimble-table-column-text.directive.ts
@@ -10,6 +10,16 @@ export type { TableColumnText };
selector: 'nimble-table-column-text'
})
export class NimbleTableColumnTextDirective {
+ public get columnId(): string | null | undefined {
+ return this.elementRef.nativeElement.columnId;
+ }
+
+ // Renaming because property should have camel casing, but attribute should not
+ // eslint-disable-next-line @angular-eslint/no-input-rename
+ @Input('column-id') public set columnId(value: string | null | undefined) {
+ this.renderer.setProperty(this.elementRef.nativeElement, 'columnId', value);
+ }
+
public get fieldName(): string | undefined {
return this.elementRef.nativeElement.fieldName;
}
diff --git a/angular-workspace/projects/ni/nimble-angular/src/directives/table-column/text/tests/nimble-table-column-text.directive.spec.ts b/angular-workspace/projects/ni/nimble-angular/src/directives/table-column/text/tests/nimble-table-column-text.directive.spec.ts
index 0d723a2d36..4151951853 100644
--- a/angular-workspace/projects/ni/nimble-angular/src/directives/table-column/text/tests/nimble-table-column-text.directive.spec.ts
+++ b/angular-workspace/projects/ni/nimble-angular/src/directives/table-column/text/tests/nimble-table-column-text.directive.spec.ts
@@ -21,7 +21,7 @@ describe('NimbleTableColumnText', () => {
@Component({
template: `
<nimble-table>
- <nimble-table-column-text #column field-name="field1" placeholder="no value"></nimble-table-column-text>
+ <nimble-table-column-text #column column-id="my-column" field-name="field1" placeholder="no value"></nimble-table-column-text>
</nimble-table>
`
})
@@ -50,17 +50,27 @@ describe('NimbleTableColumnText', () => {
expect(nativeElement.fieldName).toBe('field1');
});
- it('cwill use template string values for placeholder', () => {
+ it('will use template string values for placeholder', () => {
expect(directive.placeholder).toBe('no value');
expect(nativeElement.placeholder).toBe('no value');
});
+
+ it('will use template string values for columnId', () => {
+ expect(directive.columnId).toBe('my-column');
+ expect(nativeElement.columnId).toBe('my-column');
+ });
});
describe('with property bound values', () => {
@Component({
template: `
<nimble-table>
- <nimble-table-column-text #column [field-name]="field" [placeholder]="placeholder"></nimble-table-column-text>
+ <nimble-table-column-text
+ #column
+ [column-id]="columnId"
+ [field-name]="field"
+ [placeholder]="placeholder"
+ ></nimble-table-column-text>
</nimble-table>
`
})
@@ -69,6 +79,7 @@ describe('NimbleTableColumnText', () => {
@ViewChild('column', { read: ElementRef }) public elementRef: ElementRef<TableColumnText>;
public field = 'field1';
public placeholder = 'no value';
+ public columnId = 'my-column';
}
let fixture: ComponentFixture<TestHostComponent>;
@@ -107,13 +118,29 @@ describe('NimbleTableColumnText', () => {
expect(directive.placeholder).toBe('foo');
expect(nativeElement.placeholder).toBe('foo');
});
+
+ it('can be configured with property binding for columnId', () => {
+ expect(directive.columnId).toBe('my-column');
+ expect(nativeElement.columnId).toBe('my-column');
+
+ fixture.componentInstance.columnId = 'new-column';
+ fixture.detectChanges();
+
+ expect(directive.columnId).toBe('new-column');
+ expect(nativeElement.columnId).toBe('new-column');
+ });
});
describe('with attribute bound values', () => {
@Component({
template: `
<nimble-table>
- <nimble-table-column-text #column [attr.field-name]="field" [attr.placeholder]="placeholder"></nimble-table-column-text>
+ <nimble-table-column-text
+ #column
+ [attr.column-id]="columnId"
+ [attr.field-name]="field"
+ [attr.placeholder]="placeholder"
+ ></nimble-table-column-text>
</nimble-table>
`
})
@@ -122,6 +149,7 @@ describe('NimbleTableColumnText', () => {
@ViewChild('column', { read: ElementRef }) public elementRef: ElementRef<TableColumnText>;
public field = 'field1';
public placeholder = 'no value';
+ public columnId = 'my-column';
}
let fixture: ComponentFixture<TestHostComponent>;
@@ -139,7 +167,7 @@ describe('NimbleTableColumnText', () => {
nativeElement = fixture.componentInstance.elementRef.nativeElement;
});
- it('can be configured with property binding for fieldName', () => {
+ it('can be configured with attribute binding for fieldName', () => {
expect(directive.fieldName).toBe('field1');
expect(nativeElement.fieldName).toBe('field1');
@@ -150,7 +178,7 @@ describe('NimbleTableColumnText', () => {
expect(nativeElement.fieldName).toBe('field2');
});
- it('can be configured with property binding for placeholder', () => {
+ it('can be configured with attribute binding for placeholder', () => {
expect(directive.placeholder).toBe('no value');
expect(nativeElement.placeholder).toBe('no value');
@@ -160,5 +188,16 @@ describe('NimbleTableColumnText', () => {
expect(directive.placeholder).toBe('foo');
expect(nativeElement.placeholder).toBe('foo');
});
+
+ it('can be configured with attribute binding for columnId', () => {
+ expect(directive.columnId).toBe('my-column');
+ expect(nativeElement.columnId).toBe('my-column');
+
+ fixture.componentInstance.columnId = 'new-column';
+ fixture.detectChanges();
+
+ expect(directive.columnId).toBe('new-column');
+ expect(nativeElement.columnId).toBe('new-column');
+ });
});
});
diff --git a/angular-workspace/projects/ni/nimble-angular/src/directives/table/tests/nimble-table.directive.spec.ts b/angular-workspace/projects/ni/nimble-angular/src/directives/table/tests/nimble-table.directive.spec.ts
index 16d614088f..1dc02fd61c 100644
--- a/angular-workspace/projects/ni/nimble-angular/src/directives/table/tests/nimble-table.directive.spec.ts
+++ b/angular-workspace/projects/ni/nimble-angular/src/directives/table/tests/nimble-table.directive.spec.ts
@@ -149,7 +149,9 @@ describe('Nimble table', () => {
const expectedValidity: TableValidity = {
duplicateRecordId: false,
invalidRecordId: false,
- missingRecordId: true
+ missingRecordId: true,
+ duplicateColumnId: false,
+ missingColumnId: false
};
expect(directive.validity).toEqual(expectedValidity);
expect(nativeElement.validity).toEqual(expectedValidity);
From 7120bde6482a785f64aa946093b9a947cb0058cd Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Fri, 3 Feb 2023 11:08:45 -0600
Subject: [PATCH 07/23] doc update
---
packages/nimble-components/src/table/tests/table.stories.ts | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/packages/nimble-components/src/table/tests/table.stories.ts b/packages/nimble-components/src/table/tests/table.stories.ts
index cd23dd5fdc..6481d9aad8 100644
--- a/packages/nimble-components/src/table/tests/table.stories.ts
+++ b/packages/nimble-components/src/table/tests/table.stories.ts
@@ -94,7 +94,9 @@ The object's type is \`TableValidityState\`, and it contains the following boole
- \`duplicateRecordId\`: \`true\` when multiple records were found with the same ID
- \`missingRecordId\`: \`true\` when a record was found that did not have a field with the name specified by \`id-field-name\`
-- \`invalidRecordId\`: \`true\` when record was found where \`id-field-name\` did not refer to a value of type \`string\`
+- \`invalidRecordId\`: \`true\` when a record was found where \`id-field-name\` did not refer to a value of type \`string\`
+- \`duplicateColumnId\`: \`true\` when multiple columns were defined with the same \`column-id\`
+- \`invalidColumnId\`: \`true\` when a \`column-id\` was specified for some, but not all, columns
`;
const metadata: Meta<TableArgs> = {
From 9138e19b61855ce7ac794e67f39edf93da3b3831 Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Fri, 3 Feb 2023 11:12:31 -0600
Subject: [PATCH 08/23] Change files
---
...imble-angular-a552c61a-2254-4ebc-a7cd-91705f4ac48d.json | 7 +++++++
...nimble-blazor-aa544dba-a6aa-439a-a7d0-46c522cd4d7a.json | 7 +++++++
...le-components-a5002c3f-0312-4437-b9eb-beb9b6e218db.json | 7 +++++++
3 files changed, 21 insertions(+)
create mode 100644 change/@ni-nimble-angular-a552c61a-2254-4ebc-a7cd-91705f4ac48d.json
create mode 100644 change/@ni-nimble-blazor-aa544dba-a6aa-439a-a7d0-46c522cd4d7a.json
create mode 100644 change/@ni-nimble-components-a5002c3f-0312-4437-b9eb-beb9b6e218db.json
diff --git a/change/@ni-nimble-angular-a552c61a-2254-4ebc-a7cd-91705f4ac48d.json b/change/@ni-nimble-angular-a552c61a-2254-4ebc-a7cd-91705f4ac48d.json
new file mode 100644
index 0000000000..8cb30ced2e
--- /dev/null
+++ b/change/@ni-nimble-angular-a552c61a-2254-4ebc-a7cd-91705f4ac48d.json
@@ -0,0 +1,7 @@
+{
+ "type": "patch",
+ "comment": "Add column IDs to table",
+ "packageName": "@ni/nimble-angular",
+ "email": "20542556+mollykreis@users.noreply.github.com",
+ "dependentChangeType": "patch"
+}
diff --git a/change/@ni-nimble-blazor-aa544dba-a6aa-439a-a7d0-46c522cd4d7a.json b/change/@ni-nimble-blazor-aa544dba-a6aa-439a-a7d0-46c522cd4d7a.json
new file mode 100644
index 0000000000..033d84559c
--- /dev/null
+++ b/change/@ni-nimble-blazor-aa544dba-a6aa-439a-a7d0-46c522cd4d7a.json
@@ -0,0 +1,7 @@
+{
+ "type": "patch",
+ "comment": "Add column IDs to table",
+ "packageName": "@ni/nimble-blazor",
+ "email": "20542556+mollykreis@users.noreply.github.com",
+ "dependentChangeType": "patch"
+}
diff --git a/change/@ni-nimble-components-a5002c3f-0312-4437-b9eb-beb9b6e218db.json b/change/@ni-nimble-components-a5002c3f-0312-4437-b9eb-beb9b6e218db.json
new file mode 100644
index 0000000000..56f56e28a9
--- /dev/null
+++ b/change/@ni-nimble-components-a5002c3f-0312-4437-b9eb-beb9b6e218db.json
@@ -0,0 +1,7 @@
+{
+ "type": "patch",
+ "comment": "Add column IDs to table",
+ "packageName": "@ni/nimble-components",
+ "email": "20542556+mollykreis@users.noreply.github.com",
+ "dependentChangeType": "patch"
+}
From 7bfd14ebac427a68d24fe9e6bc7843c895c57da8 Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Fri, 3 Feb 2023 11:13:25 -0600
Subject: [PATCH 09/23] format
---
packages/nimble-components/src/table/index.ts | 17 ++-
.../src/table/models/table-validator.spec.ts | 126 ++++++++++++++----
.../src/table/models/table-validator.ts | 13 +-
.../nimble-components/src/table/template.ts | 8 +-
4 files changed, 123 insertions(+), 41 deletions(-)
diff --git a/packages/nimble-components/src/table/index.ts b/packages/nimble-components/src/table/index.ts
index 2e301cb9f5..eaca0e2696 100644
--- a/packages/nimble-components/src/table/index.ts
+++ b/packages/nimble-components/src/table/index.ts
@@ -91,7 +91,9 @@ export class Table<
super.connectedCallback();
this.virtualizer.connectedCallback();
- this.tableValidator.validateColumnIds(this.columns.map(x => x.columnId));
+ this.tableValidator.validateColumnIds(
+ this.columns.map(x => x.columnId)
+ );
this.canRenderRows = this.checkValidity();
}
@@ -120,12 +122,19 @@ export class Table<
return;
}
- this.tableValidator.validateColumnIds(this.columns.map(x => x.columnId));
+ this.tableValidator.validateColumnIds(
+ this.columns.map(x => x.columnId)
+ );
this.canRenderRows = this.checkValidity();
}
- private childItemsChanged(_prev: undefined | Element[], next: Element[]): void {
- this.columns = next.filter(x => x instanceof TableColumn).map(x => x as TableColumn);
+ private childItemsChanged(
+ _prev: undefined | Element[],
+ next: Element[]
+ ): void {
+ this.columns = next
+ .filter(x => x instanceof TableColumn)
+ .map(x => x as TableColumn);
}
private setTableData(newData: readonly TData[]): void {
diff --git a/packages/nimble-components/src/table/models/table-validator.spec.ts b/packages/nimble-components/src/table/models/table-validator.spec.ts
index 965e348288..3126a5b9ec 100644
--- a/packages/nimble-components/src/table/models/table-validator.spec.ts
+++ b/packages/nimble-components/src/table/models/table-validator.spec.ts
@@ -12,11 +12,21 @@ describe('TableValidator', () => {
expect(validator.isValid()).toEqual(invalidKeys.length === 0);
const validity = validator.getValidity();
- expect(validity.duplicateRecordId).toBe(invalidKeys.some(x => x === 'duplicateRecordId'));
- expect(validity.missingRecordId).toBe(invalidKeys.some(x => x === 'missingRecordId'));
- expect(validity.invalidRecordId).toBe(invalidKeys.some(x => x === 'invalidRecordId'));
- expect(validity.duplicateColumnId).toBe(invalidKeys.some(x => x === 'duplicateColumnId'));
- expect(validity.missingColumnId).toBe(invalidKeys.some(x => x === 'missingColumnId'));
+ expect(validity.duplicateRecordId).toBe(
+ invalidKeys.some(x => x === 'duplicateRecordId')
+ );
+ expect(validity.missingRecordId).toBe(
+ invalidKeys.some(x => x === 'missingRecordId')
+ );
+ expect(validity.invalidRecordId).toBe(
+ invalidKeys.some(x => x === 'invalidRecordId')
+ );
+ expect(validity.duplicateColumnId).toBe(
+ invalidKeys.some(x => x === 'duplicateColumnId')
+ );
+ expect(validity.missingColumnId).toBe(
+ invalidKeys.some(x => x === 'missingColumnId')
+ );
}
describe('record ID validation', () => {
@@ -130,7 +140,11 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, 'stringField');
expect(isValid).toBeFalse();
- validateValidity(['missingRecordId', 'duplicateRecordId', 'invalidRecordId']);
+ validateValidity([
+ 'missingRecordId',
+ 'duplicateRecordId',
+ 'invalidRecordId'
+ ]);
});
it('setting ID field name to undefined makes configuration valid', () => {
@@ -206,22 +220,64 @@ describe('TableValidator', () => {
});
describe('column ID validation', () => {
- const columnConfigurations: { columnIds: (string | undefined | null)[], invalidKeys: (keyof TableValidity)[], testDescription: string }[] = [
- { columnIds: [null, null, undefined, undefined], invalidKeys: [], testDescription: 'does not require column IDs' },
- { columnIds: [null, 'my-id', undefined, undefined], invalidKeys: ['missingColumnId'], testDescription: 'requires column IDs for all columns if a column ID is defined for at least one' },
- { columnIds: ['my-id-1', 'my-id-2', 'my-id-3'], invalidKeys: [], testDescription: 'unique defined IDs are valid' },
- { columnIds: ['my-id-1', 'my-id-2', 'my-id-2'], invalidKeys: ['duplicateColumnId'], testDescription: 'duplicate column IDs is invalid' },
- { columnIds: ['my-id-1', 'my-id-2', 'my-id-2', undefined], invalidKeys: ['missingColumnId', 'duplicateColumnId'], testDescription: 'reports multiple column ID validation errors' },
- { columnIds: ['my-id-1', 'my-id-2', ''], invalidKeys: [], testDescription: 'empty string is a valid column ID' },
- { columnIds: ['my-id-1', '', ''], invalidKeys: ['duplicateColumnId'], testDescription: 'detects duplicate empty string column IDs' },
- { columnIds: [null, undefined, ''], invalidKeys: ['missingColumnId'], testDescription: 'requires column IDs for all columns if one is set to empty string' },
+ const columnConfigurations: {
+ columnIds: (string | undefined | null)[],
+ invalidKeys: (keyof TableValidity)[],
+ testDescription: string
+ }[] = [
+ {
+ columnIds: [null, null, undefined, undefined],
+ invalidKeys: [],
+ testDescription: 'does not require column IDs'
+ },
+ {
+ columnIds: [null, 'my-id', undefined, undefined],
+ invalidKeys: ['missingColumnId'],
+ testDescription:
+ 'requires column IDs for all columns if a column ID is defined for at least one'
+ },
+ {
+ columnIds: ['my-id-1', 'my-id-2', 'my-id-3'],
+ invalidKeys: [],
+ testDescription: 'unique defined IDs are valid'
+ },
+ {
+ columnIds: ['my-id-1', 'my-id-2', 'my-id-2'],
+ invalidKeys: ['duplicateColumnId'],
+ testDescription: 'duplicate column IDs is invalid'
+ },
+ {
+ columnIds: ['my-id-1', 'my-id-2', 'my-id-2', undefined],
+ invalidKeys: ['missingColumnId', 'duplicateColumnId'],
+ testDescription: 'reports multiple column ID validation errors'
+ },
+ {
+ columnIds: ['my-id-1', 'my-id-2', ''],
+ invalidKeys: [],
+ testDescription: 'empty string is a valid column ID'
+ },
+ {
+ columnIds: ['my-id-1', '', ''],
+ invalidKeys: ['duplicateColumnId'],
+ testDescription: 'detects duplicate empty string column IDs'
+ },
+ {
+ columnIds: [null, undefined, ''],
+ invalidKeys: ['missingColumnId'],
+ testDescription:
+ 'requires column IDs for all columns if one is set to empty string'
+ }
];
for (const columnConfiguration of columnConfigurations) {
// eslint-disable-next-line @typescript-eslint/no-loop-func
it(columnConfiguration.testDescription, () => {
- const isValid = validator.validateColumnIds(columnConfiguration.columnIds);
- expect(isValid).toBe(columnConfiguration.invalidKeys.length === 0);
+ const isValid = validator.validateColumnIds(
+ columnConfiguration.columnIds
+ );
+ expect(isValid).toBe(
+ columnConfiguration.invalidKeys.length === 0
+ );
validateValidity(columnConfiguration.invalidKeys);
});
}
@@ -237,21 +293,43 @@ describe('TableValidator', () => {
{ stringField: true, numberField: 12 }
];
- const recordIdsAreValid = validator.validateRecordIds(data, 'stringField');
+ const recordIdsAreValid = validator.validateRecordIds(
+ data,
+ 'stringField'
+ );
expect(recordIdsAreValid).toBeFalse();
- validateValidity(['missingRecordId', 'duplicateRecordId', 'invalidRecordId']);
-
- const columnIdsAreValid = validator.validateColumnIds(['id-1', 'id-2', 'id-3']);
+ validateValidity([
+ 'missingRecordId',
+ 'duplicateRecordId',
+ 'invalidRecordId'
+ ]);
+
+ const columnIdsAreValid = validator.validateColumnIds([
+ 'id-1',
+ 'id-2',
+ 'id-3'
+ ]);
expect(columnIdsAreValid).toBeTrue();
- validateValidity(['missingRecordId', 'duplicateRecordId', 'invalidRecordId']);
+ validateValidity([
+ 'missingRecordId',
+ 'duplicateRecordId',
+ 'invalidRecordId'
+ ]);
});
it('invalid column IDs stay invalid when validating record IDs', () => {
- const columnIdsAreValid = validator.validateColumnIds(['id-1', 'id-1', undefined]);
+ const columnIdsAreValid = validator.validateColumnIds([
+ 'id-1',
+ 'id-1',
+ undefined
+ ]);
expect(columnIdsAreValid).toBeFalse();
validateValidity(['missingColumnId', 'duplicateColumnId']);
- const recordIdsAreValid = validator.validateRecordIds([], undefined);
+ const recordIdsAreValid = validator.validateRecordIds(
+ [],
+ undefined
+ );
expect(recordIdsAreValid).toBeTrue();
validateValidity(['missingColumnId', 'duplicateColumnId']);
});
diff --git a/packages/nimble-components/src/table/models/table-validator.ts b/packages/nimble-components/src/table/models/table-validator.ts
index 76e1d71b9c..941bf0c5c3 100644
--- a/packages/nimble-components/src/table/models/table-validator.ts
+++ b/packages/nimble-components/src/table/models/table-validator.ts
@@ -64,11 +64,15 @@ export class TableValidator<TData extends TableRecord> {
);
}
- public validateColumnIds(columnIds: (string | null | undefined)[]): boolean {
+ public validateColumnIds(
+ columnIds: (string | null | undefined)[]
+ ): boolean {
this.missingColumnId = false;
this.duplicateColumnId = false;
- const anyColumnsHaveIds = columnIds.some(columnId => columnId !== undefined && columnId !== null);
+ const anyColumnsHaveIds = columnIds.some(
+ columnId => columnId !== undefined && columnId !== null
+ );
if (!anyColumnsHaveIds) {
return true;
@@ -87,9 +91,6 @@ export class TableValidator<TData extends TableRecord> {
idSet.add(columnId);
}
- return (
- !this.missingColumnId
- && !this.duplicateColumnId
- );
+ return !this.missingColumnId && !this.duplicateColumnId;
}
}
diff --git a/packages/nimble-components/src/table/template.ts b/packages/nimble-components/src/table/template.ts
index 22ff9d9542..f4606841c3 100644
--- a/packages/nimble-components/src/table/template.ts
+++ b/packages/nimble-components/src/table/template.ts
@@ -1,10 +1,4 @@
-import {
- children,
- html,
- ref,
- repeat,
- when
-} from '@microsoft/fast-element';
+import { children, html, ref, repeat, when } from '@microsoft/fast-element';
import { DesignSystem } from '@microsoft/fast-foundation';
import type { VirtualItem } from '@tanstack/virtual-core';
import type { Table } from '.';
From 641af9c533516ca9749f6ea0d79803ed00231d77 Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Tue, 7 Feb 2023 10:08:35 -0600
Subject: [PATCH 10/23] remove old test (merge issue)
---
.../src/table/models/table-validator.spec.ts | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/packages/nimble-components/src/table/models/table-validator.spec.ts b/packages/nimble-components/src/table/models/table-validator.spec.ts
index 3126a5b9ec..a01445a104 100644
--- a/packages/nimble-components/src/table/models/table-validator.spec.ts
+++ b/packages/nimble-components/src/table/models/table-validator.spec.ts
@@ -52,17 +52,6 @@ describe('TableValidator', () => {
validateValidity([]);
});
- it('setting `null` field for ID is valid', () => {
- const data = [
- { stringField: 'value-1', numberField: 10 },
- { stringField: 'value-2', numberField: 11 }
- ];
-
- const isValid = validator.validateRecordIds(data, null);
- expect(isValid).toBeTrue();
- validateValidity([]);
- });
-
it('setting data with duplicate IDs is invalid', () => {
const data = [
{ stringField: 'value-1', numberField: 10 },
From ab84077dccb56cb719422c39864d89e721918eda Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Tue, 7 Feb 2023 11:48:07 -0600
Subject: [PATCH 11/23] clean up + don't allow null column ids
---
.../src/table-column/base/index.ts | 2 +-
packages/nimble-components/src/table/index.ts | 18 ++++++++--------
.../src/table/models/table-validator.spec.ts | 21 +++++--------------
.../src/table/models/table-validator.ts | 8 +++----
.../src/table/tests/table.stories.ts | 8 +++----
5 files changed, 22 insertions(+), 35 deletions(-)
diff --git a/packages/nimble-components/src/table-column/base/index.ts b/packages/nimble-components/src/table-column/base/index.ts
index 6696c5d699..4e578f4763 100644
--- a/packages/nimble-components/src/table-column/base/index.ts
+++ b/packages/nimble-components/src/table-column/base/index.ts
@@ -14,7 +14,7 @@ export abstract class TableColumn<
TColumnConfig = unknown
> extends FoundationElement {
@attr({ attribute: 'column-id' })
- public columnId?: string | null;
+ public columnId?: string;
/**
* The template to use to render the cell content for the column
diff --git a/packages/nimble-components/src/table/index.ts b/packages/nimble-components/src/table/index.ts
index c1338214dd..9da901f06e 100644
--- a/packages/nimble-components/src/table/index.ts
+++ b/packages/nimble-components/src/table/index.ts
@@ -90,11 +90,7 @@ export class Table<
public override connectedCallback(): void {
super.connectedCallback();
this.virtualizer.connectedCallback();
-
- this.tableValidator.validateColumnIds(
- this.columns.map(x => x.columnId)
- );
- this.canRenderRows = this.checkValidity();
+ this.validateColumnIds();
}
public override disconnectedCallback(): void {
@@ -122,10 +118,7 @@ export class Table<
return;
}
- this.tableValidator.validateColumnIds(
- this.columns.map(x => x.columnId)
- );
- this.canRenderRows = this.checkValidity();
+ this.validateColumnIds();
}
private childItemsChanged(
@@ -137,6 +130,13 @@ export class Table<
.map(x => x as TableColumn);
}
+ private validateColumnIds(): void {
+ this.tableValidator.validateColumnIds(
+ this.columns.map(x => x.columnId)
+ );
+ this.canRenderRows = this.checkValidity();
+ }
+
private setTableData(newData: readonly TData[]): void {
const data = newData.map(record => {
return { ...record };
diff --git a/packages/nimble-components/src/table/models/table-validator.spec.ts b/packages/nimble-components/src/table/models/table-validator.spec.ts
index a01445a104..6246610af0 100644
--- a/packages/nimble-components/src/table/models/table-validator.spec.ts
+++ b/packages/nimble-components/src/table/models/table-validator.spec.ts
@@ -210,17 +210,17 @@ describe('TableValidator', () => {
describe('column ID validation', () => {
const columnConfigurations: {
- columnIds: (string | undefined | null)[],
+ columnIds: (string | undefined)[],
invalidKeys: (keyof TableValidity)[],
testDescription: string
}[] = [
{
- columnIds: [null, null, undefined, undefined],
+ columnIds: [undefined, ''],
invalidKeys: [],
testDescription: 'does not require column IDs'
},
{
- columnIds: [null, 'my-id', undefined, undefined],
+ columnIds: ['my-id', undefined, undefined],
invalidKeys: ['missingColumnId'],
testDescription:
'requires column IDs for all columns if a column ID is defined for at least one'
@@ -241,20 +241,9 @@ describe('TableValidator', () => {
testDescription: 'reports multiple column ID validation errors'
},
{
- columnIds: ['my-id-1', 'my-id-2', ''],
- invalidKeys: [],
- testDescription: 'empty string is a valid column ID'
- },
- {
- columnIds: ['my-id-1', '', ''],
- invalidKeys: ['duplicateColumnId'],
- testDescription: 'detects duplicate empty string column IDs'
- },
- {
- columnIds: [null, undefined, ''],
+ columnIds: ['my-id-1', ''],
invalidKeys: ['missingColumnId'],
- testDescription:
- 'requires column IDs for all columns if one is set to empty string'
+ testDescription: 'does not allow empty string as a defined column ID'
}
];
diff --git a/packages/nimble-components/src/table/models/table-validator.ts b/packages/nimble-components/src/table/models/table-validator.ts
index 6b6bd17dd5..ad6cbdc49b 100644
--- a/packages/nimble-components/src/table/models/table-validator.ts
+++ b/packages/nimble-components/src/table/models/table-validator.ts
@@ -65,14 +65,12 @@ export class TableValidator<TData extends TableRecord> {
}
public validateColumnIds(
- columnIds: (string | null | undefined)[]
+ columnIds: (string | undefined)[]
): boolean {
this.missingColumnId = false;
this.duplicateColumnId = false;
- const anyColumnsHaveIds = columnIds.some(
- columnId => columnId !== undefined && columnId !== null
- );
+ const anyColumnsHaveIds = columnIds.some(id => id);
if (!anyColumnsHaveIds) {
return true;
@@ -80,7 +78,7 @@ export class TableValidator<TData extends TableRecord> {
const idSet = new Set<string>();
for (const columnId of columnIds) {
- if (typeof columnId !== 'string') {
+ if (!columnId) {
this.missingColumnId = true;
continue;
}
diff --git a/packages/nimble-components/src/table/tests/table.stories.ts b/packages/nimble-components/src/table/tests/table.stories.ts
index 6481d9aad8..5cd3030052 100644
--- a/packages/nimble-components/src/table/tests/table.stories.ts
+++ b/packages/nimble-components/src/table/tests/table.stories.ts
@@ -127,10 +127,10 @@ const metadata: Meta<TableArgs> = {
id-field-name="${x => dataSetIdFieldNames[x.data]}"
data-unused="${x => x.updateData(x)}"
>
- <nimble-table-column-text field-name="firstName" placeholder="no value" column-id="foo">First Name</nimble-table-column-text>
- <nimble-table-column-text field-name="lastName" placeholder="no value" column-id="food">Last Name</nimble-table-column-text>
- <nimble-table-column-text field-name="favoriteColor" placeholder="no value" column-id="bar">Favorite Color</nimble-table-column-text>
- <nimble-table-column-text field-name="quote" placeholder="no value" column-id="baz">Quote</nimble-table-column-text>
+ <nimble-table-column-text field-name="firstName" placeholder="no value" column-id="first-name-column">First Name</nimble-table-column-text>
+ <nimble-table-column-text field-name="lastName" placeholder="no value" column-id="last-name-column">Last Name</nimble-table-column-text>
+ <nimble-table-column-text field-name="favoriteColor" placeholder="no value" column-id="favorite-color-column">Favorite Color</nimble-table-column-text>
+ <nimble-table-column-text field-name="quote" placeholder="no value" column-id="quote-column">Quote</nimble-table-column-text>
</nimble-table>
<style class="code-hide">
#usage-warning {
From 1e1c59eb5b934c90aa0a503485e07efe0b960324 Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Tue, 7 Feb 2023 11:50:04 -0600
Subject: [PATCH 12/23] format
---
.../src/table/models/table-validator.spec.ts | 3 ++-
.../nimble-components/src/table/models/table-validator.ts | 4 +---
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/packages/nimble-components/src/table/models/table-validator.spec.ts b/packages/nimble-components/src/table/models/table-validator.spec.ts
index 6246610af0..9dd03cabb9 100644
--- a/packages/nimble-components/src/table/models/table-validator.spec.ts
+++ b/packages/nimble-components/src/table/models/table-validator.spec.ts
@@ -243,7 +243,8 @@ describe('TableValidator', () => {
{
columnIds: ['my-id-1', ''],
invalidKeys: ['missingColumnId'],
- testDescription: 'does not allow empty string as a defined column ID'
+ testDescription:
+ 'does not allow empty string as a defined column ID'
}
];
diff --git a/packages/nimble-components/src/table/models/table-validator.ts b/packages/nimble-components/src/table/models/table-validator.ts
index ad6cbdc49b..36c1221eec 100644
--- a/packages/nimble-components/src/table/models/table-validator.ts
+++ b/packages/nimble-components/src/table/models/table-validator.ts
@@ -64,9 +64,7 @@ export class TableValidator<TData extends TableRecord> {
);
}
- public validateColumnIds(
- columnIds: (string | undefined)[]
- ): boolean {
+ public validateColumnIds(columnIds: (string | undefined)[]): boolean {
this.missingColumnId = false;
this.duplicateColumnId = false;
From 77c49eb92898a09977868c3cf246c8f089be59a4 Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Tue, 7 Feb 2023 11:51:46 -0600
Subject: [PATCH 13/23] remove null as type in Angular
---
.../table-column/text/nimble-table-column-text.directive.ts | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/angular-workspace/projects/ni/nimble-angular/src/directives/table-column/text/nimble-table-column-text.directive.ts b/angular-workspace/projects/ni/nimble-angular/src/directives/table-column/text/nimble-table-column-text.directive.ts
index 66416fa0bf..05bb21f9be 100644
--- a/angular-workspace/projects/ni/nimble-angular/src/directives/table-column/text/nimble-table-column-text.directive.ts
+++ b/angular-workspace/projects/ni/nimble-angular/src/directives/table-column/text/nimble-table-column-text.directive.ts
@@ -10,13 +10,13 @@ export type { TableColumnText };
selector: 'nimble-table-column-text'
})
export class NimbleTableColumnTextDirective {
- public get columnId(): string | null | undefined {
+ public get columnId(): string | undefined {
return this.elementRef.nativeElement.columnId;
}
// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
- @Input('column-id') public set columnId(value: string | null | undefined) {
+ @Input('column-id') public set columnId(value: string | undefined) {
this.renderer.setProperty(this.elementRef.nativeElement, 'columnId', value);
}
From 7e4997552860ddc0ca127f16ca57d2fb2ea3d805 Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Tue, 7 Feb 2023 12:26:46 -0600
Subject: [PATCH 14/23] simplify
---
packages/nimble-components/src/table/index.ts | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/packages/nimble-components/src/table/index.ts b/packages/nimble-components/src/table/index.ts
index 9da901f06e..ce3c06b4e9 100644
--- a/packages/nimble-components/src/table/index.ts
+++ b/packages/nimble-components/src/table/index.ts
@@ -126,8 +126,7 @@ export class Table<
next: Element[]
): void {
this.columns = next
- .filter(x => x instanceof TableColumn)
- .map(x => x as TableColumn);
+ .filter((x): x is TableColumn => x instanceof TableColumn);
}
private validateColumnIds(): void {
From d019e9e763453f36dff0f3bd33c6f57848ea83b5 Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Wed, 8 Feb 2023 14:06:32 -0600
Subject: [PATCH 15/23] change observation pattern
---
packages/nimble-components/src/table/index.ts | 41 +++++++++++++------
.../nimble-components/src/table/template.ts | 25 ++++++-----
2 files changed, 43 insertions(+), 23 deletions(-)
diff --git a/packages/nimble-components/src/table/index.ts b/packages/nimble-components/src/table/index.ts
index ce3c06b4e9..1d721cd1b9 100644
--- a/packages/nimble-components/src/table/index.ts
+++ b/packages/nimble-components/src/table/index.ts
@@ -1,4 +1,4 @@
-import { attr, observable } from '@microsoft/fast-element';
+import { attr, Observable, observable, Notifier } from '@microsoft/fast-element';
import { DesignSystem, FoundationElement } from '@microsoft/fast-foundation';
import {
ColumnDef as TanStackColumnDef,
@@ -9,12 +9,12 @@ import {
getCoreRowModel as tanStackGetCoreRowModel,
TableOptionsResolved as TanStackTableOptionsResolved
} from '@tanstack/table-core';
-import { TableColumn } from '../table-column/base';
import { TableValidator } from './models/table-validator';
import { styles } from './styles';
import { template } from './template';
import type { TableRecord, TableRowState, TableValidity } from './types';
import { Virtualizer } from './models/virtualizer';
+import { TableColumn } from '../table-column/base';
declare global {
interface HTMLElementTagNameMap {
@@ -38,7 +38,7 @@ export class Table<
public tableData: TableRowState<TData>[] = [];
@observable
- public columns: TableColumn[] = [];
+ public readonly columns: TableColumn[] = [];
/**
* @internal
@@ -55,9 +55,6 @@ export class Table<
*/
public readonly viewport!: HTMLElement;
- @observable
- public childItems: Element[] = [];
-
/**
* @internal
*/
@@ -66,6 +63,7 @@ export class Table<
private readonly table: TanStackTable<TData>;
private options: TanStackTableOptionsResolved<TData>;
private readonly tableValidator = new TableValidator();
+ private notifiers: Notifier[] = [];
public constructor() {
super();
@@ -90,6 +88,7 @@ export class Table<
public override connectedCallback(): void {
super.connectedCallback();
this.virtualizer.connectedCallback();
+ this.observeColumns();
this.validateColumnIds();
}
@@ -101,6 +100,19 @@ export class Table<
return this.tableValidator.isValid();
}
+ /**
+ * @internal
+ */
+ // 'handleChange' is an API exposed by FAST that we need to implement. Disable lint rules caused by its signature.
+ // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/no-explicit-any
+ public handleChange(source: any, args: any): void {
+ if (source instanceof TableColumn) {
+ if (args === 'columnId') {
+ this.validateColumnIds();
+ }
+ }
+ }
+
private idFieldNameChanged(
_prev: string | undefined,
_next: string | undefined
@@ -118,15 +130,20 @@ export class Table<
return;
}
+ this.observeColumns();
this.validateColumnIds();
}
- private childItemsChanged(
- _prev: undefined | Element[],
- next: Element[]
- ): void {
- this.columns = next
- .filter((x): x is TableColumn => x instanceof TableColumn);
+ private observeColumns(): void {
+ this.notifiers.forEach(notifier => {
+ notifier.unsubscribe(this);
+ });
+ this.notifiers = [];
+
+ for (const column of this.columns) {
+ const notifier = Observable.getNotifier(column);
+ notifier.subscribe(this);
+ }
}
private validateColumnIds(): void {
diff --git a/packages/nimble-components/src/table/template.ts b/packages/nimble-components/src/table/template.ts
index f4606841c3..da64b703da 100644
--- a/packages/nimble-components/src/table/template.ts
+++ b/packages/nimble-components/src/table/template.ts
@@ -1,22 +1,25 @@
-import { children, html, ref, repeat, when } from '@microsoft/fast-element';
+import { children, ElementsFilter, html, ref, repeat, when } from '@microsoft/fast-element';
import { DesignSystem } from '@microsoft/fast-foundation';
import type { VirtualItem } from '@tanstack/virtual-core';
import type { Table } from '.';
import { TableHeader } from './components/header';
import { TableRow } from './components/row';
-import type { TableColumn } from '../table-column/base';
+import { TableColumn } from '../table-column/base';
+
+const isTableColumn = (): ElementsFilter => {
+ const filter: ElementsFilter = (
+ value: Node,
+ _: number,
+ __: Node[]
+ ): boolean => {
+ return value instanceof TableColumn;
+ };
+ return filter;
+};
// prettier-ignore
export const template = html<Table>`
- <template
- role="table"
- ${children({
- property: 'childItems',
- attributeFilter: ['column-id'],
- subtree: true,
- selector: '*'
- })}
- >
+ <template role="table" ${children({ property: 'columns', filter: isTableColumn() })}>
<div class="table-container">
<div role="rowgroup" class="header-container" style="margin-right: ${x => x.virtualizer.headerContainerMarginRight}px;">
<div class="header-row" role="row">
From f5590a7770f9cbafa36b7324ac62c795ddebe15a Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Wed, 8 Feb 2023 14:26:48 -0600
Subject: [PATCH 16/23] Update table-validator.spec.ts
---
.../src/table/models/table-validator.spec.ts | 77 ++++++++++---------
1 file changed, 39 insertions(+), 38 deletions(-)
diff --git a/packages/nimble-components/src/table/models/table-validator.spec.ts b/packages/nimble-components/src/table/models/table-validator.spec.ts
index 9dd03cabb9..ad6400e6cc 100644
--- a/packages/nimble-components/src/table/models/table-validator.spec.ts
+++ b/packages/nimble-components/src/table/models/table-validator.spec.ts
@@ -8,25 +8,10 @@ describe('TableValidator', () => {
validator = new TableValidator();
});
- function validateValidity(invalidKeys: (keyof TableValidity)[]): void {
- expect(validator.isValid()).toEqual(invalidKeys.length === 0);
-
- const validity = validator.getValidity();
- expect(validity.duplicateRecordId).toBe(
- invalidKeys.some(x => x === 'duplicateRecordId')
- );
- expect(validity.missingRecordId).toBe(
- invalidKeys.some(x => x === 'missingRecordId')
- );
- expect(validity.invalidRecordId).toBe(
- invalidKeys.some(x => x === 'invalidRecordId')
- );
- expect(validity.duplicateColumnId).toBe(
- invalidKeys.some(x => x === 'duplicateColumnId')
- );
- expect(validity.missingColumnId).toBe(
- invalidKeys.some(x => x === 'missingColumnId')
- );
+ function getInvalidKeys(tableValidator: TableValidator<TableRecord>): string[] {
+ return Object.entries(tableValidator)
+ .filter(([_, value]) => value)
+ .map(([key, _]) => key);
}
describe('record ID validation', () => {
@@ -38,7 +23,8 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, 'stringField');
expect(isValid).toBeTrue();
- validateValidity([]);
+ expect(validator.isValid()).toBeTrue();
+ expect(getInvalidKeys(validator)).toEqual([]);
});
it('setting `undefined` field for ID is valid', () => {
@@ -49,7 +35,8 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, undefined);
expect(isValid).toBeTrue();
- validateValidity([]);
+ expect(validator.isValid()).toBeTrue();
+ expect(getInvalidKeys(validator)).toEqual([]);
});
it('setting data with duplicate IDs is invalid', () => {
@@ -60,7 +47,8 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, 'stringField');
expect(isValid).toBeFalse();
- validateValidity(['duplicateRecordId']);
+ expect(validator.isValid()).toBeFalse();
+ expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(['duplicateRecordId']));
});
it('setting data with invalid ID value type is invalid', () => {
@@ -71,7 +59,8 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, 'numberField');
expect(isValid).toBeFalse();
- validateValidity(['invalidRecordId']);
+ expect(validator.isValid()).toBeFalse();
+ expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(['invalidRecordId']));
});
it('setting data with empty ID value is valid', () => {
@@ -82,7 +71,8 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, 'stringField');
expect(isValid).toBeTrue();
- validateValidity([]);
+ expect(validator.isValid()).toBeTrue();
+ expect(getInvalidKeys(validator)).toEqual([]);
});
it('setting data with undefined ID value is invalid', () => {
@@ -93,7 +83,8 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, 'stringField');
expect(isValid).toBeFalse();
- validateValidity(['invalidRecordId']);
+ expect(validator.isValid()).toBeFalse();
+ expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(['invalidRecordId']));
});
it('setting data with null ID value is invalid', () => {
@@ -104,7 +95,8 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, 'stringField');
expect(isValid).toBeFalse();
- validateValidity(['invalidRecordId']);
+ expect(validator.isValid()).toBeFalse();
+ expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(['invalidRecordId']));
});
it('setting data with missing IDs is invalid', () => {
@@ -115,7 +107,8 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, 'missingField');
expect(isValid).toBeFalse();
- validateValidity(['missingRecordId']);
+ expect(validator.isValid()).toBeFalse();
+ expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(['missingRecordId']));
});
it('multiple errors are reported during validation', () => {
@@ -129,11 +122,12 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, 'stringField');
expect(isValid).toBeFalse();
- validateValidity([
+ expect(validator.isValid()).toBeFalse();
+ expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents([
'missingRecordId',
'duplicateRecordId',
'invalidRecordId'
- ]);
+ ]));
});
it('setting ID field name to undefined makes configuration valid', () => {
@@ -191,7 +185,8 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, '');
expect(isValid).toBeTrue();
- validateValidity([]);
+ expect(validator.isValid()).toBeTrue();
+ expect(getInvalidKeys(validator)).toEqual([]);
});
it('validation occurs when ID field name is an empty string', () => {
@@ -204,7 +199,8 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, '');
expect(isValid).toBeFalse();
- validateValidity(['duplicateRecordId']);
+ expect(validator.isValid()).toBeFalse();
+ expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(['duplicateRecordId']));
});
});
@@ -257,7 +253,8 @@ describe('TableValidator', () => {
expect(isValid).toBe(
columnConfiguration.invalidKeys.length === 0
);
- validateValidity(columnConfiguration.invalidKeys);
+ expect(validator.isValid()).toBe(columnConfiguration.invalidKeys.length === 0);
+ expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(columnConfiguration.invalidKeys));
});
}
});
@@ -277,11 +274,12 @@ describe('TableValidator', () => {
'stringField'
);
expect(recordIdsAreValid).toBeFalse();
- validateValidity([
+ expect(validator.isValid()).toBeFalse();
+ expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents([
'missingRecordId',
'duplicateRecordId',
'invalidRecordId'
- ]);
+ ]));
const columnIdsAreValid = validator.validateColumnIds([
'id-1',
@@ -289,11 +287,12 @@ describe('TableValidator', () => {
'id-3'
]);
expect(columnIdsAreValid).toBeTrue();
- validateValidity([
+ expect(validator.isValid()).toBeFalse();
+ expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents([
'missingRecordId',
'duplicateRecordId',
'invalidRecordId'
- ]);
+ ]));
});
it('invalid column IDs stay invalid when validating record IDs', () => {
@@ -303,14 +302,16 @@ describe('TableValidator', () => {
undefined
]);
expect(columnIdsAreValid).toBeFalse();
- validateValidity(['missingColumnId', 'duplicateColumnId']);
+ expect(validator.isValid()).toBeFalse();
+ expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(['missingColumnId', 'duplicateColumnId']));
const recordIdsAreValid = validator.validateRecordIds(
[],
undefined
);
expect(recordIdsAreValid).toBeTrue();
- validateValidity(['missingColumnId', 'duplicateColumnId']);
+ expect(validator.isValid()).toBeFalse();
+ expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(['missingColumnId', 'duplicateColumnId']));
});
});
});
From 646bec5d208556e55301d512e0d8143a039d09db Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Wed, 8 Feb 2023 14:28:52 -0600
Subject: [PATCH 17/23] format
---
packages/nimble-components/src/table/index.ts | 7 +-
.../src/table/models/table-validator.spec.ts | 88 +++++++++++++------
.../nimble-components/src/table/template.ts | 9 +-
3 files changed, 76 insertions(+), 28 deletions(-)
diff --git a/packages/nimble-components/src/table/index.ts b/packages/nimble-components/src/table/index.ts
index 1d721cd1b9..d42efb2e43 100644
--- a/packages/nimble-components/src/table/index.ts
+++ b/packages/nimble-components/src/table/index.ts
@@ -1,4 +1,9 @@
-import { attr, Observable, observable, Notifier } from '@microsoft/fast-element';
+import {
+ attr,
+ Observable,
+ observable,
+ Notifier
+} from '@microsoft/fast-element';
import { DesignSystem, FoundationElement } from '@microsoft/fast-foundation';
import {
ColumnDef as TanStackColumnDef,
diff --git a/packages/nimble-components/src/table/models/table-validator.spec.ts b/packages/nimble-components/src/table/models/table-validator.spec.ts
index ad6400e6cc..db69de06a6 100644
--- a/packages/nimble-components/src/table/models/table-validator.spec.ts
+++ b/packages/nimble-components/src/table/models/table-validator.spec.ts
@@ -8,7 +8,9 @@ describe('TableValidator', () => {
validator = new TableValidator();
});
- function getInvalidKeys(tableValidator: TableValidator<TableRecord>): string[] {
+ function getInvalidKeys(
+ tableValidator: TableValidator<TableRecord>
+ ): string[] {
return Object.entries(tableValidator)
.filter(([_, value]) => value)
.map(([key, _]) => key);
@@ -48,7 +50,9 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, 'stringField');
expect(isValid).toBeFalse();
expect(validator.isValid()).toBeFalse();
- expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(['duplicateRecordId']));
+ expect(getInvalidKeys(validator)).toEqual(
+ jasmine.arrayWithExactContents(['duplicateRecordId'])
+ );
});
it('setting data with invalid ID value type is invalid', () => {
@@ -60,7 +64,9 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, 'numberField');
expect(isValid).toBeFalse();
expect(validator.isValid()).toBeFalse();
- expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(['invalidRecordId']));
+ expect(getInvalidKeys(validator)).toEqual(
+ jasmine.arrayWithExactContents(['invalidRecordId'])
+ );
});
it('setting data with empty ID value is valid', () => {
@@ -84,7 +90,9 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, 'stringField');
expect(isValid).toBeFalse();
expect(validator.isValid()).toBeFalse();
- expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(['invalidRecordId']));
+ expect(getInvalidKeys(validator)).toEqual(
+ jasmine.arrayWithExactContents(['invalidRecordId'])
+ );
});
it('setting data with null ID value is invalid', () => {
@@ -96,7 +104,9 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, 'stringField');
expect(isValid).toBeFalse();
expect(validator.isValid()).toBeFalse();
- expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(['invalidRecordId']));
+ expect(getInvalidKeys(validator)).toEqual(
+ jasmine.arrayWithExactContents(['invalidRecordId'])
+ );
});
it('setting data with missing IDs is invalid', () => {
@@ -108,7 +118,9 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, 'missingField');
expect(isValid).toBeFalse();
expect(validator.isValid()).toBeFalse();
- expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(['missingRecordId']));
+ expect(getInvalidKeys(validator)).toEqual(
+ jasmine.arrayWithExactContents(['missingRecordId'])
+ );
});
it('multiple errors are reported during validation', () => {
@@ -123,11 +135,13 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, 'stringField');
expect(isValid).toBeFalse();
expect(validator.isValid()).toBeFalse();
- expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents([
- 'missingRecordId',
- 'duplicateRecordId',
- 'invalidRecordId'
- ]));
+ expect(getInvalidKeys(validator)).toEqual(
+ jasmine.arrayWithExactContents([
+ 'missingRecordId',
+ 'duplicateRecordId',
+ 'invalidRecordId'
+ ])
+ );
});
it('setting ID field name to undefined makes configuration valid', () => {
@@ -200,7 +214,9 @@ describe('TableValidator', () => {
const isValid = validator.validateRecordIds(data, '');
expect(isValid).toBeFalse();
expect(validator.isValid()).toBeFalse();
- expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(['duplicateRecordId']));
+ expect(getInvalidKeys(validator)).toEqual(
+ jasmine.arrayWithExactContents(['duplicateRecordId'])
+ );
});
});
@@ -253,8 +269,14 @@ describe('TableValidator', () => {
expect(isValid).toBe(
columnConfiguration.invalidKeys.length === 0
);
- expect(validator.isValid()).toBe(columnConfiguration.invalidKeys.length === 0);
- expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(columnConfiguration.invalidKeys));
+ expect(validator.isValid()).toBe(
+ columnConfiguration.invalidKeys.length === 0
+ );
+ expect(getInvalidKeys(validator)).toEqual(
+ jasmine.arrayWithExactContents(
+ columnConfiguration.invalidKeys
+ )
+ );
});
}
});
@@ -275,11 +297,13 @@ describe('TableValidator', () => {
);
expect(recordIdsAreValid).toBeFalse();
expect(validator.isValid()).toBeFalse();
- expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents([
- 'missingRecordId',
- 'duplicateRecordId',
- 'invalidRecordId'
- ]));
+ expect(getInvalidKeys(validator)).toEqual(
+ jasmine.arrayWithExactContents([
+ 'missingRecordId',
+ 'duplicateRecordId',
+ 'invalidRecordId'
+ ])
+ );
const columnIdsAreValid = validator.validateColumnIds([
'id-1',
@@ -288,11 +312,13 @@ describe('TableValidator', () => {
]);
expect(columnIdsAreValid).toBeTrue();
expect(validator.isValid()).toBeFalse();
- expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents([
- 'missingRecordId',
- 'duplicateRecordId',
- 'invalidRecordId'
- ]));
+ expect(getInvalidKeys(validator)).toEqual(
+ jasmine.arrayWithExactContents([
+ 'missingRecordId',
+ 'duplicateRecordId',
+ 'invalidRecordId'
+ ])
+ );
});
it('invalid column IDs stay invalid when validating record IDs', () => {
@@ -303,7 +329,12 @@ describe('TableValidator', () => {
]);
expect(columnIdsAreValid).toBeFalse();
expect(validator.isValid()).toBeFalse();
- expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(['missingColumnId', 'duplicateColumnId']));
+ expect(getInvalidKeys(validator)).toEqual(
+ jasmine.arrayWithExactContents([
+ 'missingColumnId',
+ 'duplicateColumnId'
+ ])
+ );
const recordIdsAreValid = validator.validateRecordIds(
[],
@@ -311,7 +342,12 @@ describe('TableValidator', () => {
);
expect(recordIdsAreValid).toBeTrue();
expect(validator.isValid()).toBeFalse();
- expect(getInvalidKeys(validator)).toEqual(jasmine.arrayWithExactContents(['missingColumnId', 'duplicateColumnId']));
+ expect(getInvalidKeys(validator)).toEqual(
+ jasmine.arrayWithExactContents([
+ 'missingColumnId',
+ 'duplicateColumnId'
+ ])
+ );
});
});
});
diff --git a/packages/nimble-components/src/table/template.ts b/packages/nimble-components/src/table/template.ts
index da64b703da..26c821d1e7 100644
--- a/packages/nimble-components/src/table/template.ts
+++ b/packages/nimble-components/src/table/template.ts
@@ -1,4 +1,11 @@
-import { children, ElementsFilter, html, ref, repeat, when } from '@microsoft/fast-element';
+import {
+ children,
+ ElementsFilter,
+ html,
+ ref,
+ repeat,
+ when
+} from '@microsoft/fast-element';
import { DesignSystem } from '@microsoft/fast-foundation';
import type { VirtualItem } from '@tanstack/virtual-core';
import type { Table } from '.';
From e7072b3ce7226a555ff4270b5614f9c08c62f6d8 Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Wed, 8 Feb 2023 14:33:16 -0600
Subject: [PATCH 18/23] remove observers on disconnect
---
packages/nimble-components/src/table/index.ts | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/packages/nimble-components/src/table/index.ts b/packages/nimble-components/src/table/index.ts
index d42efb2e43..79e97e8739 100644
--- a/packages/nimble-components/src/table/index.ts
+++ b/packages/nimble-components/src/table/index.ts
@@ -14,12 +14,12 @@ import {
getCoreRowModel as tanStackGetCoreRowModel,
TableOptionsResolved as TanStackTableOptionsResolved
} from '@tanstack/table-core';
+import { TableColumn } from '../table-column/base';
import { TableValidator } from './models/table-validator';
import { styles } from './styles';
import { template } from './template';
import type { TableRecord, TableRowState, TableValidity } from './types';
import { Virtualizer } from './models/virtualizer';
-import { TableColumn } from '../table-column/base';
declare global {
interface HTMLElementTagNameMap {
@@ -99,6 +99,7 @@ export class Table<
public override disconnectedCallback(): void {
this.virtualizer.disconnectedCallback();
+ this.removeColumnObservers();
}
public checkValidity(): boolean {
@@ -139,11 +140,15 @@ export class Table<
this.validateColumnIds();
}
- private observeColumns(): void {
+ private removeColumnObservers(): void {
this.notifiers.forEach(notifier => {
notifier.unsubscribe(this);
});
this.notifiers = [];
+ }
+
+ private observeColumns(): void {
+ this.removeColumnObservers();
for (const column of this.columns) {
const notifier = Observable.getNotifier(column);
From 2f2c549ddcab60f5a4c5b145158c7a8382310be9 Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Thu, 9 Feb 2023 11:40:20 -0600
Subject: [PATCH 19/23] create column base class
---
.../Components/NimbleTableColumnText.razor | 1 +
.../Components/NimbleTableColumnText.razor.cs | 8 +-------
.../nimble-blazor/NimbleBlazor/NimbleTableColumn.cs | 12 ++++++++++++
3 files changed, 14 insertions(+), 7 deletions(-)
create mode 100644 packages/nimble-blazor/NimbleBlazor/NimbleTableColumn.cs
diff --git a/packages/nimble-blazor/NimbleBlazor/Components/NimbleTableColumnText.razor b/packages/nimble-blazor/NimbleBlazor/Components/NimbleTableColumnText.razor
index 2ed123db5f..79a67309c8 100644
--- a/packages/nimble-blazor/NimbleBlazor/Components/NimbleTableColumnText.razor
+++ b/packages/nimble-blazor/NimbleBlazor/Components/NimbleTableColumnText.razor
@@ -1,4 +1,5 @@
@namespace NimbleBlazor
+@inherits NimbleTableColumn
<nimble-table-column-text column-id="@ColumnId" field-name="@FieldName" placeholder="@Placeholder">
@ChildContent
diff --git a/packages/nimble-blazor/NimbleBlazor/Components/NimbleTableColumnText.razor.cs b/packages/nimble-blazor/NimbleBlazor/Components/NimbleTableColumnText.razor.cs
index a7aef0a30c..53e508ee90 100644
--- a/packages/nimble-blazor/NimbleBlazor/Components/NimbleTableColumnText.razor.cs
+++ b/packages/nimble-blazor/NimbleBlazor/Components/NimbleTableColumnText.razor.cs
@@ -3,14 +3,8 @@
namespace NimbleBlazor;
-public partial class NimbleTableColumnText : ComponentBase
+public partial class NimbleTableColumnText : NimbleTableColumn
{
- /// <summary>
- /// The ID of the column of a <see cref="NimbleTable{TData}"/>
- /// </summary>
- [Parameter]
- public string? ColumnId { get; set; }
-
/// <summary>
/// Gets or sets the field in the element representing a row of data in a <see cref="NimbleTable{TData}"/>to display
/// </summary>
diff --git a/packages/nimble-blazor/NimbleBlazor/NimbleTableColumn.cs b/packages/nimble-blazor/NimbleBlazor/NimbleTableColumn.cs
new file mode 100644
index 0000000000..bc26a16ced
--- /dev/null
+++ b/packages/nimble-blazor/NimbleBlazor/NimbleTableColumn.cs
@@ -0,0 +1,12 @@
+using Microsoft.AspNetCore.Components;
+
+namespace NimbleBlazor;
+
+public abstract class NimbleTableColumn : ComponentBase
+{
+ /// <summary>
+ /// The ID of the column of a <see cref="NimbleTable{TData}"/>
+ /// </summary>
+ [Parameter]
+ public string? ColumnId { get; set; }
+}
From b212100637721d5dfcbc70e0d0994cc9b71a9a4f Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Fri, 10 Feb 2023 11:35:01 -0600
Subject: [PATCH 20/23] PR feedback
---
packages/nimble-components/src/table/index.ts | 26 +++++++-----
.../src/table/models/table-validator.spec.ts | 42 +++++++++++--------
2 files changed, 40 insertions(+), 28 deletions(-)
diff --git a/packages/nimble-components/src/table/index.ts b/packages/nimble-components/src/table/index.ts
index 79e97e8739..fe88f9d9e2 100644
--- a/packages/nimble-components/src/table/index.ts
+++ b/packages/nimble-components/src/table/index.ts
@@ -68,7 +68,7 @@ export class Table<
private readonly table: TanStackTable<TData>;
private options: TanStackTableOptionsResolved<TData>;
private readonly tableValidator = new TableValidator();
- private notifiers: Notifier[] = [];
+ private columnNotifiers: Notifier[] = [];
public constructor() {
super();
@@ -93,11 +93,11 @@ export class Table<
public override connectedCallback(): void {
super.connectedCallback();
this.virtualizer.connectedCallback();
- this.observeColumns();
- this.validateColumnIds();
+ this.validateAndObserveColumns();
}
public override disconnectedCallback(): void {
+ super.disconnectedCallback();
this.virtualizer.disconnectedCallback();
this.removeColumnObservers();
}
@@ -108,10 +108,12 @@ export class Table<
/**
* @internal
+ *
+ * The event handler that is called when a notifier detects a change. Notifiers are added
+ * to each column, so `source` is expected to be an instance of `TableColumn`, and `args`
+ * is the string name of the property that changed on that column.
*/
- // 'handleChange' is an API exposed by FAST that we need to implement. Disable lint rules caused by its signature.
- // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/no-explicit-any
- public handleChange(source: any, args: any): void {
+ public handleChange(source: unknown, args: unknown): void {
if (source instanceof TableColumn) {
if (args === 'columnId') {
this.validateColumnIds();
@@ -136,24 +138,26 @@ export class Table<
return;
}
- this.observeColumns();
- this.validateColumnIds();
+ this.validateAndObserveColumns();
}
private removeColumnObservers(): void {
- this.notifiers.forEach(notifier => {
+ this.columnNotifiers.forEach(notifier => {
notifier.unsubscribe(this);
});
- this.notifiers = [];
+ this.columnNotifiers = [];
}
- private observeColumns(): void {
+ private validateAndObserveColumns(): void {
this.removeColumnObservers();
for (const column of this.columns) {
const notifier = Observable.getNotifier(column);
notifier.subscribe(this);
+ this.columnNotifiers.push(notifier);
}
+
+ this.validateColumnIds();
}
private validateColumnIds(): void {
diff --git a/packages/nimble-components/src/table/models/table-validator.spec.ts b/packages/nimble-components/src/table/models/table-validator.spec.ts
index db69de06a6..e5ed64559d 100644
--- a/packages/nimble-components/src/table/models/table-validator.spec.ts
+++ b/packages/nimble-components/src/table/models/table-validator.spec.ts
@@ -1,5 +1,6 @@
import type { TableRecord, TableValidity } from '../types';
import { TableValidator } from './table-validator';
+import { getSpecTypeByNamedList } from '../../utilities/tests/parameterized';
describe('TableValidator', () => {
let validator: TableValidator<TableRecord>;
@@ -223,56 +224,63 @@ describe('TableValidator', () => {
describe('column ID validation', () => {
const columnConfigurations: {
columnIds: (string | undefined)[],
+ isValid: boolean,
invalidKeys: (keyof TableValidity)[],
- testDescription: string
+ name: string
}[] = [
{
columnIds: [undefined, ''],
+ isValid: true,
invalidKeys: [],
- testDescription: 'does not require column IDs'
+ name: 'does not require column IDs'
},
{
columnIds: ['my-id', undefined, undefined],
+ isValid: false,
invalidKeys: ['missingColumnId'],
- testDescription:
+ name:
'requires column IDs for all columns if a column ID is defined for at least one'
},
{
columnIds: ['my-id-1', 'my-id-2', 'my-id-3'],
+ isValid: true,
invalidKeys: [],
- testDescription: 'unique defined IDs are valid'
+ name: 'unique defined IDs are valid'
},
{
columnIds: ['my-id-1', 'my-id-2', 'my-id-2'],
+ isValid: false,
invalidKeys: ['duplicateColumnId'],
- testDescription: 'duplicate column IDs is invalid'
+ name: 'duplicate column IDs is invalid'
},
{
columnIds: ['my-id-1', 'my-id-2', 'my-id-2', undefined],
+ isValid: false,
invalidKeys: ['missingColumnId', 'duplicateColumnId'],
- testDescription: 'reports multiple column ID validation errors'
+ name: 'reports multiple column ID validation errors'
},
{
columnIds: ['my-id-1', ''],
+ isValid: false,
invalidKeys: ['missingColumnId'],
- testDescription:
+ name:
'does not allow empty string as a defined column ID'
}
];
+ const focused: string[] = [];
+ const disabled: string[] = [];
for (const columnConfiguration of columnConfigurations) {
- // eslint-disable-next-line @typescript-eslint/no-loop-func
- it(columnConfiguration.testDescription, () => {
- const isValid = validator.validateColumnIds(
+ const specType = getSpecTypeByNamedList(columnConfiguration, focused, disabled);
+ specType(columnConfiguration.name, () => {
+ const tableValidator = new TableValidator();
+ const isValid = tableValidator.validateColumnIds(
columnConfiguration.columnIds
);
- expect(isValid).toBe(
- columnConfiguration.invalidKeys.length === 0
- );
- expect(validator.isValid()).toBe(
- columnConfiguration.invalidKeys.length === 0
- );
- expect(getInvalidKeys(validator)).toEqual(
+
+ expect(isValid).toBe(columnConfiguration.isValid);
+ expect(tableValidator.isValid()).toBe(columnConfiguration.invalidKeys.length === 0);
+ expect(getInvalidKeys(tableValidator)).toEqual(
jasmine.arrayWithExactContents(
columnConfiguration.invalidKeys
)
From 3d958f47ba2efef64b2e613ee127ba3d65664e7c Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Fri, 10 Feb 2023 11:37:01 -0600
Subject: [PATCH 21/23] format
---
.../src/table/models/table-validator.spec.ts | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/packages/nimble-components/src/table/models/table-validator.spec.ts b/packages/nimble-components/src/table/models/table-validator.spec.ts
index e5ed64559d..eee0c02f71 100644
--- a/packages/nimble-components/src/table/models/table-validator.spec.ts
+++ b/packages/nimble-components/src/table/models/table-validator.spec.ts
@@ -238,8 +238,7 @@ describe('TableValidator', () => {
columnIds: ['my-id', undefined, undefined],
isValid: false,
invalidKeys: ['missingColumnId'],
- name:
- 'requires column IDs for all columns if a column ID is defined for at least one'
+ name: 'requires column IDs for all columns if a column ID is defined for at least one'
},
{
columnIds: ['my-id-1', 'my-id-2', 'my-id-3'],
@@ -263,15 +262,18 @@ describe('TableValidator', () => {
columnIds: ['my-id-1', ''],
isValid: false,
invalidKeys: ['missingColumnId'],
- name:
- 'does not allow empty string as a defined column ID'
+ name: 'does not allow empty string as a defined column ID'
}
];
const focused: string[] = [];
const disabled: string[] = [];
for (const columnConfiguration of columnConfigurations) {
- const specType = getSpecTypeByNamedList(columnConfiguration, focused, disabled);
+ const specType = getSpecTypeByNamedList(
+ columnConfiguration,
+ focused,
+ disabled
+ );
specType(columnConfiguration.name, () => {
const tableValidator = new TableValidator();
const isValid = tableValidator.validateColumnIds(
@@ -279,7 +281,9 @@ describe('TableValidator', () => {
);
expect(isValid).toBe(columnConfiguration.isValid);
- expect(tableValidator.isValid()).toBe(columnConfiguration.invalidKeys.length === 0);
+ expect(tableValidator.isValid()).toBe(
+ columnConfiguration.invalidKeys.length === 0
+ );
expect(getInvalidKeys(tableValidator)).toEqual(
jasmine.arrayWithExactContents(
columnConfiguration.invalidKeys
From dfc0cc0eb5b887725a310bde56a66646eebf193c Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Tue, 14 Feb 2023 10:16:31 -0600
Subject: [PATCH 22/23] fix merge issues
---
packages/nimble-components/src/table/index.ts | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/packages/nimble-components/src/table/index.ts b/packages/nimble-components/src/table/index.ts
index 1a4b16a3dd..bcd413d28c 100644
--- a/packages/nimble-components/src/table/index.ts
+++ b/packages/nimble-components/src/table/index.ts
@@ -130,7 +130,11 @@ export class Table<
}
}
- private idFieldNameChanged(
+ protected childItemsChanged(): void {
+ void this.updateColumnsFromChildItems();
+ }
+
+ protected idFieldNameChanged(
_prev: string | undefined,
_next: string | undefined
): void {
@@ -139,7 +143,7 @@ export class Table<
this.setTableData(this.table.options.data);
}
- private columnsChanged(
+ protected columnsChanged(
_prev: TableColumn[] | undefined,
_next: TableColumn[]
): void {
@@ -176,10 +180,6 @@ export class Table<
this.canRenderRows = this.checkValidity();
}
- protected childItemsChanged(): void {
- void this.updateColumnsFromChildItems();
- }
-
private async updateColumnsFromChildItems(): Promise<void> {
const definedElements = this.childItems.map(async item => (item.matches(':not(:defined)')
? customElements.whenDefined(item.localName)
From 30a542335a3c295aaaa560b0d0153637a7a673a6 Mon Sep 17 00:00:00 2001
From: mollykreis <20542556+mollykreis@users.noreply.github.com>
Date: Tue, 14 Feb 2023 10:40:09 -0600
Subject: [PATCH 23/23] format
---
packages/nimble-components/src/table/tests/table.spec.ts | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/packages/nimble-components/src/table/tests/table.spec.ts b/packages/nimble-components/src/table/tests/table.spec.ts
index 24a1149142..2eda452999 100644
--- a/packages/nimble-components/src/table/tests/table.spec.ts
+++ b/packages/nimble-components/src/table/tests/table.spec.ts
@@ -114,7 +114,9 @@ describe('Table', () => {
column1.textContent = 'stringData';
(column1 as TableColumnText).fieldName = 'stringData';
- const checkIcon = document.createElement(DesignSystem.tagFor(IconCheck));
+ const checkIcon = document.createElement(
+ DesignSystem.tagFor(IconCheck)
+ );
column2 = document.createElement(tableColumnTextTag) as TableColumn;
column2.appendChild(checkIcon);
(column2 as TableColumnText).fieldName = 'moreStringData';