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

RemoveWidget() doesn't also trigger change events when it should #1142

Closed
ronvanpol opened this issue Feb 10, 2020 · 5 comments · Fixed by #1148
Closed

RemoveWidget() doesn't also trigger change events when it should #1142

ronvanpol opened this issue Feb 10, 2020 · 5 comments · Fixed by #1148
Labels

Comments

@ronvanpol
Copy link

ronvanpol commented Feb 10, 2020

The function RemoveWidget should trigger change events in case that other nodes are changed. For example take the simple case where one has a simple grid containing to adjacing nodes 1 and 2. When node 1 is removed by using the RemoveWidget function, node 2 moves to the place of node 1. This is a change for node 2 and as such it should trigger the change event.

I see in the code that the triggering of this change event is commented out in several places. Why was the decision made not to trigger the change event anymore and for what reason?

@ronvanpol ronvanpol changed the title RemoveWidget and AddWidget should trigger change events RemoveWidget should trigger change events Feb 10, 2020
@adumesny
Copy link
Member

adumesny commented Feb 10, 2020

@ronvanpol do you have an example where the change event doesn't get called when it should ?

I commented some of the change events code because it was calling it when adding/removing a node that had NO OTHER changes to siblings, so only when x,y,w,h is changed should it now trigger (still does call it more often than absolutely necessary but tons better than before, especially when loading 5 widgets we used to get called 5 times with an ever increasing number).

if you run https://gridstackjs.com/demo/two.html and open the console you will see that removing a node in the float=false grid that causes re-ordering WILL call remove+change, in the float=true, just remove.

Please re-open if you can provide a test. use the default template https://jsfiddle.net/adumesny/jqhkry7g. thank you!

@ronvanpol
Copy link
Author

This bug seems to be related to nested grids. I have prepared a jsfiddle. Click on nested items "1" or "2" and the log will only show the removed events and no change events.

https://jsfiddle.net/a7bo92vn/

@adumesny
Copy link
Member

adumesny commented Feb 10, 2020

thanks, I can see the issue now. will take a look.

@adumesny adumesny reopened this Feb 10, 2020
@adumesny adumesny changed the title RemoveWidget should trigger change events nested grid: RemoveWidget doesn't also trigger change events when it should Feb 10, 2020
@adumesny
Copy link
Member

adumesny commented Feb 10, 2020

turns out you don't need nested grid, this simpler example shows the issue as well
https://jsfiddle.net/adumesny/znqx4dmr/

@adumesny adumesny changed the title nested grid: RemoveWidget doesn't also trigger change events when it should RemoveWidget() doesn't also trigger change events when it should Feb 10, 2020
@adumesny
Copy link
Member

adumesny commented Feb 16, 2020

fixed in next release 0.6.4

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

Successfully merging a pull request may close this issue.

2 participants