Update usage of Intl.NumberFormat options (#1791)

# Pull Request ## 🤨 Rationale Fixes: #1442 ## 👩‍💻 Implementation The options cited in the issue are: - `useGrouping: 'auto'` - We previously were setting this to `true`, which means "display grouping separators even if the locale prefers otherwise." Instead we want `"auto"`, which is "display grouping separators based on the locale preference." The default for this option is `"auto"` unless `notation` is `"compact"`. We never set `notation` to `"compact"`, so we can just leave `useGrouping` to use the default. - `signDisplay: 'negative'` - Though this option value has been [supported in all browsers since 8/23](https://caniuse.com/mdn-javascript_builtins_intl_numberformat_numberformat_options_signdisplay_parameter_negative), there does not seem to be a version of Typescript whose definition of `Intl.NumberFormatOptions` allows it. There is an [open issue for adding support](microsoft/TypeScript#56269). As a workaround, I've just added type assertions where necessary. - `roundingPriority: 'lessPrecision'` - Just like `signDisplay: 'negative'`, the `roundingPriority` option has been [supported in browsers since 8/23](https://caniuse.com/mdn-javascript_builtins_intl_numberformat_numberformat_options_roundingpriority_parameter), but Typescript doesn't have support yet. It is tracked by the same issue linked above, and our type assertion serves as a workaround for this, too. By using this option, we can get rid of the separate `leadingZeroFormatter` in `DefaultUnitFormat` and simplify the logic. ## 🧪 Testing Existing tests pass. ## ✅ Checklist - [x] I have updated the project documentation to reflect my changes or determined no changes are needed.
ni · Feb 26, 2024 · d74f49e · d74f49e
1 parent f2d6322
commit d74f49e
Show file tree

Hide file tree

Showing 11 changed files with 123 additions and 179 deletions.
diff --git a/change/@ni-nimble-components-9d33a263-abcb-4b8e-a5f1-1ab02c9b4974.json b/change/@ni-nimble-components-9d33a263-abcb-4b8e-a5f1-1ab02c9b4974.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Update usage of Intl.NumberFormat options",
+  "packageName": "@ni/nimble-components",
+  "email": "7282195+m-akinc@users.noreply.github.com",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/nimble-components/src/dialog/index.ts b/packages/nimble-components/src/dialog/index.ts
@@ -17,16 +17,6 @@ declare global {
     }
 }
 
-/**
- * This is a workaround for an incomplete definition of the native dialog element. Remove when using Typescript >=4.8.3.
- * https://github.com/microsoft/TypeScript/issues/48267
- * @internal
- */
-export interface ExtendedDialog extends HTMLDialogElement {
-    showModal(): void;
-    close(): void;
-}
-
 /**
  * A nimble-styled dialog.
  */
@@ -65,7 +55,7 @@ export class Dialog<CloseReason = void> extends FoundationElement {
      *
      * @internal
      */
-    public readonly dialogElement!: ExtendedDialog;
+    public readonly dialogElement!: HTMLDialogElement;
 
     /** @internal */
     @observable

diff --git a/packages/nimble-components/src/dialog/tests/dialog.spec.ts b/packages/nimble-components/src/dialog/tests/dialog.spec.ts
@@ -1,6 +1,6 @@
 import { html } from '@microsoft/fast-element';
 import { fixture, Fixture } from '../../utilities/tests/fixture';
-import { Dialog, dialogTag, ExtendedDialog, UserDismissed } from '..';
+import { Dialog, dialogTag, UserDismissed } from '..';
 import { waitForUpdatesAsync } from '../../testing/async-helpers';
 
 // eslint-disable-next-line @typescript-eslint/no-invalid-void-type
@@ -19,10 +19,10 @@ async function setup<CloseReason = void>(
 }
 
 describe('Dialog', () => {
-    function nativeDialogElement(nimbleDialogElement: Dialog): ExtendedDialog {
-        return nimbleDialogElement.shadowRoot!.querySelector(
-            'dialog'
-        ) as ExtendedDialog;
+    function nativeDialogElement(
+        nimbleDialogElement: Dialog
+    ): HTMLDialogElement {
+        return nimbleDialogElement.shadowRoot!.querySelector('dialog')!;
     }
 
     it('should export its tag', () => {

diff --git a/packages/nimble-components/src/drawer/index.ts b/packages/nimble-components/src/drawer/index.ts
@@ -6,7 +6,6 @@ import {
     FoundationElement
 } from '@microsoft/fast-foundation';
 import { eventAnimationEnd } from '@microsoft/fast-web-utilities';
-import type { ExtendedDialog } from '../dialog';
 import { UserDismissed } from '../patterns/dialog/types';
 import { styles } from './styles';
 import { template } from './template';
@@ -36,7 +35,7 @@ export class Drawer<CloseReason = void> extends FoundationElement {
     @attr({ attribute: 'prevent-dismiss', mode: 'boolean' })
     public preventDismiss = false;
 
-    public dialog!: ExtendedDialog;
+    public dialog!: HTMLDialogElement;
     private closing = false;
 
     private resolveShow?: (reason: CloseReason | UserDismissed) => void;

diff --git a/packages/nimble-components/src/global-type-extensions.d.ts b/packages/nimble-components/src/global-type-extensions.d.ts
@@ -0,0 +1,28 @@
+// This file has modifications to global types used for nimble-components builds:
+// - The types file should not be in the build output
+//   - You should not see a file with these types created in the dist build
+// - Type additions added to this file should not appear in any public API definitions
+//   - The fields added to global types should not be observable in any built .d.ts file by TypeScript. To verify you can do a text search of generated .d.ts files in the dist folder for the fields being added and verify they are not associated with the augmented types.
+// -  This file should only impact the nimble-components library build and not users of nimble-components. Any type changes that impact clients should not be placed here.
+
+/**
+ * This is a workaround for an incomplete definition of the native dialog element. Remove when using Typescript >=4.8.3.
+ * https://github.com/microsoft/TypeScript/issues/48267
+ * @internal
+ */
+interface HTMLDialogElement {
+    showModal(): void;
+    close(returnValue?: string): void;
+}
+
+declare namespace Intl {
+    // roundingPriority has been supported by browsers since 8/23, but TypeScript still hasn't
+    // added it to the type definition. See https://github.com/microsoft/TypeScript/issues/56269
+    interface NumberFormatOptions {
+        roundingPriority?:
+        | 'auto'
+        | 'morePrecision'
+        | 'lessPrecision'
+        | undefined;
+    }
+}
diff --git a/...-components/src/table-column/number-text/tests/table-column-number-text-matrix.stories.ts b/...-components/src/table-column/number-text/tests/table-column-number-text-matrix.stories.ts
@@ -30,7 +30,7 @@ const data = [
     },
     {
         id: '1',
-        number: -9.54021
+        number: -9.5402111111
     },
     {
         id: '2'

diff --git a/packages/nimble-components/src/utilities/unit-format/decimal-unit-format.ts b/packages/nimble-components/src/utilities/unit-format/decimal-unit-format.ts
@@ -18,7 +18,6 @@ export class DecimalUnitFormat extends UnitFormat<DecimalUnitFormatOptions> {
     private readonly maximumFractionDigits: number;
 
     private readonly scaledUnitFormatters = new Map<number, ScaledUnitFormat>();
-    private readonly tenPowDecimalDigits: number;
 
     public constructor(
         locale: string,
@@ -33,10 +32,14 @@ export class DecimalUnitFormat extends UnitFormat<DecimalUnitFormatOptions> {
         }
     ) {
         super();
+        // Workaround to avoid a ts error about signDisplay not accepting the value 'negative'.
+        // It has been supported by browsers since 8/23, but TypeScript still hasn't
+        // added it to the type definitions. See https://github.com/microsoft/TypeScript/issues/56269
+        const signDisplay = 'negative' as Intl.NumberFormatOptions['signDisplay'];
         const intlNumberFormatOptions = {
             maximumFractionDigits,
             minimumFractionDigits,
-            useGrouping: true
+            signDisplay
         };
         for (const scaledUnit of unitScale.supportedScaledUnits) {
             this.scaledUnitFormatters.set(
@@ -47,7 +50,6 @@ export class DecimalUnitFormat extends UnitFormat<DecimalUnitFormatOptions> {
                 })
             );
         }
-        this.tenPowDecimalDigits = 10 ** maximumFractionDigits;
         this.unitScale = unitScale;
         this.minimumFractionDigits = minimumFractionDigits;
         this.maximumFractionDigits = maximumFractionDigits;
@@ -63,19 +65,9 @@ export class DecimalUnitFormat extends UnitFormat<DecimalUnitFormatOptions> {
 
     protected tryFormat(number: number): string {
         const { scaledValue, scaledUnit } = this.unitScale.scaleNumber(number);
-
-        const numberNormalized = this.willRoundToZero(scaledValue)
-            ? 0
-            : scaledValue;
         const scaledUnitFormatter = this.scaledUnitFormatters.get(
             scaledUnit.scaleFactor
         )!;
-        return scaledUnitFormatter.format(numberNormalized);
-    }
-
-    private willRoundToZero(number: number): boolean {
-        // Multiply the value by 10 raised to maximumFractionDigits so that Math.round can be used to emulate rounding to
-        // maximumFractionDigits decimal places. If that rounded value is 0, then the value will be rendered with only 0s.
-        return Math.round(number * this.tenPowDecimalDigits) === 0;
+        return scaledUnitFormatter.format(scaledValue);
     }
 }
diff --git a/packages/nimble-components/src/utilities/unit-format/default-unit-format.ts b/packages/nimble-components/src/utilities/unit-format/default-unit-format.ts
@@ -3,7 +3,10 @@ import type { ScaledUnitFormat } from './scaled-unit-format/scaled-unit-format';
 import type { UnitScale } from './unit-scale/unit-scale';
 import { passthroughUnitScale } from './unit-scale/passthrough-unit-scale';
 
-type NumberStyle = 'default' | 'leadingZero' | 'exponential';
+// Workaround to avoid ts errors about signDisplay not accepting the value 'negative'.
+// It has been supported by browsers since 8/23, but TypeScript still hasn't
+// added it to the type definitions. See https://github.com/microsoft/TypeScript/issues/56269
+const signDisplay = 'negative' as Intl.NumberFormatOptions['signDisplay'];
 
 // Allow consistent pattern for defining Options and ResolvedOptions
 // eslint-disable-next-line @typescript-eslint/no-empty-interface
@@ -28,27 +31,15 @@ export class DefaultUnitFormat extends UnitFormat {
     private static readonly exponentialUpperBound = 999999.5;
 
     private readonly unitScale: UnitScale;
-
-    // Format options to use by default. It renders the number with a maximum of 6 signficant digits.
+    // Format options to use by default. It renders the number with a maximum of 6 signficant digits (including zero before decimal point).
     private readonly defaultIntlNumberFormatOptions: Intl.NumberFormatOptions = {
         maximumSignificantDigits: DefaultUnitFormat.maximumDigits,
-        useGrouping: true
-    };
-
-    private readonly defaultScaledUnitFormatters = new Map<
-    number,
-    ScaledUnitFormat
-    >();
-
-    // Format options 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 readonly leadingZeroIntlNumberFormatOptions: Intl.NumberFormatOptions = {
         maximumFractionDigits: DefaultUnitFormat.maximumDigits - 1,
-        useGrouping: true
+        roundingPriority: 'lessPrecision',
+        signDisplay
     };
 
-    private readonly leadingZeroScaledUnitFormatters = new Map<
+    private readonly defaultScaledUnitFormatters = new Map<
     number,
     ScaledUnitFormat
     >();
@@ -57,7 +48,8 @@ export class DefaultUnitFormat extends UnitFormat {
     // for numbers with magintudes over 'exponentialUpperBound' or under 'exponentialLowerBound'.
     private readonly exponentialIntlNumberFormatOptions: Intl.NumberFormatOptions = {
         maximumSignificantDigits: DefaultUnitFormat.maximumDigits,
-        notation: 'scientific'
+        notation: 'scientific',
+        signDisplay
     };
 
     private readonly exponentialScaledUnitFormatter: ScaledUnitFormat;
@@ -77,14 +69,6 @@ export class DefaultUnitFormat extends UnitFormat {
                     intlNumberFormatOptions: this.defaultIntlNumberFormatOptions
                 })
             );
-            this.leadingZeroScaledUnitFormatters.set(
-                unit.scaleFactor,
-                unit.scaledUnitFormatFactory({
-                    locale,
-                    intlNumberFormatOptions:
-                        this.leadingZeroIntlNumberFormatOptions
-                })
-            );
         }
         this.exponentialScaledUnitFormatter = unitScale.baseScaledUnit.scaledUnitFormatFactory({
             locale,
@@ -100,52 +84,16 @@ export class DefaultUnitFormat extends UnitFormat {
     }
 
     protected tryFormat(number: number): string {
-        // Normalize +0 / -0 --> +0
-        const numberNormalized = number === 0 ? 0 : number;
-
-        const { scaledValue, scaledUnit } = this.unitScale.scaleNumber(numberNormalized);
-
-        const numberStyle = this.resolveNumberStyle(scaledValue);
-        switch (numberStyle) {
-            case 'default': {
-                const scaledUnitFormatter = this.defaultScaledUnitFormatters.get(
-                    scaledUnit.scaleFactor
-                )!;
-                return scaledUnitFormatter.format(scaledValue);
-            }
-            case 'leadingZero': {
-                const scaledUnitFormatter = this.leadingZeroScaledUnitFormatters.get(
-                    scaledUnit.scaleFactor
-                )!;
-                return scaledUnitFormatter.format(scaledValue);
-            }
-            case 'exponential': {
-                const scaledUnitFormatter = this.exponentialScaledUnitFormatter;
-                return scaledUnitFormatter.format(numberNormalized);
-            }
-            default:
-                throw new Error('Unexpected number format style');
-        }
-    }
-
-    private resolveNumberStyle(number: number): NumberStyle {
-        if (number === 0) {
-            return 'default';
-        }
-
-        const absoluteValue = Math.abs(number);
-        if (
-            absoluteValue >= DefaultUnitFormat.exponentialUpperBound
-            || absoluteValue < DefaultUnitFormat.exponentialLowerBound
-        ) {
-            return 'exponential';
-        }
-        // 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.
-        if (absoluteValue < 1) {
-            return 'leadingZero';
-        }
-        return 'default';
+        const { scaledValue, scaledUnit } = this.unitScale.scaleNumber(number);
+
+        const absoluteValue = Math.abs(scaledValue);
+        const useExponential = absoluteValue !== 0
+            && (absoluteValue >= DefaultUnitFormat.exponentialUpperBound
+                || absoluteValue < DefaultUnitFormat.exponentialLowerBound);
+        return useExponential
+            ? this.exponentialScaledUnitFormatter.format(number)
+            : this.defaultScaledUnitFormatters
+                .get(scaledUnit.scaleFactor)!
+                .format(scaledValue);
     }
 }