Skip to content

Commit

Permalink
Add configuration for icon mappings and spinner mappings to show/hide…
Browse files Browse the repository at this point in the history
… their text within a table's cells (#2012)

# Pull Request

## 🤨 Rationale

This updates the `nimble-mapping-icon` and `nimble-mapping-spinner`
elements to be aligned with the changes described in #1992.

## 👩‍💻 Implementation

- Added `text-hidden` to `nimble-mapping-icon` and
`nimble-mapping-spinner`
- I also added these to Blazor and Angular so that when the change is
published clients in any framework can uptake the new package and have
the opportunity to add `text-hidden` to mappings in order to keep their
existing behavior
- Reworked the `MappingIconConfig` and `MappingSpinnerConfig` to be more
aligned
- They both create the appropriate `ViewTemplate` to render a spinner,
icon, or blank
- They both add the appropriate `title` and `aria-*` attributes based on
the configuration
- Neither adds css classes that are styled in the group header styles or
cell view styles
- Update the templates for the icon column's cell view and group header
share code for rendering icons and spinners (enabled because of aligning
`MappingIconConfig` and `MappingSpinnerConfig`) and to render the text
based on the `text-hidden` attribute

## 🧪 Testing

- Updated storybook & matrix tests
- Manually tested
- Wrote/updated a number of tests for the icon column
- To enable this, created a page object for the icon column to
encapsulate the logic around finding the icon/spinner and the text

## ✅ 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.
  • Loading branch information
mollykreis authored Apr 17, 2024
1 parent ffd3aaf commit 6d50261
Show file tree
Hide file tree
Showing 33 changed files with 770 additions and 177 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { NimbleMappingDirective } from '@ni/nimble-angular/mapping/base';
import { type MappingIcon, mappingIconTag } from '@ni/nimble-components/dist/esm/mapping/icon';
import type { MappingKey } from '@ni/nimble-components/dist/esm/mapping/base/types';
import type { IconSeverity } from '@ni/nimble-components/dist/esm/icon-base/types';
import { BooleanValueOrAttribute, toBooleanProperty } from '@ni/nimble-angular/internal-utilities';

export type { MappingIcon };
export { mappingIconTag };
Expand Down Expand Up @@ -38,6 +39,14 @@ export class NimbleMappingIconDirective extends NimbleMappingDirective<MappingKe
this.renderer.setProperty(this.elementRef.nativeElement, 'severity', value);
}

public get textHidden(): boolean {
return this.elementRef.nativeElement.textHidden;
}

@Input('text-hidden') public set textHidden(value: BooleanValueOrAttribute) {
this.renderer.setProperty(this.elementRef.nativeElement, 'textHidden', toBooleanProperty(value));
}

public constructor(protected readonly renderer: Renderer2, protected readonly elementRef: ElementRef<MappingIcon>) {
super(renderer, elementRef);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ describe('NimbleMappingIcon', () => {
text="nope"
icon="nimble-icon-xmark"
severity="error"
text-hidden
>
</nimble-mapping-icon>
</nimble-table-column-icon>
Expand Down Expand Up @@ -75,6 +76,11 @@ describe('NimbleMappingIcon', () => {
expect(directive.severity).toBe('error');
expect(nativeElement.severity).toBe('error');
});

it('will use template string values for textHidden', () => {
expect(directive.textHidden).toBeTrue();
expect(nativeElement.textHidden).toBeTrue();
});
});

describe('with property bound values', () => {
Expand All @@ -88,6 +94,7 @@ describe('NimbleMappingIcon', () => {
[text]="text"
[icon]="icon"
[severity]="severity"
[textHidden]="textHidden"
>
</nimble-mapping-icon>
</nimble-table-column-icon>
Expand All @@ -101,6 +108,7 @@ describe('NimbleMappingIcon', () => {
public text = 'nope';
public icon = 'nimble-icon-xmark';
public severity: IconSeverity = IconSeverity.error;
public textHidden = true;
}

let fixture: ComponentFixture<TestHostComponent>;
Expand Down Expand Up @@ -161,6 +169,17 @@ describe('NimbleMappingIcon', () => {
expect(directive.severity).toBe('success');
expect(nativeElement.severity).toBe('success');
});

it('can be configured with property binding for textHidden', () => {
expect(directive.textHidden).toBeTrue();
expect(nativeElement.textHidden).toBeTrue();

fixture.componentInstance.textHidden = false;
fixture.detectChanges();

expect(directive.textHidden).toBeFalse();
expect(nativeElement.textHidden).toBeFalse();
});
});

describe('with attribute bound values', () => {
Expand All @@ -174,6 +193,7 @@ describe('NimbleMappingIcon', () => {
[attr.text]="text"
[attr.icon]="icon"
[attr.severity]="severity"
[attr.text-hidden]="textHidden"
>
</nimble-mapping-icon>
</nimble-table-column-icon>
Expand All @@ -187,6 +207,7 @@ describe('NimbleMappingIcon', () => {
public text = 'nope';
public icon = 'nimble-icon-xmark';
public severity: IconSeverity = IconSeverity.error;
public textHidden = true;
}

let fixture: ComponentFixture<TestHostComponent>;
Expand Down Expand Up @@ -226,7 +247,7 @@ describe('NimbleMappingIcon', () => {
expect(nativeElement.text).toBe('yep');
});

it('can be configured with property binding for icon', () => {
it('can be configured with attribute binding for icon', () => {
expect(directive.icon).toBe('nimble-icon-xmark');
expect(nativeElement.icon).toBe('nimble-icon-xmark');

Expand All @@ -237,7 +258,7 @@ describe('NimbleMappingIcon', () => {
expect(nativeElement.icon).toBe('nimble-icon-check');
});

it('can be configured with property binding for severity', () => {
it('can be configured with attribute binding for severity', () => {
expect(directive.severity).toBe('error');
expect(nativeElement.severity).toBe('error');

Expand All @@ -247,5 +268,16 @@ describe('NimbleMappingIcon', () => {
expect(directive.severity).toBe('success');
expect(nativeElement.severity).toBe('success');
});

it('can be configured with attribute binding for textHidden', () => {
expect(directive.textHidden).toBeTrue();
expect(nativeElement.textHidden).toBeTrue();

fixture.componentInstance.textHidden = false;
fixture.detectChanges();

expect(directive.textHidden).toBeFalse();
expect(nativeElement.textHidden).toBeFalse();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Directive, ElementRef, Input, Renderer2 } from '@angular/core';
import { NimbleMappingDirective } from '@ni/nimble-angular/mapping/base';
import { type MappingSpinner, mappingSpinnerTag } from '@ni/nimble-components/dist/esm/mapping/spinner';
import type { MappingKey } from '@ni/nimble-components/dist/esm/mapping/base/types';
import { BooleanValueOrAttribute, toBooleanProperty } from '@ni/nimble-angular/internal-utilities';

export type { MappingSpinner };
export { mappingSpinnerTag };
Expand All @@ -21,6 +22,14 @@ export class NimbleMappingSpinnerDirective extends NimbleMappingDirective<Mappin
this.renderer.setProperty(this.elementRef.nativeElement, 'text', value);
}

public get textHidden(): boolean {
return this.elementRef.nativeElement.textHidden;
}

@Input('text-hidden') public set textHidden(value: BooleanValueOrAttribute) {
this.renderer.setProperty(this.elementRef.nativeElement, 'textHidden', toBooleanProperty(value));
}

public constructor(protected readonly renderer: Renderer2, protected readonly elementRef: ElementRef<MappingSpinner>) {
super(renderer, elementRef);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe('NimbleMappingSpinner', () => {
#mapping
key="false"
text="nope"
text-hidden
>
</nimble-mapping-spinner>
</nimble-table-column-icon>
Expand Down Expand Up @@ -62,6 +63,11 @@ describe('NimbleMappingSpinner', () => {
expect(directive.text).toBe('nope');
expect(nativeElement.text).toBe('nope');
});

it('will use template string values for textHidden', () => {
expect(directive.textHidden).toBeTrue();
expect(nativeElement.textHidden).toBeTrue();
});
});

describe('with property bound values', () => {
Expand All @@ -73,6 +79,7 @@ describe('NimbleMappingSpinner', () => {
#mapping
[key]="key"
[text]="text"
[textHidden]="textHidden"
>
</nimble-mapping-spinner>
</nimble-table-column-icon>
Expand All @@ -84,6 +91,7 @@ describe('NimbleMappingSpinner', () => {
@ViewChild('mapping', { read: ElementRef }) public elementRef: ElementRef<MappingSpinner>;
public key = false;
public text = 'nope';
public textHidden = true;
}

let fixture: ComponentFixture<TestHostComponent>;
Expand Down Expand Up @@ -122,6 +130,17 @@ describe('NimbleMappingSpinner', () => {
expect(directive.text).toBe('yep');
expect(nativeElement.text).toBe('yep');
});

it('can be configured with property binding for textHidden', () => {
expect(directive.textHidden).toBeTrue();
expect(nativeElement.textHidden).toBeTrue();

fixture.componentInstance.textHidden = false;
fixture.detectChanges();

expect(directive.textHidden).toBeFalse();
expect(nativeElement.textHidden).toBeFalse();
});
});

describe('with attribute bound values', () => {
Expand All @@ -133,6 +152,7 @@ describe('NimbleMappingSpinner', () => {
#mapping
[attr.key]="key"
[attr.text]="text"
[attr.text-hidden]="textHidden"
>
</nimble-mapping-spinner>
</nimble-table-column-icon>
Expand All @@ -144,6 +164,7 @@ describe('NimbleMappingSpinner', () => {
@ViewChild('mapping', { read: ElementRef }) public elementRef: ElementRef<MappingSpinner>;
public key = false;
public text = 'nope';
public textHidden = true;
}

let fixture: ComponentFixture<TestHostComponent>;
Expand Down Expand Up @@ -182,5 +203,16 @@ describe('NimbleMappingSpinner', () => {
expect(directive.text).toBe('yep');
expect(nativeElement.text).toBe('yep');
});

it('can be configured with attribute binding for textHidden', () => {
expect(directive.textHidden).toBeTrue();
expect(nativeElement.textHidden).toBeTrue();

fixture.componentInstance.textHidden = false;
fixture.detectChanges();

expect(directive.textHidden).toBeFalse();
expect(nativeElement.textHidden).toBeFalse();
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "major",
"comment": "Add configuration for icon mappings and spinner mappings to show/hide their text within a table's cells. **Breaking change:** The icon mappings and spinner mappings now default to showing their text within a table's cells in addition to on group rows.",
"packageName": "@ni/nimble-angular",
"email": "20542556+mollykreis@users.noreply.github.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "major",
"comment": "Add configuration for icon mappings and spinner mappings to show/hide their text within a table's cells. **Breaking change:** The icon mappings and spinner mappings now default to showing their text within a table's cells in addition to on group rows.",
"packageName": "@ni/nimble-blazor",
"email": "20542556+mollykreis@users.noreply.github.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "major",
"comment": "Add configuration for icon mappings and spinner mappings to show/hide their text within a table's cells. **Breaking change:** The icon mappings and spinner mappings now default to showing their text within a table's cells in addition to on group rows.",
"packageName": "@ni/nimble-components",
"email": "20542556+mollykreis@users.noreply.github.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@
text="@Text"
icon="@Icon"
severity="@Severity.ToAttributeValue()"
text-hidden="@TextHidden"
@attributes="AdditionalAttributes">
</nimble-mapping-icon>
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,10 @@ public partial class NimbleMappingIcon<TKey> : NimbleMappingBase<TKey>
/// </summary>
[Parameter]
public string? Text { get; set; }

/// <summary>
/// Gets or sets whether or not the mapping's text should be rendered in the table's cells.
/// </summary>
[Parameter]
public bool? TextHidden { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@
<nimble-mapping-spinner
key="@FormattedKey"
text="@Text"
text-hidden="@TextHidden"
@attributes="AdditionalAttributes">
</nimble-mapping-spinner>
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,10 @@ public partial class NimbleMappingSpinner<TKey> : NimbleMappingBase<TKey>
/// </summary>
[Parameter]
public string? Text { get; set; }

/// <summary>
/// Gets or sets whether or not the mapping's text should be rendered in the table's cells.
/// </summary>
[Parameter]
public bool? TextHidden { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ public void NimbleMappingIconSeverityAttribute_HasCorrectMarkup()
Assert.Contains($"severity=\"success\"", element.Markup);
}

[Fact]
public void NimbleMappingIconTextHiddenAttribute_HasCorrectMarkup()
{
var element = RenderWithPropertySet<int, bool?>(x => x.TextHidden, true);

Assert.Contains($"text-hidden", element.Markup);
}

private IRenderedComponent<NimbleMappingIcon<TKey>> RenderWithPropertySet<TKey, TProperty>(Expression<Func<NimbleMappingIcon<TKey>, TProperty>> propertyGetter, TProperty propertyValue)
{
var context = new TestContext();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ public void NimbleMappingSpinnerTextAttribute_HasCorrectMarkup()
Assert.Contains($"text=\"foo\"", element.Markup);
}

[Fact]
public void NimbleMappingSpinnerTextHiddenAttribute_HasCorrectMarkup()
{
var element = RenderWithPropertySet<int, bool?>(x => x.TextHidden, true);

Assert.Contains($"text-hidden", element.Markup);
}

private IRenderedComponent<NimbleMappingSpinner<TKey>> RenderWithPropertySet<TKey, TProperty>(Expression<Func<NimbleMappingSpinner<TKey>, TProperty>> propertyGetter, TProperty propertyValue)
{
var context = new TestContext();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export const icons: StoryObj<IconArgs> = {
icon="${x => x.tag}"
text="${x => x.tag}"
severity="${(_, c) => c.parent.severity}"
text-hidden
></${mappingIconTag}>
`)}
</${tableColumnIconTag}>
Expand Down
3 changes: 3 additions & 0 deletions packages/nimble-components/src/mapping/icon/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ export class MappingIcon extends Mapping<MappingKey> {
@attr()
public text?: string;

@attr({ attribute: 'text-hidden', mode: 'boolean' })
public textHidden = false;

/**
* @internal
* Calculated asynchronously by the icon mapping based on the configured icon value.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,26 @@ export const iconMapping: StoryObj = {
},
icon: {
control: { type: 'none' },
description:
'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.'
description: `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. Space will always be reserved for the icon so
that the text in cells and group rows associated with icon mappings will always be aligned.`
},
severity: {
control: { type: 'none' },
description:
'Must be one of the values in the `IconSeverity` enum. Controls the color of the icon.'
},
text: {
control: { type: 'none' },
description: `A textual description of the value. The text will be displayed next to the icon in a cell if \`text-hidden\` is not set,
or as the tooltip and accessible name of the icon if \`text-hidden\` is set. The text is also displayed next to the icon
in a group header. This attribute is required.`
},
textHidden: {
control: { type: 'none' },
name: 'text-hidden',
description:
'A textual description of the value which will be used as the tooltip and accessible name of the icon. The text is also displayed next to the icon in a group header. This attribute is required.'
"When set, the text is hidden within the table's cells. When unset, the text is rendered next to the icon within the cell."
}
},
args: {}
Expand Down
3 changes: 3 additions & 0 deletions packages/nimble-components/src/mapping/spinner/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ declare global {
export class MappingSpinner extends Mapping<MappingKey> {
@attr()
public text?: string;

@attr({ attribute: 'text-hidden', mode: 'boolean' })
public textHidden = false;
}

const spinnerMapping = MappingSpinner.compose({
Expand Down
Loading

0 comments on commit 6d50261

Please sign in to comment.