-
Notifications
You must be signed in to change notification settings - Fork 150
Fix: Dragging Panels Moving Outside Canvas & Ensure Proper Z-Index #540
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?
Conversation
WalkthroughThe changes adjust the z-index values used during the drag event in the dragging function. Both the main and the versioned files modify the z-index for the draggable element and non-drag elements from 99 to 101, ensuring the stacking order is modified during a mouse down event. No modifications were made to the exported or public entities. Changes
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
cc : @vedant-jain03 @niladrix719 PTAL! |
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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: 0
🧹 Nitpick comments (2)
v0/src/simulator/src/drag.ts (1)
68-71
: Existing constraint properly prevents elements from moving outside the visible areaThe
restriction: 'body'
constraint already ensures that draggable elements remain within the body element, addressing the issue of panels moving outside the canvas. This keeps them accessible to users at all times.However, for better clarity, consider adding a comment that explains how this addresses the specific issue mentioned in the PR:
interact.modifiers.restrictRect({ // Restrict the draggable element within its parent container + // Prevents draggable panels from moving outside the visible area restriction: 'body', }),
src/simulator/src/drag.ts (1)
68-71
: Existing constraint properly prevents elements from moving outside the visible areaThe
restriction: 'body'
constraint already ensures that draggable elements remain within the body element, addressing the issue of panels moving outside the canvas. This keeps them accessible to users at all times.However, for better clarity, consider adding a comment that explains how this addresses the specific issue mentioned in the PR:
interact.modifiers.restrictRect({ // Restrict the draggable element within its parent container + // Prevents draggable panels from moving outside the visible area restriction: 'body', }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/simulator/src/drag.ts
(1 hunks)v0/src/simulator/src/drag.ts
(1 hunks)
🔇 Additional comments (2)
v0/src/simulator/src/drag.ts (1)
76-77
: Z-index change addresses elements appearing behind the navbar and properties panelThe increase of the z-index value from 99 to a higher value of 101 ensures that draggable panels will be displayed above other UI elements like the navbar and properties panel, which typically have z-index values of 100 or lower. This change successfully addresses one of the key issues mentioned in the PR objectives.
The z-index is properly applied to both the active dragged element and other draggable panels to maintain a consistent UI hierarchy during drag operations.
src/simulator/src/drag.ts (1)
76-77
: Z-index change addresses elements appearing behind the navbar and properties panelThe increase of the z-index value from 99 to a higher value of 101 ensures that draggable panels will be displayed above other UI elements like the navbar and properties panel, which typically have z-index values of 100 or lower. This change successfully addresses one of the key issues mentioned in the PR objectives.
The z-index is properly applied to both the active dragged element and other draggable panels to maintain a consistent UI hierarchy during drag operations.
Fixes #506
Describe the changes you have made in this PR -
This PR addresses two issues related to draggable elements like Testbench and Timing Diagram in the simulator:
Elements Moving Outside the Canvas:
Z-Index Issue:
This Ensure synchronous behaviour with circuitverse legacy simulator.
Screenshots of the changes (If any) -
Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.
Summary by CodeRabbit