Move gizmo: keyboard controls#556
Conversation
📝 WalkthroughWalkthroughThis PR centralizes gizmo interaction logic in Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/gizmos.js`:
- Around line 502-518: pickMeshFromScene creates both keyboard picking and a
pointer observer but the abort/mouse/keyboard paths each clean up different
pieces, leaving the pointer observer or keyboard mode active; centralize cleanup
into a single function (either extend exitGizmoState or add cleanupScenePick)
that: stops keyboard picking (stopAxisKeyboard/clear its variable), removes the
pointer observer (the pointer observer variable used in pickMeshFromScene),
removes positionGizmoObserver via
gizmoManager.onAttachedToMeshObservable.remove(positionGizmoObserver) and clears
positionGizmoObserver, removes active classes, disables gizmos and resets
cursor; then call this single cleanup from pickMeshFromScene’s mouse-pick
handler, keyboard-pick handler, and any abort/cancel paths so no stale observers
remain.
- Around line 593-598: The early return when the gizmo button is active skips
teardown for duplicate-mode, leaving activeDuplicatePickHandler and its window
click listener armed; before calling exitGizmoState() (in the block checking
button.classList.contains("active")), run the same duplicate-mode cleanup that's
done later (remove/clear activeDuplicatePickHandler and remove the window click
listener) or refactor so duplicate teardown is moved out of disableGizmos() into
explicit handlers (e.g., toggleGizmo() and Escape handler) and invoke that
teardown here to ensure no duplicate placement remains active.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Summary
Add keyboard controls for move gizmo
When the move gizmo is selected:
Arrow keys plus page up/page down
X, Y or Z to fix on an axis, then use arrow keys to move only on that axis
Known issues
Summary by CodeRabbit