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

Widget width reported incorrectly by save() after dragging from wider nested grid #2394

Closed
akosfekete opened this issue Jul 17, 2023 · 2 comments · Fixed by #2461 or #2473
Closed

Widget width reported incorrectly by save() after dragging from wider nested grid #2394

akosfekete opened this issue Jul 17, 2023 · 2 comments · Fixed by #2461 or #2473
Labels

Comments

@akosfekete
Copy link
Contributor

Subject of the issue

Widget width is incorrect in JSON produced by the save() method, if the given widget has been dragged from a nested grid that has more columns than the outer grid. The grid and widgets behave correctly, the problem is only with data generated by save(). Loading back this data into the grid will cause an incorrect state (since the JSON contains an impossible layout)

The save() method in gridstack-engine.ts takes the width value from this._layouts if present. The resized node's actual width seems correct, the incorrect value seems to come from this._layouts
https://github.com/gridstack/gridstack.js/blob/89ae9d6fd163137c5fde895ae14d1c5d2bb7a495/src/gridstack-engine.ts#L746C7-L746C7
image

Your environment

  • version of gridstack.js - 8.2.3
  • which browser/OS - Firefox 116 / Chrome 114, Windows

Steps to reproduce

  1. Drag a widget from a grid with 2 columns into a nested grid with 3 columns
  2. Resize the widget to be as wide as possible
  3. Drag the widget back into the main grid from the nested grid
  4. Resize the widget
    The widget will have its width reported incorrectly in the JSON data generated by save()

https://jsfiddle.net/akosfekete/Lpv2au4w/11/
firefox_RNua2gRDXP

Expected behavior

The width should be the same as it appears in the layout.

@adumesny
Copy link
Member

adumesny commented Jul 17, 2023

ha, good catch. ._layouts has higher resolution column count so we tend to save from that (say 12 -> 1 then save should remember the 12 column full layout not 1 column layout). the bug here is that when moving an item to another grid (parent) we don't delete those entries. I'm actually surprised we use _layouts from another grid essentially...

@akosfekete
Copy link
Contributor Author

akosfekete commented Sep 13, 2023

The issue seems to stem from nodeBoundFix (), where the layout is cached as you mentioned: https://github.com/gridstack/gridstack.js/blob/768b8ef6c508c6c47ad44c0748f0f11405da4716/src/gridstack-engine.ts#L398C37-L398C37

The comment at line 389 mentions my use case (the "target" grid has fewer columns than the "source" nested grid). I created a fork where I remove the added nodes from the layout cache when an add event is triggered. I'm not sure if this could cause issues in other use cases, but the width is reported correctly for me after this.
https://github.com/gridstack/gridstack.js/compare/master...akosfekete:gridstack.js:2394_fix?expand=1

adumesny added a commit to adumesny/gridstack.js that referenced this issue Sep 27, 2023
* fix gridstack#2394]
* better fix that makes sure we keep wanted size when an item is dropped on us (so we remember in case we change grid column)
* also changed to not save x,y during dropping since the drag behavior will figure where to put it, and that means we don't cache that layout that might not apply to the new grid anyway.
* added a running example showing issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants