gridstack / gridstack.js

Build interactive dashboards in minutes.
https://gridstackjs.com
MIT License
6.79k stars 1.29k forks source link

OnChange event not triggered correctly due to scrollbar show up #2823

Open quick691fr opened 2 weeks ago

quick691fr commented 2 weeks ago

Subject of the issue

OnChange event not triggered correctly when scrollbar shows up after dragging a widget on bottom of grid.

It seems coming from the scrollbar which modify the clientWidth and then trigger a BatchUpdate on onResize method since prevWidth !== el.clientWidth (of course the scrollbar shows up). the saveInitial methof then erase previous _orig and _dirty flag.

Your environment

gridstack.js v 10.3.1 Brave v1.70.126

Steps to reproduce

https://jsfiddle.net/drp0jbf5/41/

If you move pink item 3 half under cyan 4 with both right side of blocks aligned, or move pink item 3 under all other to the side of orange block item 2 then the onChange event ifs not triggered.

chrome-extension-nlipoenfbbikpbjkfpfillcgkoblgpmj-setup-react-html-from-install.webm

Expected behavior

The onChangeEvent should be triggered on every move of a widget.

dylemma commented 2 weeks ago

Holy moly, the timing of this report! I was just trying out this library and debugging why my change handler wasn't being called, and sure enough, it was this.

quick691fr commented 2 weeks ago

I found this few days ago and just did the report about it.

I think it's not simple to fix since this is coming from an external behavior. I didn't deep dive into the code so don't know exactly how to fix it for now.

My guess is a special test addition to https://github.com/gridstack/gridstack.js/blob/6f2fce414baa86bfc7b45d15c435819f671aec25/src/gridstack.ts#L1772 in order to test if it's a scrollbar or a real resize of gridstack container.

I hope @adumesny will get a clue.

adumesny commented 2 weeks ago

hey thanks for this report. curious what onChange critical work do you depend on ? Also I would recommend using grid.load() (or pass in init() rather than using DOM attributes since not everything is gs- and you need to serialize your grid edits anyway. JSON is much better.

quick691fr commented 2 weeks ago

Hey @adumesny,

I did this simple code example but I'm using gridstack in React and use the following code to load my widgets after put them in the DOM of a component

grid.batchUpdate();
grid.removeAll();
items.forEach((item) => {
  if (item.id) grid.makeWidget(item.id.toString());
});
grid.batchUpdate(false);

I'm using DOM since my gridstack items are containing custom React components and I need them to be loaded by React instead of using the content property which is static.

I rely on onChange event to get moved items and save the position / size in a database.

I did see that there were an example of React gridstack using Context and so in the repository but didn't look at so much since I was able to make my things worked without.

dylemma commented 2 weeks ago

My usage was similar... https://jsfiddle.net/4qu6p9bt/ - React + localStorage so the grid layout could be restored from a page load

adumesny commented 2 weeks ago

I don't know React but there is a Grdistack.addRemoveCB that I use for Angualr to create the items dynamically for JSON and works with complex grids (mutiple, nested, etc...) much much better than DOM + init/makeWidget() because it lets GS do it's things when drag and dropping form sidebar, between grids, etc... the static content is really for demos, not real app IMO.