collision: load after init() fix

* fix gridstack#1693 * broke in 4.x. load() now correctly works again (batch mode saving initial values multiple times) * added unit test and demo as well, so this shouldn't happen again.
adumesny · Mar 28, 2021 · 5a42d09 · 5a42d09
1 parent ed7e0cb
commit 5a42d09
Show file tree

Hide file tree

Showing 6 changed files with 103 additions and 12 deletions.
diff --git a/doc/CHANGES.md b/doc/CHANGES.md
@@ -5,6 +5,7 @@ Change log
 <!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
 **Table of Contents**  *generated with [DocToc](http://doctoc.herokuapp.com/)*
 
+- [4.0.2-dev](#402-dev)
 - [4.0.2 (2021-3-27)](#402-2021-3-27)
 - [4.0.1 (2021-3-20)](#401-2021-3-20)
 - [4.0.0 (2021-3-19)](#400-2021-3-19)
@@ -50,9 +51,13 @@ Change log
 - [v0.1.0 (2014-11-18)](#v010-2014-11-18)
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
+## 4.0.2-dev
+
+- fix [#1693](https://github.com/gridstack/gridstack.js/issues/1693) `load` after `init()` broken in 4.x
+
 ## 4.0.2 (2021-3-27)
 
-- fix [#1679](https://github.com/gridstack/gridstack.js/issues/1679) `Resizable: {handles:'w/sw'}` work again in 4.x
+- fix [#1679](https://github.com/gridstack/gridstack.js/issues/1679) `Resizable: {handles:'w/sw'}` broken in 4.x
 - fix [#1658](https://github.com/gridstack/gridstack.js/issues/1658) `enableMove(T/F)` not working correctly
 - fix `helper: myFunction` now working for H5 case for `dragInOptions` & `setupDragIn()`
 - fix prevent `addGrid()` from creating nested div grid if container already is a '.grid-stack' div

diff --git a/spec/e2e/html/1693_load_after.html b/spec/e2e/html/1693_load_after.html
@@ -0,0 +1,35 @@
+<!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>load after init</title>
+
+  <link rel="stylesheet" href="../../../demo/demo.css"/>
+  <script src="../../../dist/gridstack-h5.js"></script>
+
+</head>
+<body>
+  <div class="container-fluid">
+    <h1>load after init</h1>
+    <div class="grid-stack">
+      <div class="grid-stack-item" gs-id="id1">
+           <div class="grid-stack-item-content">
+               widget 1 
+            </div>
+      </div>
+       <div class="grid-stack-item" gs-id="id2" >
+          <div class="grid-stack-item-content">
+             widget 2 
+          </div>
+      </div>
+  </div>  </div>
+  <script src="../../../demo/events.js"></script>
+  <script type="text/javascript">
+    var data = [{id: "id1",x:0,y:0,w:2,h:1}, {id: "id2",x:3,y:0,w:3,h:2} ];
+    var grid = GridStack.init();
+    grid.load(data);
+   </script>
+</body>
+</html>
diff --git a/spec/gridstack-spec.ts b/spec/gridstack-spec.ts
@@ -1770,6 +1770,55 @@ describe('gridstack', function() {
     });
   });
 
+  describe('load', function() {
+    beforeEach(function() {
+      document.body.insertAdjacentHTML('afterbegin', gridstackHTML);
+    });
+    afterEach(function() {
+      document.body.removeChild(document.getElementById('gs-cont'));
+    });
+    it('after init #1693', function() {
+      let grid = GridStack.init();
+      grid.load([{id:'gsItem1',x:0,y:0,w:5,h:1},{id:'gsItem2',x:6,y:0,w:2,h:2}]);
+
+      let el1 = document.getElementById('item1')
+      expect(parseInt(el1.getAttribute('gs-x'))).toBe(0);
+      expect(parseInt(el1.getAttribute('gs-y'))).toBe(0);
+      expect(parseInt(el1.getAttribute('gs-w'))).toBe(5);
+      expect(parseInt(el1.getAttribute('gs-h'))).toBe(1);
+
+      let el2 = document.getElementById('item2')
+      expect(parseInt(el2.getAttribute('gs-x'))).toBe(6);
+      expect(parseInt(el2.getAttribute('gs-y'))).toBe(0);
+      expect(parseInt(el2.getAttribute('gs-w'))).toBe(2);
+      expect(parseInt(el2.getAttribute('gs-h'))).toBe(2);
+    });
+    it('after init replace nodes', function() {
+      let grid = GridStack.init();
+      expect(document.getElementById('item1')).not.toBe(null);
+      expect(document.getElementById('item2')).not.toBe(null);
+
+      // this will replace with 2 new nodes
+      grid.load([{id:'new1',x:0,y:0,w:5,h:1},{id:'new2',x:6,y:0,w:2,h:2}]);
+      expect(grid.engine.nodes.length).toBe(2);
+
+      expect(document.getElementById('item1')).toBe(null);
+      let el1 = grid.engine.nodes.find(n => n.id === 'new1').el;
+      expect(parseInt(el1.getAttribute('gs-x'))).toBe(0);
+      expect(parseInt(el1.getAttribute('gs-y'))).toBe(0);
+      expect(parseInt(el1.getAttribute('gs-w'))).toBe(5);
+      expect(parseInt(el1.getAttribute('gs-h'))).toBe(1);
+
+      expect(document.getElementById('item2')).toBe(null);
+      let el2 = grid.engine.nodes.find(n => n.id === 'new2').el;
+      expect(parseInt(el2.getAttribute('gs-x'))).toBe(6);
+      expect(parseInt(el2.getAttribute('gs-y'))).toBe(0);
+      expect(parseInt(el2.getAttribute('gs-w'))).toBe(2);
+      expect(parseInt(el2.getAttribute('gs-h'))).toBe(2);
+    });
+
+  });
+
  // ..and finally track log warnings at the end, instead of displaying them....
   describe('obsolete warnings', function() {
     console.warn = jasmine.createSpy('log'); // track warnings instead of displaying them

diff --git a/src/gridstack-dd.ts b/src/gridstack-dd.ts
@@ -193,7 +193,7 @@ GridStack.prototype._setupAcceptWidget = function(): GridStack {
         // 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;
+        node._temporaryRemoved = true; // so it can be insert onDrag below
       } else {
         node.w = w; node.h = h;
         node._temporaryRemoved = true; // so we can insert it
@@ -326,6 +326,15 @@ GridStack.prototype._setupRemoveDrop = function(): GridStack {
  * is dynamically create and needs to change later.
  **/
 GridStack.setupDragIn = function(_dragIn?: string, _dragInOptions?: DDDragInOpt) {
+  let dragIn: string;
+  let dragInOptions: DDDragInOpt;
+  const dragInDefaultOptions: DDDragInOpt = {
+    revert: 'invalid',
+    handle: '.grid-stack-item-content',
+    scroll: false,
+    appendTo: 'body'
+  };
+
   // cache in the passed in values (form grid init?) so they don't have to resend them each time
   if (_dragIn) {
     dragIn = _dragIn;
@@ -337,14 +346,6 @@ GridStack.setupDragIn = function(_dragIn?: string, _dragInOptions?: DDDragInOpt)
     if (!dd.isDraggable(el)) dd.dragIn(el, dragInOptions);
   });
 }
-let dragIn: string;
-let dragInOptions: DDDragInOpt;
-const dragInDefaultOptions: DDDragInOpt = {
-  revert: 'invalid',
-  handle: '.grid-stack-item-content',
-  scroll: false,
-  appendTo: 'body'
-};
 
 /** @internal prepares the element for drag&drop **/
 GridStack.prototype._prepareDragDropByNode = function(node: GridStackNode): GridStack {

diff --git a/src/gridstack-ddi.ts b/src/gridstack-ddi.ts
@@ -24,7 +24,7 @@ export class GridStackDDI {
   }
 
   /** removes any drag&drop present (called during destroy) */
-  public remove(el: GridItemHTMLElement): GridStackDDI {
+  public remove(_el: GridItemHTMLElement): GridStackDDI {
     return this; // no-op for static grids
   }
 }
diff --git a/src/gridstack-engine.ts b/src/gridstack-engine.ts
@@ -56,6 +56,7 @@ export class GridStackEngine {
     this.batchMode = true;
     this._prevFloat = this._float;
     this._float = true; // let things go anywhere for now... commit() will restore and possibly reposition
+    this.saveInitial(); // since begin update (which is called multiple times) won't do this
     return this;
   }
 
@@ -658,7 +659,7 @@ export class GridStackEngine {
     if (!node._updating) {
       node._updating = true;
       delete node._skipDown;
-      this.saveInitial();
+      if (!this.batchMode) this.saveInitial();
     }
     return this;
   }