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

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

Open
quick691fr opened this issue Oct 16, 2024 · 8 comments
Open

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

quick691fr opened this issue Oct 16, 2024 · 8 comments
Labels

Comments

@quick691fr
Copy link

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
Copy link

dylemma commented Oct 16, 2024

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
Copy link
Author

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

if (this.prevWidth === this.el.clientWidth) return; // no-op
in order to test if it's a scrollbar or a real resize of gridstack container.

I hope @adumesny will get a clue.

@adumesny
Copy link
Member

adumesny commented Oct 16, 2024

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
Copy link
Author

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
Copy link

dylemma commented Oct 16, 2024

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

@adumesny
Copy link
Member

adumesny commented Oct 16, 2024

I don't know React but there is a Gridstack.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 thing when drag and dropping from sidebar, between grids, etc... the static content is really for demos, not real app IMO. The Angular demo uses a custom selector: string to actually create the right component stored in in the widget definition (instead of content)

@quick691fr
Copy link
Author

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.

So I guess, this is not going to be fixed soon.
Can you explain how is working this addRemoveCB, I don't see it in the API documentation.

@adumesny
Copy link
Member

adumesny commented Nov 4, 2024

I updated my comment. you need to look at the provide angular code... plain JS demos don't use it, just dummy content field.

If you want the issue fixed, a donation would help prioritize it up, or create a fix that tracks when scrollbar show/disapear and ignore size changed in that case.

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

No branches or pull requests

3 participants