Skip to content
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

Take canvas offsets into account when dropping a new block on the canvas. #4387

Merged
merged 3 commits into from
Jun 18, 2022

Conversation

contentfree
Copy link
Contributor

Relates to #2824 / #3770

I'm not sure this is how you'll want to solve this globally, @artf, but it does seem to fix the dropping of blocks in designer mode on a narrow canvas. I believe there's similar issues with the initial position of the dragged element: Canvas.getMouseRelativeCanvas doesn't correctly take into account the canvas offset but I think the main cause may be CanvasView.getPosition and the way it combines frame offset & canvas offset in a strange way.

@contentfree
Copy link
Contributor Author

Also, I'm a grapesjs newb and it's likely there's a more elegant way to solve this more broadly.

Copy link
Member

@artf artf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess there is a lot of stuff to check/improve for the absolute mode but at least I see how it fixes the current issue, so at the moment is better than nothing.

Thank you 🙇

@artf artf merged commit 6712e58 into GrapesJS:dev Jun 18, 2022
@contentfree
Copy link
Contributor Author

contentfree commented Jun 18, 2022 via email

@artf
Copy link
Member

artf commented Jun 20, 2022

Honestly, I'm not sure if this is something I'd suggest to someone new to GrapesJS 😅
There are different places where the D&D of components can start (from blocks, inside the iframe, native HTML5 D&D outside the editor) and in each case, there are different constraints and you have to make it work in different modes (absolute/translate/standard).

@contentfree
Copy link
Contributor Author

Strangely, this line const canvasOffset = editor.Canvas.getOffset(); works for me locally but doesn't once incorporated to grapesjs. I might need to push an update to you that does something like em.get("Canvas") instead.

@artf
Copy link
Member

artf commented Aug 2, 2022

@contentfree you're right, my bad I didn't notice it. I'll push a fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants