Skip to content

Commit 3d35013

Browse files
patyorkpre-commit-ci[bot]mscolnick
authored
columns.tsx LocaleNumber to use maximumFractionalDigits (#7263)
Default formatting for number data-table columns updated to use calculated maxFractionalDigits based on locale. ## 📝 Summary By default, a data-table uses `Intl.NumberFormat` with no options to display number types that have no explicit format. This can lead to rounding-when-displayed by default, which is antithetical to the expected behavior when displaying raw data (e.g. without explicit formatting). ## 🔍 Description of Changes Used the existing `maxFractionalDigits` function from the number utils to invoke the `useNumberFormatter` function with a value for the `maximumSignificantDigits` parameter. ## 📋 Checklist - [X] I have read the [contributor guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md). - [X] For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on [Discord](https://marimo.io/discord?ref=pr), or the community [discussions](https://github.com/marimo-team/marimo/discussions) (Please provide a link if applicable). - [ ] I have added tests for the changes made. - [X] I have run the code and verified that it works as expected. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Make data-table number rendering locale-aware with max fractional digits and add comprehensive LocaleNumber tests; minor test cleanups. > > - **Data Table**: > - **`LocaleNumber`**: Exported and updated in `frontend/src/components/data-table/columns.tsx` to use `useLocale` and `maxFractionalDigits(locale)` via `useNumberFormatter({ maximumFractionDigits })`. > - **Tests**: > - **Columns**: Added extensive tests in `__tests__/columns.test.tsx` for `LocaleNumber` across locales (`en-US`, `de-DE`, `fr-FR`, `ja-JP`) and edge cases (integers, zeros, negatives, large/small numbers, Infinity, -Infinity, NaN, scientific notation). > - **Logs**: Updated timestamps in `frontend/src/core/cells/__tests__/logs.test.ts` to use numeric separators; no behavior changes. > - **ProgressPlugin**: Minor type tweak in `frontend/src/plugins/layout/__test__/ProgressPlugin.test.ts` for `Cases` tuple typing. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 69f8f7d. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Myles Scolnick <myles@marimo.io>
1 parent 284ad6a commit 3d35013

File tree

4 files changed

+169
-23
lines changed

4 files changed

+169
-23
lines changed

‎frontend/src/components/data-table/__tests__/columns.test.tsx‎

Lines changed: 143 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
/* Copyright 2024 Marimo. All rights reserved. */
22

33
import { render } from "@testing-library/react";
4+
import { I18nProvider } from "react-aria";
45
import { describe, expect, it, test } from "vitest";
56
import { TooltipProvider } from "@/components/ui/tooltip";
6-
import { generateColumns, inferFieldTypes } from "../columns";
7+
import { generateColumns, inferFieldTypes, LocaleNumber } from "../columns";
78
import { getMimeValues, isMimeValue, MimeCell } from "../mime-cell";
89
import type { FieldTypesWithExternalType } from "../types";
910
import { uniformSample } from "../uniformSample";
@@ -377,3 +378,144 @@ describe("getMimeValues", () => {
377378
expect(getMimeValues(values)).toBeUndefined();
378379
});
379380
});
381+
382+
describe("LocaleNumber", () => {
383+
it("should format numbers correctly for en-US locale", () => {
384+
const { container } = render(
385+
<I18nProvider locale="en-US">
386+
<LocaleNumber value={1_234_567.89} />
387+
</I18nProvider>,
388+
);
389+
expect(container.textContent).toMatchInlineSnapshot(`"1,234,567.89"`);
390+
});
391+
392+
it("should format numbers correctly for de-DE locale", () => {
393+
const { container } = render(
394+
<I18nProvider locale="de-DE">
395+
<LocaleNumber value={1_234_567.89} />
396+
</I18nProvider>,
397+
);
398+
expect(container.textContent).toMatchInlineSnapshot(`"1.234.567,89"`);
399+
});
400+
401+
it("should format integers correctly", () => {
402+
const { container } = render(
403+
<I18nProvider locale="en-US">
404+
<LocaleNumber value={1000} />
405+
</I18nProvider>,
406+
);
407+
expect(container.textContent).toMatchInlineSnapshot(`"1,000"`);
408+
});
409+
410+
it("should format zero correctly", () => {
411+
const { container } = render(
412+
<I18nProvider locale="en-US">
413+
<LocaleNumber value={0} />
414+
</I18nProvider>,
415+
);
416+
expect(container.textContent).toMatchInlineSnapshot(`"0"`);
417+
});
418+
419+
it("should format negative numbers correctly", () => {
420+
const { container } = render(
421+
<I18nProvider locale="en-US">
422+
<LocaleNumber value={-1234.56} />
423+
</I18nProvider>,
424+
);
425+
expect(container.textContent).toMatchInlineSnapshot(`"-1,234.56"`);
426+
});
427+
428+
it("should format small decimal numbers correctly", () => {
429+
const { container } = render(
430+
<I18nProvider locale="en-US">
431+
<LocaleNumber value={0.123_456_789} />
432+
</I18nProvider>,
433+
);
434+
expect(container.textContent).toMatchInlineSnapshot(`"0.123456789"`);
435+
});
436+
437+
it("should format large numbers correctly", () => {
438+
const { container } = render(
439+
<I18nProvider locale="en-US">
440+
<LocaleNumber value={999_999_999.99} />
441+
</I18nProvider>,
442+
);
443+
expect(container.textContent).toMatchInlineSnapshot(`"999,999,999.99"`);
444+
});
445+
446+
it("should format numbers correctly for fr-FR locale", () => {
447+
const { container } = render(
448+
<I18nProvider locale="fr-FR">
449+
<LocaleNumber value={1_234_567.89} />
450+
</I18nProvider>,
451+
);
452+
// eslint-disable-next-line no-irregular-whitespace
453+
expect(container.textContent).toMatchInlineSnapshot(`"1 234 567,89"`);
454+
});
455+
456+
it("should format numbers correctly for ja-JP locale", () => {
457+
const { container } = render(
458+
<I18nProvider locale="ja-JP">
459+
<LocaleNumber value={1_234_567.89} />
460+
</I18nProvider>,
461+
);
462+
expect(container.textContent).toMatchInlineSnapshot(`"1,234,567.89"`);
463+
});
464+
465+
it("should respect maximumFractionDigits based on locale", () => {
466+
// Test with a number that has many decimal places
467+
const { container } = render(
468+
<I18nProvider locale="en-US">
469+
<LocaleNumber value={1.123_456_789_012_345_7} />
470+
</I18nProvider>,
471+
);
472+
expect(container.textContent).toMatchInlineSnapshot(`"1.1234567890123457"`);
473+
});
474+
475+
it("should handle very large numbers", () => {
476+
const { container } = render(
477+
<I18nProvider locale="en-US">
478+
<LocaleNumber value={123_456_789_012_345.67} />
479+
</I18nProvider>,
480+
);
481+
expect(container.textContent).toMatchInlineSnapshot(
482+
`"123,456,789,012,345.67"`,
483+
);
484+
});
485+
486+
it("should handle Infinity", () => {
487+
const { container } = render(
488+
<I18nProvider locale="en-US">
489+
<LocaleNumber value={Infinity} />
490+
</I18nProvider>,
491+
);
492+
expect(container.textContent).toMatchInlineSnapshot(`"∞"`);
493+
});
494+
495+
it("should handle negative Infinity", () => {
496+
const { container } = render(
497+
<I18nProvider locale="en-US">
498+
<LocaleNumber value={-Infinity} />
499+
</I18nProvider>,
500+
);
501+
expect(container.textContent).toMatchInlineSnapshot(`"-∞"`);
502+
});
503+
504+
it("should handle NaN", () => {
505+
const { container } = render(
506+
<I18nProvider locale="en-US">
507+
<LocaleNumber value={Number.NaN} />
508+
</I18nProvider>,
509+
);
510+
expect(container.textContent).toMatchInlineSnapshot(`"NaN"`);
511+
});
512+
513+
it("should handle numbers with scientific notation", () => {
514+
const { container } = render(
515+
<I18nProvider locale="en-US">
516+
<LocaleNumber value={1e10} />
517+
</I18nProvider>,
518+
);
519+
expect(container.textContent).toMatchInlineSnapshot(`"10,000,000,000"`);
520+
});
521+
});

‎frontend/src/components/data-table/columns.tsx‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,15 @@
44
import { PopoverClose } from "@radix-ui/react-popover";
55
import type { Column, ColumnDef } from "@tanstack/react-table";
66
import { formatDate } from "date-fns";
7-
import { useNumberFormatter } from "react-aria";
7+
import { useLocale, useNumberFormatter } from "react-aria";
88
import { WithLocale } from "@/core/i18n/with-locale";
99
import type { DataType } from "@/core/kernel/messages";
1010
import type { CalculateTopKRows } from "@/plugins/impl/DataTablePlugin";
1111
import { cn } from "@/utils/cn";
1212
import { type DateFormat, exactDateTime, getDateFormat } from "@/utils/dates";
1313
import { Logger } from "@/utils/Logger";
1414
import { Maps } from "@/utils/maps";
15+
import { maxFractionalDigits } from "@/utils/numbers";
1516
import { Objects } from "@/utils/objects";
1617
import { EmotionCacheProvider } from "../editor/output/EmotionCacheProvider";
1718
import { JsonOutput } from "../editor/output/JsonOutput";
@@ -602,7 +603,10 @@ export function renderCellValue<TData, TValue>({
602603
);
603604
}
604605

605-
const LocaleNumber = ({ value }: { value: number }) => {
606-
const format = useNumberFormatter();
606+
export const LocaleNumber = ({ value }: { value: number }) => {
607+
const { locale } = useLocale();
608+
const format = useNumberFormatter({
609+
maximumFractionDigits: maxFractionalDigits(locale),
610+
});
607611
return format.format(value);
608612
};

‎frontend/src/core/cells/__tests__/logs.test.ts‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ describe("getCellLogsForMessage", () => {
2424
mimetype: "text/plain",
2525
channel: "stdout",
2626
data: "Hello, World!",
27-
timestamp: 1234567890,
27+
timestamp: 1_234_567_890,
2828
},
2929
],
3030
output: null,
@@ -37,7 +37,7 @@ describe("getCellLogsForMessage", () => {
3737

3838
expect(logs).toHaveLength(1);
3939
expect(logs[0]).toEqual({
40-
timestamp: 1234567890,
40+
timestamp: 1_234_567_890,
4141
level: "stdout",
4242
message: "Hello, World!",
4343
cellId: "cell-1",
@@ -52,7 +52,7 @@ describe("getCellLogsForMessage", () => {
5252
mimetype: "text/plain",
5353
channel: "stderr",
5454
data: "Error occurred",
55-
timestamp: 1234567890,
55+
timestamp: 1_234_567_890,
5656
},
5757
],
5858
output: null,
@@ -65,7 +65,7 @@ describe("getCellLogsForMessage", () => {
6565

6666
expect(logs).toHaveLength(1);
6767
expect(logs[0]).toEqual({
68-
timestamp: 1234567890,
68+
timestamp: 1_234_567_890,
6969
level: "stderr",
7070
message: "Error occurred",
7171
cellId: "cell-2",
@@ -80,7 +80,7 @@ describe("getCellLogsForMessage", () => {
8080
mimetype: "text/html",
8181
channel: "stdout",
8282
data: '<span style="color: red;">Error: Something went wrong</span>',
83-
timestamp: 1234567890,
83+
timestamp: 1_234_567_890,
8484
},
8585
],
8686
output: null,
@@ -93,7 +93,7 @@ describe("getCellLogsForMessage", () => {
9393

9494
expect(logs).toHaveLength(1);
9595
expect(logs[0]).toEqual({
96-
timestamp: 1234567890,
96+
timestamp: 1_234_567_890,
9797
level: "stdout",
9898
message: "Error: Something went wrong",
9999
cellId: "cell-3",
@@ -108,7 +108,7 @@ describe("getCellLogsForMessage", () => {
108108
mimetype: "text/html",
109109
channel: "stderr",
110110
data: "<div><strong>Critical Error:</strong> System failure</div>",
111-
timestamp: 1234567890,
111+
timestamp: 1_234_567_890,
112112
},
113113
],
114114
output: null,
@@ -121,7 +121,7 @@ describe("getCellLogsForMessage", () => {
121121

122122
expect(logs).toHaveLength(1);
123123
expect(logs[0]).toEqual({
124-
timestamp: 1234567890,
124+
timestamp: 1_234_567_890,
125125
level: "stderr",
126126
message: "Critical Error: System failure",
127127
cellId: "cell-4",
@@ -136,7 +136,7 @@ describe("getCellLogsForMessage", () => {
136136
mimetype: "application/vnd.marimo+traceback",
137137
channel: "marimo-error",
138138
data: '<div class="traceback"><span style="color: red;">Traceback (most recent call last):</span><pre> File "test.py", line 1</pre></div>',
139-
timestamp: 1234567890,
139+
timestamp: 1_234_567_890,
140140
},
141141
],
142142
output: null,
@@ -162,19 +162,19 @@ describe("getCellLogsForMessage", () => {
162162
mimetype: "text/plain",
163163
channel: "stdout",
164164
data: "Plain text output",
165-
timestamp: 1234567890,
165+
timestamp: 1_234_567_890,
166166
},
167167
{
168168
mimetype: "text/html",
169169
channel: "stdout",
170170
data: "<span>HTML output</span>",
171-
timestamp: 1234567891,
171+
timestamp: 1_234_567_891,
172172
},
173173
{
174174
mimetype: "application/vnd.marimo+traceback",
175175
channel: "stderr",
176176
data: "<div>Traceback error</div>",
177-
timestamp: 1234567892,
177+
timestamp: 1_234_567_892,
178178
},
179179
],
180180
output: null,
@@ -225,7 +225,7 @@ describe("getCellLogsForMessage", () => {
225225
mimetype: "application/json",
226226
channel: "stdout",
227227
data: '{"key": "value"}',
228-
timestamp: 1234567890,
228+
timestamp: 1_234_567_890,
229229
},
230230
],
231231
output: null,
@@ -247,7 +247,7 @@ describe("getCellLogsForMessage", () => {
247247
mimetype: "text/plain",
248248
channel: "pdb" as unknown as "stdout", // Non-logging channel
249249
data: "Should be ignored",
250-
timestamp: 1234567890,
250+
timestamp: 1_234_567_890,
251251
},
252252
],
253253
output: null,
@@ -284,7 +284,7 @@ describe("getCellLogsForMessage", () => {
284284
mimetype: "text/html",
285285
channel: "stdout",
286286
data: "<div><span>Nested</span> <strong>HTML</strong> <em>content</em></div>",
287-
timestamp: 1234567890,
287+
timestamp: 1_234_567_890,
288288
},
289289
],
290290
output: null,
@@ -307,7 +307,7 @@ describe("getCellLogsForMessage", () => {
307307
mimetype: "text/plain",
308308
channel: "marimo-error",
309309
data: "Internal error",
310-
timestamp: 1234567890,
310+
timestamp: 1_234_567_890,
311311
},
312312
],
313313
output: null,
@@ -326,7 +326,7 @@ describe("getCellLogsForMessage", () => {
326326
describe("formatLogTimestamp", () => {
327327
test("formats unix timestamp correctly", () => {
328328
// January 1, 2024, 12:00:00 PM UTC
329-
const timestamp = 1704110400;
329+
const timestamp = 1_704_110_400;
330330
const result = formatLogTimestamp(timestamp);
331331

332332
// The result depends on the timezone, so we just check it's not empty
@@ -336,7 +336,7 @@ describe("formatLogTimestamp", () => {
336336
});
337337

338338
test("formats timestamp with AM/PM notation", () => {
339-
const timestamp = 1704110400; // Noon
339+
const timestamp = 1_704_110_400; // Noon
340340
const result = formatLogTimestamp(timestamp);
341341

342342
// Should contain AM or PM

‎frontend/src/plugins/layout/__test__/ProgressPlugin.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import { expect, test } from "vitest";
33
import { prettyTime } from "../ProgressPlugin";
44

5-
const Cases: Array<[number, string]> = [
5+
const Cases: [number, string][] = [
66
// exact values
77
[0, "0s"],
88
[1, "1s"],

0 commit comments

Comments
 (0)