Fix undefined in capacity bar (#1955)

* failing test catches the bug

* fix decimal split math

* delete unused CapacityMetric component

* use Intl.NumberFormat to incredible effect

* might as well use Intl.NumberFormat for round() too

* make sure we're handling numbers between -1 and 1

Author: david-crespo

app/components/CapacityBar.tsx

@@ -43,8 +43,8 @@ export const CapacityBar = ({
           <span className="ml-1 !normal-case text-mono-sm text-quaternary">({unit})</span>
         </div>
         <div className="flex -translate-y-0.5 items-baseline">
-          <div className="font-light text-sans-2xl">{wholeNumber.toLocaleString()}</div>
-          <div className="text-sans-xl text-quaternary">{decimal || ''}%</div>
+          <div className="font-light text-sans-2xl">{wholeNumber}</div>
+          <div className="text-sans-xl text-quaternary">{decimal}%</div>
         </div>
       </div>
       <div className="p-3 pt-1">

app/components/CapacityMetric.tsx

@@ -8,7 +8,7 @@
 import { type ReactElement } from 'react'
 import { expect, test } from 'vitest'
 
-import { groupBy, intersperse, lowestBy, sortBy, sumBy } from './array'
+import { groupBy, intersperse, lowestBy, sortBy, splitOnceBy, sumBy } from './array'
 
 test('sortBy', () => {
   expect(sortBy(['d', 'b', 'c', 'a'])).toEqual(['a', 'b', 'c', 'd'])
@@ -97,3 +97,10 @@ test('intersperse', () => {
   expect(result.map(getText)).toEqual(['a', ',', 'b', ',', 'or', 'c'])
   expect(result.map(getKey)).toEqual(['a', 'sep-1', 'b', 'sep-2', 'conj', 'c'])
 })
+
+test('splitOnceBy', () => {
+  expect(splitOnceBy([], () => false)).toEqual([[], []])
+  expect(splitOnceBy([1, 2, 3], () => false)).toEqual([[1, 2, 3], []])
+  expect(splitOnceBy([1, 2, 3], () => true)).toEqual([[], [1, 2, 3]])
+  expect(splitOnceBy([1, 2, 3], (x) => x % 2 === 0)).toEqual([[1], [2, 3]])
+})

libs/util/array.spec.tsx

@@ -102,3 +102,11 @@ export function intersperse(
     return [sep0, item]
   })
 }
+
+/**
+ * Split array at first element where `by` is true. That element lands in the second array.
+ */
+export function splitOnceBy<T>(array: T[], by: (t: T) => boolean) {
+  const i = array.findIndex(by)
+  return i === -1 ? [array, []] : [array.slice(0, i), array.slice(i)]
+}

libs/util/array.ts

@@ -5,12 +5,14 @@
  *
  * Copyright Oxide Computer Company
  */
-import { expect, it } from 'vitest'
+import { afterAll, beforeAll, describe, expect, it } from 'vitest'
 
 import { GiB } from '.'
 import { round, splitDecimal } from './math'
 
 it('rounds properly', () => {
+  expect(round(0.456, 2)).toEqual(0.46)
+  expect(round(-0.456, 2)).toEqual(-0.46)
   expect(round(123.456, 0)).toEqual(123)
   expect(round(123.456, 1)).toEqual(123.5)
   expect(round(123.456, 2)).toEqual(123.46)
@@ -19,18 +21,79 @@ it('rounds properly', () => {
   expect(round(123.0001, 3)).toEqual(123) // period is culled if decimals are all zeros
   expect(round(1.9, 0)).toEqual(2)
   expect(round(1.9, 1)).toEqual(1.9)
+  expect(round(4.997, 2)).toEqual(5)
   expect(round(5 / 2, 2)).toEqual(2.5) // math expressions are resolved
   expect(round(1879048192 / GiB, 2)).toEqual(1.75) // constants can be evaluated
 })
 
-it.each([
-  [1.23, ['1', '.23']],
-  [1, ['1', '']], // whole number decimal should be an empty string
-  [1.252525, ['1', '.25']],
-  [1.259, ['1', '.26']], // should correctly round the decimal
-  [-50.2, ['-50', '.2']], // should correctly not round down to -51
-  [1000.5, ['1,000', '.5']], // testing localeString
-  [49.00000001, ['49', '']],
-])('splitDecimal', (input, output) => {
-  expect(splitDecimal(input)).toEqual(output)
+describe('splitDecimal', () => {
+  describe('with default locale', () => {
+    it.each([
+      [0.23, ['0', '.23']],
+      [0.236, ['0', '.24']],
+      [-0.236, ['-0', '.24']],
+      [1.23, ['1', '.23']],
+      [1, ['1', '']], // whole number decimal should be an empty string
+
+      // values just below whole numbers
+      [5 - Number.EPSILON, ['5', '']],
+      [4.997, ['5', '']],
+      [-4.997, ['-5', '']],
+      [0.997, ['1', '']],
+
+      // values just above whole numbers
+      [49.00000001, ['49', '']],
+      [5 + Number.EPSILON, ['5', '']],
+
+      [1.252525, ['1', '.25']],
+      [1.259, ['1', '.26']], // should correctly round the decimal
+      [-50.2, ['-50', '.2']], // should correctly not round down to -51
+      [1000.5, ['1,000', '.5']], // test localeString grouping
+    ])('splitDecimal %d -> %s', (input, output) => {
+      expect(splitDecimal(input)).toEqual(output)
+    })
+  })
+
+  describe('with de-DE locale', () => {
+    const originalLanguage = global.navigator.language
+
+    beforeAll(() => {
+      Object.defineProperty(global.navigator, 'language', {
+        value: 'de-DE',
+        writable: true,
+      })
+    })
+
+    it.each([
+      [0.23, ['0', ',23']],
+      [0.236, ['0', ',24']],
+      [-0.236, ['-0', ',24']],
+      [1.23, ['1', ',23']],
+      [1, ['1', '']], // whole number decimal should be an empty string
+
+      // values just below whole numbers
+      [5 - Number.EPSILON, ['5', '']],
+      [4.997, ['5', '']],
+      [-4.997, ['-5', '']],
+      [0.997, ['1', '']],
+
+      // values just above whole numbers
+      [49.00000001, ['49', '']],
+      [5 + Number.EPSILON, ['5', '']],
+
+      [1.252525, ['1', ',25']],
+      [1.259, ['1', ',26']], // should correctly round the decimal
+      [-50.2, ['-50', ',2']], // should correctly not round down to -51
+      [1000.5, ['1.000', ',5']], // test localeString grouping
+    ])('splitDecimal %d -> %s', (input, output) => {
+      expect(splitDecimal(input)).toEqual(output)
+    })
+
+    afterAll(() => {
+      Object.defineProperty(global.navigator, 'language', {
+        value: originalLanguage,
+        writable: true,
+      })
+    })
+  })
 })

libs/util/math.spec.ts

@@ -6,15 +6,29 @@
  * Copyright Oxide Computer Company
  */
 
-export function splitDecimal(value: number) {
-  const wholeNumber = Math.trunc(value)
-  const decimal = value % 1 !== 0 ? round(value % 1, 2) : null
+import { splitOnceBy } from '.'
+
+/**
+ * Get the two parts of a number (before decimal and after-and-including
+ * decimal) as strings. Round to 2 decimal points if necessary.
+ *
+ * If there is no decimal, we will only have whole parts (which can include
+ * minus sign, group separators [comma in en-US], and of course actual number
+ * groups). Those will get joined and the decimal part will be the empty string.
+ */
+export function splitDecimal(value: number): [string, string] {
+  const nf = Intl.NumberFormat(navigator.language, { maximumFractionDigits: 2 })
+  const parts = nf.formatToParts(value)
+
+  const [wholeParts, decimalParts] = splitOnceBy(parts, (p) => p.type === 'decimal')
+
   return [
-    wholeNumber.toLocaleString(),
-    decimal ? '.' + decimal.toLocaleString().split('.')[1] : '',
+    wholeParts.map((p) => p.value).join(''),
+    decimalParts.map((p) => p.value).join(''),
   ]
 }
 
 export function round(num: number, digits: number) {
-  return Number(num.toFixed(digits))
+  const nf = Intl.NumberFormat(navigator.language, { maximumFractionDigits: digits })
+  return Number(nf.format(num))
 }