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

BUG: Component model and view inits every time it's dragged over every component present in Canvas on which it can be dropped into #6269

Closed
2 tasks done
bt-mkt opened this issue Oct 25, 2024 · 6 comments · Fixed by #6297

Comments

@bt-mkt
Copy link

bt-mkt commented Oct 25, 2024

GrapesJS version

  • I confirm to use the latest version of GrapesJS

What browser are you using?

Chrome Version 130.0.6723.59 (Official Build) (arm64)

Reproducible demo link

https://codepen.io/btmkt/pen/QWeOJRB?editors=1111

Describe the bug

How to reproduce the bug?

  1. Open console.
  2. Drop multiple components inside the canvas(easy to reproduce with 3 columns for example).
  3. Drag the test-comp component added at the end in the extra category over the added components.

What is the expected behavior?
The component model and view init events (at least, not sure if there are others) should trigger only when dropping the component into the Canvas and not every time it's dragged above a component in which it can be dropped.

What is the current behavior?
The component model and view init events are triggered every time you drag it over a component it can be dropped into, and twice when dropped. This causes a lot of issues especially if for example you trigger a modal open( it will add as many modals, if not conditioned, as many times init is triggered).

I've created a pen for v0.21.13 (https://codepen.io/btmkt/pen/OJKOrPQ?editors=1111) also so you can see the difference, same steps to reproduce the issue apply.
I am currently on v0.21.10 which works the same as v0.21.13, from my tests at least, and would like to bump to v.0.22.1, but can't due to this new behaviour.

Please let me know if you need any other information.
Thank you.

Code of Conduct

  • I agree to follow this project's Code of Conduct
@bt-mkt
Copy link
Author

bt-mkt commented Oct 29, 2024

@artf any chance you could have a look at this? Thank you.

@artf
Copy link
Member

artf commented Nov 5, 2024

@mohamedsalem401 this is probably happening due to the shallow editor.
I think it would make sense to leverage the temporary option (which we already use to skip some actions) in canMove. wdyt?

const wrapper = this.getShallowWrapper();
srcModel = wrapper?.append(source, { temporary: true })[0] || null;

@mohamedsalem401
Copy link
Member

@artf
Yes, I suspect it has to do with the shallow editor. Hopefully the temporary option will work and then I'll open a PR

@artf artf linked a pull request Nov 5, 2024 that will close this issue
@artf artf closed this as completed in #6297 Nov 5, 2024
@bt-mkt
Copy link
Author

bt-mkt commented Nov 8, 2024

@mohamedsalem401 this is probably happening due to the shallow editor. I think it would make sense to leverage the temporary option (which we already use to skip some actions) in canMove. wdyt?

const wrapper = this.getShallowWrapper();
srcModel = wrapper?.append(source, { temporary: true })[0] || null;

Hello @artf,

The change above, which I saw was implemented, doesn't seem to fix the behaviour I encountered. It's still the same as in version 0.22.1.
I've updated the pen to 0.22.3 (https://codepen.io/btmkt/pen/QWeOJRB?editors=1111) and I've also ran some tests in my app which confirms the above.

Thank you. Hope you will give it another try.

@mohamedsalem401
Copy link
Member

@bt-mkt
The fix will be released in the next version. The fix was added after V 0.22.3.

@bt-mkt
Copy link
Author

bt-mkt commented Nov 8, 2024

@mohamedsalem401 Understood. I assumed it was included when I saw it was closed the same day of the last release, should have checked the release to see if the pr was indeed included.

Thank you.

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 a pull request may close this issue.

3 participants