Skip to content

Commit

Permalink
Add cell placeholders (#1950)
Browse files Browse the repository at this point in the history
# Pull Request

## 🤨 Rationale

This PR adds a `placeholder` configuration to the following table
columns as described in #1869:
- anchor
- date-text
- duration-text
- number-text
- text

This is part of #1538. The remaining work is to add the `placeholder`
attributes to the Angular and Blazor wrappers for each column.

As part of this PR, I also included best practices established for each
column in the storybook docs. These are adapted from the placeholder
HLD.

## 👩‍💻 Implementation

I implemented this change in a similar way as #1914. Specifically, I
updated `TableColumnTextCellViewBase` to have its own implementation of
`columnConfigChanged` and `cellRecordChanged` that calls a new
`applyPlaceholderTextIfNeeded` function. This required me to create a
`TableColumnTextBaseCellRecord` interface to enforce that the cell
records used by `TableColumnTextCellViewBase` have a `value` property
and to create a `TableColumnTextBaseColumnConfig` interface to enforce
that the column configs used by `TableColumnTextCellViewBase` have an
optional `placeholder` property.

## 🧪 Testing

- Updated/created unit tests
- Manually tested in storybook
- Updated matrix tests to test each relevant column with & without a
placeholder

## ✅ Checklist

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

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

---------

Co-authored-by: Milan Raj <rajsite@users.noreply.github.com>
  • Loading branch information
mollykreis and rajsite authored Mar 27, 2024
1 parent 6310ab5 commit a01932b
Show file tree
Hide file tree
Showing 45 changed files with 1,074 additions and 326 deletions.
Original file line number Diff line number Diff line change
@@ -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"
}
Original file line number Diff line number Diff line change
@@ -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"`.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -22,4 +27,9 @@ export const styles = css`
overflow: hidden;
text-overflow: ellipsis;
}
:host(.placeholder) span {
font: ${placeholderFont};
color: ${placeholderFontColor};
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const template = html<TableColumnAnchorCellView>`
}
return true;
}}"
class="${x => (x.isPlaceholder ? 'placeholder' : '')}"
>
${when(x => typeof x.cellRecord?.href === 'string', html<TableColumnAnchorCellView>`
<${anchorTag}
Expand Down
15 changes: 13 additions & 2 deletions packages/nimble-components/src/table-column/anchor/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -23,6 +24,7 @@ export interface TableColumnAnchorColumnConfig {
target?: string;
type?: string;
download?: string;
placeholder?: string;
}

declare global {
Expand All @@ -35,7 +37,11 @@ declare global {
* A table column for displaying links.
*/
export class TableColumnAnchor extends mixinGroupableColumnAPI(
mixinFractionalWidthColumnAPI(TableColumn<TableColumnAnchorColumnConfig>)
mixinFractionalWidthColumnAPI(
mixinColumnWithPlaceholderAPI(
TableColumn<TableColumnAnchorColumnConfig>
)
)
) {
@attr({ attribute: 'label-field-name' })
public labelFieldName?: string;
Expand Down Expand Up @@ -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'],
Expand Down Expand Up @@ -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
};
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -50,33 +54,41 @@ 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`
<label style="color: var(${controlLabelFontColor.cssCustomProperty}); font: var(${controlLabelFont.cssCustomProperty})">${appearanceName} ${underlineHiddenName} Anchor Table Column</label>
<label style="color: var(${controlLabelFontColor.cssCustomProperty}); font: var(${controlLabelFont.cssCustomProperty})">
${appearanceName} ${underlineHiddenName} Anchor Table Column ${placeholderName}
</label>
<${tableTag} id-field-name="id" style="height: 320px">
<${tableColumnAnchorTag}
label-field-name="firstName"
href-field-name="link"
group-index="0"
appearance="${() => appearance}"
?underline-hidden="${() => underlineHidden}"
placeholder="${() => placeholder}"
>
<${iconUserTag}></${iconUserTag}>
</${tableColumnAnchorTag}>
</${tableTag}>
`;

export const tableColumnAnchorThemeMatrix: StoryFn = createMatrixThemeStory(
createMatrix(component, [appearanceStates, underlineHiddenStates])
createMatrix(component, [
appearanceStates,
underlineHiddenStates,
placeholderStates
])
);

tableColumnAnchorThemeMatrix.play = async (): Promise<void> => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,21 @@ The <Tag name={tableColumnAnchorTag}/> column is used to display string fields a
<Canvas of={tableColumnAnchorStories.anchorColumn} />
<Controls of={tableColumnAnchorStories.anchorColumn} />

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

<TargetDocs />

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

Expand Down
Loading

0 comments on commit a01932b

Please sign in to comment.