Move column validator into columnInternals (#1999)

# Pull Request ## 🤨 Rationale Resolves #1389 ## 👩‍💻 Implementation - Updated `TableColumn`, `ColumnInternals`, and `ColumnInternalsOptions` to be generic with `TColumnValidator extends ColumnValidator<[]>` - Updated `ColumnInternals` to have a `validator` property whose type is enforced by the type of `TColumnValidator` - Made `validator` a required input on `ColumnInternalsOptions`, so now every column has to specify a validator. In some cases this is a "default" validator with no validity keys, but making this required simplified a few different code paths. This included keeping columns with validators from having to use the `!` operator to assert that they knew that a validator had been set within their options. - Updated columns to specify a validator and to use that validator through `ColumnInternals` rather than a class member on the column. ## 🧪 Testing Existing tests still pass ## ✅ Checklist  - [ ] I have updated the project documentation to reflect my changes or determined no changes are needed.
ni · Apr 8, 2024 · d16c59e · d16c59e
1 parent 967f9e3
commit d16c59e
Show file tree

Hide file tree

Showing 28 changed files with 155 additions and 150 deletions.
diff --git a/change/@ni-nimble-components-90fb9043-d42e-4be9-827d-e6f94ab7b96b.json b/change/@ni-nimble-components-90fb9043-d42e-4be9-827d-e6f94ab7b96b.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Move validator into ColumnInternals",
+  "packageName": "@ni/nimble-components",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/nimble-components/src/table-column/anchor/index.ts b/packages/nimble-components/src/table-column/anchor/index.ts
@@ -12,6 +12,7 @@ import { tableColumnAnchorCellViewTag } from './cell-view';
 import { tableColumnTextGroupHeaderViewTag } from '../text/group-header-view';
 import type { AnchorAppearance } from '../../anchor/types';
 import type { ColumnInternalsOptions } from '../base/models/column-internals';
+import { ColumnValidator } from '../base/models/column-validator';
 
 export type TableColumnAnchorCellRecord = TableStringField<'label' | 'href'>;
 export interface TableColumnAnchorColumnConfig {
@@ -86,7 +87,8 @@ export class TableColumnAnchor extends mixinGroupableColumnAPI(
             cellViewTag: tableColumnAnchorCellViewTag,
             groupHeaderViewTag: tableColumnTextGroupHeaderViewTag,
             delegatedEvents: ['click'],
-            sortOperation: TableColumnSortOperation.localeAwareCaseSensitive
+            sortOperation: TableColumnSortOperation.localeAwareCaseSensitive,
+            validator: new ColumnValidator<[]>([])
         };
     }
 

diff --git a/packages/nimble-components/src/table-column/base/index.ts b/packages/nimble-components/src/table-column/base/index.ts
@@ -10,19 +10,24 @@ import {
     ColumnInternalsOptions
 } from './models/column-internals';
 import type { TableColumnValidity } from './types';
+import type { ColumnValidator } from './models/column-validator';
 
 /**
  * The base class for table columns
  */
 export abstract class TableColumn<
-    TColumnConfig = unknown
+    TColumnConfig = unknown,
+    TColumnValidator extends ColumnValidator<[]> = ColumnValidator<[]>
 > extends FoundationElement {
     /**
      * @internal
      *
      * Column properties configurable by plugin authors
      */
-    public readonly columnInternals: ColumnInternals<TColumnConfig> = new ColumnInternals(this.getColumnInternalsOptions());
+    public readonly columnInternals: ColumnInternals<
+    TColumnConfig,
+    TColumnValidator
+    > = new ColumnInternals(this.getColumnInternalsOptions());
 
     @attr({ attribute: 'column-id' })
     public columnId?: string;
@@ -53,11 +58,11 @@ export abstract class TableColumn<
     public contentSlot!: HTMLSlotElement;
 
     public checkValidity(): boolean {
-        return this.columnInternals.validConfiguration;
+        return this.columnInternals.validator.isColumnValid;
     }
 
     public get validity(): TableColumnValidity {
-        return {};
+        return this.columnInternals.validator.getValidity();
     }
 
     /** @internal */
@@ -75,7 +80,7 @@ export abstract class TableColumn<
         this.setAttribute('slot', this.columnInternals.uniqueId);
     }
 
-    protected abstract getColumnInternalsOptions(): ColumnInternalsOptions;
+    protected abstract getColumnInternalsOptions(): ColumnInternalsOptions<TColumnValidator>;
 
     protected sortDirectionChanged(): void {
         if (!this.sortingDisabled) {

diff --git a/packages/nimble-components/src/table-column/base/models/column-internals.ts b/packages/nimble-components/src/table-column/base/models/column-internals.ts
@@ -10,8 +10,11 @@ import {
 import type { TableGroupRow } from '../../../table/components/group-row';
 import { createGroupHeaderViewTemplate } from '../group-header-view/template';
 import { createCellViewTemplate } from '../cell-view/template';
+import type { ColumnValidator } from './column-validator';
 
-export interface ColumnInternalsOptions {
+export interface ColumnInternalsOptions<
+    TColumnValidator extends ColumnValidator<[]> = ColumnValidator<[]>
+> {
     /**
      * The tag (element name) of the custom element that renders the cell content for the column.
      * That element should derive from TableCellView<TCellRecord, TColumnConfig>.
@@ -39,13 +42,21 @@ export interface ColumnInternalsOptions {
      * The sort operation to use for the column (defaults to TableColumnSortOperation.basic)
      */
     readonly sortOperation?: TableColumnSortOperation;
+
+    /**
+     * The validator for the column
+     */
+    readonly validator: TColumnValidator;
 }
 
 /**
  * Internal column state configured by plugin authors
  * @internal
  */
-export class ColumnInternals<TColumnConfig> {
+export class ColumnInternals<
+    TColumnConfig,
+    TColumnValidator extends ColumnValidator<[]>
+> {
     /**
      * @see ColumnInternalsOptions.cellRecordFieldNames
      */
@@ -72,12 +83,6 @@ export class ColumnInternals<TColumnConfig> {
     @observable
     public columnConfig?: TColumnConfig;
 
-    /**
-     * Whether this column has a valid configuration.
-     */
-    @observable
-    public validConfiguration = true;
-
     /**
      * The name of the data field that will be used for operations on the table, such as sorting and grouping.
      */
@@ -163,14 +168,17 @@ export class ColumnInternals<TColumnConfig> {
     @observable
     public currentSortDirection: TableColumnSortDirection = TableColumnSortDirection.none;
 
-    public constructor(options: ColumnInternalsOptions) {
+    public readonly validator: TColumnValidator;
+
+    public constructor(options: ColumnInternalsOptions<TColumnValidator>) {
         this.cellRecordFieldNames = options.cellRecordFieldNames;
         this.cellViewTemplate = createCellViewTemplate(options.cellViewTag);
         this.groupHeaderViewTemplate = createGroupHeaderViewTemplate(
             options.groupHeaderViewTag
         );
         this.delegatedEvents = options.delegatedEvents;
         this.sortOperation = options.sortOperation ?? TableColumnSortOperation.basic;
+        this.validator = options.validator;
     }
 
     protected fractionalWidthChanged(): void {
@@ -184,7 +192,7 @@ export class ColumnInternals<TColumnConfig> {
 
 export function isColumnInternalsProperty(
     changedProperty: string,
-    ...args: (keyof ColumnInternals<unknown>)[]
+    ...args: (keyof ColumnInternals<unknown, ColumnValidator<[]>>)[]
 ): boolean {
     for (const arg of args) {
         if (changedProperty === arg) {

diff --git a/packages/nimble-components/src/table-column/base/models/column-validator.ts b/packages/nimble-components/src/table-column/base/models/column-validator.ts
@@ -1,17 +1,17 @@
+import { observable } from '@microsoft/fast-element';
 import { Validator } from '../../../utilities/models/validator';
 import type { TableColumnValidity } from '../types';
-import type { ColumnInternals } from './column-internals';
 
 /**
  * Base column validator
  */
 export class ColumnValidator<
     ValidityFlagNames extends readonly string[]
 > extends Validator<ValidityFlagNames> {
-    public constructor(
-        private readonly columnInternals: ColumnInternals<unknown>,
-        configValidityKeys: ValidityFlagNames
-    ) {
+    @observable
+    public isColumnValid = true;
+
+    public constructor(configValidityKeys: ValidityFlagNames) {
         super(configValidityKeys);
     }
 
@@ -38,6 +38,6 @@ export class ColumnValidator<
     }
 
     private updateColumnInternalsFlag(): void {
-        this.columnInternals.validConfiguration = this.isValid();
+        this.isColumnValid = this.isValid();
     }
 }
diff --git a/packages/nimble-components/src/table-column/base/tests/table-cell-view.spec.ts b/packages/nimble-components/src/table-column/base/tests/table-cell-view.spec.ts
@@ -13,6 +13,7 @@ import {
     tableColumnEmptyTag
 } from './table-column.fixtures';
 import type { ColumnInternalsOptions } from '../models/column-internals';
+import { ColumnValidator } from '../models/column-validator';
 
 async function setup(): Promise<Fixture<TableCellView>> {
     return fixture(tableColumnEmptyCellViewTag);
@@ -36,7 +37,8 @@ describe('TableCellView', () => {
                 cellRecordFieldNames: [],
                 cellViewTag: tableColumnEmptyCellViewTag,
                 groupHeaderViewTag: tableColumnEmptyGroupHeaderViewTag,
-                delegatedEvents: ['click', 'keydown']
+                delegatedEvents: ['click', 'keydown'],
+                validator: new ColumnValidator<[]>([])
             };
         }
     }

diff --git a/packages/nimble-components/src/table-column/base/tests/table-column.fixtures.ts b/packages/nimble-components/src/table-column/base/tests/table-column.fixtures.ts
@@ -4,10 +4,7 @@ import { attr, customElement } from '@microsoft/fast-element';
 import { TableCellView } from '../cell-view';
 import { TableGroupHeaderView } from '../group-header-view';
 import { TableColumn } from '..';
-import type {
-    ColumnInternalsOptions,
-    ColumnInternals
-} from '../models/column-internals';
+import type { ColumnInternalsOptions } from '../models/column-internals';
 import { ColumnValidator } from '../models/column-validator';
 
 export const tableColumnEmptyCellViewTag = 'nimble-test-table-column-empty-cell-view';
@@ -56,7 +53,8 @@ export class TableColumnEmpty extends TableColumn {
             cellRecordFieldNames: [],
             cellViewTag: tableColumnEmptyCellViewTag,
             groupHeaderViewTag: tableColumnEmptyGroupHeaderViewTag,
-            delegatedEvents: []
+            delegatedEvents: [],
+            validator: new ColumnValidator<[]>([])
         };
     }
 }
@@ -69,8 +67,8 @@ const configValidity = ['invalidFoo', 'invalidBar'] as const;
 export class TestColumnValidator extends ColumnValidator<
     typeof configValidity
 > {
-    public constructor(columnInternals: ColumnInternals<unknown>) {
-        super(columnInternals, configValidity);
+    public constructor() {
+        super(configValidity);
     }
 
     public validateFoo(isValid: boolean): void {
@@ -95,30 +93,31 @@ declare global {
 @customElement({
     name: tableColumnValidationTestTag
 })
-export class TableColumnValidationTest extends TableColumn {
-    /* @internal */
-    public readonly validator = new TestColumnValidator(this.columnInternals);
-
+export class TableColumnValidationTest extends TableColumn<
+unknown,
+TestColumnValidator
+> {
     @attr({ mode: 'boolean' })
     public foo = false;
 
     @attr({ mode: 'boolean' })
     public bar = false;
 
-    protected override getColumnInternalsOptions(): ColumnInternalsOptions {
+    protected override getColumnInternalsOptions(): ColumnInternalsOptions<TestColumnValidator> {
         return {
             cellRecordFieldNames: [],
             cellViewTag: tableColumnEmptyCellViewTag,
             groupHeaderViewTag: tableColumnEmptyGroupHeaderViewTag,
-            delegatedEvents: []
+            delegatedEvents: [],
+            validator: new TestColumnValidator()
         };
     }
 
     private fooChanged(): void {
-        this.validator.validateFoo(this.foo);
+        this.columnInternals.validator.validateFoo(this.foo);
     }
 
     private barChanged(): void {
-        this.validator.validateBar(this.bar);
+        this.columnInternals.validator.validateBar(this.bar);
     }
 }
diff --git a/packages/nimble-components/src/table-column/base/tests/table-column.spec.ts b/packages/nimble-components/src/table-column/base/tests/table-column.spec.ts
@@ -14,6 +14,7 @@ import {
 import { TableColumn } from '..';
 import { TableColumnSortDirection } from '../../../table/types';
 import type { ColumnInternalsOptions } from '../models/column-internals';
+import { ColumnValidator } from '../models/column-validator';
 
 async function setup(): Promise<Fixture<TableColumnEmpty>> {
     return fixture(tableColumnEmptyTag);
@@ -123,7 +124,8 @@ describe('TableColumn', () => {
                         cellRecordFieldNames: [],
                         cellViewTag: 'div',
                         groupHeaderViewTag: tableColumnEmptyGroupHeaderViewTag,
-                        delegatedEvents: []
+                        delegatedEvents: [],
+                        validator: new ColumnValidator<[]>([])
                     };
                 }
             }
@@ -153,7 +155,8 @@ describe('TableColumn', () => {
                         cellRecordFieldNames: [],
                         cellViewTag: tableColumnEmptyCellViewTag,
                         groupHeaderViewTag: 'div',
-                        delegatedEvents: []
+                        delegatedEvents: [],
+                        validator: new ColumnValidator<[]>([])
                     };
                 }
             }

diff --git a/packages/nimble-components/src/table-column/date-text/index.ts b/packages/nimble-components/src/table-column/date-text/index.ts
@@ -7,7 +7,7 @@ import { styles } from '../base/styles';
 import { template } from '../base/template';
 import type { TableNumberField } from '../../table/types';
 import { TableColumnTextBase, mixinTextBase } from '../text-base';
-import { TableColumnSortOperation, TableColumnValidity } from '../base/types';
+import { TableColumnSortOperation } from '../base/types';
 import { tableColumnDateTextGroupHeaderViewTag } from './group-header-view';
 import { tableColumnDateTextCellViewTag } from './cell-view';
 import type { ColumnInternalsOptions } from '../base/models/column-internals';
@@ -50,11 +50,11 @@ declare global {
  * The table column for displaying dates/times as text.
  */
 export class TableColumnDateText extends mixinTextBase(
-    TableColumnTextBase<TableColumnDateTextColumnConfig>
+    TableColumnTextBase<
+    TableColumnDateTextColumnConfig,
+    TableColumnDateTextValidator
+    >
 ) {
-    /** @internal */
-    public validator = new TableColumnDateTextValidator(this.columnInternals);
-
     @attr
     public format: DateTextFormat;
 
@@ -135,21 +135,18 @@ export class TableColumnDateText extends mixinTextBase(
         lang.unsubscribe(this.langSubscriber, this);
     }
 
-    public override get validity(): TableColumnValidity {
-        return this.validator.getValidity();
-    }
-
     public placeholderChanged(): void {
         this.updateColumnConfig();
     }
 
-    protected override getColumnInternalsOptions(): ColumnInternalsOptions {
+    protected override getColumnInternalsOptions(): ColumnInternalsOptions<TableColumnDateTextValidator> {
         return {
             cellRecordFieldNames: ['value'],
             cellViewTag: tableColumnDateTextCellViewTag,
             groupHeaderViewTag: tableColumnDateTextGroupHeaderViewTag,
             delegatedEvents: [],
-            sortOperation: TableColumnSortOperation.basic
+            sortOperation: TableColumnSortOperation.basic,
+            validator: new TableColumnDateTextValidator()
         };
     }
 
@@ -242,10 +239,10 @@ export class TableColumnDateText extends mixinTextBase(
                 placeholder: this.placeholder
             };
             this.columnInternals.columnConfig = columnConfig;
-            this.validator.setCustomOptionsValidity(true);
+            this.columnInternals.validator.setCustomOptionsValidity(true);
         } else {
             this.columnInternals.columnConfig = undefined;
-            this.validator.setCustomOptionsValidity(false);
+            this.columnInternals.validator.setCustomOptionsValidity(false);
         }
     }
 

diff --git a/...s/nimble-components/src/table-column/date-text/models/table-column-date-text-validator.ts b/...s/nimble-components/src/table-column/date-text/models/table-column-date-text-validator.ts
@@ -1,4 +1,3 @@
-import type { ColumnInternals } from '../../base/models/column-internals';
 import { ColumnValidator } from '../../base/models/column-validator';
 
 const dateTextValidityFlagNames = ['invalidCustomOptionsCombination'] as const;
@@ -9,8 +8,8 @@ const dateTextValidityFlagNames = ['invalidCustomOptionsCombination'] as const;
 export class TableColumnDateTextValidator extends ColumnValidator<
     typeof dateTextValidityFlagNames
 > {
-    public constructor(columnInternals: ColumnInternals<unknown>) {
-        super(columnInternals, dateTextValidityFlagNames);
+    public constructor() {
+        super(dateTextValidityFlagNames);
     }
 
     public setCustomOptionsValidity(valid: boolean): void {

diff --git a/packages/nimble-components/src/table-column/duration-text/index.ts b/packages/nimble-components/src/table-column/duration-text/index.ts
@@ -13,6 +13,7 @@ import { DurationFormatter } from './models/duration-formatter';
 import { tableColumnDurationTextGroupHeaderViewTag } from './group-header-view';
 import type { TableColumnTextBaseColumnConfig } from '../text-base/cell-view';
 import { TableColumnTextBase, mixinTextBase } from '../text-base';
+import { ColumnValidator } from '../base/models/column-validator';
 
 export type TableColumnDurationTextCellRecord = TableNumberField<'value'>;
 export interface TableColumnDurationTextColumnConfig
@@ -59,7 +60,8 @@ export class TableColumnDurationText extends mixinTextBase(
             cellViewTag: tableColumnDurationTextCellViewTag,
             groupHeaderViewTag: tableColumnDurationTextGroupHeaderViewTag,
             delegatedEvents: [],
-            sortOperation: TableColumnSortOperation.basic
+            sortOperation: TableColumnSortOperation.basic,
+            validator: new ColumnValidator<[]>([])
         };
     }