diff --git a/change/@ni-nimble-components-a55ae070-d85f-4102-9219-b84655baab32.json b/change/@ni-nimble-components-a55ae070-d85f-4102-9219-b84655baab32.json new file mode 100644 index 0000000000..a892019618 --- /dev/null +++ b/change/@ni-nimble-components-a55ae070-d85f-4102-9219-b84655baab32.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Add placeholders to table columns", + "packageName": "@ni/nimble-components", + "email": "20542556+mollykreis@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/packages/nimble-components/src/patterns/table-column/tests/no-null-and-undefined-best-practice-docs.mdx b/packages/nimble-components/src/patterns/table-column/tests/no-null-and-undefined-best-practice-docs.mdx new file mode 100644 index 0000000000..d939555476 --- /dev/null +++ b/packages/nimble-components/src/patterns/table-column/tests/no-null-and-undefined-best-practice-docs.mdx @@ -0,0 +1 @@ +Avoid mixing `undefined` and `null` as values for the same field. When grouping this will lead to two groups (one for `null` values and one for `undefined` values) that both have the text `"No value"`. diff --git a/packages/nimble-components/src/table-column/anchor/cell-view/index.ts b/packages/nimble-components/src/table-column/anchor/cell-view/index.ts index 4d84b21ba8..9f97ac797a 100644 --- a/packages/nimble-components/src/table-column/anchor/cell-view/index.ts +++ b/packages/nimble-components/src/table-column/anchor/cell-view/index.ts @@ -26,11 +26,25 @@ TableColumnAnchorColumnConfig @observable public hasOverflow = false; + /** @internal */ + @observable + public isPlaceholder = false; + /** @internal */ public anchor?: Anchor; @volatile public get text(): string { + const displayedText = this.cellRecord?.label ?? this.cellRecord?.href; + if ( + (displayedText === undefined || displayedText === null) + && this.columnConfig?.placeholder + ) { + this.isPlaceholder = true; + return this.columnConfig.placeholder; + } + + this.isPlaceholder = false; if (typeof this.cellRecord?.label === 'string') { return this.cellRecord.label; } diff --git a/packages/nimble-components/src/table-column/anchor/cell-view/styles.ts b/packages/nimble-components/src/table-column/anchor/cell-view/styles.ts index e2dba54296..21eb833b32 100644 --- a/packages/nimble-components/src/table-column/anchor/cell-view/styles.ts +++ b/packages/nimble-components/src/table-column/anchor/cell-view/styles.ts @@ -1,5 +1,10 @@ import { css } from '@microsoft/fast-element'; -import { bodyFont, bodyFontColor } from '../../../theme-provider/design-tokens'; +import { + bodyFont, + bodyFontColor, + placeholderFont, + placeholderFontColor +} from '../../../theme-provider/design-tokens'; export const styles = css` :host { @@ -22,4 +27,9 @@ export const styles = css` overflow: hidden; text-overflow: ellipsis; } + + :host(.placeholder) span { + font: ${placeholderFont}; + color: ${placeholderFontColor}; + } `; diff --git a/packages/nimble-components/src/table-column/anchor/cell-view/template.ts b/packages/nimble-components/src/table-column/anchor/cell-view/template.ts index d65f7f03bd..d369615412 100644 --- a/packages/nimble-components/src/table-column/anchor/cell-view/template.ts +++ b/packages/nimble-components/src/table-column/anchor/cell-view/template.ts @@ -13,6 +13,7 @@ export const template = html` } return true; }}" + class="${x => (x.isPlaceholder ? 'placeholder' : '')}" > ${when(x => typeof x.cellRecord?.href === 'string', html` <${anchorTag} diff --git a/packages/nimble-components/src/table-column/anchor/index.ts b/packages/nimble-components/src/table-column/anchor/index.ts index 732362f2cc..4f15d7034c 100644 --- a/packages/nimble-components/src/table-column/anchor/index.ts +++ b/packages/nimble-components/src/table-column/anchor/index.ts @@ -6,6 +6,7 @@ import { template } from '../base/template'; import { TableColumnSortOperation } from '../base/types'; import { mixinFractionalWidthColumnAPI } from '../mixins/fractional-width-column'; import { mixinGroupableColumnAPI } from '../mixins/groupable-column'; +import { mixinColumnWithPlaceholderAPI } from '../mixins/placeholder'; import type { TableStringField } from '../../table/types'; import { tableColumnAnchorCellViewTag } from './cell-view'; import { tableColumnTextGroupHeaderViewTag } from '../text/group-header-view'; @@ -23,6 +24,7 @@ export interface TableColumnAnchorColumnConfig { target?: string; type?: string; download?: string; + placeholder?: string; } declare global { @@ -35,7 +37,11 @@ declare global { * A table column for displaying links. */ export class TableColumnAnchor extends mixinGroupableColumnAPI( - mixinFractionalWidthColumnAPI(TableColumn) + mixinFractionalWidthColumnAPI( + mixinColumnWithPlaceholderAPI( + TableColumn + ) + ) ) { @attr({ attribute: 'label-field-name' }) public labelFieldName?: string; @@ -70,6 +76,10 @@ export class TableColumnAnchor extends mixinGroupableColumnAPI( @attr public download?: string; + public placeholderChanged(): void { + this.updateColumnConfig(); + } + protected override getColumnInternalsOptions(): ColumnInternalsOptions { return { cellRecordFieldNames: ['label', 'href'], @@ -141,7 +151,8 @@ export class TableColumnAnchor extends mixinGroupableColumnAPI( rel: this.rel, target: this.target, type: this.type, - download: this.download + download: this.download, + placeholder: this.placeholder }; } } diff --git a/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor-matrix.stories.ts b/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor-matrix.stories.ts index a2606a7d3f..8b20f18351 100644 --- a/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor-matrix.stories.ts +++ b/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor-matrix.stories.ts @@ -14,6 +14,10 @@ import { controlLabelFont, controlLabelFontColor } from '../../../theme-provider/design-tokens'; +import { + placeholderStates, + type PlaceholderState +} from '../../../utilities/tests/states'; const metadata: Meta = { title: 'Tests/Table Column: Anchor', @@ -50,18 +54,21 @@ const appearanceStates: [string, string | undefined][] = Object.entries( ).map(([key, value]) => [pascalCase(key), value]); type AppearanceState = (typeof appearanceStates)[number]; -const underlineHiddenStates: [string, boolean][] = [ +const underlineHiddenStates = [ ['Underline Hidden', true], ['', false] -]; +] as const; type UnderlineHiddenState = (typeof underlineHiddenStates)[number]; // prettier-ignore const component = ( [appearanceName, appearance]: AppearanceState, - [underlineHiddenName, underlineHidden]: UnderlineHiddenState + [underlineHiddenName, underlineHidden]: UnderlineHiddenState, + [placeholderName, placeholder]: PlaceholderState ): ViewTemplate => html` - + <${tableTag} id-field-name="id" style="height: 320px"> <${tableColumnAnchorTag} label-field-name="firstName" @@ -69,6 +76,7 @@ const component = ( group-index="0" appearance="${() => appearance}" ?underline-hidden="${() => underlineHidden}" + placeholder="${() => placeholder}" > <${iconUserTag}> @@ -76,7 +84,11 @@ const component = ( `; export const tableColumnAnchorThemeMatrix: StoryFn = createMatrixThemeStory( - createMatrix(component, [appearanceStates, underlineHiddenStates]) + createMatrix(component, [ + appearanceStates, + underlineHiddenStates, + placeholderStates + ]) ); tableColumnAnchorThemeMatrix.play = async (): Promise => { diff --git a/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor.mdx b/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor.mdx index 683b4e2126..f793121ecf 100644 --- a/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor.mdx +++ b/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor.mdx @@ -12,11 +12,21 @@ The column is used to display string fields a -## Target Configuration +## Usage + +### Best Practices + +- Provide meaningful, distinct labels for records that have a non-empty url. + - Do not provide `null`, `undefined`, empty, or whitespace labels with non-empty urls. + - Do not provide duplicate labels for different urls. + - If a label is not available or known for a url, the url itself may be explicitly provided for the label to ensure each distinct url has a distinct label. As grouping is done by label value, this prevents unrelated URLs from being grouped together. +- For records without a url, the label may also be omitted. All omitted labels should consistently use `null`, `undefined`, or empty string but not a combination of those values. + +### Target Configuration -## Angular Usage +### Angular Usage In an Angular app, you can configure a callback to intercept clicks so that you may invoke the router to perform the navigation instead of the default handler: diff --git a/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor.spec.ts b/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor.spec.ts index 89a8ad203c..27b6c2dc5a 100644 --- a/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor.spec.ts +++ b/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor.spec.ts @@ -1,6 +1,6 @@ -import { html } from '@microsoft/fast-element'; +import { html, ref } from '@microsoft/fast-element'; import { parameterizeSpec } from '@ni/jasmine-parameterized'; -import type { Table } from '../../../table'; +import { tableTag, type Table } from '../../../table'; import { TableColumnAnchor, tableColumnAnchorTag } from '..'; import { waitForUpdatesAsync } from '../../../testing/async-helpers'; import { type Fixture, fixture } from '../../../utilities/tests/fixture'; @@ -16,11 +16,17 @@ interface SimpleTableRecord extends TableRecord { otherLink?: string | null; } +class ElementReferences { + public table!: Table; + public column!: TableColumnAnchor; +} + // prettier-ignore -async function setup(): Promise>> { +async function setup(source: ElementReferences): Promise>> { return fixture>( - html` + html`<${tableTag} style="width: 700px" ${ref('table')}> <${tableColumnAnchorTag} + ${ref('column')} label-field-name="label" href-field-name="link" appearance="prominent" @@ -38,19 +44,24 @@ async function setup(): Promise>> { <${tableColumnAnchorTag}> Column 2 - ` + `, + { source } ); } describe('TableColumnAnchor', () => { - let element: Table; + let table: Table; + let column: TableColumnAnchor; let connect: () => Promise; let disconnect: () => Promise; let pageObject: TablePageObject; beforeEach(async () => { - ({ element, connect, disconnect } = await setup()); - pageObject = new TablePageObject(element); + const elementReferences = new ElementReferences(); + ({ connect, disconnect } = await setup(elementReferences)); + table = elementReferences.table; + column = elementReferences.column; + pageObject = new TablePageObject(table); }); afterEach(async () => { @@ -71,62 +82,40 @@ describe('TableColumnAnchor', () => { await connect(); await waitForUpdatesAsync(); - const firstColumn = element.columns[0] as TableColumnAnchor; - - expect(firstColumn.checkValidity()).toBeTrue(); + expect(column.checkValidity()).toBeTrue(); }); describe('with no href', () => { - const noValueData = [ - { name: 'field not present', data: [{ unused: 'foo' }] }, - { name: 'value is null', data: [{ label: null }] }, - { name: 'value is undefined', data: [{ label: undefined }] }, - { - name: 'value is not a string', - data: [{ label: 10 as unknown as string }] - } - ] as const; - parameterizeSpec(noValueData, (spec, name, value) => { - spec(`displays empty string when label ${name}`, async () => { - await element.setData(value.data); - await connect(); - await waitForUpdatesAsync(); - - expect(pageObject.getRenderedCellTextContent(0, 0)).toBe(''); - }); - }); - it('changing labelFieldName updates display', async () => { - await element.setData([{ label: 'foo', otherLabel: 'bar' }]); + await table.setData([{ label: 'foo', otherLabel: 'bar' }]); await connect(); await waitForUpdatesAsync(); - const firstColumn = element.columns[0] as TableColumnAnchor; - firstColumn.labelFieldName = 'otherLabel'; + column.labelFieldName = 'otherLabel'; await waitForUpdatesAsync(); expect(pageObject.getRenderedCellTextContent(0, 0)).toBe('bar'); }); it('changing data from value to null displays blank', async () => { - await element.setData([{ label: 'foo' }]); + await table.setData([{ label: 'foo' }]); await connect(); await waitForUpdatesAsync(); expect(pageObject.getRenderedCellTextContent(0, 0)).toBe('foo'); - await element.setData([{ label: null }]); + await table.setData([{ label: null }]); await waitForUpdatesAsync(); expect(pageObject.getRenderedCellTextContent(0, 0)).toBe(''); }); it('changing data from null to value displays value', async () => { - await element.setData([{ label: null }]); + await table.setData([{ label: null }]); await connect(); await waitForUpdatesAsync(); expect(pageObject.getRenderedCellTextContent(0, 0)).toBe(''); - await element.setData([{ label: 'foo' }]); + await table.setData([{ label: 'foo' }]); await waitForUpdatesAsync(); expect(pageObject.getRenderedCellTextContent(0, 0)).toBe('foo'); @@ -136,9 +125,8 @@ describe('TableColumnAnchor', () => { await connect(); await waitForUpdatesAsync(); - const firstColumn = element.columns[0] as TableColumnAnchor; - firstColumn.labelFieldName = undefined; - await element.setData([{ field: 'foo' }]); + column.labelFieldName = undefined; + await table.setData([{ field: 'foo' }]); await waitForUpdatesAsync(); expect(pageObject.getRenderedCellTextContent(0, 0)).toBe(''); @@ -146,7 +134,7 @@ describe('TableColumnAnchor', () => { it('sets title when cell text is ellipsized', async () => { const cellContents = 'a very long value that should get ellipsized due to not fitting within the default cell width'; - await element.setData([{ label: cellContents }]); + await table.setData([{ label: cellContents }]); await connect(); await waitForUpdatesAsync(); pageObject.dispatchEventToCell(0, 0, new MouseEvent('mouseover')); @@ -156,7 +144,7 @@ describe('TableColumnAnchor', () => { it('does not set title when cell text is fully visible', async () => { const cellContents = 'short value'; - await element.setData([{ label: cellContents }]); + await table.setData([{ label: cellContents }]); await connect(); await waitForUpdatesAsync(); pageObject.dispatchEventToCell(0, 0, new MouseEvent('mouseover')); @@ -166,7 +154,7 @@ describe('TableColumnAnchor', () => { it('removes title on mouseout of cell', async () => { const cellContents = 'a very long value that should get ellipsized due to not fitting within the default cell width'; - await element.setData([{ label: cellContents }]); + await table.setData([{ label: cellContents }]); await connect(); await waitForUpdatesAsync(); pageObject.dispatchEventToCell(0, 0, new MouseEvent('mouseover')); @@ -181,7 +169,7 @@ describe('TableColumnAnchor', () => { spec(`data "${name}" renders correctly`, async () => { await connect(); - await element.setData([{ label: name }]); + await table.setData([{ label: name }]); await waitForUpdatesAsync(); expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( @@ -194,7 +182,7 @@ describe('TableColumnAnchor', () => { describe('with href', () => { it('displays label when href is not string', async () => { - await element.setData([ + await table.setData([ { label: 'foo', link: 10 as unknown as string } ]); await connect(); @@ -204,33 +192,31 @@ describe('TableColumnAnchor', () => { }); it('changing labelFieldName updates display', async () => { - await element.setData([ + await table.setData([ { label: 'foo', otherLabel: 'bar', link: 'url' } ]); await connect(); await waitForUpdatesAsync(); - const firstColumn = element.columns[0] as TableColumnAnchor; - firstColumn.labelFieldName = 'otherLabel'; + column.labelFieldName = 'otherLabel'; await waitForUpdatesAsync(); expect(pageObject.getRenderedCellTextContent(0, 0)).toBe('bar'); }); it('changing hrefFieldName updates href', async () => { - await element.setData([{ link: 'foo', otherLink: 'bar' }]); + await table.setData([{ link: 'foo', otherLink: 'bar' }]); await connect(); await waitForUpdatesAsync(); - const firstColumn = element.columns[0] as TableColumnAnchor; - firstColumn.hrefFieldName = 'otherLink'; + column.hrefFieldName = 'otherLink'; await waitForUpdatesAsync(); expect(pageObject.getRenderedCellAnchor(0, 0).href).toBe('bar'); }); it('sets appearance on anchor', async () => { - await element.setData([{ link: 'foo' }]); + await table.setData([{ link: 'foo' }]); await connect(); await waitForUpdatesAsync(); @@ -240,12 +226,11 @@ describe('TableColumnAnchor', () => { }); it('updating underline-hidden from true to false removes the underline-hidden attribute from the anchor', async () => { - await element.setData([{ link: 'foo' }]); + await table.setData([{ link: 'foo' }]); await connect(); await waitForUpdatesAsync(); - const firstColumn = element.columns[0] as TableColumnAnchor; - firstColumn.underlineHidden = true; + column.underlineHidden = true; await waitForUpdatesAsync(); expect( pageObject @@ -253,7 +238,7 @@ describe('TableColumnAnchor', () => { .hasAttribute('underline-hidden') ).toBeTrue(); - firstColumn.underlineHidden = false; + column.underlineHidden = false; await waitForUpdatesAsync(); expect( pageObject @@ -276,7 +261,7 @@ describe('TableColumnAnchor', () => { ] as const; parameterizeSpec(linkOptionData, (spec, name, value) => { spec(`sets ${name} on anchor`, async () => { - await element.setData([{ link: 'foo' }]); + await table.setData([{ link: 'foo' }]); await connect(); await waitForUpdatesAsync(); @@ -288,7 +273,7 @@ describe('TableColumnAnchor', () => { describe('with no label', () => { it('displays empty string when href is not string', async () => { - await element.setData([{ link: 10 as unknown as string }]); + await table.setData([{ link: 10 as unknown as string }]); await connect(); await waitForUpdatesAsync(); @@ -296,7 +281,7 @@ describe('TableColumnAnchor', () => { }); it('displays url', async () => { - await element.setData([{ link: 'foo' }]); + await table.setData([{ link: 'foo' }]); await connect(); await waitForUpdatesAsync(); @@ -304,24 +289,24 @@ describe('TableColumnAnchor', () => { }); it('changing url from value to null displays blank', async () => { - await element.setData([{ link: 'foo' }]); + await table.setData([{ link: 'foo' }]); await connect(); await waitForUpdatesAsync(); expect(pageObject.getRenderedCellTextContent(0, 0)).toBe('foo'); - await element.setData([{ link: null }]); + await table.setData([{ link: null }]); await waitForUpdatesAsync(); expect(pageObject.getRenderedCellTextContent(0, 0)).toBe(''); }); it('changing url from null to value displays value', async () => { - await element.setData([{ link: null }]); + await table.setData([{ link: null }]); await connect(); await waitForUpdatesAsync(); expect(pageObject.getRenderedCellTextContent(0, 0)).toBe(''); - await element.setData([{ link: 'foo' }]); + await table.setData([{ link: 'foo' }]); await waitForUpdatesAsync(); expect(pageObject.getRenderedCellTextContent(0, 0)).toBe('foo'); @@ -330,7 +315,7 @@ describe('TableColumnAnchor', () => { it('sets title when cell text is ellipsized', async () => { const cellContents = 'a very long value that should get ellipsized due to not fitting within the default cell width'; - await element.setData([{ label: cellContents, link: 'url' }]); + await table.setData([{ label: cellContents, link: 'url' }]); await connect(); await waitForUpdatesAsync(); pageObject.dispatchEventToCell(0, 0, new MouseEvent('mouseover')); @@ -340,7 +325,7 @@ describe('TableColumnAnchor', () => { it('does not set title when cell text is fully visible', async () => { const cellContents = 'short value'; - await element.setData([{ label: cellContents, link: 'url' }]); + await table.setData([{ label: cellContents, link: 'url' }]); await connect(); await waitForUpdatesAsync(); pageObject.dispatchEventToCell(0, 0, new MouseEvent('mouseover')); @@ -350,7 +335,7 @@ describe('TableColumnAnchor', () => { it('removes title on mouseout of cell', async () => { const cellContents = 'a very long value that should get ellipsized due to not fitting within the default cell width'; - await element.setData([{ label: cellContents, link: 'url' }]); + await table.setData([{ label: cellContents, link: 'url' }]); await connect(); await waitForUpdatesAsync(); pageObject.dispatchEventToCell(0, 0, new MouseEvent('mouseover')); @@ -362,8 +347,8 @@ describe('TableColumnAnchor', () => { it('sets title when group header text is ellipsized', async () => { const cellContents = 'a very long value that should get ellipsized due to not fitting within the default cell width'; - await element.setData([{ label: cellContents, link: 'url' }]); - element.style.width = '200px'; + await table.setData([{ label: cellContents, link: 'url' }]); + table.style.width = '200px'; await connect(); await waitForUpdatesAsync(); pageObject.dispatchEventToGroupHeader( @@ -376,7 +361,7 @@ describe('TableColumnAnchor', () => { it('does not set title when group header text is fully visible', async () => { const cellContents = 'foo'; - await element.setData([{ label: cellContents, link: 'url' }]); + await table.setData([{ label: cellContents, link: 'url' }]); await connect(); await waitForUpdatesAsync(); pageObject.dispatchEventToGroupHeader( @@ -389,7 +374,7 @@ describe('TableColumnAnchor', () => { it('removes title on mouseout of group header', async () => { const cellContents = 'a very long value that should get ellipsized due to not fitting within the default cell width'; - await element.setData([{ label: cellContents, link: 'url' }]); + await table.setData([{ label: cellContents, link: 'url' }]); await connect(); await waitForUpdatesAsync(); pageObject.dispatchEventToGroupHeader( @@ -406,14 +391,13 @@ describe('TableColumnAnchor', () => { }); it('sorts by the label field', async () => { - element.idFieldName = 'label'; + table.idFieldName = 'label'; await connect(); await waitForUpdatesAsync(); - const firstColumn = element.columns[0] as TableColumnAnchor; - firstColumn.sortDirection = TableColumnSortDirection.ascending; - firstColumn.sortIndex = 0; - await element.setData([ + column.sortDirection = TableColumnSortDirection.ascending; + column.sortIndex = 0; + await table.setData([ { label: 'd', link: 'foo3' }, { label: 'a', link: 'foo4' }, { label: 'c', link: 'foo1' }, @@ -440,7 +424,7 @@ describe('TableColumnAnchor', () => { spec(`data "${name}" renders correctly`, async () => { await connect(); - await element.setData([{ label: name, link: 'url' }]); + await table.setData([{ label: name, link: 'url' }]); await waitForUpdatesAsync(); expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( @@ -455,7 +439,7 @@ describe('TableColumnAnchor', () => { spec(`data "${name}" renders correctly`, async () => { await connect(); - await element.setData([{ label: name, link: 'url' }]); + await table.setData([{ label: name, link: 'url' }]); await waitForUpdatesAsync(); expect( @@ -465,4 +449,172 @@ describe('TableColumnAnchor', () => { }); }); }); + + describe('placeholder', () => { + const testCases = [ + { + name: 'label and href are both defined', + data: [{ label: 'my label', link: 'url' }], + cellValue: 'my label', + groupValue: 'my label', + usesColumnPlaceholder: false + }, + { + name: 'label and href are both missing', + data: [{}], + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true + }, + { + name: 'label and href are both undefined', + data: [{ label: undefined, link: undefined }], + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true + }, + { + name: 'label and href are both null', + data: [{ label: null, link: null }], + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true + }, + { + name: 'label is null and href is undefined', + data: [{ label: null, link: undefined }], + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true + }, + { + name: 'label is undefined and href is null', + data: [{ label: undefined, link: null }], + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true + }, + { + name: 'label is empty with defined href', + data: [{ label: '', link: 'link' }], + cellValue: '', + groupValue: 'Empty', + usesColumnPlaceholder: false + }, + { + name: 'label is non-empty with undefined href', + data: [{ label: 'my label', link: undefined }], + cellValue: 'my label', + groupValue: 'my label', + usesColumnPlaceholder: false + }, + { + name: 'label is not a string with no href', + data: [{ label: 10 as unknown as string }], + cellValue: '', + groupValue: '', + usesColumnPlaceholder: false + }, + { + name: 'label is not a string with a defined href', + data: [{ label: 10 as unknown as string, link: 'link' }], + cellValue: 'link', + groupValue: '', + usesColumnPlaceholder: false + } + ] as const; + + async function initializeColumnAndTable( + data: readonly SimpleTableRecord[], + placeholder?: string + ): Promise { + column.placeholder = placeholder; + await table.setData(data); + await connect(); + await waitForUpdatesAsync(); + } + + parameterizeSpec(testCases, (spec, name, value) => { + spec( + `cell and group row render expected value when ${name} and placeholder is configured`, + async () => { + const placeholder = 'Custom placeholder'; + await initializeColumnAndTable(value.data, placeholder); + + const expectedCellText = value.usesColumnPlaceholder + ? placeholder + : value.cellValue; + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + expectedCellText + ); + expect( + pageObject.getRenderedGroupHeaderTextContent(0) + ).toBe(value.groupValue); + } + ); + }); + + parameterizeSpec(testCases, (spec, name, value) => { + spec( + `cell and group row render expected value when ${name} and placeholder is not configured`, + async () => { + await initializeColumnAndTable(value.data); + + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + value.cellValue + ); + expect( + pageObject.getRenderedGroupHeaderTextContent(0) + ).toBe(value.groupValue); + } + ); + }); + + it('setting placeholder to undefined updates cells from displaying placeholder to displaying blank', async () => { + const placeholder = 'My placeholder'; + await initializeColumnAndTable([{}], placeholder); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + placeholder + ); + + column.placeholder = undefined; + await waitForUpdatesAsync(); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe(''); + }); + + it('setting placeholder to defined string updates cells from displaying blank to displaying placeholder', async () => { + await initializeColumnAndTable([{}]); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe(''); + + const placeholder = 'placeholder'; + column.placeholder = placeholder; + await waitForUpdatesAsync(); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + placeholder + ); + }); + + it('updating placeholder from one string to another updates cell', async () => { + const placeholder1 = 'My first placeholder'; + await initializeColumnAndTable([{}], placeholder1); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + placeholder1 + ); + + const placeholder2 = 'My second placeholder'; + column.placeholder = placeholder2; + await waitForUpdatesAsync(); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + placeholder2 + ); + }); + + it('can configure empty placeholder', async () => { + const placeholder = ''; + await initializeColumnAndTable([{}], placeholder); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + placeholder + ); + }); + }); }); diff --git a/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor.stories.ts b/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor.stories.ts index 4cdca5af2f..7cd871af4b 100644 --- a/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor.stories.ts +++ b/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor.stories.ts @@ -69,6 +69,7 @@ interface AnchorColumnTableArgs extends SharedTableArgs { hrefFieldName: string; appearance: keyof typeof AnchorAppearance; underlineHidden: boolean; + placeholder: string; } export const anchorColumn: StoryObj = { @@ -84,6 +85,7 @@ export const anchorColumn: StoryObj = { href-field-name="${x => x.hrefFieldName}" appearance="${x => x.appearance}" ?underline-hidden="${x => x.underlineHidden}" + placeholder="${x => x.placeholder}" > Link Column @@ -118,6 +120,10 @@ export const anchorColumn: StoryObj = { name: 'underline-hidden', defaultValue: { summary: 'false' }, description: 'Causes the underline to be hidden except on hover.' + }, + placeholder: { + description: + 'The placeholder text to display when the label and href are both `undefined` or `null` for a record.' } }, args: { @@ -125,6 +131,7 @@ export const anchorColumn: StoryObj = { hrefFieldName: 'url', appearance: 'default', underlineHidden: false, + placeholder: 'Mystery', ...sharedTableArgs(simpleData) } }; diff --git a/packages/nimble-components/src/table-column/date-text/cell-view/index.ts b/packages/nimble-components/src/table-column/date-text/cell-view/index.ts index 676c4904f0..73cefad31f 100644 --- a/packages/nimble-components/src/table-column/date-text/cell-view/index.ts +++ b/packages/nimble-components/src/table-column/date-text/cell-view/index.ts @@ -21,15 +21,7 @@ export class TableColumnDateTextCellView extends TableColumnTextCellViewBase< TableColumnDateTextCellRecord, TableColumnDateTextColumnConfig > { - private columnConfigChanged(): void { - this.updateText(); - } - - private cellRecordChanged(): void { - this.updateText(); - } - - private updateText(): void { + protected updateText(): void { if (this.columnConfig) { this.text = formatNumericDate( this.columnConfig.formatter, diff --git a/packages/nimble-components/src/table-column/date-text/index.ts b/packages/nimble-components/src/table-column/date-text/index.ts index 578b1da9d8..16612f24a9 100644 --- a/packages/nimble-components/src/table-column/date-text/index.ts +++ b/packages/nimble-components/src/table-column/date-text/index.ts @@ -32,9 +32,11 @@ import { import { TableColumnDateTextValidator } from './models/table-column-date-text-validator'; import { lang } from '../../theme-provider'; import { optionalBooleanConverter } from '../../utilities/models/converter'; +import type { TableColumnTextBaseColumnConfig } from '../text-base/cell-view'; export type TableColumnDateTextCellRecord = TableNumberField<'value'>; -export interface TableColumnDateTextColumnConfig { +export interface TableColumnDateTextColumnConfig + extends TableColumnTextBaseColumnConfig { formatter: Intl.DateTimeFormat; } @@ -135,6 +137,10 @@ export class TableColumnDateText extends TableColumnTextBase { return this.validator.getValidity(); } + public placeholderChanged(): void { + this.updateColumnConfig(); + } + protected override getColumnInternalsOptions(): ColumnInternalsOptions { return { cellRecordFieldNames: ['value'], @@ -230,7 +236,8 @@ export class TableColumnDateText extends TableColumnTextBase { if (formatter) { const columnConfig: TableColumnDateTextColumnConfig = { - formatter + formatter, + placeholder: this.placeholder }; this.columnInternals.columnConfig = columnConfig; this.validator.setCustomOptionsValidity(true); diff --git a/packages/nimble-components/src/table-column/date-text/tests/table-column-date-text-matrix.stories.ts b/packages/nimble-components/src/table-column/date-text/tests/table-column-date-text-matrix.stories.ts index 7f4ff268a9..10107e30f5 100644 --- a/packages/nimble-components/src/table-column/date-text/tests/table-column-date-text-matrix.stories.ts +++ b/packages/nimble-components/src/table-column/date-text/tests/table-column-date-text-matrix.stories.ts @@ -12,6 +12,10 @@ import { controlLabelFont, controlLabelFontColor } from '../../../theme-provider/design-tokens'; +import { + placeholderStates, + type PlaceholderState +} from '../../../utilities/tests/states'; const metadata: Meta = { title: 'Tests/Table Column: Date Text', @@ -37,12 +41,17 @@ const data = [ ] as const; // prettier-ignore -const component = (): ViewTemplate => html` - +const component = ( + [placeholderName, placeholder]: PlaceholderState +): ViewTemplate => html` + <${tableTag} id-field-name="id" style="height: 250px"> <${tableColumnDateTextTag} field-name="date" group-index="0" + placeholder="${() => placeholder}" > <${iconUserTag}> @@ -50,7 +59,7 @@ const component = (): ViewTemplate => html` `; export const tableColumnDateTextThemeMatrix: StoryFn = createMatrixThemeStory( - createMatrix(component) + createMatrix(component, [placeholderStates]) ); tableColumnDateTextThemeMatrix.play = async (): Promise => { diff --git a/packages/nimble-components/src/table-column/date-text/tests/table-column-date-text.mdx b/packages/nimble-components/src/table-column/date-text/tests/table-column-date-text.mdx index 28422d38a7..867822d1a9 100644 --- a/packages/nimble-components/src/table-column/date-text/tests/table-column-date-text.mdx +++ b/packages/nimble-components/src/table-column/date-text/tests/table-column-date-text.mdx @@ -3,6 +3,7 @@ import { NimbleTableColumnDateText } from './table-column-date-text.react'; import * as tableColumnDateTextStories from './table-column-date-text.stories'; import { tableColumnDateTextTag } from '..'; import { tableTag } from '../../../table'; +import NoNullAndUndefinedBestPractice from '../../../patterns/table-column/tests/no-null-and-undefined-best-practice-docs.mdx'; @@ -16,7 +17,11 @@ The <Tag name={tableColumnDateTextTag}/> column is used to display date-time fie {/* ## Appearance Variants */} -{/* ## Usage */} +## Usage + +### Best Practices + +- <NoNullAndUndefinedBestPractice /> {/* ## Examples */} diff --git a/packages/nimble-components/src/table-column/date-text/tests/table-column-date-text.spec.ts b/packages/nimble-components/src/table-column/date-text/tests/table-column-date-text.spec.ts index 9bcaefed03..10f1077218 100644 --- a/packages/nimble-components/src/table-column/date-text/tests/table-column-date-text.spec.ts +++ b/packages/nimble-components/src/table-column/date-text/tests/table-column-date-text.spec.ts @@ -78,48 +78,6 @@ describe('TableColumnDateText', () => { expect(column.checkValidity()).toBeTrue(); }); - describe('displays blank when', () => { - const badValueData = [ - { name: 'field not present', data: [{ unused: 'foo' }] }, - { name: 'value is null', data: [{ field: null }] }, - { name: 'value is undefined', data: [{ field: undefined }] }, - { - name: 'value is Inf', - data: [{ field: Number.POSITIVE_INFINITY }] - }, - { - name: 'value is -Inf', - data: [{ field: Number.NEGATIVE_INFINITY }] - }, - { name: 'value is NaN', data: [{ field: Number.NaN }] }, - { - name: 'value is MAX_VALUE', - data: [{ field: Number.MAX_VALUE }] - }, - { - name: 'value is too large for Date', - data: [{ field: 8640000000000000 + 1 }] - }, - { - name: 'value is too small for Date', - data: [{ field: -8640000000000000 - 1 }] - }, - { - name: 'value is not a number', - data: [{ field: 'foo' as unknown as number }] - } - ] as const; - - parameterizeSpec(badValueData, (spec, name, value) => { - spec(name, async () => { - await table.setData(value.data); - await waitForUpdatesAsync(); - - expect(pageObject.getRenderedCellContent(0, 0)).toEqual(''); - }); - }); - }); - // WebKit skipped, see https://github.com/ni/nimble/issues/1940 it('changing fieldName updates display #SkipWebkit', async () => { await table.setData([ @@ -522,83 +480,185 @@ describe('TableColumnDateText', () => { { name: 'value is not specified', data: [{}], - groupValue: 'No value' + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true }, { name: 'value is undefined', data: [{ field: undefined }], - groupValue: 'No value' + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true }, { name: 'value is null', data: [{ field: null }], - groupValue: 'No value' + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true }, { name: 'value is Number.NaN', data: [{ field: Number.NaN }], - groupValue: '' + cellValue: '', + groupValue: '', + usesColumnPlaceholder: false }, { name: 'value is valid and non-zero', data: [{ field: 1708984169258 }], - groupValue: '2/26/2024' + cellValue: '2/26/2024', + groupValue: '2/26/2024', + usesColumnPlaceholder: false }, { name: 'value is incorrect type', data: [{ field: 'not a number' as unknown as number }], - groupValue: '' + cellValue: '', + groupValue: '', + usesColumnPlaceholder: false }, { name: 'value is specified and falsey', data: [{ field: 0 }], - groupValue: '1/1/1970' + cellValue: '1/1/1970', + groupValue: '1/1/1970', + usesColumnPlaceholder: false }, { name: 'value is Inf', data: [{ field: Number.POSITIVE_INFINITY }], - groupValue: '' + cellValue: '', + groupValue: '', + usesColumnPlaceholder: false }, { name: 'value is -Inf', data: [{ field: Number.NEGATIVE_INFINITY }], - groupValue: '' + cellValue: '', + groupValue: '', + usesColumnPlaceholder: false }, { name: 'value is MAX_VALUE', data: [{ field: Number.MAX_VALUE }], - groupValue: '' + cellValue: '', + groupValue: '', + usesColumnPlaceholder: false }, { name: 'value is too large for Date', data: [{ field: 8640000000000000 + 1 }], - groupValue: '' + cellValue: '', + groupValue: '', + usesColumnPlaceholder: false }, { name: 'value is too small for Date', data: [{ field: -8640000000000000 - 1 }], - groupValue: '' + cellValue: '', + groupValue: '', + usesColumnPlaceholder: false } - ]; + ] as const; + + async function initializeColumnAndTable( + data: readonly SimpleTableRecord[], + placeholder?: string + ): Promise<void> { + // Set a custom time zone so that the behavior of the test does not + // depend on the configuration of the computer running the tests. + column.format = DateTextFormat.custom; + column.customTimeZone = 'UTC'; + column.placeholder = placeholder; + await table.setData(data); + await connect(); + await waitForUpdatesAsync(); + } parameterizeSpec(testCases, (spec, name, value) => { spec( - `group row renders expected value when ${name}`, + `cell and group row render expected value when ${name} and placeholder is configured`, async () => { - // Set a custom time zone so that the behavior of the test does not - // depend on the configuration of the computer running the tests. - column.format = DateTextFormat.custom; - column.customTimeZone = 'UTC'; - await table.setData(value.data); - await connect(); - await waitForUpdatesAsync(); + const placeholder = 'Custom placeholder'; + await initializeColumnAndTable(value.data, placeholder); + + const expectedCellText = value.usesColumnPlaceholder + ? placeholder + : value.cellValue; + expect(pageObject.getRenderedCellContent(0, 0)).toBe( + expectedCellText + ); + expect( + pageObject.getRenderedGroupHeaderContent(0) + ).toBe(value.groupValue); + } + ); + }); + parameterizeSpec(testCases, (spec, name, value) => { + spec( + `cell and group row render expected value when ${name} and placeholder is not configured`, + async () => { + await initializeColumnAndTable(value.data); + + expect(pageObject.getRenderedCellContent(0, 0)).toBe( + value.cellValue + ); expect( pageObject.getRenderedGroupHeaderContent(0) ).toBe(value.groupValue); } ); }); + + it('setting placeholder to undefined updates cells from displaying placeholder to displaying blank', async () => { + const placeholder = 'My placeholder'; + await initializeColumnAndTable([{}], placeholder); + expect(pageObject.getRenderedCellContent(0, 0)).toBe( + placeholder + ); + + column.placeholder = undefined; + await waitForUpdatesAsync(); + expect(pageObject.getRenderedCellContent(0, 0)).toBe(''); + }); + + it('setting placeholder to defined string updates cells from displaying blank to displaying placeholder', async () => { + await initializeColumnAndTable([{}]); + expect(pageObject.getRenderedCellContent(0, 0)).toBe(''); + + const placeholder = 'placeholder'; + column.placeholder = placeholder; + await waitForUpdatesAsync(); + expect(pageObject.getRenderedCellContent(0, 0)).toBe( + placeholder + ); + }); + + it('updating placeholder from one string to another updates cell', async () => { + const placeholder1 = 'My first placeholder'; + await initializeColumnAndTable([{}], placeholder1); + expect(pageObject.getRenderedCellContent(0, 0)).toBe( + placeholder1 + ); + + const placeholder2 = 'My second placeholder'; + column.placeholder = placeholder2; + await waitForUpdatesAsync(); + expect(pageObject.getRenderedCellContent(0, 0)).toBe( + placeholder2 + ); + }); + + it('can configure empty placeholder', async () => { + const placeholder = ''; + await initializeColumnAndTable([{}], placeholder); + expect(pageObject.getRenderedCellContent(0, 0)).toBe( + placeholder + ); + }); }); }); diff --git a/packages/nimble-components/src/table-column/date-text/tests/table-column-date-text.stories.ts b/packages/nimble-components/src/table-column/date-text/tests/table-column-date-text.stories.ts index bb6c6796e2..6c3effd969 100644 --- a/packages/nimble-components/src/table-column/date-text/tests/table-column-date-text.stories.ts +++ b/packages/nimble-components/src/table-column/date-text/tests/table-column-date-text.stories.ts @@ -47,9 +47,9 @@ const simpleData = [ birthday: new Date(2013, 3, 1, 20, 4, 37, 975).valueOf() }, { - firstName: 'Maggie', - lastName: 'Simpson', - birthday: new Date(2022, 0, 12, 20, 4, 37, 975).valueOf() + firstName: 'Abbey', + lastName: 'Simpson?', + birthday: undefined } ] as const; @@ -79,6 +79,7 @@ export default metadata; interface TextColumnTableArgs extends SharedTableArgs { fieldName: string; + placeholder: string; format: keyof typeof DateTextFormat; customDateStyle: DateStyle; customTimeStyle: TimeStyle; @@ -123,6 +124,7 @@ export const dateTextColumn: StoryObj<TextColumnTableArgs> = { </${tableColumnTextTag}> <${tableColumnDateTextTag} field-name="birthday" + placeholder="${x => x.placeholder}" format="${x => DateTextFormat[x.format]}" custom-date-style="${x => x.customDateStyle}" custom-time-style="${x => x.customTimeStyle}" @@ -155,6 +157,10 @@ export const dateTextColumn: StoryObj<TextColumnTableArgs> = { 'Set this attribute to identify which field in the data record should be displayed in each column. The field values must be of type `number` and represent the number of milliseconds since January 1, 1970 UTC. This is the representation used by the `Date` type.', control: { type: 'none' } }, + placeholder: { + description: + 'The placeholder text to display when the field value is `undefined` or `null` for a record.' + }, format: { description: 'By default, dates are formatted like "Jan 1, 2023, 12:00:00 AM". To use a different format, set this attribute to `custom` and provide additional attributes corresponding to [`Intl.DateTimeFormat()` options](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/DateTimeFormat/DateTimeFormat). Each `Intl.DateTimeFormat()` option has a corresponding attribute whose name is kebab-cased and prefixed with `custom-` e.g. `custom-date-style` corresponds to `dateStyle`.', @@ -333,6 +339,7 @@ export const dateTextColumn: StoryObj<TextColumnTableArgs> = { }, args: { fieldName: 'firstName', + placeholder: 'Unknown birthday', format: 'default', customDateStyle: undefined, customTimeStyle: undefined, diff --git a/packages/nimble-components/src/table-column/duration-text/cell-view/index.ts b/packages/nimble-components/src/table-column/duration-text/cell-view/index.ts index 0b59bde91b..9dd48bbef1 100644 --- a/packages/nimble-components/src/table-column/duration-text/cell-view/index.ts +++ b/packages/nimble-components/src/table-column/duration-text/cell-view/index.ts @@ -20,15 +20,7 @@ export class TableColumnDurationTextCellView extends TableColumnTextCellViewBase TableColumnDurationTextCellRecord, TableColumnDurationTextColumnConfig > { - private columnConfigChanged(): void { - this.updateText(); - } - - private cellRecordChanged(): void { - this.updateText(); - } - - private updateText(): void { + protected updateText(): void { this.text = this.columnConfig?.formatter.format(this.cellRecord?.value) ?? ''; } } diff --git a/packages/nimble-components/src/table-column/duration-text/index.ts b/packages/nimble-components/src/table-column/duration-text/index.ts index f66b291f1b..bf99c10274 100644 --- a/packages/nimble-components/src/table-column/duration-text/index.ts +++ b/packages/nimble-components/src/table-column/duration-text/index.ts @@ -12,9 +12,11 @@ import type { ColumnInternalsOptions } from '../base/models/column-internals'; import { lang } from '../../theme-provider'; import { DurationFormatter } from './models/duration-formatter'; import { tableColumnDurationTextGroupHeaderViewTag } from './group-header-view'; +import type { TableColumnTextBaseColumnConfig } from '../text-base/cell-view'; export type TableColumnDurationTextCellRecord = TableNumberField<'value'>; -export interface TableColumnDurationTextColumnConfig { +export interface TableColumnDurationTextColumnConfig + extends TableColumnTextBaseColumnConfig { formatter: DurationFormatter; } @@ -45,6 +47,10 @@ export class TableColumnDurationText extends TableColumnTextBase { lang.unsubscribe(this.langSubscriber, this); } + public placeholderChanged(): void { + this.updateColumnConfig(); + } + protected override getColumnInternalsOptions(): ColumnInternalsOptions { return { cellRecordFieldNames: ['value'], @@ -58,14 +64,11 @@ export class TableColumnDurationText extends TableColumnTextBase { private updateColumnConfig(): void { const formatter = new DurationFormatter(lang.getValueFor(this)); - if (formatter) { - const columnConfig: TableColumnDurationTextColumnConfig = { - formatter - }; - this.columnInternals.columnConfig = columnConfig; - } else { - this.columnInternals.columnConfig = undefined; - } + const columnConfig: TableColumnDurationTextColumnConfig = { + formatter, + placeholder: this.placeholder + }; + this.columnInternals.columnConfig = columnConfig; } } diff --git a/packages/nimble-components/src/table-column/duration-text/tests/table-column-duration-text-matrix.stories.ts b/packages/nimble-components/src/table-column/duration-text/tests/table-column-duration-text-matrix.stories.ts index b4f6daf048..71d0e29980 100644 --- a/packages/nimble-components/src/table-column/duration-text/tests/table-column-duration-text-matrix.stories.ts +++ b/packages/nimble-components/src/table-column/duration-text/tests/table-column-duration-text-matrix.stories.ts @@ -11,6 +11,10 @@ import { controlLabelFont, controlLabelFontColor } from '../../../theme-provider/design-tokens'; +import { + placeholderStates, + type PlaceholderState +} from '../../../utilities/tests/states'; const metadata: Meta = { title: 'Tests/Table Column: Duration Text', @@ -36,19 +40,24 @@ const data = [ ] as const; // prettier-ignore -const component = (): ViewTemplate => html` - <label style="color: var(${controlLabelFontColor.cssCustomProperty}); font: var(${controlLabelFont.cssCustomProperty})">Duration Text Table Column</label> +const component = ( + [placeholderName, placeholder]: PlaceholderState +): ViewTemplate => html` + <label style="color: var(${controlLabelFontColor.cssCustomProperty}); font: var(${controlLabelFont.cssCustomProperty})"> + Duration Text Table Column ${placeholderName} + </label> <${tableTag} id-field-name="id" style="height: 250px"> <${tableColumnDurationTextTag} field-name="duration" group-index="0" + placeholder="${() => placeholder}" > Duration </${tableColumnDurationTextTag}> </${tableTag}> `; -export const tableColumnDurationTextThemeMatrix: StoryFn = createMatrixThemeStory(createMatrix(component)); +export const tableColumnDurationTextThemeMatrix: StoryFn = createMatrixThemeStory(createMatrix(component, [placeholderStates])); tableColumnDurationTextThemeMatrix.play = async (): Promise<void> => { await Promise.all( diff --git a/packages/nimble-components/src/table-column/duration-text/tests/table-column-duration-text.mdx b/packages/nimble-components/src/table-column/duration-text/tests/table-column-duration-text.mdx index c5ecbd6153..2c39e8606a 100644 --- a/packages/nimble-components/src/table-column/duration-text/tests/table-column-duration-text.mdx +++ b/packages/nimble-components/src/table-column/duration-text/tests/table-column-duration-text.mdx @@ -2,6 +2,7 @@ import { Controls, Canvas, Meta, Title } from '@storybook/blocks'; import * as tableColumnDurationTextStories from './table-column-duration-text.stories'; import { tableColumnDurationTextTag } from '..'; import { tableTag } from '../../../table'; +import NoNullAndUndefinedBestPractice from '../../../patterns/table-column/tests/no-null-and-undefined-best-practice-docs.mdx'; <Meta of={tableColumnDurationTextStories} /> <Title of={tableColumnDurationTextStories} /> @@ -14,9 +15,15 @@ The <Tag name={tableColumnDurationTextTag}/> column is used to display a period /> <Controls of={tableColumnDurationTextStories.durationTextColumn} /> -## Angular Usage +## Usage -### Duration Pipe +### Best Practices + +- <NoNullAndUndefinedBestPractice /> + +### Angular Usage + +#### Duration Pipe An Angular pipe is provided for formatting duration values into strings outside of the Nimble table. This pipe uses the same formatting rules (and localization) as the duration table column. diff --git a/packages/nimble-components/src/table-column/duration-text/tests/table-column-duration-text.spec.ts b/packages/nimble-components/src/table-column/duration-text/tests/table-column-duration-text.spec.ts index 419d200602..2c44a1a890 100644 --- a/packages/nimble-components/src/table-column/duration-text/tests/table-column-duration-text.spec.ts +++ b/packages/nimble-components/src/table-column/duration-text/tests/table-column-duration-text.spec.ts @@ -185,60 +185,149 @@ describe('TableColumnDurationText', () => { { name: 'value is not specified', data: [{}], - groupValue: 'No value' + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true }, { name: 'value is undefined', data: [{ field: undefined }], - groupValue: 'No value' + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true }, { name: 'value is null', data: [{ field: null }], - groupValue: 'No value' + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true }, { name: 'value is Number.NaN', data: [{ field: Number.NaN }], - groupValue: '' + cellValue: '', + groupValue: '', + usesColumnPlaceholder: false }, { name: 'value is valid and non-zero', data: [{ field: 20000 }], - groupValue: '20 sec' + cellValue: '20 sec', + groupValue: '20 sec', + usesColumnPlaceholder: false }, { name: 'value is incorrect type', data: [{ field: 'not a number' as unknown as number }], - groupValue: '' + cellValue: '', + groupValue: '', + usesColumnPlaceholder: false }, { name: 'value is specified and falsey', data: [{ field: 0 }], - groupValue: '0 sec' + cellValue: '0 sec', + groupValue: '0 sec', + usesColumnPlaceholder: false }, { name: 'value is Inf', data: [{ field: Number.POSITIVE_INFINITY }], - groupValue: '' + cellValue: '', + groupValue: '', + usesColumnPlaceholder: false }, { name: 'value is negative', data: [{ field: -5 }], - groupValue: '' + cellValue: '', + groupValue: '', + usesColumnPlaceholder: false } - ]; + ] as const; + + async function initializeColumnAndTable( + data: readonly SimpleTableRecord[], + placeholder?: string + ): Promise<void> { + column.placeholder = placeholder; + await table.setData(data); + await connect(); + await waitForUpdatesAsync(); + } + + parameterizeSpec(testCases, (spec, name, value) => { + spec( + `cell and group row render expected value when ${name} and placeholder is configured`, + async () => { + const placeholder = 'Custom placeholder'; + await initializeColumnAndTable(value.data, placeholder); + + const expectedCellText = value.usesColumnPlaceholder + ? placeholder + : value.cellValue; + expect(pageObject.getRenderedCellContent(0, 0)).toBe( + expectedCellText + ); + expect(pageObject.getRenderedGroupHeaderContent(0)).toBe( + value.groupValue + ); + } + ); + }); parameterizeSpec(testCases, (spec, name, value) => { - spec(`group row renders expected value when ${name}`, async () => { - await table.setData(value.data); - await connect(); - await waitForUpdatesAsync(); - - expect(pageObject.getRenderedGroupHeaderContent(0)).toBe( - value.groupValue - ); - }); + spec( + `cell and group row render expected value when ${name} and placeholder is not configured`, + async () => { + await initializeColumnAndTable(value.data); + + expect(pageObject.getRenderedCellContent(0, 0)).toBe( + value.cellValue + ); + expect(pageObject.getRenderedGroupHeaderContent(0)).toBe( + value.groupValue + ); + } + ); + }); + + it('setting placeholder to undefined updates cells from displaying placeholder to displaying blank', async () => { + const placeholder = 'My placeholder'; + await initializeColumnAndTable([{}], placeholder); + expect(pageObject.getRenderedCellContent(0, 0)).toBe(placeholder); + + column.placeholder = undefined; + await waitForUpdatesAsync(); + expect(pageObject.getRenderedCellContent(0, 0)).toBe(''); + }); + + it('setting placeholder to defined string updates cells from displaying blank to displaying placeholder', async () => { + await initializeColumnAndTable([{}]); + expect(pageObject.getRenderedCellContent(0, 0)).toBe(''); + + const placeholder = 'placeholder'; + column.placeholder = placeholder; + await waitForUpdatesAsync(); + expect(pageObject.getRenderedCellContent(0, 0)).toBe(placeholder); + }); + + it('updating placeholder from one string to another updates cell', async () => { + const placeholder1 = 'My first placeholder'; + await initializeColumnAndTable([{}], placeholder1); + expect(pageObject.getRenderedCellContent(0, 0)).toBe(placeholder1); + + const placeholder2 = 'My second placeholder'; + column.placeholder = placeholder2; + await waitForUpdatesAsync(); + expect(pageObject.getRenderedCellContent(0, 0)).toBe(placeholder2); + }); + + it('can configure empty placeholder', async () => { + const placeholder = ''; + await initializeColumnAndTable([{}], placeholder); + expect(pageObject.getRenderedCellContent(0, 0)).toBe(placeholder); }); }); }); diff --git a/packages/nimble-components/src/table-column/duration-text/tests/table-column-duration-text.stories.ts b/packages/nimble-components/src/table-column/duration-text/tests/table-column-duration-text.stories.ts index 86ea0921cf..fa43f06b4e 100644 --- a/packages/nimble-components/src/table-column/duration-text/tests/table-column-duration-text.stories.ts +++ b/packages/nimble-components/src/table-column/duration-text/tests/table-column-duration-text.stories.ts @@ -36,7 +36,7 @@ const simpleData = [ { firstName: 'Montgomery', lastName: 'Burns', - swearWordCadence: 3.78e12 + swearWordCadence: undefined } ] as const; @@ -66,6 +66,7 @@ export default metadata; interface TextColumnTableArgs extends SharedTableArgs { fieldName: string; + placeholder: string; } export const durationTextColumn: StoryObj<TextColumnTableArgs> = { @@ -83,6 +84,7 @@ export const durationTextColumn: StoryObj<TextColumnTableArgs> = { </${tableColumnTextTag}> <${tableColumnDurationTextTag} field-name="swearWordCadence" + placeholder="${x => x.placeholder}" > Time since last swear word </${tableColumnDurationTextTag}> @@ -94,9 +96,14 @@ export const durationTextColumn: StoryObj<TextColumnTableArgs> = { description: 'Set this attribute to identify which field in the data record should be displayed in each column. The field values must be of type `number` and represent a total number of milliseconds.', control: { type: 'none' } + }, + placeholder: { + description: + 'The placeholder text to display when the field value is `undefined` or `null` for a record.' } }, args: { - fieldName: 'firstName' + fieldName: 'firstName', + placeholder: 'Unknown time' } }; diff --git a/packages/nimble-components/src/table-column/enum-base/index.ts b/packages/nimble-components/src/table-column/enum-base/index.ts index 9e65ecf819..e89dfb9806 100644 --- a/packages/nimble-components/src/table-column/enum-base/index.ts +++ b/packages/nimble-components/src/table-column/enum-base/index.ts @@ -17,13 +17,15 @@ import type { MappingConfig } from './models/mapping-config'; import type { MappingKey } from '../../mapping/base/types'; import { resolveKeyWithType } from './models/mapping-key-resolver'; import type { TableColumnEnumBaseValidator } from './models/table-column-enum-base-validator'; +import type { TableColumnTextBaseColumnConfig } from '../text-base/cell-view'; export type TableColumnEnumCellRecord = | TableStringField<'value'> | TableBooleanField<'value'> | TableNumberField<'value'>; export type MappingConfigs = Map<MappingKey, MappingConfig>; -export interface TableColumnEnumColumnConfig { +export interface TableColumnEnumColumnConfig + extends TableColumnTextBaseColumnConfig { mappingConfigs: MappingConfigs; } diff --git a/packages/nimble-components/src/table-column/enum-text/cell-view/index.ts b/packages/nimble-components/src/table-column/enum-text/cell-view/index.ts index e5eed59ced..5205d7cab3 100644 --- a/packages/nimble-components/src/table-column/enum-text/cell-view/index.ts +++ b/packages/nimble-components/src/table-column/enum-text/cell-view/index.ts @@ -21,15 +21,7 @@ export class TableColumnEnumTextCellView extends TableColumnTextCellViewBase< TableColumnEnumCellRecord, TableColumnEnumColumnConfig > { - private columnConfigChanged(): void { - this.updateText(); - } - - private cellRecordChanged(): void { - this.updateText(); - } - - private updateText(): void { + protected updateText(): void { const value = this.cellRecord?.value; if (value === undefined || value === null) { this.text = ''; diff --git a/packages/nimble-components/src/table-column/enum-text/tests/table-column-enum-text.mdx b/packages/nimble-components/src/table-column/enum-text/tests/table-column-enum-text.mdx index 6e23e98c83..54f1e0b005 100644 --- a/packages/nimble-components/src/table-column/enum-text/tests/table-column-enum-text.mdx +++ b/packages/nimble-components/src/table-column/enum-text/tests/table-column-enum-text.mdx @@ -15,7 +15,14 @@ The <Tag name={tableColumnEnumTextTag}/> column renders string, number, or boole <Canvas of={tableColumnEnumTextStories.enumTextColumn} /> <Controls of={tableColumnEnumTextStories.enumTextColumn} /> -## Blazor Usage +## Usage + +### Best Practices + +- Provide a mapping for every expected record value. Because grouping is performed on the record value, non-mapped record values can result in multiple groups without group labels. +- Avoid having multiple values that map to the same label because grouping and sorting is done on the record value rather than the mapped label. + +### Blazor Usage When setting a child mapping element's `Key` value to a string, you must wrap it in `@()` so that the compiler treats it as a string, e.g. `<NimbleMappingText Key=@("foo") ...>` diff --git a/packages/nimble-components/src/table-column/icon/tests/table-column-icon.mdx b/packages/nimble-components/src/table-column/icon/tests/table-column-icon.mdx index 0602423db5..036c980ba0 100644 --- a/packages/nimble-components/src/table-column/icon/tests/table-column-icon.mdx +++ b/packages/nimble-components/src/table-column/icon/tests/table-column-icon.mdx @@ -17,7 +17,15 @@ The <Tag name={tableColumnIconTag}/> column renders string, number, or boolean v <Canvas of={tableColumnIconStories.iconColumn} /> <Controls of={tableColumnIconStories.iconColumn} /> -## Blazor Usage +## Usage + +### Best Practices + +- Provide a mapping for every expected record value. Because grouping is performed on the record value, non-mapped record values can result in multiple groups without group labels. + - To improve grouping behavior of values that don't correspond to icons, explicitly map those values to an `undefined` icon. +- Avoid having multiple values that map to the same label because grouping and sorting is done on the record value rather than the mapped label. + +### Blazor Usage When setting a child mapping element's `Key` value to a string, you must wrap it in `@()` so that the compiler treats it as a string, e.g. `<NimbleMappingIcon Key=@("foo") ...>` diff --git a/packages/nimble-components/src/table-column/mixins/placeholder.ts b/packages/nimble-components/src/table-column/mixins/placeholder.ts new file mode 100644 index 0000000000..72556bdbdc --- /dev/null +++ b/packages/nimble-components/src/table-column/mixins/placeholder.ts @@ -0,0 +1,33 @@ +import { attr } from '@microsoft/fast-element'; +import type { TableColumn } from '../base'; + +// Pick just the relevant properties the mixin depends on (typescript complains if the mixin declares private / protected base exports) +// Because the 'placeholder' mixin doesn't depend on any properties of the TableColumn, there are no properties to pick. +type TableColumnWithPlaceholder = Pick<TableColumn, never>; +// prettier-ignore +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type TableColumnWithPlaceholderConstructor = abstract new (...args: any[]) => TableColumnWithPlaceholder; + +// As the returned class is internal to the function, we can't write a signature that uses it directly, so rely on inference +// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/explicit-function-return-type +export function mixinColumnWithPlaceholderAPI< + TBase extends TableColumnWithPlaceholderConstructor +>(base: TBase) { + /** + * The Mixin that provides a concrete column with the API to allow grouping + * by the values in that column. + */ + abstract class ColumnWithPlaceholder extends base { + public placeholder?: string; + + /** @internal */ + public abstract placeholderChanged(): void; + } + attr({ attribute: 'placeholder' })( + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + ColumnWithPlaceholder.prototype, + 'placeholder' + ); + + return ColumnWithPlaceholder; +} diff --git a/packages/nimble-components/src/table-column/number-text/cell-view/index.ts b/packages/nimble-components/src/table-column/number-text/cell-view/index.ts index 0866f88414..7d61e699d8 100644 --- a/packages/nimble-components/src/table-column/number-text/cell-view/index.ts +++ b/packages/nimble-components/src/table-column/number-text/cell-view/index.ts @@ -21,16 +21,12 @@ export class TableColumnNumberTextCellView extends TableColumnTextCellViewBase< TableColumnNumberTextCellRecord, TableColumnNumberTextColumnConfig > { - private columnConfigChanged(): void { - this.updateText(); + protected override columnConfigChanged(): void { + super.columnConfigChanged(); this.alignment = this.columnConfig?.alignment ?? TextCellViewBaseAlignment.left; } - private cellRecordChanged(): void { - this.updateText(); - } - - private updateText(): void { + protected updateText(): void { this.text = this.columnConfig?.formatter?.format(this.cellRecord?.value) ?? ''; } } diff --git a/packages/nimble-components/src/table-column/number-text/index.ts b/packages/nimble-components/src/table-column/number-text/index.ts index 2b0ea4a246..d6ee298639 100644 --- a/packages/nimble-components/src/table-column/number-text/index.ts +++ b/packages/nimble-components/src/table-column/number-text/index.ts @@ -26,9 +26,11 @@ import { TextCellViewBaseAlignment } from '../text-base/cell-view/types'; import { lang } from '../../theme-provider'; import { Unit } from '../../unit/base/unit'; import { waitUntilCustomElementsDefinedAsync } from '../../utilities/wait-until-custom-elements-defined-async'; +import type { TableColumnTextBaseColumnConfig } from '../text-base/cell-view'; export type TableColumnNumberTextCellRecord = TableNumberField<'value'>; -export interface TableColumnNumberTextColumnConfig { +export interface TableColumnNumberTextColumnConfig + extends TableColumnTextBaseColumnConfig { formatter: UnitFormat; alignment: TextCellViewBaseAlignment; } @@ -97,6 +99,10 @@ export class TableColumnNumberText extends TableColumnTextBase { return this.validator.getValidity(); } + public placeholderChanged(): void { + this.updateColumnConfig(); + } + protected override getColumnInternalsOptions(): ColumnInternalsOptions { return { cellRecordFieldNames: ['value'], @@ -171,7 +177,8 @@ export class TableColumnNumberText extends TableColumnTextBase { if (this.validator.isValid()) { const columnConfig: TableColumnNumberTextColumnConfig = { formatter: this.createFormatter(), - alignment: this.determineCellContentAlignment() + alignment: this.determineCellContentAlignment(), + placeholder: this.placeholder }; this.columnInternals.columnConfig = columnConfig; } else { diff --git a/packages/nimble-components/src/table-column/number-text/tests/table-column-number-text-matrix.stories.ts b/packages/nimble-components/src/table-column/number-text/tests/table-column-number-text-matrix.stories.ts index 7568e1abe9..42132e52da 100644 --- a/packages/nimble-components/src/table-column/number-text/tests/table-column-number-text-matrix.stories.ts +++ b/packages/nimble-components/src/table-column/number-text/tests/table-column-number-text-matrix.stories.ts @@ -13,6 +13,10 @@ import { controlLabelFontColor } from '../../../theme-provider/design-tokens'; import { NumberTextAlignment } from '../types'; +import { + placeholderStates, + type PlaceholderState +} from '../../../utilities/tests/states'; const metadata: Meta = { title: 'Tests/Table Column: Number Text', @@ -44,16 +48,18 @@ type AlignmentState = (typeof alignmentStates)[number]; // prettier-ignore const component = ( - [alignmentName, alignment]: AlignmentState + [alignmentName, alignment]: AlignmentState, + [placeholderName, placeholder]: PlaceholderState ): ViewTemplate => html` <label style="color: var(${controlLabelFontColor.cssCustomProperty}); font: var(${controlLabelFont.cssCustomProperty})"> - Number Text Table Column with ${alignmentName} alignment + Number Text Table Column With ${alignmentName} Alignment ${placeholderName} </label> <${tableTag} id-field-name="id" style="height: 450px"> <${tableColumnNumberTextTag} field-name="number" group-index="0" alignment="${() => alignment}" + placeholder="${() => placeholder}" > Default </${tableColumnNumberTextTag}> @@ -63,6 +69,7 @@ const component = ( field-name="number" group-index="2" alignment="${() => alignment}" + placeholder="${() => placeholder}" > Decimal (3 digits) </${tableColumnNumberTextTag}> @@ -70,7 +77,7 @@ const component = ( `; export const tableColumnNumberTextThemeMatrix: StoryFn = createMatrixThemeStory( - createMatrix(component, [alignmentStates]) + createMatrix(component, [alignmentStates, placeholderStates]) ); tableColumnNumberTextThemeMatrix.play = async (): Promise<void> => { diff --git a/packages/nimble-components/src/table-column/number-text/tests/table-column-number-text.mdx b/packages/nimble-components/src/table-column/number-text/tests/table-column-number-text.mdx index 7fcfebf185..32ce82d5bc 100644 --- a/packages/nimble-components/src/table-column/number-text/tests/table-column-number-text.mdx +++ b/packages/nimble-components/src/table-column/number-text/tests/table-column-number-text.mdx @@ -1,9 +1,13 @@ import { Canvas, Meta, Controls, Title } from '@storybook/blocks'; import { NimbleTableColumnNumberText } from './table-column-number-text.react'; -import { columnOperationBehavior } from '../../base/tests/table-column-stories-utils'; +import { + columnOperationBehavior, + noNullAndUndefinedBestPractice +} from '../../base/tests/table-column-stories-utils'; import * as tableColumnNumberTextStories from './table-column-number-text.stories'; import { tableColumnNumberTextTag } from '..'; import { tableTag } from '../../../table'; +import NoNullAndUndefinedBestPractice from '../../../patterns/table-column/tests/no-null-and-undefined-best-practice-docs.mdx'; <Meta of={tableColumnNumberTextStories} /> <Title of={tableColumnNumberTextStories} /> @@ -20,11 +24,20 @@ based on the value of the `lang` token, which can be set via the [nimble-theme-p {/* ## Appearance Variants */} -{/* ## Usage */} +## Usage -## Angular Usage +### Best Practices -### Number Format Pipe +- <NoNullAndUndefinedBestPractice /> +- If relevant to your data source, make sure to consider the IEEE 754 special cases of `-Inf`, `+Inf`, `-0`, `+0`, and `NaN`. Those values are handled as follows in the table: + - `-Inf` will be rendered as `-∞` in the table. It is considered the smallest possible number when sorting. + - `+Inf` will be rendered as `∞` in the table. It is considered the largest possible number when sorting. + - `-0`, `0`, and `+0` will be rendered as `0` in the table. No distinction is made between those values when grouping. + - `NaN` will be rendered as `NaN` in the table. It is sorted between non-defined values of `undefined`/`null` and `-Inf`. + +### Angular Usage + +#### Number Format Pipe An Angular pipe is provided for formatting number values into strings outside of the Nimble table. It uses the same formatting rules (and localization) as the number-text table column. diff --git a/packages/nimble-components/src/table-column/number-text/tests/table-column-number-text.spec.ts b/packages/nimble-components/src/table-column/number-text/tests/table-column-number-text.spec.ts index cc1163995f..84ca9b282a 100644 --- a/packages/nimble-components/src/table-column/number-text/tests/table-column-number-text.spec.ts +++ b/packages/nimble-components/src/table-column/number-text/tests/table-column-number-text.spec.ts @@ -78,25 +78,6 @@ describe('TableColumnNumberText', () => { expect(elementReferences.column2.checkValidity()).toBeTrue(); }); - const noValueData = [ - { name: 'field not present', data: [{ unused: 'foo' }] }, - { name: 'value is null', data: [{ number1: null }] }, - { name: 'value is undefined', data: [{ number1: undefined }] }, - { - name: 'value is not a number', - data: [{ number1: 'hello world' as unknown as number }] - } - ] as const; - parameterizeSpec(noValueData, (spec, name, value) => { - spec(`displays empty string when ${name}`, async () => { - await table.setData(value.data); - await connect(); - await waitForUpdatesAsync(); - - expect(pageObject.getRenderedCellTextContent(0, 0)).toBe(''); - }); - }); - it('defaults to "default" format', () => { expect(elementReferences.column1.format).toBe(NumberTextFormat.default); }); @@ -739,50 +720,145 @@ describe('TableColumnNumberText', () => { { name: 'value is not specified', data: [{}], - groupValue: 'No value' + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true }, { name: 'value is undefined', data: [{ number1: undefined }], - groupValue: 'No value' + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true }, { name: 'value is null', data: [{ number1: null }], - groupValue: 'No value' + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true }, { name: 'value is Number.NaN', data: [{ number1: Number.NaN }], - groupValue: 'NaN' + cellValue: 'NaN', + groupValue: 'NaN', + usesColumnPlaceholder: false }, { name: 'value is valid and non-zero', data: [{ number1: 100 }], - groupValue: '100' + cellValue: '100', + groupValue: '100', + usesColumnPlaceholder: false }, { name: 'value is incorrect type', data: [{ number1: 'not a number' as unknown as number }], - groupValue: '' + cellValue: '', + groupValue: '', + usesColumnPlaceholder: false }, { name: 'value is specified and falsey', data: [{ number1: 0 }], - groupValue: '0' + cellValue: '0', + groupValue: '0', + usesColumnPlaceholder: false } - ]; + ] as const; + + async function initializeColumnAndTable( + data: readonly SimpleTableRecord[], + placeholder?: string + ): Promise<void> { + elementReferences.column1.placeholder = placeholder; + await table.setData(data); + await connect(); + await waitForUpdatesAsync(); + } parameterizeSpec(testCases, (spec, name, value) => { - spec(`group row renders expected value when ${name}`, async () => { - await table.setData(value.data); - await connect(); - await waitForUpdatesAsync(); + spec( + `cell and group row render expected value when ${name} and placeholder is configured`, + async () => { + const placeholder = 'Custom placeholder'; + await initializeColumnAndTable(value.data, placeholder); + + const expectedCellText = value.usesColumnPlaceholder + ? placeholder + : value.cellValue; + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + expectedCellText + ); + expect( + pageObject.getRenderedGroupHeaderTextContent(0) + ).toBe(value.groupValue); + } + ); + }); - expect(pageObject.getRenderedGroupHeaderTextContent(0)).toBe( - value.groupValue - ); - }); + parameterizeSpec(testCases, (spec, name, value) => { + spec( + `cell and group row render expected value when ${name} and placeholder is not configured`, + async () => { + await initializeColumnAndTable(value.data); + + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + value.cellValue + ); + expect( + pageObject.getRenderedGroupHeaderTextContent(0) + ).toBe(value.groupValue); + } + ); + }); + + it('setting placeholder to undefined updates cells from displaying placeholder to displaying blank', async () => { + const placeholder = 'My placeholder'; + await initializeColumnAndTable([{}], placeholder); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + placeholder + ); + + elementReferences.column1.placeholder = undefined; + await waitForUpdatesAsync(); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe(''); + }); + + it('setting placeholder to defined string updates cells from displaying blank to displaying placeholder', async () => { + await initializeColumnAndTable([{}]); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe(''); + + const placeholder = 'placeholder'; + elementReferences.column1.placeholder = placeholder; + await waitForUpdatesAsync(); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + placeholder + ); + }); + + it('updating placeholder from one string to another updates cell', async () => { + const placeholder1 = 'My first placeholder'; + await initializeColumnAndTable([{}], placeholder1); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + placeholder1 + ); + + const placeholder2 = 'My second placeholder'; + elementReferences.column1.placeholder = placeholder2; + await waitForUpdatesAsync(); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + placeholder2 + ); + }); + + it('can configure empty placeholder', async () => { + const placeholder = ''; + await initializeColumnAndTable([{}], placeholder); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + placeholder + ); }); }); }); diff --git a/packages/nimble-components/src/table-column/number-text/tests/table-column-number-text.stories.ts b/packages/nimble-components/src/table-column/number-text/tests/table-column-number-text.stories.ts index 431a9393ce..01e49af525 100644 --- a/packages/nimble-components/src/table-column/number-text/tests/table-column-number-text.stories.ts +++ b/packages/nimble-components/src/table-column/number-text/tests/table-column-number-text.stories.ts @@ -49,7 +49,7 @@ const simpleData = [ lastName: 'Van Houten', age: 14.1, favoriteNumber: -0.00000064532623, - measurement: -0.00000064532623 + measurement: undefined } ] as const; @@ -79,6 +79,7 @@ export default metadata; interface NumberTextColumnTableArgs extends SharedTableArgs { fieldName: string; + placeholder: string; format: keyof typeof NumberTextFormat; alignment: keyof typeof NumberTextAlignment; decimalDigits: number; @@ -161,10 +162,10 @@ export const numberTextColumn: StoryObj<NumberTextColumnTableArgs> = { <${tableColumnNumberTextTag} field-name="age" format="${x => NumberTextFormat[x.format]}" alignment="${x => NumberTextAlignment[x.alignment]}" decimal-digits="${x => x.decimalDigits}" decimal-maximum-digits="${x => x.decimalMaximumDigits}"> Age </${tableColumnNumberTextTag}> - <${tableColumnNumberTextTag} field-name="favoriteNumber" format="${x => NumberTextFormat[x.format]}" alignment="${x => NumberTextAlignment[x.alignment]}" decimal-digits="${x => x.decimalDigits}" decimal-maximum-digits="${x => x.decimalMaximumDigits}"> + <${tableColumnNumberTextTag} field-name="favoriteNumber" format="${x => NumberTextFormat[x.format]}" alignment="${x => NumberTextAlignment[x.alignment]}" decimal-digits="${x => x.decimalDigits}" decimal-maximum-digits="${x => x.decimalMaximumDigits}" placeholder="${x => x.placeholder}"> Favorite Number </${tableColumnNumberTextTag}> - <${tableColumnNumberTextTag} field-name="measurement" format="${x => NumberTextFormat[x.format]}" alignment="${x => NumberTextAlignment[x.alignment]}" decimal-digits="${x => x.decimalDigits}" decimal-maximum-digits="${x => x.decimalMaximumDigits}"> + <${tableColumnNumberTextTag} field-name="measurement" format="${x => NumberTextFormat[x.format]}" alignment="${x => NumberTextAlignment[x.alignment]}" decimal-digits="${x => x.decimalDigits}" decimal-maximum-digits="${x => x.decimalMaximumDigits}" placeholder="${x => x.placeholder}"> Measurement ${when(x => x.unit === 'byte', html`<${unitByteTag}></${unitByteTag}>`)} ${when(x => x.unit === 'byte (1024)', html`<${unitByteTag} binary></${unitByteTag}>`)} @@ -179,6 +180,10 @@ export const numberTextColumn: StoryObj<NumberTextColumnTableArgs> = { 'Set this attribute to identify which field in the data record should be displayed in each column. The field values must be of type `number`.', control: { type: 'none' } }, + placeholder: { + description: + 'The placeholder text to display when the field value is `undefined` or `null` for a record.' + }, format: { description: formatDescription, options: Object.keys(NumberTextFormat), @@ -223,6 +228,7 @@ export const numberTextColumn: StoryObj<NumberTextColumnTableArgs> = { alignment: 'default', decimalDigits: 2, decimalMaximumDigits: undefined, - unit: 'volt' + unit: 'volt', + placeholder: 'Unknown voltage' } }; diff --git a/packages/nimble-components/src/table-column/text-base/cell-view/index.ts b/packages/nimble-components/src/table-column/text-base/cell-view/index.ts index 870f701b25..5ec79b5839 100644 --- a/packages/nimble-components/src/table-column/text-base/cell-view/index.ts +++ b/packages/nimble-components/src/table-column/text-base/cell-view/index.ts @@ -2,13 +2,24 @@ import { observable } from '@microsoft/fast-element'; import { TableCellView } from '../../base/cell-view'; import type { TableCellRecord } from '../../base/types'; import { TextCellViewBaseAlignment } from './types'; +import type { TableFieldValue } from '../../../table/types'; + +export interface TableColumnTextBaseCellRecord extends TableCellRecord { + value: TableFieldValue; +} + +export interface TableColumnTextBaseColumnConfig { + placeholder?: string; +} /** * The cell view base class for displaying fields of any type as text. */ export abstract class TableColumnTextCellViewBase< - TCellRecord extends TableCellRecord = TableCellRecord, - TColumnConfig = unknown + TCellRecord extends + TableColumnTextBaseCellRecord = TableColumnTextBaseCellRecord, + TColumnConfig extends + TableColumnTextBaseColumnConfig = TableColumnTextBaseColumnConfig > extends TableCellView<TCellRecord, TColumnConfig> { /** @internal */ @observable @@ -25,4 +36,44 @@ export abstract class TableColumnTextCellViewBase< */ @observable public alignment: TextCellViewBaseAlignment = TextCellViewBaseAlignment.left; + + /** + * Whether or not the text being displayed in the cell view is a placeholder. + */ + @observable + public isPlaceholder = false; + + protected abstract updateText(): void; + + protected columnConfigChanged(): void { + if (!this.applyPlaceholderTextIfNeeded()) { + this.updateText(); + } + } + + private cellRecordChanged(): void { + if (!this.applyPlaceholderTextIfNeeded()) { + this.updateText(); + } + } + + /** + * Sets `this.text` to the appropriate placeholder if `cellValue` warrants it. + * @returns `true` if `this.text` was set to a placeholder, `false` otherwise. + */ + private applyPlaceholderTextIfNeeded(): boolean { + const cellValue = this.cellRecord?.value; + const placeholder = this.columnConfig?.placeholder; + if ( + typeof placeholder === 'string' + && (cellValue === null || cellValue === undefined) + ) { + this.text = placeholder; + this.isPlaceholder = true; + } else { + this.isPlaceholder = false; + } + + return this.isPlaceholder; + } } diff --git a/packages/nimble-components/src/table-column/text-base/cell-view/styles.ts b/packages/nimble-components/src/table-column/text-base/cell-view/styles.ts index db6a7a94ca..af765c9141 100644 --- a/packages/nimble-components/src/table-column/text-base/cell-view/styles.ts +++ b/packages/nimble-components/src/table-column/text-base/cell-view/styles.ts @@ -1,5 +1,10 @@ import { css } from '@microsoft/fast-element'; -import { bodyFont, bodyFontColor } from '../../../theme-provider/design-tokens'; +import { + bodyFont, + bodyFontColor, + placeholderFont, + placeholderFontColor +} from '../../../theme-provider/design-tokens'; export const styles = css` :host(.right-align) { @@ -13,4 +18,9 @@ export const styles = css` overflow: hidden; text-overflow: ellipsis; } + + :host(.placeholder) span { + font: ${placeholderFont}; + color: ${placeholderFontColor}; + } `; diff --git a/packages/nimble-components/src/table-column/text-base/cell-view/template.ts b/packages/nimble-components/src/table-column/text-base/cell-view/template.ts index 3ace67962e..bb68d303e6 100644 --- a/packages/nimble-components/src/table-column/text-base/cell-view/template.ts +++ b/packages/nimble-components/src/table-column/text-base/cell-view/template.ts @@ -4,11 +4,13 @@ import type { TableColumnTextCellViewBase } from '.'; import { overflow } from '../../../utilities/directive/overflow'; import { TextCellViewBaseAlignment } from './types'; +// prettier-ignore export const template = html<TableColumnTextCellViewBase>` <template - class="${x => (x.alignment === TextCellViewBaseAlignment.right - ? 'right-align' - : '')}" + class=" + ${x => (x.alignment === TextCellViewBaseAlignment.right ? 'right-align' : '')} + ${x => (x.isPlaceholder ? 'placeholder' : '')} + " > <span ${overflow('hasOverflow')} diff --git a/packages/nimble-components/src/table-column/text-base/cell-view/tests/table-column-text-base-cell-view.spec.ts b/packages/nimble-components/src/table-column/text-base/cell-view/tests/table-column-text-base-cell-view.spec.ts index 8705096068..6a64d6e8ce 100644 --- a/packages/nimble-components/src/table-column/text-base/cell-view/tests/table-column-text-base-cell-view.spec.ts +++ b/packages/nimble-components/src/table-column/text-base/cell-view/tests/table-column-text-base-cell-view.spec.ts @@ -25,7 +25,9 @@ describe('TableColumnTextCellViewBase', () => { styles: textBaseCellViewStyles }) // eslint-disable-next-line @typescript-eslint/no-unused-vars - class TestTextBaseCellView extends TableColumnTextCellViewBase {} + class TestTextBaseCellView extends TableColumnTextCellViewBase { + protected override updateText(): void {} + } async function setup(): Promise<Fixture<TableColumnTextCellViewBase>> { return fixture(testTextBaseCellViewTag); diff --git a/packages/nimble-components/src/table-column/text-base/index.ts b/packages/nimble-components/src/table-column/text-base/index.ts index be3578fc64..a0a68faa51 100644 --- a/packages/nimble-components/src/table-column/text-base/index.ts +++ b/packages/nimble-components/src/table-column/text-base/index.ts @@ -2,12 +2,13 @@ import { attr } from '@microsoft/fast-element'; import { mixinFractionalWidthColumnAPI } from '../mixins/fractional-width-column'; import { TableColumn } from '../base'; import { mixinGroupableColumnAPI } from '../mixins/groupable-column'; +import { mixinColumnWithPlaceholderAPI } from '../mixins/placeholder'; /** * The base class for table columns that display fields of any type as text. */ export abstract class TableColumnTextBase extends mixinGroupableColumnAPI( - mixinFractionalWidthColumnAPI(TableColumn) + mixinFractionalWidthColumnAPI(mixinColumnWithPlaceholderAPI(TableColumn)) ) { @attr({ attribute: 'field-name' }) public fieldName?: string; diff --git a/packages/nimble-components/src/table-column/text/cell-view/index.ts b/packages/nimble-components/src/table-column/text/cell-view/index.ts index f0609eb377..278a9ca28a 100644 --- a/packages/nimble-components/src/table-column/text/cell-view/index.ts +++ b/packages/nimble-components/src/table-column/text/cell-view/index.ts @@ -20,7 +20,7 @@ export class TableColumnTextCellView extends TableColumnTextCellViewBase< TableColumnTextCellRecord, TableColumnTextColumnConfig > { - private cellRecordChanged(): void { + protected updateText(): void { this.text = typeof this.cellRecord?.value === 'string' ? this.cellRecord.value : ''; diff --git a/packages/nimble-components/src/table-column/text/index.ts b/packages/nimble-components/src/table-column/text/index.ts index e6f0c60c09..2b21cdee6e 100644 --- a/packages/nimble-components/src/table-column/text/index.ts +++ b/packages/nimble-components/src/table-column/text/index.ts @@ -7,10 +7,13 @@ import { TableColumnSortOperation } from '../base/types'; import { tableColumnTextGroupHeaderViewTag } from './group-header-view'; import { tableColumnTextCellViewTag } from './cell-view'; import type { ColumnInternalsOptions } from '../base/models/column-internals'; +import type { TableColumnTextBaseColumnConfig } from '../text-base/cell-view'; export type TableColumnTextCellRecord = TableStringField<'value'>; + // eslint-disable-next-line @typescript-eslint/no-empty-interface -export interface TableColumnTextColumnConfig {} +export interface TableColumnTextColumnConfig + extends TableColumnTextBaseColumnConfig {} declare global { interface HTMLElementTagNameMap { @@ -22,6 +25,12 @@ declare global { * The table column for displaying string fields as text. */ export class TableColumnText extends TableColumnTextBase { + public placeholderChanged(): void { + this.columnInternals.columnConfig = { + placeholder: this.placeholder + }; + } + protected override getColumnInternalsOptions(): ColumnInternalsOptions { return { cellRecordFieldNames: ['value'], diff --git a/packages/nimble-components/src/table-column/text/tests/table-column-text-matrix.stories.ts b/packages/nimble-components/src/table-column/text/tests/table-column-text-matrix.stories.ts index 30e96422bf..ea23c0f494 100644 --- a/packages/nimble-components/src/table-column/text/tests/table-column-text-matrix.stories.ts +++ b/packages/nimble-components/src/table-column/text/tests/table-column-text-matrix.stories.ts @@ -7,6 +7,14 @@ import { } from '../../../utilities/tests/matrix'; import { Table, tableTag } from '../../../table'; import { tableColumnTextTag } from '..'; +import { + controlLabelFont, + controlLabelFontColor +} from '../../../theme-provider/design-tokens'; +import { + placeholderStates, + type PlaceholderState +} from '../../../utilities/tests/states'; const metadata: Meta = { title: 'Tests/Table Column: Text', @@ -37,16 +45,23 @@ const data = [ ] as const; // prettier-ignore -const component = (): ViewTemplate => html` +const component = ( + [placeholderName, placeholder]: PlaceholderState +): ViewTemplate => html` + <label style="color: var(${controlLabelFontColor.cssCustomProperty}); font: var(${controlLabelFont.cssCustomProperty})"> + Text Table Column ${placeholderName} + </label> <${tableTag} id-field-name="id" style="height: 320px"> <${tableColumnTextTag} field-name="id" + placeholder="${() => placeholder}" > ID </${tableColumnTextTag}> <${tableColumnTextTag} field-name="firstName" group-index="0" + placeholder="${() => placeholder}" > First name </${tableColumnTextTag}> @@ -54,7 +69,7 @@ const component = (): ViewTemplate => html` `; export const tableColumnTextThemeMatrix: StoryFn = createMatrixThemeStory( - createMatrix(component) + createMatrix(component, [placeholderStates]) ); tableColumnTextThemeMatrix.play = async (): Promise<void> => { diff --git a/packages/nimble-components/src/table-column/text/tests/table-column-text.mdx b/packages/nimble-components/src/table-column/text/tests/table-column-text.mdx index 5d15e30550..009308208d 100644 --- a/packages/nimble-components/src/table-column/text/tests/table-column-text.mdx +++ b/packages/nimble-components/src/table-column/text/tests/table-column-text.mdx @@ -3,6 +3,7 @@ import { NimbleTableColumnText } from './table-column-text.react'; import * as tableColumnTextStories from './table-column-text.stories'; import { tableColumnTextTag } from '..'; import { tableTag } from '../../../table'; +import NoNullAndUndefinedBestPractice from '../../../patterns/table-column/tests/no-null-and-undefined-best-practice-docs.mdx'; <Meta of={tableColumnTextStories} /> <Title of={tableColumnTextStories} /> @@ -16,7 +17,13 @@ The <Tag name={tableColumnTextTag}/> column is used to display string fields as {/* ## Appearance Variants */} -{/* ## Usage */} +## Usage + +### Best Practices + +- <NoNullAndUndefinedBestPractice /> +- Avoid mixing empty string with `undefined`/`null`. The distinction when grouping between `"No value"` and `"Empty"` is not likely meaningful to a user. +- Avoid displaying whitespace values that are not empty string (`''`) as these values will be rendered as-is in group rows. {/* ## Examples */} 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 0061dcc639..e6f3016fa9 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 @@ -72,25 +72,6 @@ describe('TableColumnText', () => { expect(column.checkValidity()).toBeTrue(); }); - const noValueData = [ - { name: 'field not present', data: [{ unused: 'foo' }] }, - { name: 'value is null', data: [{ field: null }] }, - { name: 'value is undefined', data: [{ field: undefined }] }, - { - name: 'value is not a string', - data: [{ field: 10 as unknown as string }] - } - ] as const; - parameterizeSpec(noValueData, (spec, name, value) => { - spec(`displays empty string when ${name}`, async () => { - await table.setData(value.data); - await connect(); - await waitForUpdatesAsync(); - - expect(pageObject.getRenderedCellTextContent(0, 0)).toBe(''); - }); - }); - it('changing fieldName updates display', async () => { await table.setData([{ field: 'foo', anotherField: 'bar' }]); await connect(); @@ -237,40 +218,138 @@ describe('TableColumnText', () => { { name: 'value is not specified', data: [{}], - groupValue: 'No value' + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true }, { name: 'value is undefined', data: [{ field: undefined }], - groupValue: 'No value' + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true }, { name: 'value is null', data: [{ field: null }], - groupValue: 'No value' + cellValue: '', + groupValue: 'No value', + usesColumnPlaceholder: true }, { name: 'value is incorrect type', data: [{ field: 100 as unknown as string }], - groupValue: '' + cellValue: '', + groupValue: '', + usesColumnPlaceholder: false }, { name: 'value is an empty string', data: [{ field: '' }], - groupValue: 'Empty' + cellValue: '', + groupValue: 'Empty', + usesColumnPlaceholder: false + }, + { + name: 'value is a string containing only whitespace', + data: [{ field: ' ' }], + cellValue: '', + groupValue: '', + usesColumnPlaceholder: false } - ]; + ] as const; + + async function initializeColumnAndTable( + data: readonly SimpleTableRecord[], + placeholder?: string + ): Promise<void> { + column.placeholder = placeholder; + await table.setData(data); + await connect(); + await waitForUpdatesAsync(); + } parameterizeSpec(testCases, (spec, name, value) => { - spec(`group row renders expected value when ${name}`, async () => { - await table.setData(value.data); - await connect(); - await waitForUpdatesAsync(); + spec( + `cell and group row render expected value when ${name} and placeholder is configured`, + async () => { + const placeholder = 'Custom placeholder'; + await initializeColumnAndTable(value.data, placeholder); + + const expectedCellText = value.usesColumnPlaceholder + ? placeholder + : value.cellValue; + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + expectedCellText + ); + expect( + pageObject.getRenderedGroupHeaderTextContent(0) + ).toBe(value.groupValue); + } + ); + }); - expect(pageObject.getRenderedGroupHeaderTextContent(0)).toBe( - value.groupValue - ); - }); + parameterizeSpec(testCases, (spec, name, value) => { + spec( + `cell and group row render expected value when ${name} and placeholder is not configured`, + async () => { + await initializeColumnAndTable(value.data); + + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + value.cellValue + ); + expect( + pageObject.getRenderedGroupHeaderTextContent(0) + ).toBe(value.groupValue); + } + ); + }); + + it('setting placeholder to undefined updates cells from displaying placeholder to displaying blank', async () => { + const placeholder = 'My placeholder'; + await initializeColumnAndTable([{}], placeholder); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + placeholder + ); + + column.placeholder = undefined; + await waitForUpdatesAsync(); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe(''); + }); + + it('setting placeholder to defined string updates cells from displaying blank to displaying placeholder', async () => { + await initializeColumnAndTable([{}]); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe(''); + + const placeholder = 'placeholder'; + column.placeholder = placeholder; + await waitForUpdatesAsync(); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + placeholder + ); + }); + + it('updating placeholder from one string to another updates cell', async () => { + const placeholder1 = 'My first placeholder'; + await initializeColumnAndTable([{}], placeholder1); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + placeholder1 + ); + + const placeholder2 = 'My second placeholder'; + column.placeholder = placeholder2; + await waitForUpdatesAsync(); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + placeholder2 + ); + }); + + it('can configure empty placeholder', async () => { + const placeholder = ''; + await initializeColumnAndTable([{}], placeholder); + expect(pageObject.getRenderedCellTextContent(0, 0)).toBe( + placeholder + ); }); }); }); diff --git a/packages/nimble-components/src/table-column/text/tests/table-column-text.stories.ts b/packages/nimble-components/src/table-column/text/tests/table-column-text.stories.ts index f5c34d201c..7e1d235a61 100644 --- a/packages/nimble-components/src/table-column/text/tests/table-column-text.stories.ts +++ b/packages/nimble-components/src/table-column/text/tests/table-column-text.stories.ts @@ -65,6 +65,7 @@ type TextColumnFieldNameOption = 'firstName' | 'lastName'; interface TextColumnTableArgs extends SharedTableArgs { fieldName: TextColumnFieldNameOption; + placeholder: string; } export const textColumn: StoryObj<TextColumnTableArgs> = { @@ -82,6 +83,7 @@ export const textColumn: StoryObj<TextColumnTableArgs> = { </${tableColumnTextTag}> <${tableColumnTextTag} field-name="quote" + placeholder="${x => x.placeholder}" > Quote </${tableColumnTextTag}> @@ -94,9 +96,14 @@ export const textColumn: StoryObj<TextColumnTableArgs> = { 'Set this attribute to identify which field in the data record should be displayed in each column. The field values must be of type `string`.', options: ['firstName', 'lastName'], control: { type: 'radio' } + }, + placeholder: { + description: + 'The placeholder text to display when the field value is `undefined` or `null` for a record.' } }, args: { - fieldName: 'firstName' + fieldName: 'firstName', + placeholder: 'Did not respond to request for comment' } }; diff --git a/packages/nimble-components/src/utilities/tests/states.ts b/packages/nimble-components/src/utilities/tests/states.ts index a5b88d786c..bbe9820e11 100644 --- a/packages/nimble-components/src/utilities/tests/states.ts +++ b/packages/nimble-components/src/utilities/tests/states.ts @@ -41,3 +41,9 @@ export type ReadOnlyState = (typeof readOnlyStates)[number]; export const iconVisibleStates = [false, true] as const; export type IconVisibleState = (typeof iconVisibleStates)[number]; + +export const placeholderStates = [ + ['With Placeholder', 'Custom placeholder'], + ['', undefined] +] as const; +export type PlaceholderState = (typeof placeholderStates)[number];