Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add column IDs to table #1033

Merged
merged 31 commits into from
Feb 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
7f37e8d
partial
mollykreis Feb 2, 2023
1c9a69b
more tests
mollykreis Feb 3, 2023
6de11a4
format
mollykreis Feb 3, 2023
037ac7e
Update TableValidity interface in Blazor
mollykreis Feb 3, 2023
6082e59
column id support in blazor
mollykreis Feb 3, 2023
03e0a8f
Angular
mollykreis Feb 3, 2023
7120bde
doc update
mollykreis Feb 3, 2023
9138e19
Change files
mollykreis Feb 3, 2023
7bfd14e
format
mollykreis Feb 3, 2023
ee981b9
Merge branch 'main' into table-column-ids
mollykreis Feb 3, 2023
5affc97
Merge branch 'main' into table-column-ids
mollykreis Feb 7, 2023
641af9c
remove old test (merge issue)
mollykreis Feb 7, 2023
ab84077
clean up + don't allow null column ids
mollykreis Feb 7, 2023
1e1c59e
format
mollykreis Feb 7, 2023
77c49eb
remove null as type in Angular
mollykreis Feb 7, 2023
1fd5af2
Merge branch 'main' into table-column-ids
mollykreis Feb 7, 2023
7e49975
simplify
mollykreis Feb 7, 2023
442a8ac
Merge branch 'table-column-ids' of https://github.com/ni/nimble into …
mollykreis Feb 7, 2023
d019e9e
change observation pattern
mollykreis Feb 8, 2023
f5590a7
Update table-validator.spec.ts
mollykreis Feb 8, 2023
646bec5
format
mollykreis Feb 8, 2023
e7072b3
remove observers on disconnect
mollykreis Feb 8, 2023
8811921
Merge branch 'main' into table-column-ids
mollykreis Feb 8, 2023
2f2c549
create column base class
mollykreis Feb 9, 2023
b212100
PR feedback
mollykreis Feb 10, 2023
3d958f4
format
mollykreis Feb 10, 2023
881613a
Merge branch 'main' into table-column-ids
mollykreis Feb 10, 2023
73c8617
Merge branch 'main' into table-column-ids
mollykreis Feb 14, 2023
dfc0cc0
fix merge issues
mollykreis Feb 14, 2023
30a5423
format
mollykreis Feb 14, 2023
c2e6395
Merge branch 'main' into table-column-ids
mollykreis Feb 14, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ export type { TableColumnText };
selector: 'nimble-table-column-text'
})
export class NimbleTableColumnTextDirective {
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 | undefined) {
this.renderer.setProperty(this.elementRef.nativeElement, 'columnId', value);
}

public get fieldName(): string | undefined {
return this.elementRef.nativeElement.fieldName;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
`
})
Expand Down Expand Up @@ -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>
`
})
Expand All @@ -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>;
Expand Down Expand Up @@ -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>
`
})
Expand All @@ -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>;
Expand All @@ -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');

Expand All @@ -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');

Expand All @@ -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');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
mollykreis marked this conversation as resolved.
Show resolved Hide resolved
"type": "patch",
"comment": "Add column IDs to table",
"packageName": "@ni/nimble-angular",
"email": "20542556+mollykreis@users.noreply.github.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -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"
}
Original file line number Diff line number Diff line change
@@ -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"
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
@namespace NimbleBlazor
@inherits NimbleTableColumn

<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>

Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

namespace NimbleBlazor;

public partial class NimbleTableColumnText : ComponentBase
public partial class NimbleTableColumnText : NimbleTableColumn
{
/// <summary>
/// Gets or sets the field in the element representing a row of data in a <see cref="NimbleTable{TData}"/>to display
Expand Down
12 changes: 12 additions & 0 deletions packages/nimble-blazor/NimbleBlazor/NimbleTableColumn.cs
Original file line number Diff line number Diff line change
@@ -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; }
}
10 changes: 10 additions & 0 deletions packages/nimble-blazor/NimbleBlazor/TableValidity.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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; }
}
5 changes: 4 additions & 1 deletion packages/nimble-components/src/table-column/base/index.ts
Original file line number Diff line number Diff line change
@@ -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 { uniqueId } from '@microsoft/fast-web-utilities';
import type {
Expand All @@ -14,6 +14,9 @@ export abstract class TableColumn<
TCellRecord extends TableCellRecord = TableCellRecord,
TColumnConfig = unknown
> extends FoundationElement {
@attr({ attribute: 'column-id' })
public columnId?: string;

/**
* The template to use to render the cell content for the column
*/
Expand Down
81 changes: 71 additions & 10 deletions packages/nimble-components/src/table/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
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,
Expand Down Expand Up @@ -72,6 +77,7 @@ export class Table<
private readonly table: TanStackTable<TData>;
private options: TanStackTableOptionsResolved<TData>;
private readonly tableValidator = new TableValidator();
private columnNotifiers: Notifier[] = [];

public constructor() {
super();
Expand All @@ -93,32 +99,87 @@ 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.validateAndObserveColumns();
}

public override disconnectedCallback(): void {
mollykreis marked this conversation as resolved.
Show resolved Hide resolved
super.disconnectedCallback();
this.virtualizer.disconnectedCallback();
this.removeColumnObservers();
}

public checkValidity(): boolean {
return this.tableValidator.isValid();
}

/**
* @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.
*/
public handleChange(source: unknown, args: unknown): void {
if (source instanceof TableColumn) {
if (args === 'columnId') {
this.validateColumnIds();
}
}
}

protected childItemsChanged(): void {
void this.updateColumnsFromChildItems();
}

protected 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);
}

protected columnsChanged(
_prev: TableColumn[] | undefined,
_next: TableColumn[]
): void {
if (!this.$fastController.isConnected) {
return;
}

this.validateAndObserveColumns();
}

private removeColumnObservers(): void {
this.columnNotifiers.forEach(notifier => {
notifier.unsubscribe(this);
});
this.columnNotifiers = [];
}

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 {
this.tableValidator.validateColumnIds(
this.columns.map(x => x.columnId)
);
this.canRenderRows = this.checkValidity();
}

private async updateColumnsFromChildItems(): Promise<void> {
const definedElements = this.childItems.map(async item => (item.matches(':not(:defined)')
? customElements.whenDefined(item.localName)
Expand Down
Loading