Repository navigation
Conversation
Co-authored-by: Aymeric Rabot <Aymericr@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Closing as a duplicate. This PR's head ( |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1dbbd10. Configure here.
| ? measurementHint(inputValue, spec, { | ||
| bareUnit: parseUnit ?? spec.unitId, | ||
| system: isImperial ? 'us' : 'metric', | ||
| locale, |
There was a problem hiding this comment.
Metric write-back stays ASCII
Medium Severity
MetricControl now parses with the scoped locale but still formats idle, edit, reset, and arrow-key values with toFixed. In a French scope the field shows 2.75 after commit instead of 2,75, so the control never writes the locale decimal separator back into the UI.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 1dbbd10. Configure here.
| unitsPerPixel: number | ||
| }) { | ||
| const unit = useViewer((state) => state.unit) | ||
| const locale = useLocale() |
There was a problem hiding this comment.
Extrusion input loses comma decimals
Medium Severity
The extrusion field now parses with the scoped locale, but its initial value and the unit-change effect still stringify with toFixed. After a unit or locale change the input is rewritten with a period, so a French 2,75 does not survive.
Reviewed by Cursor Bugbot for commit 1dbbd10. Configure here.
| /** Lingo-parse the free-text real-length input in the dropdown's unit — a | ||
| * bare number means the dropdown unit, while `180cm`, `1m80` or `5'11"` | ||
| * override it. Returns `null` when the text isn't a readable length. */ | ||
| function parseReferenceScaleLength(raw: string, unit: ReferenceScaleUnit): number | null { |
There was a problem hiding this comment.
Reference-scale labels stay ASCII
Medium Severity
Reference-scale parsing now takes the scoped locale, but labels and scale previews still format through the panel's local toFixed helper. Committed lengths and area previews keep a period in French instead of a comma.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 1dbbd10. Configure here.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| unit = '', | ||
| restoreOnCommit = true, | ||
| }: MetricControlProps) { | ||
| const locale = useLocale() |
There was a problem hiding this comment.
When an editor is scoped to a decimal-comma locale such as French, this locale is used only while parsing; the control still initializes, resets, and renders its value with toFixed, so users see 2.75 even though the equivalent SliderControl shows and accepts 2,75. Use a locale-aware formatter for the displayed and editable values as well as for parsing.
Useful? React with 👍 / 👎.
| return `${formatNumber(squareMetersToAreaUnit(squareMeters, unit), { | ||
| min: fractionDigits, | ||
| max: fractionDigits, | ||
| })}${getAreaUnitLabel(unit)}` |
There was a problem hiding this comment.
With <Editor locale="fr">, area, volume, and metric-length labels still use English decimal punctuation because these helpers call formatNumber without a locale, whose documented default is English. This affects measurement overlays and quantity panels throughout the editor; accept a locale in these formatters and pass the scoped useLocale() value from rendering paths.
Useful? React with 👍 / 👎.
What does this PR do?
Follows up on #1148’s five remaining Bugbot findings:
ClientBootstrapsubscribes withdefaultI18n.subscribeLocale, which excludes catalog refresh notifications. Registering a later compiled pack therefore refreshes messages without applying the global English locale to the request scope. The regression now explicitly registers a compiled catalog while globalenand request-localfrdiffer, and verifies no locale notification or reset occurs.SliderControlgetslocalefromuseLocale()and callsparseSliderBareValue(text, locale), which forwards it toparseLocalizedNumber. The regression now explicitly keeps global localeenwhile rendering and parsing through a French scope.formatNumberno longer reads the live registry locale; its default is deterministic English and React renderers pass theuseLocale()snapshot explicitly. The SSR regression now asserts global English and request-local French simultaneously.Intl.NumberFormatfor idle, edit, reset, and arrow-key values. Slider write-back was already locale-aware and remains covered. The related floor-plan extrusion input now also preserves the scoped decimal separator after unit or locale changes.apps/editor/proxy.tsis unchanged.How to test
bun test packages/core/src/i18n packages/editor/src/components/ui/controls/slider-control.test.tsx packages/editor/src/components/ui/controls/metric-control.test.ts packages/editor/src/components/editor/floorplan-panel.test.ts packages/editor/src/components/editor-2d/floorplan-measurement-tool-layer.test.ts.bun run ci.2,75remains comma-formatted after committing metric, slider, extrusion, and reference-scale values.Screenshots / screen recording
N/A — non-visual formatting correctness change.
Checklist
bun devbun checkto verify)mainbranch