Skip to content

Commit

Permalink
Add locale support to number-text column (#1499)
Browse files Browse the repository at this point in the history
# Pull Request

## 🤨 Rationale

This is part of #1011 

This PR adds support for the number-text column using the `lang` token
on the theme provider to format numbers in a locale-aware way.

## 👩‍💻 Implementation

I followed the same pattern used in the date-text column to listen for
changes to the `lang` theme provider token and pass that into the
formatters for the number-text column.

## 🧪 Testing

- Manually tested in storybook
- New unit test for the column that verifies the formatter is updated
with the `lang` changes
- New unit tests for each formatter that verify the locale is used when
formatting
- Note: I updated the formatter tests to run all the test cases with an
additional locale. On one hand this potentially seemed like "too many"
locale tests, but I thought it was the best way to ensure that the
locale was used in all internal formatters (e.g. the `default` formatter
uses 3 formatters internally) and that we had coverage for a wide range
of possible values to be formatted.

## ✅ 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 Sep 12, 2023
1 parent d791161 commit b49755b
Show file tree
Hide file tree
Showing 10 changed files with 448 additions and 206 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Add locale support to number-text column",
"packageName": "@ni/nimble-components",
"email": "[email protected]",
"dependentChangeType": "patch"
}
23 changes: 20 additions & 3 deletions packages/nimble-components/src/table-column/number-text/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { DesignSystem } from '@microsoft/fast-foundation';
import {
DesignSystem,
DesignTokenSubscriber
} from '@microsoft/fast-foundation';
import { attr, nullableNumberConverter } from '@microsoft/fast-element';
import { styles } from '../base/styles';
import { template } from '../base/template';
Expand All @@ -15,6 +18,7 @@ import { DefaultFormatter } from './models/default-formatter';
import { DecimalFormatter } from './models/decimal-formatter';
import { TableColumnNumberTextValidator } from './models/table-column-number-text-validator';
import { TextCellViewBaseAlignment } from '../text-base/cell-view/types';
import { lang } from '../../theme-provider';

export type TableColumnNumberTextCellRecord = TableNumberField<'value'>;
export interface TableColumnNumberTextColumnConfig {
Expand Down Expand Up @@ -46,11 +50,23 @@ export class TableColumnNumberText extends TableColumnTextBase {
@attr({ attribute: 'decimal-digits', converter: nullableNumberConverter })
public decimalDigits?: number;

private readonly langSubscriber: DesignTokenSubscriber<typeof lang> = {
handleChange: () => {
this.updateColumnConfig();
}
};

public override connectedCallback(): void {
super.connectedCallback();
lang.subscribe(this.langSubscriber, this);
this.updateColumnConfig();
}

public override disconnectedCallback(): void {
super.disconnectedCallback();
lang.unsubscribe(this.langSubscriber, this);
}

public override get validity(): TableColumnValidity {
return this.validator.getValidity();
}
Expand Down Expand Up @@ -94,13 +110,14 @@ export class TableColumnNumberText extends TableColumnTextBase {
private createFormatter(): NumberFormatter {
switch (this.format) {
case NumberTextFormat.roundToInteger:
return new RoundToIntegerFormatter();
return new RoundToIntegerFormatter(lang.getValueFor(this));
case NumberTextFormat.decimal:
return new DecimalFormatter(
lang.getValueFor(this),
this.decimalDigits ?? defaultDecimalDigits
);
default:
return new DefaultFormatter();
return new DefaultFormatter(lang.getValueFor(this));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ export class DecimalFormatter extends NumberFormatter {
private readonly formatter: Intl.NumberFormat;
private readonly tenPowDecimalDigits: number;

public constructor(decimalsToDisplay: number) {
public constructor(locale: string, decimalsToDisplay: number) {
super();
this.formatter = new Intl.NumberFormat(undefined, {
this.formatter = new Intl.NumberFormat(locale, {
maximumFractionDigits: decimalsToDisplay,
minimumFractionDigits: decimalsToDisplay,
useGrouping: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,34 +17,32 @@ export class DefaultFormatter extends NumberFormatter {
private static readonly exponentialUpperBound = 999999.5;

// Formatter to use by default. It renders the number with a maximum of 6 signficant digits.
private static readonly defaultFormatter = new Intl.NumberFormat(
undefined,
{
maximumSignificantDigits: DefaultFormatter.maximumDigits,
useGrouping: true
}
);
private readonly defaultFormatter: Intl.NumberFormat;

// Formatter to use for numbers that have leading zeros. It limits the number of rendered
// digits using 'maximumFractionDigits', which will result in less than 6 significant digits
// in order to render no more than 6 total digits.
private static readonly leadingZeroFormatter = new Intl.NumberFormat(
undefined,
{
maximumFractionDigits: DefaultFormatter.maximumDigits - 1,
useGrouping: true
}
);
private readonly leadingZeroFormatter: Intl.NumberFormat;

// Formatter for numbers that should be displayed in exponential notation. This should be used
// for numbers with magintudes over 'exponentialUpperBound' or under 'exponentialLowerBound'.
private static readonly exponentialFormatter = new Intl.NumberFormat(
undefined,
{
private readonly exponentialFormatter: Intl.NumberFormat;

public constructor(locale: string) {
super();
this.defaultFormatter = new Intl.NumberFormat(locale, {
maximumSignificantDigits: DefaultFormatter.maximumDigits,
useGrouping: true
});
this.leadingZeroFormatter = new Intl.NumberFormat(locale, {
maximumFractionDigits: DefaultFormatter.maximumDigits - 1,
useGrouping: true
});
this.exponentialFormatter = new Intl.NumberFormat(locale, {
maximumSignificantDigits: DefaultFormatter.maximumDigits,
notation: 'scientific'
}
);
});
}

protected format(number: number): string {
// The NumberFormat option of `signDisplay: "negative"` is not supported in all browsers nimble supports.
Expand All @@ -56,22 +54,22 @@ export class DefaultFormatter extends NumberFormatter {

private getFormatterForNumber(number: number): Intl.NumberFormat {
if (number === 0) {
return DefaultFormatter.defaultFormatter;
return this.defaultFormatter;
}

const absoluteValue = Math.abs(number);
if (
absoluteValue >= DefaultFormatter.exponentialUpperBound
|| absoluteValue < DefaultFormatter.exponentialLowerBound
) {
return DefaultFormatter.exponentialFormatter;
return this.exponentialFormatter;
}
// Ideally, we could set 'roundingPriority: "lessPrecision"' with a formatter that has both 'maximumSignificantDigits' and
// 'maximumFractionDigits' configured instead of having two different formatters that we conditionally choose between. However,
// 'roundingPrioirty' is not supported yet in all browsers nimble supports.
if (absoluteValue < 1) {
return DefaultFormatter.leadingZeroFormatter;
return this.leadingZeroFormatter;
}
return DefaultFormatter.defaultFormatter;
return this.defaultFormatter;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,21 @@ import { NumberFormatter } from './number-formatter';
* The formatter for a number-text column whose format is configured to be 'roundToInteger'.
*/
export class RoundToIntegerFormatter extends NumberFormatter {
private static readonly formatter = new Intl.NumberFormat(undefined, {
maximumFractionDigits: 0,
useGrouping: true
});
private readonly formatter: Intl.NumberFormat;

public constructor(locale: string) {
super();
this.formatter = new Intl.NumberFormat(locale, {
maximumFractionDigits: 0,
useGrouping: true
});
}

protected format(number: number): string {
// The NumberFormat option of `signDisplay: "negative"` is not supported in all browsers nimble supports.
// Because that option cannot be used to avoid rendering "-0", coerce the values that will round to -0 to 0
// prior to formatting.
const valueToFormat = Math.round(number) === 0 ? 0 : number;
return RoundToIntegerFormatter.formatter.format(valueToFormat);
return this.formatter.format(valueToFormat);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,81 +2,134 @@ import { getSpecTypeByNamedList } from '../../../../utilities/tests/parameterize
import { DecimalFormatter } from '../decimal-formatter';

describe('DecimalFormatter', () => {
const locales = ['en', 'de'] as const;
const testCases: readonly {
name: string,
decimalDigits: number,
value: number,
expectedFormattedValue: string
expectedFormattedValue: {
en: string,
de: string
}
}[] = [
{
name: 'NEGATIVE_INFINITY renders as -∞',
decimalDigits: 1,
value: Number.NEGATIVE_INFINITY,
expectedFormattedValue: '-∞'
expectedFormattedValue: {
en: '-∞',
de: '-∞'
}
},
{
name: 'POSITIVE_INFINITY renders as ∞',
decimalDigits: 1,
value: Number.POSITIVE_INFINITY,
expectedFormattedValue: '∞'
expectedFormattedValue: {
en: '∞',
de: '∞'
}
},
{
name: 'NaN renders as NaN',
decimalDigits: 1,
value: Number.NaN,
expectedFormattedValue: 'NaN'
expectedFormattedValue: {
en: 'NaN',
de: 'NaN'
}
},
{
name: '-0 renders without negative sign',
decimalDigits: 2,
value: -0,
expectedFormattedValue: '0.00'
expectedFormattedValue: {
en: '0.00',
de: '0,00'
}
},
{
name: 'does not round to -0',
decimalDigits: 2,
value: -0.00001,
expectedFormattedValue: '0.00'
expectedFormattedValue: {
en: '0.00',
de: '0,00'
}
},
{
name: '+0 renders without positive sign',
decimalDigits: 2,
value: 0,
expectedFormattedValue: '0.00'
expectedFormattedValue: {
en: '0.00',
de: '0,00'
}
},
{
name: 'limits to "decimal-digits" decimals with rounding up',
decimalDigits: 7,
value: 1.23456789,
expectedFormattedValue: '1.2345679'
expectedFormattedValue: {
en: '1.2345679',
de: '1,2345679'
}
},
{
name: 'limits to "decimal-digits" decimals with rounding down',
decimalDigits: 5,
value: 10.001122,
expectedFormattedValue: '10.00112'
expectedFormattedValue: {
en: '10.00112',
de: '10,00112'
}
},
{
name: 'adds zeros to reach "decimal-digits" decimals',
decimalDigits: 3,
value: 16,
expectedFormattedValue: '16.000'
expectedFormattedValue: {
en: '16.000',
de: '16,000'
}
},
{
name: 'uses grouping',
decimalDigits: 3,
value: 123456.789,
expectedFormattedValue: {
en: '123,456.789',
de: '123.456,789'
}
}
] as const;

const focused: string[] = [];
const disabled: string[] = [];
for (const testCase of testCases) {
const specType = getSpecTypeByNamedList(testCase, focused, disabled);
// eslint-disable-next-line @typescript-eslint/no-loop-func
specType(
`${testCase.name}`,
for (const locale of locales) {
for (const testCase of testCases) {
const specType = getSpecTypeByNamedList(
testCase,
focused,
disabled
);
// eslint-disable-next-line @typescript-eslint/no-loop-func
() => {
const formatter = new DecimalFormatter(testCase.decimalDigits);
const formattedValue = formatter.formatValue(testCase.value);
expect(formattedValue).toEqual(testCase.expectedFormattedValue);
}
);
specType(
`${testCase.name} with '${locale}' locale`,
// eslint-disable-next-line @typescript-eslint/no-loop-func
() => {
const formatter = new DecimalFormatter(
locale,
testCase.decimalDigits
);
const formattedValue = formatter.formatValue(
testCase.value
);
expect(formattedValue).toEqual(
testCase.expectedFormattedValue[locale]
);
}
);
}
}
});
Loading

0 comments on commit b49755b

Please sign in to comment.