-
Notifications
You must be signed in to change notification settings - Fork 2
ui: thousands separators on delta percentages #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
06645da
f72e3e2
0199b3c
531a62b
2b458c4
61904ad
fdca653
1372542
deb66c9
508ec18
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| // Copyright 2026 Oddbit (https://oddbit.id) | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import { describe, expect, it } from "vitest"; | ||
| import { jsx } from "hono/jsx"; | ||
| import { Delta } from "../../components/delta"; | ||
|
|
||
| function render(props: { pct: number; lang: string }): string { | ||
| return String(jsx(Delta, props)); | ||
| } | ||
|
|
||
| describe("Delta component", () => { | ||
| it("renders +1,400% for an en delta of 1400", () => { | ||
| const out = render({ pct: 1400, lang: "en" }); | ||
| expect(out).toContain("+1,400%"); | ||
| expect(out).toContain("delta up"); | ||
| expect(out).toContain("trending_up"); | ||
| }); | ||
|
|
||
| it("normalizes -0 to 0 so the flat delta does not render -0%", () => { | ||
| // Math.round(((99.9 - 100) / 100) * 100) === -0, which Intl.NumberFormat | ||
| // would otherwise render as "-0". | ||
| const out = render({ pct: -0, lang: "en" }); | ||
| expect(out).toContain(">0%<"); | ||
| expect(out).not.toContain("-0%"); | ||
| expect(out).not.toContain("−0%"); | ||
| expect(out).toContain("delta flat"); | ||
| expect(out).toContain("trending_flat"); | ||
| }); | ||
|
|
||
| it("groups thousands using the active locale", () => { | ||
| expect(render({ pct: 1400, lang: "id" })).toContain("+1.400%"); | ||
| }); | ||
|
|
||
| it("renders a negative delta with a minus and the down direction", () => { | ||
| const out = render({ pct: -25, lang: "en" }); | ||
| expect(out).toContain("-25%"); | ||
| expect(out).toContain("delta down"); | ||
| expect(out).toContain("trending_down"); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ type KpiCardProps = { | |
| valueId?: string; | ||
| deltaPct?: number | null; | ||
| deltaId?: string; | ||
| lang: string; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same answer as the Generated by Claude Code
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Settled in the prior |
||
| hint?: string; | ||
| sparkline?: number[]; | ||
| span?: 1 | 2 | 3; | ||
|
|
@@ -26,6 +27,7 @@ export const KpiCard: FC<KpiCardProps> = ({ | |
| valueId, | ||
| deltaPct, | ||
| deltaId, | ||
| lang, | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| hint, | ||
| sparkline, | ||
| span = 1, | ||
|
|
@@ -38,7 +40,7 @@ export const KpiCard: FC<KpiCardProps> = ({ | |
| {icon && <span class="icon">{icon}</span>} | ||
| <span>{label}</span> | ||
| </div> | ||
| {deltaPct !== undefined && deltaPct !== null && <Delta pct={deltaPct} id={deltaId} />} | ||
| {deltaPct !== undefined && deltaPct !== null && <Delta pct={deltaPct} lang={lang} id={deltaId} />} | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| </div> | ||
| <div class="kpi-value" id={valueId}>{value}</div> | ||
| {hint && <div class="kpi-hint">{hint}</div>} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping
langrequired here. There are only threeDeltacallers (dashboard.tsxviaKpiCard,bundles.tsx,links.tsx) andlangis already in scope at each — the prop drilling is one line per call site. A silent fallback would mask future i18n bugs (the server has no meaningful "user locale" outside the cookie/settings, soIntl.NumberFormat().resolvedOptions().localewould just return the Worker's runtime locale, not the user's). The matching server-sidefmtNumber(n, lang)helper takeslangas required for the same reason.Generated by Claude Code