GrapesJS / grapesjs

Free and Open source Web Builder Framework. Next generation tool for building templates without coding
https://grapesjs.com
Other
22.69k stars 4.11k forks source link

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 bt-mkt closed 2 weeks ago

bt-mkt commented 4 weeks ago

GrapesJS version

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

bt-mkt commented 3 weeks ago

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

artf commented 2 weeks ago

@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 commented 2 weeks ago

@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

bt-mkt commented 2 weeks ago

@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 commented 2 weeks ago

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

bt-mkt commented 2 weeks ago

@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.