GrapesJS / grapesjs

Free and Open source Web Builder Framework. Next generation tool for building templates without coding
https://grapesjs.com
BSD 3-Clause "New" or "Revised" License
22.36k stars 4.05k forks source link

BUG: Component tools misalign when the editor is resized #5705

Closed bernesto closed 7 months ago

bernesto commented 7 months ago

GrapesJS version

What browser are you using?

Chrome v122

Reproducible demo link

https://jsfiddle.net/Lk2bwhp4/

Describe the bug

How to reproduce the bug?

  1. Place the editor inside of a container, then programmatically resize the container's width.

What is the expected behavior? The editor and all of its children should resize accordingly.

What is the current behavior? All component's canvas spots in the editor retain their offsets from prior to the canvas resize.

image

Code of Conduct

artf commented 7 months ago

Thanks @bernesto for the report but as we can't detect programmatic layout changes without impacting the performances, in this case, you have to update the editor layout manually via editor.refresh() (eg. in your example you would put the call right at the end of your resizeEditor function)

bernesto commented 7 months ago

Hi @artf, I actually had this accidentally mixed in the last pull request you accepted (src/canvas/model/Frame.ts). You might want to take a look at what I added and see if it is causing an issue. We have it in testing right now and haven't seen a degradation in performance and it addresses the issue.

artf commented 7 months ago

I actually had this accidentally mixed in the last pull request you accepted

and I actually removed it cause it seemed to be unrelated 🤣

Not a problem to add it back but consider that your example will work only because of the desktop Device. The width is filling the container and that triggers the iframe window resize (which then will trigger the added event). It won't work in case you change to a smaller Device, in that case no resize event will be triggered so you still need the refresh call.

bernesto commented 7 months ago

LOL. Thanks for understanding @artf. So, just to clarify, this isn't related to the window resizing, but to the programatic resizing of a container that effects the size of the Iframe and needing to refresh the canvas spot locations. In theory, this should be device agnostic.

P.s. do you mind if I pm you on X. Doing a lot of core work in GrapesJS to address some edge case scenarios, make some performance and usability improvements? I have found myself asking "why are they doing this?" more than a few times, where there may a very good reason that I am not seeing, then and coming to the same conclusion after a lot of work. LMAO

artf commented 7 months ago

So, just to clarify, this isn't related to the window resizing, but to the programatic resizing of a container that effects the size of the Iframe and needing to refresh the canvas spot locations. In theory, this should be device agnostic.

Yeah I got it, but I'm not referring to the top window triggering the resize but the window inside the canvas iframe. I mean, placing your additional even trigger is fixing your demo, but not if you change the GrapesJS device to a smaller size.

Screenshot 2024-03-08 at 01 17 54

do you mind if I pm you on X. Doing a lot of core work in GrapesJS to address some edge case scenarios, make some performance and usability improvements? I have found myself asking "why are they doing this?" more than a few times, where there may a very good reason that I am not seeing, then and coming to the same conclusion after a lot of work. LMAO

Sure.