better fix: nested grid size issue

* 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.
WebMaid · Sep 27, 2023 · aaf4773 · aaf4773
1 parent 1ff1f42
commit aaf4773
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 9 deletions.
diff --git a/doc/CHANGES.md b/doc/CHANGES.md
@@ -103,6 +103,7 @@ Change log
 ## 9.2.1-dev (TBD)
 * fix - sub-grid styles now look for immediate correct parent, not any depth above.
 * fix [#2469](https://github.com/gridstack/gridstack.js/issues/2469) "Invalid height" error CSS minHeight
+* fix [#2394](https://github.com/gridstack/gridstack.js/issues/2394) nested grid size issue when sub-items moved up/down
 
 ## 9.2.1 (2023-09-20)
 * fix _updateContainerHeight() to use height rather than min-height again (apart for nested grids which need it) and partial getComputedStyle CSS minHeight support

diff --git a/spec/e2e/html/2394_save_sub_item_moved.html b/spec/e2e/html/2394_save_sub_item_moved.html
@@ -0,0 +1,60 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="utf-8">
+  <meta http-equiv="X-UA-Compatible" content="IE=edge">
+  <meta name="viewport" content="width=device-width, initial-scale=1">
+  <title>#2394 Save sub item moved</title>
+
+  <link rel="stylesheet" href="../../../demo/demo.css" />
+  <link rel="stylesheet" href="../../../dist/gridstack-extra.min.css"/>
+  <script src="../../../dist/gridstack-all.js"></script>
+
+</head>
+<body>
+  <h1>#2394 Save sub item moved</h1>
+  <button onClick="print()">Save</button>
+  <!-- <button onClick="reset()">Reset</button> -->
+  <button onClick="load()">Load</button>
+  <br><br>
+  <div class="grid-stack"></div>
+
+  <script type="text/javascript">
+    let subGridOpts = {
+        cellHeight: 50,
+        column: 3,
+        padding: 5,
+        minRow: 2, // don't collapse when empty
+        disableOneColumnMode: true,
+        acceptWidgets: true,
+        children: [ {id:0, x:0, y:0, w:3, content:'move to parent grid col=2'}]
+      };
+
+    var options = { // put in gridstack options here
+      disableOneColumnMode: true, // for jfiddle small window size
+      cellHeight: 70,
+      column: 2,
+      minRow: 3,
+      acceptWidgets: true,
+      children: [
+        {id:1, x:0, y:0, w:2, h:2, content:'3 columns', subGridOpts: subGridOpts },
+        {id:2, x:0, y:2, w:1, y:1, content:'widget' }
+      ]
+    };
+    var grid = GridStack.init(options);
+    var layout;
+
+    print = function() {
+      layout = grid.save(false, false);
+      console.log(layout);
+    }
+    load = function() {
+      grid.load(layout);
+    }
+    // reset = function() {
+    //   grid.removeAll();
+    // }
+    print()
+  </script>
+</body>
+</html>
diff --git a/src/gridstack-engine.ts b/src/gridstack-engine.ts
@@ -392,9 +392,9 @@ export class GridStackEngine {
     const saveOrig = (node.x || 0) + (node.w || 1) > this.column;
     if (saveOrig && this.column < 12 && !this._inColumnResize && node._id && this.findCacheLayout(node, 12) === -1) {
       let copy = {...node}; // need _id + positions
-      if (copy.autoPosition) { delete copy.x; delete copy.y; }
+      if (copy.autoPosition || copy.x === undefined) { delete copy.x; delete copy.y; }
       else copy.x = Math.min(11, copy.x);
-      copy.w = Math.min(12, copy.w);
+      copy.w = Math.min(12, copy.w || 1);
       this.cacheOneLayout(copy, 12);
     }
 
@@ -744,9 +744,8 @@ export class GridStackEngine {
     this.sortNodes();
     this.nodes.forEach(n => {
       let wl = layout?.find(l => l._id === n._id);
-      let w: GridStackNode = {...n};
-      // use layout info instead if set
-      if (wl) { w.x = wl.x; w.y = wl.y; w.w = wl.w; }
+      // use layout info fields instead if set
+      let w: GridStackNode = {...n, ...(wl || {})};
       Utils.removeInternalForSave(w, !saveElement);
       if (saveCB) saveCB(n, w);
       list.push(w);
@@ -943,7 +942,7 @@ export class GridStackEngine {
   public cacheOneLayout(n: GridStackNode, column: number): GridStackEngine {
     n._id = n._id ?? GridStackEngine._idSeq++;
     let l: GridStackNode = {x: n.x, y: n.y, w: n.w, _id: n._id}
-    if (n.autoPosition) { delete l.x; delete l.y; l.autoPosition = true; }
+    if (n.autoPosition || n.x === undefined) { delete l.x; delete l.y; if (n.autoPosition) l.autoPosition = true; }
     this._layouts = this._layouts || [];
     this._layouts[column] = this._layouts[column] || [];
     let index = this.findCacheLayout(n, column);

diff --git a/src/gridstack.ts b/src/gridstack.ts
@@ -2042,12 +2042,14 @@ export class GridStack {
         // console.log('dropover cloning node'); // TEST
           if (!el._gridstackNodeOrig) el._gridstackNodeOrig = node; // shouldn't have multiple nested!
           el.gridstackNode = node = {...node, w, h, grid: this};
+          delete node.x;
+          delete node.y;
           this.engine.cleanupNode(node)
             .nodeBoundFix(node);
           // restore some internal fields we need after clearing them all
           node._initDD =
-        node._isExternal =  // DOM needs to be re-parented on a drop
-        node._temporaryRemoved = true; // so it can be inserted onDrag below
+          node._isExternal =  // DOM needs to be re-parented on a drop
+          node._temporaryRemoved = true; // so it can be inserted onDrag below
         } else {
           node.w = w; node.h = h;
           node._temporaryRemoved = true; // so we can insert it
@@ -2096,6 +2098,7 @@ export class GridStack {
         delete el._gridstackNodeOrig;
         if (wasAdded && origNode?.grid && origNode.grid !== this) {
           let oGrid = origNode.grid;
+          oGrid.engine.removeNodeFromLayoutCache(origNode);
           oGrid.engine.removedNodes.push(origNode);
           oGrid._triggerRemoveEvent()._triggerChangeEvent();
           // if it's an empty sub-grid that got auto-created, nuke it
@@ -2141,7 +2144,6 @@ export class GridStack {
         this._updateContainerHeight();
         this.engine.addedNodes.push(node);// @ts-ignore
         this._triggerAddEvent();// @ts-ignore
-        this.engine.removeNodeFromLayoutCache(node);
         this._triggerChangeEvent();
 
         this.engine.endUpdate();