Prefer calculated anchor offsets when moving#553
Prefer calculated anchor offsets when moving#553rushabhcodes wants to merge 1 commit intotscircuit:mainfrom
Conversation
Always show computed Δx/Δy while a component is moving or when stored offsets don't match the current position (0.01mm tolerance). Add isMovingComponent state and selection logic for displayed labels.
rushabhcodes
requested review from
ShiboSoftwareDev and
seveibar
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR enhances the anchor offset overlay display logic in both BoardAnchorOffsetOverlay and GroupAnchorOffsetOverlay components to provide more accurate real-time feedback when components are being moved. The changes ensure that calculated offsets are always displayed during component movement and when stored offsets don't match the current position, improving user experience during interactive operations.
Key changes:
- Added
isMovingComponentstate subscription to both overlay components to detect when a component is being dragged - Implemented tolerance-based comparison logic to determine when calculated offsets should be displayed instead of stored values
- Fixed a pre-existing bug in
BoardAnchorOffsetOverlaywhere the Y-axis label was incorrectly referencing the X offset value
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/components/AnchorOffsetOverlay/Board/index.tsx | Added logic to prefer calculated offsets during component movement and when stored offsets are out of sync; includes fix for Y-label bug |
| src/components/AnchorOffsetOverlay/Group/index.tsx | Added identical logic to prefer calculated offsets during component movement and when stored offsets are out of sync |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Always show calculated offset when component is being moved or when stored offset doesn't match current position | ||
| const storedOffsetX = displayOffsetX | ||
| ? parseFloat(displayOffsetX) | ||
| : null | ||
| const storedOffsetY = displayOffsetY | ||
| ? parseFloat(displayOffsetY) | ||
| : null | ||
| const offsetMatchesStored = | ||
| storedOffsetX !== null && | ||
| storedOffsetY !== null && | ||
| Math.abs(offsetX - storedOffsetX) < 0.01 && // Allow small tolerance for floating point precision | ||
| Math.abs(offsetY - storedOffsetY) < 0.01 | ||
|
|
||
| const shouldUseCalculatedOffset = | ||
| isMovingComponent || | ||
| !displayOffsetX || | ||
| !displayOffsetY || | ||
| !offsetMatchesStored |
There was a problem hiding this comment.
The conditions !displayOffsetX and !displayOffsetY in shouldUseCalculatedOffset are redundant with the !offsetMatchesStored check, since offsetMatchesStored already requires both stored offsets to be non-null. Consider simplifying the logic to just check isMovingComponent || !offsetMatchesStored for better maintainability.
| // Always show calculated offset when component is being moved or when stored offset doesn't match current position | ||
| const storedOffsetX = displayOffsetX | ||
| ? parseFloat(displayOffsetX) | ||
| : null | ||
| const storedOffsetY = displayOffsetY | ||
| ? parseFloat(displayOffsetY) | ||
| : null | ||
| const offsetMatchesStored = | ||
| storedOffsetX !== null && | ||
| storedOffsetY !== null && | ||
| Math.abs(offsetX - storedOffsetX) < 0.01 && // Allow small tolerance for floating point precision | ||
| Math.abs(offsetY - storedOffsetY) < 0.01 | ||
|
|
||
| const shouldUseCalculatedOffset = | ||
| isMovingComponent || | ||
| !displayOffsetX || | ||
| !displayOffsetY || | ||
| !offsetMatchesStored |
There was a problem hiding this comment.
The conditions !displayOffsetX and !displayOffsetY in shouldUseCalculatedOffset are redundant with the !offsetMatchesStored check, since offsetMatchesStored already requires both stored offsets to be non-null. Consider simplifying the logic to just check isMovingComponent || !offsetMatchesStored for better maintainability.
| Math.abs(offsetX - storedOffsetX) < 0.01 && // Allow small tolerance for floating point precision | ||
| Math.abs(offsetY - storedOffsetY) < 0.01 |
There was a problem hiding this comment.
The tolerance value 0.01 is a magic number that appears in both Board and Group overlay components. Consider extracting it to a named constant in VISUAL_CONFIG or a shared constants file for better maintainability and consistency.
| Math.abs(offsetX - storedOffsetX) < 0.01 && // Allow small tolerance for floating point precision | ||
| Math.abs(offsetY - storedOffsetY) < 0.01 |
There was a problem hiding this comment.
The tolerance value 0.01 is a magic number that appears in both Board and Group overlay components. Consider extracting it to a named constant in VISUAL_CONFIG or a shared constants file for better maintainability and consistency.
seveibar
left a comment
seveibar
left a comment
There was a problem hiding this comment.
I dont like the specific handling of moving components, i think we’re slowly going to deprecate moving components so i dont want to add a bunch of code for it
This pull request updates the anchor offset overlays for both board and group components to ensure that the displayed offset labels always reflect the most accurate position, especially when a component is being moved or when the stored offset does not match the current position. The logic for determining when to show the calculated offset versus the stored offset has been improved for better accuracy and user feedback.
Enhancements to anchor offset overlay display:
BoardAnchorOffsetOverlayandGroupAnchorOffsetOverlaynow check if a component is being moved (isMovingComponent) and always show the calculated offset if so, or if the stored offset is missing or out of sync with the current position. [1] [2]Before
Screencast_20251217_010334.mp4
After
Screencast_20251217_010243.mp4