Skip to content

Commit

Permalink
Nimble Table: cell view (custom element) and cell state changes (#1138)
Browse files Browse the repository at this point in the history
# Pull Request

## 🤨 Rationale

Fixes #997 

## 👩‍💻 Implementation

Implementation is similar to what was outlined in the the
[HLD](https://github.com/ni/nimble/blob/main/packages/nimble-components/src/table/specs/cell-state-when-scrolling-hld.md)
([PR](#1052)):
- Add a `TableCellView<TCellRecord, TColumnConfig>` class. TableCellView
instances will be used in `TableCell`s, instead of the old approach
(column-provided cellTemplate and cellStyles).
- Add `cellViewTag` property to `TableColumn` (links the column with the
associated `TableCellView` type)
- `TableCellView` defines a `focusedRecycleCallback()` method which is a
no-op by default. If a cell view contains a focusable element, and a
cell is focused, and then a scroll (row recycle) occurs, this gives the
cell view an opportunity to commit/blur/etc.
- We don't yet have a column type exercising this, but there's a new
autotest for this code path.
- If a cell action menu is open when a scroll/ row recycle occurs, the
action menu is closed (via the associated menu button).

## 🧪 Testing
- Updated existing autotests
- Added new tests for `focusedRecycleCallback` and the code that closes
action menus on scroll
- Manually tested table in Storybook + Angular + Blazor example apps

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
  • Loading branch information
msmithNI authored Apr 4, 2023
1 parent 044441a commit 91172d9
Show file tree
Hide file tree
Showing 24 changed files with 367 additions and 244 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Table updates: use custom element (TableCellView) in cells, cells are notified of row recycling, action menus are closed on scroll/ row recycles.",
"packageName": "@ni/nimble-components",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { observable } from '@microsoft/fast-element';
import { FoundationElement } from '@microsoft/fast-foundation';
import type { TableCellRecord, TableCellState } from '../types';

/**
* Base class for table cell views, which are used within the nimble-table-cell.
* Each TableColumn type has a corresponding TableCellView type (linked via TableColumn.cellViewTag).
*/
export abstract class TableCellView<
TCellRecord extends TableCellRecord = TableCellRecord,
TColumnConfig = unknown
>
extends FoundationElement
implements TableCellState<TCellRecord, TColumnConfig> {
@observable
public cellRecord!: TCellRecord;

@observable
public columnConfig!: TColumnConfig;

/**
* Called if an element inside this cell view has focus, and this row/cell is being recycled.
* Expected implementation is to commit changes as needed, and blur the focusable element (or close
* the menu/popup/etc).
*/
public focusedRecycleCallback(): void {}
}
31 changes: 16 additions & 15 deletions packages/nimble-components/src/table-column/base/index.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,24 @@
import {
attr,
ElementStyles,
nullableNumberConverter,
observable,
ViewTemplate
} from '@microsoft/fast-element';
import { FoundationElement } from '@microsoft/fast-foundation';
import { uniqueId } from '@microsoft/fast-web-utilities';
import type { TableCell } from '../../table/components/cell';
import { createCellViewTemplate } from '../../table/components/cell/template';
import { TableColumnSortDirection, TableFieldName } from '../../table/types';
import {
defaultFractionalWidth,
defaultMinPixelWidth,
TableCellRecord,
TableCellState,
TableColumnSortOperation
} from './types';

/**
* The base class for table columns
*/
export abstract class TableColumn<
TCellRecord extends TableCellRecord = TableCellRecord,
TColumnConfig = unknown
> extends FoundationElement {
@attr({ attribute: 'column-id' })
Expand Down Expand Up @@ -81,18 +79,15 @@ export abstract class TableColumn<

/**
* @internal
*
* The template to use to render the cell content for the column
* The tag (element name) of the custom element that renders the cell content for the column.
* That element should derive from TableCellView<TCellRecord, TColumnConfig>.
*/
// prettier-ignore
public abstract readonly cellTemplate: ViewTemplate<TableCellState<TCellRecord, TColumnConfig>>;
@observable
public abstract readonly cellViewTag: string;

/**
* @internal
*
* The style to apply to the cellTemplate
*/
public abstract readonly cellStyles?: ElementStyles;
/* @internal */
@observable
public currentCellViewTemplate?: ViewTemplate<TableCell>;

/**
* @internal
Expand All @@ -114,7 +109,7 @@ export abstract class TableColumn<
/**
* @internal
*
* The relevant, static configuration a column requires its cellTemplate to have access to.
* The relevant, static configuration a column requires its cell view to have access to.
*/
@observable
public columnConfig?: TColumnConfig;
Expand Down Expand Up @@ -156,6 +151,12 @@ export abstract class TableColumn<
this.setAttribute('slot', this.internalUniqueId);
}

protected cellViewTagChanged(): void {
this.currentCellViewTemplate = this.cellViewTag
? createCellViewTemplate(this.cellViewTag)
: undefined;
}

protected internalFractionalWidthChanged(): void {
this.currentFractionalWidth = this.internalFractionalWidth;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,27 @@
import {
ViewTemplate,
ElementStyles,
html,
customElement
} from '@microsoft/fast-element';
/* eslint-disable max-classes-per-file */
import { customElement } from '@microsoft/fast-element';
import {
fixture,
Fixture,
uniqueElementName
} from '../../../utilities/tests/fixture';
import type { TableCellState } from '../types';
import { TableColumn } from '..';
import { TableCellView } from '../cell-view';

const columnName = uniqueElementName();
const columnCellViewName = uniqueElementName();

@customElement({
name: columnCellViewName
})
// eslint-disable-next-line @typescript-eslint/no-unused-vars
class TestTableColumnCellView extends TableCellView {}

@customElement({
name: columnName
})
class TestTableColumn extends TableColumn {
public cellTemplate: ViewTemplate<TableCellState> = html``;
public cellStyles?: ElementStyles | undefined;
public cellViewTag = columnCellViewName;
public cellRecordFieldNames: readonly string[] = [];
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,24 @@
/* eslint-disable max-classes-per-file */
import {
ViewTemplate,
ElementStyles,
html,
customElement
} from '@microsoft/fast-element';
import { customElement } from '@microsoft/fast-element';
import {
fixture,
Fixture,
uniqueElementName
} from '../../../utilities/tests/fixture';
import type { TableCellState } from '../../base/types';
import { TableColumn } from '../../base';
import { mixinFractionalWidthColumnAPI } from '../fractional-width-column';
import { TableCellView } from '../../base/cell-view';

const columnCellViewName = uniqueElementName();

@customElement({
name: columnCellViewName
})
// eslint-disable-next-line @typescript-eslint/no-unused-vars
class TestTableColumnCellView extends TableCellView {}

class TestTableColumnBase extends TableColumn {
public cellTemplate: ViewTemplate<TableCellState> = html``;
public cellStyles?: ElementStyles | undefined;
public cellViewTag = columnCellViewName;
public cellRecordFieldNames: readonly string[] = [];
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { observable, volatile } from '@microsoft/fast-element';
import { DesignSystem } from '@microsoft/fast-foundation';
import type {
TableColumnTextCellRecord,
TableColumnTextColumnConfig
} from '..';
import { TableCellView } from '../../base/cell-view';
import { styles } from './styles';
import { template } from './template';

declare global {
interface HTMLElementTagNameMap {
'nimble-table-column-text-cell-view': TableColumnTextCellView;
}
}

/**
* A cell view for displaying strings
*/
export class TableColumnTextCellView extends TableCellView<
TableColumnTextCellRecord,
TableColumnTextColumnConfig
> {
/** @internal */
@observable
public isValidContentAndHasOverflow = false;

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

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

const textCellView = TableColumnTextCellView.compose({
baseName: 'table-column-text-cell-view',
template,
styles
});
DesignSystem.getOrCreate().withPrefix('nimble').register(textCellView());
export const tableColumnTextCellViewTag = DesignSystem.tagFor(
TableColumnTextCellView
);
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import {
bodyFont,
bodyFontColor,
controlLabelFontColor
} from '../../theme-provider/design-tokens';
} from '../../../theme-provider/design-tokens';

export const cellStyles = css`
export const styles = css`
span {
font: ${bodyFont};
color: ${bodyFontColor};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { html, ref } from '@microsoft/fast-element';

import type { TableColumnTextCellView } from '.';

export const template = html<TableColumnTextCellView>`
<span
${ref('textSpan')}
class="${x => (typeof x.cellRecord.value === 'string' ? '' : 'placeholder')}"
@mouseover="${x => {
x.isValidContentAndHasOverflow = !!x.content && x.textSpan.offsetWidth < x.textSpan.scrollWidth;
}}"
@mouseout="${x => {
x.isValidContentAndHasOverflow = false;
}}"
title=${x => (x.isValidContentAndHasOverflow ? x.content : null)}
>
${x => x.content}
</span>
`;
12 changes: 3 additions & 9 deletions packages/nimble-components/src/table-column/text/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ import { mixinFractionalWidthColumnAPI } from '../mixins/fractional-width-column
import type { TableStringField } from '../../table/types';
import { TableColumn } from '../base';
import { TableColumnSortOperation } from '../base/types';
import { cellStyles } from './styles';
import { cellTemplate } from './template';
import { tableColumnTextCellViewTag } from './cell-view';

export type TableColumnTextCellRecord = TableStringField<'value'>;
export interface TableColumnTextColumnConfig {
Expand All @@ -24,10 +23,7 @@ declare global {
/**
* The base class for a table column for displaying strings.
*/
class TableColumnTextBase extends TableColumn<
TableColumnTextCellRecord,
TableColumnTextColumnConfig
> {
class TableColumnTextBase extends TableColumn<TableColumnTextColumnConfig> {
public cellRecordFieldNames = ['value'] as const;

@attr({ attribute: 'field-name' })
Expand All @@ -36,9 +32,7 @@ TableColumnTextColumnConfig
@attr
public placeholder?: string;

public readonly cellStyles = cellStyles;

public readonly cellTemplate = cellTemplate;
public readonly cellViewTag = tableColumnTextCellViewTag;

public constructor() {
super();
Expand Down
36 changes: 0 additions & 36 deletions packages/nimble-components/src/table-column/text/template.ts

This file was deleted.

Loading

0 comments on commit 91172d9

Please sign in to comment.