Vps 31/number input objects#444
Conversation
Co-authored-by: Leo Wang <git@leowla.com>
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe authoring sidebar now reads selected components from visual-scene state and includes an object property editor for position, dimensions, rotation, and axis flips. Geometry updates use vertex mutations, and line and speech defaults are now clickable. ChangesCanvas object editing
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CanvasSideBar
participant ComponentProperties
participant ObjectPropertyEditor
participant modifyComponentProp
participant modifyVerts
CanvasSideBar->>ComponentProperties: provide selected component
ComponentProperties->>ObjectPropertyEditor: render geometry controls
ObjectPropertyEditor->>modifyComponentProp: apply position, size, rotation, or flip
modifyComponentProp->>modifyVerts: update component vertices
modifyVerts-->>ObjectPropertyEditor: updated component bounds
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
In terms of requested changes ans coderabbits requests, all has been completed just awaiting final reviews before merge to main |
harbassan
left a comment
There was a problem hiding this comment.
nice, everything looks good and seems to works well except for the number inputs acting wierd on backspace press:
Virtual.Patient.System.-.UoA.Mozilla.Firefox.2026-07-12.22-28-52.mp4
|
the unable to fully backspace thing was cause i initially had it set to that val to 0 which deletes the component so i changed it to 10 thats why it was changing to 1, i changed it to retain its value but just show empty in the field |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/features/authoring/CanvasSideBar/ComponentProperties.jsx`:
- Line 47: Update the component-selected rendering flow around the
!component?.clickable guard so ObjectPropertyEditor renders whenever a component
is selected, including non-clickable components. Keep only the link and state
controls conditional on component.clickable, and preserve the existing behavior
for no selected component.
In `@frontend/src/features/authoring/CanvasSideBar/ObjectPropertyEditor.jsx`:
- Around line 14-22: Update the dimension write logic in ObjectPropertyEditor to
preserve each axis’s current orientation: derive the signed direction from the
existing bounds vertices and apply it when calculating the new endpoint from the
edited width or height. Keep the absolute values used for displaying input
dimensions, and update both width and height handlers so flipped bounds remain
flipped.
- Around line 95-128: Add id/htmlFor associations between the Position X and
Position Y labels and their corresponding controls in ObjectPropertyEditor, and
change those controls to semantic numeric inputs using type="number". Preserve
the existing value and saveProp behavior for both fields.
- Around line 47-56: Update saveProp to reject partial or non-finite numeric
input before calling set: validate the complete trimmed value rather than
relying on parseFloat, require Number.isFinite, and reject negative values when
editing width or height. Preserve empty-string handling, and provide visible
validation feedback for rejected input if the editor’s existing validation
mechanism supports it.
- Around line 58-67: Update the x and y branches in the bounds-vertex update
flow to calculate each translation delta from the updater callback’s current
`prev` vertices, using the target value minus `prev[0].x` or `prev[0].y`. Remove
the render-time `component.bounds.verts`-based delta calculation while
preserving the existing translate behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3482747b-2041-4de7-b9f2-76612fb7eb3c
📒 Files selected for processing (5)
frontend/src/features/authoring/CanvasSideBar/CanvasSideBar.jsxfrontend/src/features/authoring/CanvasSideBar/ComponentProperties.jsxfrontend/src/features/authoring/CanvasSideBar/ObjectPropertyEditor.jsxfrontend/src/features/authoring/handlers/pointer/resize.tsfrontend/src/features/authoring/scene/operations/component.ts
There was a problem hiding this comment.
CleanShot.2026-07-13.at.15.42.37.mp4
- Unable to delete negative number
- Unable to enter negative number, expected to allow
-as first character - Unable to enter decimal number, expected to allow
.after a whole number - Not working for line and speech box
- Every digit edit gets saved so I need to undo multiple times to get back to my original number
it was outdated, i needed an icon from 0.8
…all digits inside input box, can enter 0 on some inputs
…ot and it makes it easier to backtrack when all the data is in a class
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
frontend/src/features/authoring/CanvasSideBar/ObjectPropertyEditor.jsx (2)
166-202: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInputs still lack semantic numeric type/label association.
Width/Height/Position X/Y remain plain text inputs with visible labels not linked via
id/htmlFor, so keyboard/mobile numeric affordances and assistive-technology association are missing. Same as the previously flagged concern.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/features/authoring/CanvasSideBar/ObjectPropertyEditor.jsx` around lines 166 - 202, Update the width, height, Position X, and Position Y inputs in ObjectPropertyEditor to use type="number" and assign unique id values. Link each visible label to its corresponding input with matching htmlFor attributes, preserving the existing value and onChange behavior.
128-137: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPosition deltas still computed from stale outer
vertssnapshot.
diffis derived from render-timecomponent.bounds.verts, whilemodifyComponentPropinvokes the updater with the current vertex state. If a drag or other update lands between render and this edit, the object jumps to the wrong position. This is unchanged from the previously flagged issue.Suggested fix
if (type === "x") { - const diff = value - verts[0].x; modifyComponentProp(component.id, "bounds.verts", (prev) => - translate(prev, { x: diff, y: 0 }) + translate(prev, { x: value - prev[0].x, y: 0 }) ); } else if (type === "y") { - const diff = value - verts[0].y; modifyComponentProp(component.id, "bounds.verts", (prev) => - translate(prev, { x: 0, y: diff }) + translate(prev, { x: 0, y: value - prev[0].y }) );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/features/authoring/CanvasSideBar/ObjectPropertyEditor.jsx` around lines 128 - 137, Update the x/y position handling in the component property edit flow so each updater computes its delta from the current vertices passed to modifyComponentProp, rather than the render-time verts snapshot. Preserve the existing translation behavior while ensuring concurrent updates use the latest first vertex coordinates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/features/authoring/CanvasSideBar/ObjectPropertyEditor.jsx`:
- Around line 82-117: Update inputValidation and parseInput to reject incomplete
or non-finite numeric strings before calling set: validate the trimmed input as
a complete numeric representation rather than relying on parseFloat alone, and
require the parsed value to be finite. Preserve the existing empty/"-" handling,
zero normalization, and negativeFlip behavior.
- Around line 82-117: Update inputValidation and parseInput so controlled inputs
retain the raw trimmed text, including a trailing decimal point, by passing the
original string to set instead of the parsed number. Continue using the
separately parsed value for zero handling, negativeFlip, and geometry
calculations, while preserving empty and "-" input behavior.
- Around line 63-80: Update negativeFlip and the width/height branches of
saveProp to compute endpoint changes from the updater callback’s prev vertices,
preserving each axis’s signed direction rather than always growing from prev[0].
Keep the flipped orientation and fixed anchor intact when editing negative
dimensions, so repeated edits change only the size without shifting the box
position.
---
Duplicate comments:
In `@frontend/src/features/authoring/CanvasSideBar/ObjectPropertyEditor.jsx`:
- Around line 166-202: Update the width, height, Position X, and Position Y
inputs in ObjectPropertyEditor to use type="number" and assign unique id values.
Link each visible label to its corresponding input with matching htmlFor
attributes, preserving the existing value and onChange behavior.
- Around line 128-137: Update the x/y position handling in the component
property edit flow so each updater computes its delta from the current vertices
passed to modifyComponentProp, rather than the render-time verts snapshot.
Preserve the existing translation behavior while ensuring concurrent updates use
the latest first vertex coordinates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0099c374-a240-438e-a470-ccef7ef39a7e
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
frontend/package.jsonfrontend/src/features/authoring/CanvasSideBar/ObjectPropertyEditor.jsx
|
Honestly idk if anyone would know that negative val is to flip component unless told might not be worth it @harbassan |
Issue
Repeated PR for accidental merged branch, awaiting reviews.
Old PR: #417
Solution
#417
Risk
Only risk is me using github on my phone ✌️
Checklist
Summary by CodeRabbit
lucide-reactdependency version.