Merge pull request #2861 from adumesny/master

loading same layout with overlapping widget

Author: adumesny

spec/e2e/html/2492_load_twice.html

@@ -24,7 +24,7 @@ <h1>load twice bug</h1>
     ];
     let count = 0;
     items.forEach(n => n.id = String(count++)); // TEST loading with ids
-    let grid = GridStack.init({cellHeight: 70, margin: 5}).load(items)
+    let grid = GridStack.init({cellHeight: 70, margin: 5, children: items})
     items.forEach(n => n.content += '*')
     grid.load(items);
   </script>

spec/gridstack-spec.ts

@@ -1,6 +1,5 @@
 import { GridItemHTMLElement, GridStack, GridStackNode, GridStackWidget } from '../src/gridstack';
 import { Utils } from '../src/utils';
-import '../dist/gridstack.css';
 
 describe('gridstack >', function() {
   'use strict';
@@ -73,10 +72,6 @@ describe('gridstack >', function() {
       grids = GridStack.initAll(undefined, 'grid-stack');
       expect(grids.length).toBe(1);
     });
-    it('initAll use wrong selector >', function() {
-      grids = GridStack.initAll(undefined, 'BAD_SELECTOR_TEST');
-      expect(grids.length).toBe(0);
-    });
   });
 
   describe('grid.setAnimation >', function() {
@@ -766,7 +761,7 @@ describe('gridstack >', function() {
       grid = GridStack.init(options);
       let items = Utils.getElements('.grid-stack-item');
       items.forEach(oldEl => {
-        let el = grid.addWidget(oldEl);
+        let el = grid.makeWidget(oldEl);
         expect(parseInt(oldEl.getAttribute('gs-x'), 10)).toBe(parseInt(el.getAttribute('gs-x'), 10));
         expect(parseInt(oldEl.getAttribute('gs-y'), 10)).toBe(parseInt(el.getAttribute('gs-y'), 10));
       })
@@ -780,7 +775,7 @@ describe('gridstack >', function() {
       let items = Utils.getElements('.grid-stack-item');
       items.forEach(oldEl => {
         let el = oldEl.cloneNode(true) as HTMLElement;
-        el = grid.addWidget(el);
+        el = grid.makeWidget(el);
         expect(parseInt(el.getAttribute('gs-y'), 10)).not.toBe(parseInt(oldEl.getAttribute('gs-y'), 10));
       });
     });

spec/regression-spec.ts

@@ -0,0 +1,58 @@
+import { GridItemHTMLElement, GridStack, GridStackWidget } from '../src/gridstack';
+
+describe('regression >', function() {
+  'use strict';
+
+  let grid: GridStack;
+  let findEl = function(id: string): GridItemHTMLElement {
+    return grid.engine.nodes.find(n => n.id === id)!.el!;
+  };
+
+  // empty grid
+  let gridstackEmptyHTML =
+  '<div style="width: 800px; height: 600px" id="gs-cont">' +
+  '  <div class="grid-stack"></div>' +
+  '</div>';
+
+  describe('2492 load() twice >', function() {
+    beforeEach(function() {
+      document.body.insertAdjacentHTML('afterbegin', gridstackEmptyHTML);
+    });
+    afterEach(function() {
+      document.body.removeChild(document.getElementById('gs-cont'));
+    });
+    it('', function() {
+      let items: GridStackWidget[] = [
+        {x: 0, y: 0, w:2, content: '0 wide'},
+        {x: 1, y: 0, content: '1 over'},
+        {x: 2, y: 1, content: '2 float'},
+      ];
+      let count = 0;
+      items.forEach(n => n.id = String(count++));
+      grid = GridStack.init({cellHeight: 70, margin: 5}).load(items);
+
+      let el0 = findEl('0');
+      let el1 = findEl('1');
+      let el2 = findEl('2');
+
+      expect(parseInt(el0.getAttribute('gs-x'), 10)).toBe(0);
+      expect(parseInt(el0.getAttribute('gs-y'), 10)).toBe(0);
+      expect(el0.children[0].innerHTML).toBe(items[0].content!);
+      expect(parseInt(el1.getAttribute('gs-x'), 10)).toBe(1);
+      expect(parseInt(el1.getAttribute('gs-y'), 10)).toBe(1);
+      expect(parseInt(el2.getAttribute('gs-x'), 10)).toBe(2);
+      expect(parseInt(el2.getAttribute('gs-y'), 10)).toBe(0);
+
+      // loading with changed content should be same positions
+      items.forEach(n => n.content += '*')
+      grid.load(items);
+      expect(parseInt(el0.getAttribute('gs-x'), 10)).toBe(0);
+      expect(parseInt(el0.getAttribute('gs-y'), 10)).toBe(0);
+      expect(el0.children[0].innerHTML).toBe(items[0].content!);
+      expect(parseInt(el1.getAttribute('gs-x'), 10)).toBe(1);
+      expect(parseInt(el1.getAttribute('gs-y'), 10)).toBe(1);
+      expect(parseInt(el2.getAttribute('gs-x'), 10)).toBe(2);
+      expect(parseInt(el2.getAttribute('gs-y'), 10)).toBe(0);
+    });
+  });
+});

src/gridstack-engine.ts

@@ -112,8 +112,12 @@ export class GridStackEngine {
       if (collide.locked || this._loading || node._moving && !node._skipDown && nn.y > node.y && !this.float &&
         // can take space we had, or before where we're going
         (!this.collide(collide, {...collide, y: node.y}, node) || !this.collide(collide, {...collide, y: nn.y - collide.h}, node))) {
+
         node._skipDown = (node._skipDown || nn.y > node.y);
-        moved = this.moveNode(node, {...nn, y: collide.y + collide.h, ...newOpt});
+        const newNN = {...nn, y: collide.y + collide.h, ...newOpt};
+        // pretent we moved to where we are now so we can continue any collision checks #2492
+        moved = this._loading && Utils.samePos(node, newNN) ? true : this.moveNode(node, newNN);
+
         if ((collide.locked || this._loading) && moved) {
           Utils.copyPos(nn, node); // moving after lock become our new desired location
         } else if (!collide.locked && moved && opt.pack) {
@@ -127,7 +131,9 @@ export class GridStackEngine {
         // move collide down *after* where we will be, ignoring where we are now (don't collide with us)
         moved = this.moveNode(collide, {...collide, y: nn.y + nn.h, skip: node, ...newOpt});
       }
-      if (!moved) { return didMove; } // break inf loop if we couldn't move after all (ex: maxRow, fixed)
+
+      if (!moved) return didMove; // break inf loop if we couldn't move after all (ex: maxRow, fixed)
+
       collide = undefined;
     }
     return didMove;
@@ -720,7 +726,7 @@ export class GridStackEngine {
     }
 
     // now move (to the original ask vs the collision version which might differ) and repack things
-    if (needToMove) {
+    if (needToMove && !Utils.samePos(node, nn)) {
       node._dirty = true;
       Utils.copyPos(node, nn);
     }

src/gridstack.ts

@@ -744,12 +744,13 @@ export class GridStack {
 
         // add back to current list BUT force a collision check if it 'appears' we didn't change to make sure we don't overlap others now
         this.engine.nodes.push(item);
-        if (Utils.samePos(item, w)) {
+        if (Utils.samePos(item, w) && this.engine.nodes.length > 1) {
           this.moveNode(item, { ...w, forceCollide: true });
-          Utils.copyPos(w, item);
+          Utils.copyPos(w, item); // use possily updated values before update() is called next (no-op since already moved)
         }
 
         this.update(item.el, w);
+
         if (w.subGridOpts?.children) { // update any sub grid as well
           const sub = item.el.querySelector('.grid-stack') as GridHTMLElement;
           if (sub && sub.gridstack) {
@@ -1312,6 +1313,7 @@ export class GridStack {
       if (w.content !== undefined) {
         const itemContent = el.querySelector('.grid-stack-item-content') as HTMLElement;
         if (itemContent && itemContent.textContent !== w.content) {
+          n.content = w.content;
           GridStack.renderCB(itemContent, w);
           // restore any sub-grid back
           if (n.subGrid?.el) {