Skip to content

Vps 31/number input objects#444

Open
GogaIV wants to merge 28 commits into
masterfrom
VPS-31/number-input-objects
Open

Vps 31/number input objects#444
GogaIV wants to merge 28 commits into
masterfrom
VPS-31/number-input-objects

Conversation

@GogaIV

@GogaIV GogaIV commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

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

  • Acceptance criteria met
  • Wiki documentation is written and up to date
  • Unit tests written and passing
  • Integration tests written and passing
  • Continuous integration build passing

Summary by CodeRabbit

  • New Features
    • Added an Object Properties panel to edit component width, height, X/Y position, and rotation from the authoring sidebar, with live updates.
    • Added controls to flip components across the X or Y axis and handle negative width/height for intuitive resizing.
  • Bug Fixes
    • Improved sidebar component selection and syncing with the current visual selection.
    • Set line and speech components to be clickable by default, restoring expected interaction in authored scenes.
  • Chores
    • Updated the lucide-react dependency version.

@GogaIV
GogaIV requested review from harbassan and leowla July 12, 2026 07:46
@linear

linear Bot commented Jul 12, 2026

Copy link
Copy Markdown

VPS-31

@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@GogaIV, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 43 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f78d4ab9-fe1f-4c48-be2e-8ac86fa27868

📥 Commits

Reviewing files that changed from the base of the PR and between 1be895b and 13dd86c.

📒 Files selected for processing (1)
  • frontend/src/features/authoring/CanvasSideBar/ObjectPropertyEditor.jsx
📝 Walkthrough

Walkthrough

The 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.

Changes

Canvas object editing

Layer / File(s) Summary
Geometry editor implementation
frontend/src/features/authoring/CanvasSideBar/ObjectPropertyEditor.jsx, frontend/src/features/authoring/handlers/pointer/resize.ts, frontend/package.json
Adds controlled geometry inputs, validation, rotation and flip controls, and applies edits through translation and vertex mutations; upgrades lucide-react for the editor icons.
Sidebar component integration
frontend/src/features/authoring/CanvasSideBar/CanvasSideBar.jsx, frontend/src/features/authoring/CanvasSideBar/ComponentProperties.jsx
Reads the selected component from useVisualScene and renders ObjectPropertyEditor in the component properties panel.
Clickable component defaults
frontend/src/features/authoring/scene/operations/component.ts
Sets clickable: true for line and speech component defaults.

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
Loading

Possibly related PRs

  • UoaWDCC/VPS#417: Contains the same sidebar integration, object property editor, exported vertex helper, and clickable defaults.

Suggested reviewers: harbassan, hazikchaudhry, leowla

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The template sections are present, but Issue and Solution are too vague and mostly just point to another PR. Add a direct summary of the problem, the fix in this PR, the risks, and any completed tests instead of linking only to the old PR.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and matches the main change: numeric input objects.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch VPS-31/number-input-objects

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@GogaIV

GogaIV commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

In terms of requested changes ans coderabbits requests, all has been completed just awaiting final reviews before merge to main

@harbassan harbassan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@GogaIV
GogaIV requested a review from harbassan July 13, 2026 00:42
@GogaIV

GogaIV commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 5da0f70 and c1b6037.

📒 Files selected for processing (5)
  • frontend/src/features/authoring/CanvasSideBar/CanvasSideBar.jsx
  • frontend/src/features/authoring/CanvasSideBar/ComponentProperties.jsx
  • frontend/src/features/authoring/CanvasSideBar/ObjectPropertyEditor.jsx
  • frontend/src/features/authoring/handlers/pointer/resize.ts
  • frontend/src/features/authoring/scene/operations/component.ts

Comment thread frontend/src/features/authoring/CanvasSideBar/ObjectPropertyEditor.jsx Outdated

@leowla leowla left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (2)
frontend/src/features/authoring/CanvasSideBar/ObjectPropertyEditor.jsx (2)

166-202: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Inputs 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 win

Position deltas still computed from stale outer verts snapshot.

diff is derived from render-time component.bounds.verts, while modifyComponentProp invokes 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

📥 Commits

Reviewing files that changed from the base of the PR and between c1b6037 and 1be895b.

⛔ Files ignored due to path filters (1)
  • frontend/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (2)
  • frontend/package.json
  • frontend/src/features/authoring/CanvasSideBar/ObjectPropertyEditor.jsx

@GogaIV

GogaIV commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Honestly idk if anyone would know that negative val is to flip component unless told might not be worth it @harbassan
Other than that all requested changes are implemented

@GogaIV
GogaIV requested a review from leowla July 23, 2026 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants