Skip to content

Commit

Permalink
Empty icon mapping (#1874)
Browse files Browse the repository at this point in the history
# Pull Request

## 🤨 Rationale

Grouping on an icon value isn't ideal if any of the cells don't have an
icon. Specifically, the group row doesn't have a value associated with
it and renders only a row count.

It would be nice if we could allow a client to leave a cell blank for
certain field values but still have the group rows have a value
associated with them. For example, have a column that shows either a
spinner or nothing based on whether a task is in progress. When
grouping, it would be nice to show a string, such as "Idle", associated
with the records that have nothing in progress without cluttering the
table with extra icons in cells.

This is the Future Work described in #1869

## 👩‍💻 Implementation

Updated the icon column to allow a `nimble-mapping-icon` to have an
`undefined` icon. In that case, the cell renders nothing but the group
row renders only the text associated with the mapping.

## 🧪 Testing

Manually tested in storybook.
Added new unit tests for having an `undefined` icon
Updated storybook/matrix tests

## ✅ 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 <[email protected]>
  • Loading branch information
mollykreis and rajsite authored Feb 29, 2024
1 parent 7a14257 commit d4ca92e
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 17 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Allow icon mappings to specify an undefined icon",
"packageName": "@ni/nimble-components",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const iconMapping: StoryObj = {
icon: {
control: { type: 'none' },
description:
'The tag name of the Nimble icon to render, e.g. `nimble-icon-check`.'
'The tag name of the Nimble icon to render, e.g. `nimble-icon-check`. Alternatively, set `icon` to `undefined` to render no icon for the mapping while still providing a label to be used when grouping.'
},
severity: {
description:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,38 @@ export interface IconView {
severity: IconSeverity;
text?: string;
}
const createIconTemplate = (icon: string): ViewTemplate<IconView> => html`
<${icon}
title="${x => x.text}"
role="img"
aria-label="${x => x.text}"
severity="${x => x.severity}"
class="no-shrink"
>
</${icon}>`;

// Create an empty template containing only a space because creating a ViewTemplate
// with an empty string throws an exception at runtime.
// prettier-ignore
const emptyTemplate = html<IconView>` `;

const createIconTemplate = (
icon: string | undefined
): ViewTemplate<IconView> => {
if (icon === undefined) {
return emptyTemplate;
}

return html`
<${icon}
title="${x => x.text}"
role="img"
aria-label="${x => x.text}"
severity="${x => x.severity}"
class="no-shrink"
>
</${icon}>
`;
};

/**
* Mapping configuration corresponding to a icon mapping
*/
export class MappingIconConfig extends MappingConfig {
public readonly iconTemplate: ViewTemplate<IconView>;
public constructor(
resolvedIcon: string,
resolvedIcon: string | undefined,
public readonly severity: IconSeverity,
text: string | undefined
) {
Expand Down
3 changes: 0 additions & 3 deletions packages/nimble-components/src/table-column/icon/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@ export class TableColumnIcon extends mixinGroupableColumnAPI(

protected createMappingConfig(mapping: Mapping<unknown>): MappingConfig {
if (mapping instanceof MappingIcon) {
if (!mapping.resolvedIcon) {
throw Error('Unresolved icon');
}
return new MappingIconConfig(
mapping.resolvedIcon,
mapping.severity,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ export class TableColumnIconValidator extends TableColumnEnumBaseValidator<
);
}

private static hasUnresolvedIcon(mappingIcon: MappingIcon): boolean {
return (
typeof mappingIcon.icon === 'string'
&& mappingIcon.resolvedIcon === undefined
);
}

public override validate(
mappings: Mapping<unknown>[],
keyType: MappingKeyType
Expand All @@ -52,7 +59,7 @@ export class TableColumnIconValidator extends TableColumnEnumBaseValidator<
private validateIconNames(mappings: Mapping<unknown>[]): void {
const invalid = mappings
.filter(TableColumnIconValidator.isIconMappingElement)
.some(mappingIcon => mappingIcon.resolvedIcon === undefined);
.some(TableColumnIconValidator.hasUnresolvedIcon);
this.setConditionValue('invalidIconName', invalid);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,23 @@ const data = [
{
id: '2',
code: 2
},
{
id: '3',
code: -1
}
] as const;

// prettier-ignore
const component = (): ViewTemplate => html`
<${tableTag} id-field-name="id" style="height: 250px; ${isChromatic() ? '--ni-private-spinner-animation-play-state:paused' : ''}">
<${tableTag} id-field-name="id" style="height: 320px; ${isChromatic() ? '--ni-private-spinner-animation-play-state:paused' : ''}">
<${tableColumnIconTag}
field-name="code"
key-type="number"
group-index="0"
>
Column 1
<${mappingIconTag} key="-1" text="Unknown value"></${mappingIconTag}>
<${mappingIconTag} key="0" text="Zero" icon="${iconCheckTag}"></${mappingIconTag}>
<${mappingSpinnerTag} key="1" text="One"></${mappingSpinnerTag}>
</${tableColumnIconTag}>
Expand All @@ -52,6 +57,7 @@ const component = (): ViewTemplate => html`
key-type="number"
>
Column 2
<${mappingIconTag} key="-1" text="Unknown value"></${mappingIconTag}>
<${mappingIconTag} key="0" text="Zero" icon="${iconCheckTag}" severity="success"></${mappingIconTag}>
<${mappingIconTag} key="1" text="One" icon="${iconCheckTag}" severity="warning"></${mappingIconTag}>
<${mappingIconTag} key="2" text="Two" icon="${iconCheckTag}" severity="error"></${mappingIconTag}>
Expand All @@ -61,6 +67,7 @@ const component = (): ViewTemplate => html`
key-type="number"
>
Column 3
<${mappingIconTag} key="-1" text="Unknown value"></${mappingIconTag}>
<${mappingIconTag} key="0" text="Zero" icon="${iconCheckTag}" severity="information"></${mappingIconTag}>
</${tableColumnIconTag}>
</${tableTag}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ interface SimpleTableRecord extends TableRecord {
interface BasicIconMapping {
key?: MappingKey;
text?: string;
icon: string;
icon?: string;
}

interface BasicSpinnerMapping {
Expand Down Expand Up @@ -154,6 +154,20 @@ describe('TableColumnIcon', () => {
expect(() => pageObject.getRenderedIconColumnCellIconTagName(0, 0)).toThrowError();
});

it('displays blank when no icon specified for mapping', async () => {
({ element, connect, disconnect, model } = await setup({
keyType: MappingKeyType.string,
iconMappings: [{ key: 'a', text: 'alpha', icon: undefined }],
spinnerMappings: []
}));
pageObject = new TablePageObject<SimpleTableRecord>(element);
await element.setData([{ field1: 'a' }]);
await connect();
await waitForUpdatesAsync();

expect(() => pageObject.getRenderedIconColumnCellIconTagName(0, 0)).toThrowError();
});

it('changing fieldName updates display', async () => {
({ element, connect, disconnect, model } = await setup({
keyType: MappingKeyType.string,
Expand Down Expand Up @@ -308,6 +322,23 @@ describe('TableColumnIcon', () => {
expect(pageObject.getRenderedGroupHeaderTextContent(0)).toBe('');
});

it('sets group header text label and no icon when icon is undefined', async () => {
({ element, connect, disconnect, model } = await setup({
keyType: MappingKeyType.string,
iconMappings: [{ key: 'b', text: 'bravo', icon: undefined }],
spinnerMappings: []
}));
pageObject = new TablePageObject<SimpleTableRecord>(element);
await element.setData([{ field1: 'b' }]);
await connect();
await waitForUpdatesAsync();
model.col1.groupIndex = 0;
await waitForUpdatesAsync();

expect(() => pageObject.getRenderedIconColumnGroupHeaderIconTagName(0)).toThrowError();
expect(pageObject.getRenderedGroupHeaderTextContent(0)).toBe('bravo');
});

it('clears cell when mappings removed', async () => {
({ element, connect, disconnect, model } = await setup({
keyType: MappingKeyType.string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ const simpleData = [
lastName: 'Simpson',
status: 'success',
isChild: true
},
{
firstName: 'Abbey',
lastName: '?',
status: 'unknown',
isChild: false
}
] as const;

Expand Down Expand Up @@ -77,6 +83,7 @@ export const iconColumn: StoryObj<IconColumnTableArgs> = {
<${mappingIconTag} key="fail" icon="${iconXmarkTag}" severity="error" text="Not a Simpson"></${mappingIconTag}>
<${mappingIconTag} key="success" icon="${iconCheckLargeTag}" severity="success" text="Is a Simpson"></${mappingIconTag}>
<${mappingSpinnerTag} key="calculating" text="Calculating"></${mappingSpinnerTag}>
<${mappingIconTag} key="unknown" text="Unknown"></${mappingIconTag}>
</${tableColumnIconTag}>
<${tableColumnIconTag} field-name="isChild" key-type="boolean">
Is Child
Expand Down

0 comments on commit d4ca92e

Please sign in to comment.