fix(fields): formatPercent groups and follows the display locale — the last tooltip channel (#4553) - #4565
Merged
Merged
Conversation
`formatPercent(value, precision)` took no locale and never touched `Intl` —
its whole body was `${percentDisplayValue(value).toFixed(precision)}%`. So
unlike its siblings it did not render in the machine's locale; it rendered
in NO locale: an ASCII decimal mark, never grouped, identical on every
machine.
Because it never grouped, `1235%` was wrong in en-US too, so grouping and
locale land together and the English output MOVES at four digits and up
(`1235%` -> `1,235%`). German changes at every magnitude (`1.235 %`), the
no-break space before the sign being part of the locale's convention.
Consumers threaded in the same change, per the repo's parameter-with-its-
consumers rule: PercentCellRenderer (both scaling paths), the gantt tooltip
row, the grid mobile card cell, and the dashboard's renderFieldValue plus
its two memoized callers.
Bumps follow each package's own .d.ts diff, measured both ways: fields and
plugin-dashboard minor (entry-visible optional parameter), plugin-gantt and
plugin-grid patch (byte-identical .d.ts).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
PM step-7 复核 — ACCEPT (session_017Qqyix2QcnpUC9XeYVDzx3) — and #4553 closes with this: the gantt tooltip's every row (number, currency, percent, date) now agrees on one display locale, pinned in a single German-convention case.
Auto-merge armed (squash) — landing verified per the merge-queue discipline. #4566 enters the pool with a PM ruling to follow. Generated by Claude Code Generated by Claude Code |
yinlianghui
marked this pull request as ready for review
August 13, 2026 10:50
This was referenced Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4553 — phase 2, closing the card. Phase 1 (numbers and currency) landed as #4557.
The defect, and why grouping comes with it
#4557 threaded the gantt tooltip's number and currency rows and measured that percent could not follow:
formatPercent(value, precision)took no locale parameter, and its whole body was`${percentDisplayValue(value).toFixed(precision)}%`. It built noIntl.NumberFormatand never reachedformatDisplayNumber— so unlike its siblings it did not render in the MACHINE's locale. It rendered in no locale: an ASCII decimal mark, never a grouping separator, byte-identical on every machine.That is why the ruled fix is grouping and locale in one change.
1235%was wrong in en-US too, not only in German:1235%1,235%1235%1.235 %80%80 %The English output MOVES, and that is the fix, not a regression. It moves only from four digits up;
80%,12.5%,33.33%are byte-identical. German changes at every magnitude because the no-break space before the sign is part of the locale's percent convention — which is what routing throughIntlbuys over appending a literal%.The scaling contract is untouched:
percentDisplayValuestill disambiguates a fraction-stored percent (0.8becomes 80%) from a whole one, so the list cell and the dashboard measure formatter still agree.Consumer census — one more than the ruling named
The ruling asked for a census. It found four call sites, in four packages:
PercentCellRendererrenderFieldValueTwo census findings worth the reviewer's attention:
PercentCellRendererhad a second, more primitive path. Its whole-percent branch (progress/completionfields, which store 0-100 and must skip the fraction scaling) was a baretoFixedcall that bypassedformatPercententirely. Threading only the other half would have made one grid internally inconsistent — aprogresscolumn ungrouped and unlocalized beside aratecolumn that was neither — so both branches now share one locale-aware body and differ only in the scaling policy, which is all the branch was ever about.formatPercentconsumer.formatMeasurein@object-ui/corehas its own percent path (toLocaleString(undefined, ...)) and never callsformatPercent, soDatasetWidget/ObjectMetricare outside this change's blast radius. It does carry the same machine-locale defect in its own right — reported separately, not fixed here.renderFieldValueis a plain function, not a component, so it takes the locale as an optional fourth parameter beside thetenantCurrencyalready threaded that way; both of its callers pass it and declare it in their memo dependency arrays.Verification
Red-first, directions predicted in writing in each test file's header before running. Runner: node v22.22.2 / ICU 78.2 / machine locale en-US.
Reverse verification by reverting the six source files (
git diffto a patch +git checkout --, nevergit stash), keeping the new expectations:18 red is the full predicted set — the en grouping move, the de forms, both renderer paths, all four consumers, and the moved gantt pin. The 14 green are the must-not-change pins. Restore confirmed byte-identical by sha256 across all six files.
Green after:
Test Files 5 passed (5),Tests 32 passed (32). Whole-package sweep over all four affected packages:Test Files 248 passed (248),Tests 2757 passed (2757)— no pre-existing percent assertion anywhere in the repo moved.Type surface reverse-verified against the REBUILT
.d.ts, both directions:formatPercent(80, 0, 'de')accepts,formatPercent(80, 0, 123)is rejected withTS2345: Argument of type 'number' is not assignable to parameter of type 'string'.Other gates: both tsc passes green for each of the four packages; eslint 0 errors in all four;
check:control-bytes,check:phantom-deps,changeset:checkgreen.Pin moved (declared)
ObjectGantt.numberLocale.test.tsxassertedRatio=1235%as #4557's inversion evidence, with a comment saying it should go red the dayformatPercentgrew a locale. This is that day, so the case now assertsRatio=1.235 %and is relabelled. Theenexpectation in that file moved too — the one line in that case that is not byte-identical, for the grouping reason above. A new case asserts every row of one tooltip finally agreeing on one convention, which is the card's original complaint, closed.One test that is honestly labelled rather than quietly wrong
The dashboard file's "re-formats when the tenant locale changes after first paint" case was written to pin the memo dependency. It does not, and says so in place. Measured both ways: with
displayLocaleremoved from the dependency array and the argument still threaded, all five cases still passed — the provider change makes that component refetch, which givesfinalDataa fresh identity and re-runs the memo on its own. Making the data source module-constant (#4554's masking-path lesson) removes one masking path but not that one. The dependency is still declared, becauseexhaustive-depsrequires it and because it is what keeps the cells correct if the refetch ever stops coinciding — but it is guarded by reasoning, not by that test, and the file records that so a later reader does not mistake it for a pin.Bumps
Per each package's own
.d.tsdiff, measured in both directions and reproducible:@object-ui/fieldsminor,@object-ui/plugin-dashboardminor — entry-visible optional parameter, on the Date formatter is half-localized: future relative forms and absolute timestamps stay en-US on a non-English locale #4272 / PR fix(fields): the date formatter's last three en-US channels thread the display locale (#4272) #4544 precedent.@object-ui/plugin-ganttpatch,@object-ui/plugin-gridpatch —.d.tsbyte-identical.Generated by Claude Code