-
Notifications
You must be signed in to change notification settings - Fork 230
fix: right-click delete in lower part of board (#829) #893
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: right-click delete in lower part of board (#829) #893
Conversation
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughImports were extended to include 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
f29fa78 to
39aa20f
Compare
Sync selection to context menu position so Delete/Copy/Cut work everywhere.
39aa20f to
9c4f4ff
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/simulator/src/ux.js`:
- Around line 58-77: In syncSelectionToContextMenuPosition(), preserve the
previous simulationArea.mouseDown value before setting it to true, then call
update(globalScope, true), and finally restore simulationArea.mouseDown to the
saved value to avoid leaving the simulator in a forced “dragging” state; locate
the assignments to simulationArea.mouseDown in the function and wrap them with a
temporary variable (e.g., prevMouseDown) to save and restore around the update
call.
In `@v1/src/simulator/src/ux.js`:
- Around line 58-77: In syncSelectionToContextMenuPosition, save the current
simulationArea.mouseDown value to a temp variable before setting
simulationArea.mouseDown = true, call update(globalScope, true), and then
restore simulationArea.mouseDown to the saved value so the forced sync for
contextmenu does not leave a persistent "mouseDown" state; update the function
around the simulationArea.mouseDown assignments to perform this save/restore.
|
hey @Nihal4777 @tachyons please review the changes and merge them if possible thanks |
Nihal4777
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that when the context menu opens in the lower part of the canvas, it renders upward. However, the outside-click detection logic assumes the menu always opens downward (using ctxPos.y to ctxPos.y + offsetHeight). Because of this, clicks on menu items are incorrectly treated as clicks outside the menu, causing it to close before the item click handler runs.
Preserve simulationArea.mouseDown value in syncSelectionToContextMenuPosition() to avoid leaving the simulator in a forced 'dragging' state after right click operations. Changed variable name from wasMouseDown to prevMouseDown for clarity.
|
@Nihal4777 I have implemented the fix by changing wasMouseDown to prevMouseDown in both ux.js files. This ensures proper mouse state preservation during right click operations, preventing the persistent dragging state. The changes maintain consistency and resolve the lower board area issue. |
…nt propagation) - Fixed DPR coordinate calculation using window.devicePixelRatio instead of undefined global - Increased context menu z-index to 1000 to ensure clicks reach menu items above canvas - Added @mousedown.stop to prevent canvas deselection when clicking context menu The original PR code had an undefined DPR variable causing NaN coordinates. Additionally, the canvas was intercepting menu clicks and deselecting components before Delete could execute, especially in lower canvas where menu appeared far from the selected component.
|
Hi @Nihal4777 pls review this PR, changes are properly working. Screen.Recording.2026-02-05.at.3.35.46.AM.movthanks for you time |

What
Right click → Delete (and Copy/Cut) was not working when the component was in the lower part of the canvas. It worked in the upper part and keyboard Delete still worked everywhere.
Why
Selection for the context menu actions was coming from the mousedown that opened the menu. In the lower part of the board that selection wasn’t reliable, so
lastSelectedwas wrong or missing when the user chose Delete.How
Before showing the context menu we now sync selection to the right click position: convert that position to canvas/simulation coords (same as
panStartin listeners), set the relevantsimulationAreamouse fields, and runupdate(globalScope, true)so the element under the cursor becomeslastSelected. That way Delete/Copy/Cut always target the element the user right-clicked on, no matter where it is on the canvas.Closes #829
Summary by CodeRabbit
Bug Fixes
Style
Behavior Changes