feat: add imperial unit support for measurements in UI controls and t…#310
feat: add imperial unit support for measurements in UI controls and t…#310PRAVEEN7230 wants to merge 8 commits into
Conversation
…ools, and remove unused env example file
…tool and measurement utilities
…ss (pascalorg#284) Backport from monorepo PR pascalorg#310 (fix/april-pass): 1. WebGPU renderer race on Canvas remount Cache the in-flight WebGPURenderer promise per canvas element so concurrent configure() calls from R3F's useLayoutEffect await the same init() instead of creating duplicate renderers. Fixes intermittent 'resolve target size does not match' errors when navigating between projects and home. 2. Duplicate item loses its scale Add scale to the field set in useDraftNode.commit() so duplicated items preserve their original scale instead of falling back to [1,1,1]. Co-authored-by: Pascal <open@pascal.app>
…aterial asset library.
…use-client Mark elevator-opening-system as client
…ve redundant wall-tool component
|
Thanks for putting in the work here, @PRAVEEN7230 — and for cross-referencing #308 to help us connect the dots. After reviewing both PRs in depth, we're planning to go with #321's approach for the core panel-display bug. Here's the reasoning: Architecture concern with #321 vs #310 on the core bug Unrelated change What's genuinely new in your PR: the keyboard dimension input 🎉 If you want to extract that feature into a new PR, here are a few things to address before it's ready:
None of that is criticism of the idea — it's a great UX addition. Happy to review a follow-up PR for it! Thanks again for contributing and for helping clarify the relationship between these two issues. |
|
Follow-up adversarial pass (Codex 5.5 / GPT-5.5 / xhigh reasoning) with the full codebase — more specific findings: 🐛 Blocker:
|
| File | Label | min | Breaks because |
|---|---|---|---|
spawn/panel.tsx:99 |
X/Y/Z pos | position ± 2 |
signed, formatted as imperial feet |
door/panel.tsx:681 |
X wall | -10 to +10 | signed position |
stair-segment/panel.tsx:238 |
X/Y/Z | -50 to +50 | signed position |
reference-panel.tsx:298 |
X/Y/Z pos | -50 to +50 | signed position |
slab/panel.tsx:161 |
Height (elevation) | -1 to +1 | signed elevation |
fence/inspector-editors.tsx:75 |
Curve | -maxCurveOffset |
signed curve offset |
For example: -0.1524m = -0.5ft. PR #310's formatMeasurement returns -1'6" for this. Typing -6" returns null from parseMeasurement. Every signed unit="m" control becomes unusable in imperial mode.
🐛 Blocker: parseMeasurement("10", "imperial") returns null
Verified by running the parser directly:
parseMeasurement("10", "imperial") => null ← raw number branch unreachable
parseMeasurement("5.5", "imperial") => null ← same
parseMeasurement("10'", "imperial") => 3.048 ✓
parseMeasurement("10' 6\"", "imperial") => 3.200 ✓
parseMeasurement("-6\"", "imperial") => null ← negative rejected
The regex /^\s*(?:(\d+(?:\.\d+)?)\s*')?\s*(?:(\d+(?:\.\d+)?)\s*")?\s*$/ requires at least one ' or " to match — a bare number like "10" doesn't satisfy it so match is null. The raw-number fallback inside the if (match) block is dead code.
Fix: check for a bare decimal number before the feet/inches regex.
🐛 Blocker: typed input leaks into the next wall segment after mouse-click commit
Repro:
- Start drawing a wall (click to place first point)
- Type
10'— draft overlay shows10' - Click to place the wall (don't press Enter)
- Wall commits, new segment starts — but
draftInputRef.currentanddraftInputstill contain10' - New segment shows stale
10'as label; further keystrokes append to it
Root cause: the mouse pointerup commit path in wall/tool.tsx:454 doesn't clear draftInputRef.current / setDraftInput(null). Also, stopDrafting() itself doesn't clear it, so any future cancel path can leave state around.
Fix: add draftInputRef.current = null; setDraftInput(null) to both stopDrafting() and the pointer commit path.
All other points from the previous comment stand. Looking forward to the standalone keyboard-input PR with these fixed!
|
Hey @PRAVEEN7230 — thanks for the work on this, and for flagging the duplicate on #322. You spotted a real overlap. After reviewing both PRs in depth, we're going to land #321 first. Its approach is a better fit for the codebase right now: a single shared That said, your PR has something #321 doesn't: the wall-tool keyboard dimension input — the ability to type Would you be open to extracting just that keyboard input piece into a new, focused PR targeting Either way, thank you for the contribution and for doing the detective work on the duplicate. |
|
Closing in favor of #321, which takes a cleaner panel-level approach to imperial unit display. The core panel-display bug is covered by #321 — it adds a shared What's unique to this PR worth preserving: the keyboard dimension input during wall drafting (type
Thanks for contributing, @PRAVEEN7230! |
What does this PR do?
This PR fixes the bug where toggling the unit only affected the display of dimensions and not the input itself, specifically in the object editing panels. It also adds keyboard-based dimensional input to the Wall tool during drafting, fully integrated into the new
nodesregistry refactor.Fixes #308
Specific changes:
measurements.tsutility to handle parsing and formatting between metric and imperial values (e.g., converting10'6"to meters). Exported viapackages/editor/src/index.tsx.SliderControl: UpdatedSliderControl(used in the Properties panels across all node kinds) to fetch the active unit state. It now properly formats values and parses imperial strings so that dimensions update correctly instead of silently reverting to metric.packages/nodes/src/wall/tool.tsx. While drawing a wall, users can now type dimensions (e.g.,10') and pressEnterto snap the wall length in the current pointer direction exactly.wall-toolor breaking existing architectural boundaries.How to test
15'2". Ensure the wall length updates accurately.10'(a text input overlay should appear near your cursor). PressEnterand verify the wall is created at exactly10'length in the direction of the cursor.Escapeto ensure it clears the text input without prematurely canceling the wall drawing itself.Screenshots / screen recording
Checklist
bun devbun checkto verify)mainbranch