Canvas: Add element snapping and alignment #80407
Conversation
nmarrs
added
add to changelog
area/panel/canvas
nmarrs
requested review from
leeoniya and
adela-almasan
and removed request for
a team
nmarrs
changed the title
docs/sources/panels-visualizations/visualizations/canvas/index.md
Outdated
Show resolved
Hide resolved
| @@ -426,9 +436,25 @@ export class Scene { | |||
| .on('dragStart', (event) => { | |||
| this.ignoreDataUpdate = true; | |||
| this.setNonTargetPointerEvents(event.target, true); | |||
|
|
|||
| if (this.moveable && this.moveable.elementGuidelines) { | |||
| const targetIndex = this.moveable.elementGuidelines.indexOf(event.target); | |||
There was a problem hiding this comment.
Nit: may not be worth it, but there's some duped code a few time here that we could maybe abstract out? Either way, this is awesome!
There was a problem hiding this comment.
ah yes that is a good point, a lot of repeated code here, thanks for pointing this out!
There was a problem hiding this comment.
on closer review breaking out this logic I think is not really in scope of these changes (honestly the scene.tsx file has been needing a cleaning up for a while now (ideally breaking out moveable / selecto config to their own file where we can better group helper functions specific to setting up config (and also cleaning up scene.tsx a ton)
For now I've included some comments to help with future readability / maintainability 🤞
|
I will take a look at the pan / zoom issues you were experiencing this week. |
|
Tested it locally and works great; however, as you mentioned, I am experiencing a couple issues during zoom, which I documented here: #81528 Because these appear to be caused by moveable itself, we will disable snappable during zoom for now until we can follow-up on this. |
Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>
| @@ -426,9 +436,25 @@ export class Scene { | |||
| .on('dragStart', (event) => { | |||
| this.ignoreDataUpdate = true; | |||
| this.setNonTargetPointerEvents(event.target, true); | |||
|
|
|||
| if (this.moveable && this.moveable.elementGuidelines) { | |||
| const targetIndex = this.moveable.elementGuidelines.indexOf(event.target); | |||
There was a problem hiding this comment.
on closer review breaking out this logic I think is not really in scope of these changes (honestly the scene.tsx file has been needing a cleaning up for a while now (ideally breaking out moveable / selecto config to their own file where we can better group helper functions specific to setting up config (and also cleaning up scene.tsx a ton)
For now I've included some comments to help with future readability / maintainability 🤞
| const scale = zoomPanPinchRef.state.scale; | ||
| scene.scale = scale; | ||
| updateMoveable(scale); | ||
| }; |
There was a problem hiding this comment.
@drew08t i've done some minor cleanup to these changes, would appreciate a quick set of 👀 to make sure I'm not doing anything strange 😬
docs/sources/panels-visualizations/visualizations/canvas/index.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>
|
Does this also include option to select multiple items and align them Left/Center/Right/Top/Middle/Bottom? |
Co-authored-by: drew08t <drew08@gmail.com> Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>
There are a few potentially low hanging fruit that we should take advantage of with improving canvas.
One of these is the snappable feature built into the moveable library. This PR pushes canvas further into a more matured product experience / helps smooth over its rougher edges.
Screen.Recording.2024-01-11.at.8.51.19.PM.mov
TODO