Skip to content

Commit

Permalink
Refactor to create base classes for table columns that display text (#…
Browse files Browse the repository at this point in the history
…1239)

# Pull Request

## 🤨 Rationale

As part of the vision described in #1224 in service of #1011 and other
column types, we will want to share rendering logic across columns that
display their data as text. This could eventually include:
 - the existing text column
 - the mapping column discussed in #1220 
 - columns that display numbers, dates, elapsed time, and more
 - columns defined in applications with custom formatting logic

## 👩‍💻 Implementation

1. Extract abstract base classes, templates, and styles from the
existing text column for:
    - the column custom element
    - the cell view custom element
    - the group header custom element
These live in `table-column/text-base`.
2. Derive concrete instances and register custom elements of each of
these for the `text` column
3. Define and use some table field types to provide stricter typing some
generic parameters

## 🧪 Testing

Relying on existing tests. 

We have a future task to add docs/examples/tests for applications
deriving from these base classes.

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Co-authored-by: Milan Raj <[email protected]>
  • Loading branch information
jattasNI and rajsite authored May 12, 2023
1 parent a94e767 commit c00161d
Show file tree
Hide file tree
Showing 14 changed files with 195 additions and 90 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Refactor to create base classes for table columns that display text",
"packageName": "@ni/nimble-components",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { observable } from '@microsoft/fast-element';
import { FoundationElement } from '@microsoft/fast-foundation';
import type { TableFieldValue } from '../../../table/types';

export interface TableGroupHeaderState<
TGroupValue = unknown,
TGroupValue = TableFieldValue,
TColumnConfig = unknown
> {
groupHeaderValue: TGroupValue;
Expand All @@ -15,7 +16,7 @@ export interface TableGroupHeaderState<
* type (linked via TableColumn.groupHeaderViewTag).
*/
export abstract class TableGroupHeaderView<
TGroupValue = unknown,
TGroupValue = TableFieldValue,
TColumnConfig = unknown
>
extends FoundationElement
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { observable, volatile } from '@microsoft/fast-element';
import { TableCellView } from '../../base/cell-view';
import type { TableCellRecord } from '../../base/types';

/**
* The cell view base class for displaying fields of any type as text.
*/
export abstract class TableColumnTextCellViewBase<
TCellRecord extends TableCellRecord = TableCellRecord,
TColumnConfig = unknown
> extends TableCellView<TCellRecord, TColumnConfig> {
/** @internal */
@observable
public isValidContentAndHasOverflow = false;

/** @internal */
public textSpan!: HTMLElement;

/**
* Returns the text to render in the cell when it contains a valid value (i.e. when shouldUsePlaceholder() is false).
* If the implementation has branching code paths then it must be marked with @volatile.
* https://www.fast.design/docs/fast-element/observables-and-state/#observable-features
*/
public abstract get text(): string;

/**
* Returns the text to render in the cell when it contains an invalid value (i.e. when shouldUsePlaceholder() is true).
* If the implementation has branching code paths then it must be marked with @volatile.
* https://www.fast.design/docs/fast-element/observables-and-state/#observable-features
*/
public abstract get placeholder(): string;

/**
* Returns whether to display the placeholder value or the text value
* If the implementation has branching code paths then it must be marked with @volatile.
* https://www.fast.design/docs/fast-element/observables-and-state/#observable-features
* */
public abstract get shouldUsePlaceholder(): boolean;

@volatile
public get content(): string {
return this.shouldUsePlaceholder ? this.placeholder : this.text;
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { html, ref } from '@microsoft/fast-element';

import type { TableColumnTextCellView } from '.';
import type { TableColumnTextCellViewBase } from '.';

export const template = html<TableColumnTextCellView>`
export const template = html<TableColumnTextCellViewBase>`
<span
${ref('textSpan')}
class="${x => (typeof x.cellRecord.value === 'string' ? '' : 'placeholder')}"
class="${x => (x.shouldUsePlaceholder ? 'placeholder' : '')}"
@mouseover="${x => {
x.isValidContentAndHasOverflow = !!x.content && x.textSpan.offsetWidth < x.textSpan.scrollWidth;
}}"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { observable, volatile } from '@microsoft/fast-element';
import { TableGroupHeaderView } from '../../base/group-header-view';
import type { TableFieldValue } from '../../../table/types';
import type { TableColumnWithPlaceholderColumnConfig } from '../../base/types';

/**
* The group header view base class for displaying fields of any type as text.
*/
export abstract class TableColumnTextGroupHeaderViewBase<
TGroupValue = TableFieldValue,
TColumnConfig = TableColumnWithPlaceholderColumnConfig
> extends TableGroupHeaderView<TGroupValue, TColumnConfig> {
/** @internal */
public textSpan!: HTMLElement;

/** @internal */
@observable
public hasOverflow = false;

/**
* Returns the text to render in the cell when it contains a valid value (i.e. when shouldUsePlaceholder() is false).
* If the implementation has branching code paths then it must be marked with @volatile.
* https://www.fast.design/docs/fast-element/observables-and-state/#observable-features
*/
public abstract get text(): string;

/**
* Returns the text to render in the cell when it contains an invalid value (i.e. when shouldUsePlaceholder() is true).
* If the implementation has branching code paths then it must be marked with @volatile.
* https://www.fast.design/docs/fast-element/observables-and-state/#observable-features
*/
public abstract get placeholder(): string;

/**
* Returns whether to display the placeholder value or the text value
* If the implementation has branching code paths then it must be marked with @volatile.
* https://www.fast.design/docs/fast-element/observables-and-state/#observable-features
* */
public abstract get shouldUsePlaceholder(): boolean;

@volatile
public get content(): string {
return this.shouldUsePlaceholder ? this.placeholder : this.text;
}

/** @internal */
public updateTitleOverflow(): void {
this.hasOverflow = this.textSpan.offsetWidth < this.textSpan.scrollWidth;
}

/** @internal */
public clearTitleOverflow(): void {
this.hasOverflow = false;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { html, ref } from '@microsoft/fast-element';
import type { TableColumnTextGroupHeaderViewBase } from '.';

export const template = html<TableColumnTextGroupHeaderViewBase>`
<span
${ref('textSpan')}
class="${x => (x.shouldUsePlaceholder ? 'placeholder' : '')}"
@mouseover="${x => x.updateTitleOverflow()}"
@mouseout="${x => x.clearTitleOverflow()}"
title="${x => (x.hasOverflow && x.content ? x.content : undefined)}"
>
${x => x.content}
</span>
`;
31 changes: 31 additions & 0 deletions packages/nimble-components/src/table-column/text-base/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { attr } from '@microsoft/fast-element';
import { mixinFractionalWidthColumnAPI } from '../mixins/fractional-width-column';
import { TableColumn } from '../base';
import type { TableColumnWithPlaceholderColumnConfig } from '../base/types';
import { mixinGroupableColumnAPI } from '../mixins/groupable-column';

/**
* The base class for table columns that display fields of any type as text.
*/
export abstract class TableColumnTextBase extends mixinGroupableColumnAPI(
mixinFractionalWidthColumnAPI(
TableColumn<TableColumnWithPlaceholderColumnConfig>
)
) {
@attr({ attribute: 'field-name' })
public fieldName?: string;

@attr
public placeholder?: string;

protected fieldNameChanged(): void {
this.columnInternals.dataRecordFieldNames = [this.fieldName];
this.columnInternals.operandDataRecordFieldName = this.fieldName;
}

protected placeholderChanged(): void {
this.columnInternals.columnConfig = {
placeholder: this.placeholder ?? ''
};
}
}
29 changes: 13 additions & 16 deletions packages/nimble-components/src/table-column/text/cell-view/index.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { observable, volatile } from '@microsoft/fast-element';
import { DesignSystem } from '@microsoft/fast-foundation';
import { template } from '../../text-base/cell-view/template';
import type {
TableColumnTextCellRecord,
TableColumnTextColumnConfig
} from '..';
import { TableCellView } from '../../base/cell-view';
import { styles } from './styles';
import { template } from './template';
import { styles } from '../../text-base/cell-view/styles';
import { TableColumnTextCellViewBase } from '../../text-base/cell-view';

declare global {
interface HTMLElementTagNameMap {
Expand All @@ -15,24 +14,22 @@ declare global {
}

/**
* A cell view for displaying strings
* A cell view for displaying string fields as text
*/
export class TableColumnTextCellView extends TableCellView<
export class TableColumnTextCellView extends TableColumnTextCellViewBase<
TableColumnTextCellRecord,
TableColumnTextColumnConfig
> {
/** @internal */
@observable
public isValidContentAndHasOverflow = false;
public override get text(): string {
return this.cellRecord.value!;
}

/** @internal */
public textSpan!: HTMLElement;
public override get placeholder(): string {
return this.columnConfig.placeholder;
}

@volatile
public get content(): string {
return typeof this.cellRecord.value === 'string'
? this.cellRecord.value
: this.columnConfig.placeholder;
public override get shouldUsePlaceholder(): boolean {
return typeof this.cellRecord.value !== 'string';
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,43 +1,32 @@
import { observable, volatile } from '@microsoft/fast-element';
import { DesignSystem } from '@microsoft/fast-foundation';
import type { TableColumnWithPlaceholderColumnConfig } from '../../base/types';
import { TableGroupHeaderView } from '../../base/group-header-view';
import { template } from './template';
import { styles } from './styles';
import type { TableStringFieldValue } from '../../../table/types';
import { TableColumnTextGroupHeaderViewBase } from '../../text-base/group-header-view';
import { template } from '../../text-base/group-header-view/template';
import { styles } from '../../text-base/group-header-view/styles';
import type { TableColumnTextColumnConfig } from '..';

declare global {
interface HTMLElementTagNameMap {
'nimble-table-column-text-group-header': TableColumnTextGroupHeaderView;
}
}
/**
* The custom element used to render a group row header for a group representing rows
* grouped by a TableColumnText column.
* The group header view for displaying string fields as text.
*/
export class TableColumnTextGroupHeaderView extends TableGroupHeaderView<
string | null | undefined,
TableColumnWithPlaceholderColumnConfig
export class TableColumnTextGroupHeaderView extends TableColumnTextGroupHeaderViewBase<
TableStringFieldValue,
TableColumnTextColumnConfig
> {
/** @internal */
public textSpan!: HTMLElement;

/** @internal */
@observable
public isValidContentAndHasOverflow = false;

@volatile
public get content(): string {
return typeof this.groupHeaderValue === 'string'
? this.groupHeaderValue
: this.columnConfig?.placeholder ?? '';
public override get text(): string {
return this.groupHeaderValue!;
}

public updateTitleOverflow(): void {
this.isValidContentAndHasOverflow = this.textSpan.offsetWidth < this.textSpan.scrollWidth;
public override get placeholder(): string {
return this.columnConfig?.placeholder ?? '';
}

public clearTitleOverflow(): void {
this.isValidContentAndHasOverflow = false;
public override get shouldUsePlaceholder(): boolean {
return typeof this.groupHeaderValue !== 'string';
}
}

Expand Down

This file was deleted.

29 changes: 3 additions & 26 deletions packages/nimble-components/src/table-column/text/index.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
/* eslint-disable max-classes-per-file */
import { DesignSystem } from '@microsoft/fast-foundation';
import { attr } from '@microsoft/fast-element';
import { styles } from '../base/styles';
import { template } from '../base/template';
import { mixinFractionalWidthColumnAPI } from '../mixins/fractional-width-column';
import type { TableStringField } from '../../table/types';
import { TableColumn } from '../base';
import { TableColumnTextBase } from '../text-base';
import {
TableColumnWithPlaceholderColumnConfig,
TableColumnSortOperation
} from '../base/types';
import { mixinGroupableColumnAPI } from '../mixins/groupable-column';
import { tableColumnTextGroupHeaderTag } from './group-header-view';
import { tableColumnTextCellViewTag } from './cell-view';

Expand All @@ -26,17 +22,9 @@ declare global {
}

/**
* The table column for displaying strings.
* The table column for displaying string fields as text.
*/
export class TableColumnText extends mixinGroupableColumnAPI(
mixinFractionalWidthColumnAPI(TableColumn<TableColumnTextColumnConfig>)
) {
@attr({ attribute: 'field-name' })
public fieldName?: string;

@attr
public placeholder?: string;

export class TableColumnText extends TableColumnTextBase {
public constructor() {
super({
cellRecordFieldNames: ['value'],
Expand All @@ -46,17 +34,6 @@ export class TableColumnText extends mixinGroupableColumnAPI(
});
this.columnInternals.sortOperation = TableColumnSortOperation.localeAwareCaseSensitive;
}

protected fieldNameChanged(): void {
this.columnInternals.dataRecordFieldNames = [this.fieldName];
this.columnInternals.operandDataRecordFieldName = this.fieldName;
}

protected placeholderChanged(): void {
this.columnInternals.columnConfig = {
placeholder: this.placeholder ?? ''
};
}
}

const nimbleTableColumnText = TableColumnText.compose({
Expand Down
8 changes: 7 additions & 1 deletion packages/nimble-components/src/table/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ export type TableFieldName = string;
*/
export type TableFieldValue = string | number | boolean | null | undefined;

/**
* TableStringFieldValue describes the type associated with values within
* a table's string records.
*/
export type TableStringFieldValue = string | null | undefined;

/**
* TableRecord describes the data structure that backs a single row in a table.
* It is made up of fields, which are key/value pairs that have a key of type
Expand All @@ -22,7 +28,7 @@ export interface TableRecord {
}

export type TableStringField<FieldName extends TableFieldName> = {
[name in FieldName]: string | null | undefined;
[name in FieldName]: TableStringFieldValue;
};

export interface TableValidity {
Expand Down

0 comments on commit c00161d

Please sign in to comment.