Merge pull request #1628 from adumesny/develop

collision overall Part II - 50% coverage check

Author: adumesny

doc/CHANGES.md

@@ -50,9 +50,9 @@ Change log
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
 ## 3.3.0-dev
 
-- fix [#149](https://github.com/gridstack/gridstack.js/issues/149) [#1094](https://github.com/gridstack/gridstack.js/issues/1094) [#1605](https://github.com/gridstack/gridstack.js/issues/1605) re-write of the **collision code**! you can now swap items of the same size (vertical/horizontal) when grid is full, and is the default in `float:false` (top gravity) as it feels more natural. Could add Alt key for swap vs push behavior later.<br>
-Dragging up and down now behave the same (used to require push WAY down past to swap/append). Also much more efficient collision code.<br>
-Still TODO: handle mid point of dragged over items rather 50% of row/column and check for the most covered when multiple items collide.
+- fix [#149](https://github.com/gridstack/gridstack.js/issues/149) [#1094](https://github.com/gridstack/gridstack.js/issues/1094) [#1605](https://github.com/gridstack/gridstack.js/issues/1605) re-write of the **collision code**! you can now swap items of the same size (vertical/horizontal) when grid is full, and is the default in `float:false` (top gravity) as it feels more natural. Could add Alt key for swap vs push behavior later.
+- Dragging up and down now behave the same (used to require push WAY down past to swap/append). Also much more efficient collision code.
+- handle mid point of dragged over items (>50%) rather than a new row/column and check for the most covered when multiple items collide.
 - fix [1617](https://github.com/gridstack/gridstack.js/issues/1617) FireFox DOM order issue. Thanks [@marcel-necker](https://github.com/marcel-necker)
 
 ## 3.3.0 (2021-2-2)

spec/e2e/html/141_1534_swap.html

@@ -34,7 +34,7 @@ <h1>Swap collision demo</h1>
 
     let count = 0;
     let items = [
-      {x:0, y:0}, {x:0, y:1}, {x:0, y:2},{x:1, y:0}, {x:1, y:1}, {x:1, y:2},
+      {x:0, y:0}, {x:0, y:1}, {x:0, y:2},{x:1, y:0}, {x:1, y:1}, {x:1, y:2, h:2, w:2},
       {x:5, y:0}, {x:4, y:1, w:3, locked: false, _content: 'locked'}, {x:5, y:2},
       {x:7, y:0}, {x:8, y:0}, {x:9, y:0}, {x:7, y:1, w:3}, {x:8, y:2},
       {x:11, y:0}, {x:11, y:1, h:2},

spec/gridstack-engine-spec.ts

@@ -333,23 +333,23 @@ describe('gridstack engine', function() {
     });
     it('should return true for changed x', function() {
       let widget = { x: 1, y: 2, w: 3, h: 4 };
-      expect(engine.changedPos(widget, 2, 2)).toEqual(true);
+      expect(engine.changedPosConstrain(widget, {x:2, y:2})).toEqual(true);
     });
     it('should return true for changed y', function() {
       let widget = { x: 1, y: 2, w: 3, h: 4 };
-      expect(engine.changedPos(widget, 1, 1)).toEqual(true);
+      expect(engine.changedPosConstrain(widget, {x:1, y:1})).toEqual(true);
     });
     it('should return true for changed width', function() {
       let widget = { x: 1, y: 2, w: 3, h: 4 };
-      expect(engine.changedPos(widget, 2, 2, 4, 4)).toEqual(true);
+      expect(engine.changedPosConstrain(widget, {x:2, y:2, w:4, h:4})).toEqual(true);
     });
     it('should return true for changed height', function() {
       let widget = { x: 1, y: 2, w: 3, h: 4 };
-      expect(engine.changedPos(widget, 1, 2, 3, 3)).toEqual(true);
+      expect(engine.changedPosConstrain(widget, {x:1, y:2, w:3, h:3})).toEqual(true);
     });
     it('should return false for unchanged position', function() {
       let widget = { x: 1, y: 2, w: 3, h: 4 };
-      expect(engine.changedPos(widget, 1, 2, 3, 4)).toEqual(false);
+      expect(engine.changedPosConstrain(widget, {x:1, y:2, w:3, h:4})).toEqual(false);
     });
   });
 
@@ -368,17 +368,17 @@ describe('gridstack engine', function() {
       engine.addNode(nodes[1])
       // add item that moves past locked one
       expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 1, w: 12, h: 1, locked: true}));
-      expect(findNode(engine, 2)).toEqual(jasmine.objectContaining({x: 1, y: 2}));
+      expect(findNode(engine, 2)).toEqual(jasmine.objectContaining({x: 1, y: 2, h: 3, id: 2}));
       // prevents moving locked item
       let node1 = findNode(engine, 1);
-      expect(engine.moveNode(node1, 6, 6)).toEqual(false);
+      expect(engine.moveNode(node1, {x:6, y:6})).toEqual(false);
       // but moves regular one (gravity ON)
       let node2 = findNode(engine, 2);
-      expect(engine.moveNode(node2, 6, 6)).toEqual(true);
+      expect(engine.moveNode(node2, {x:6, y:6})).toEqual(true);
       expect(node2).toEqual(jasmine.objectContaining({x: 6, y: 2, w: 2, h: 3}));
       // but moves regular one (gravity OFF)
       engine.float = true;
-      expect(engine.moveNode(node2, 7, 6)).toEqual(true);
+      expect(engine.moveNode(node2, {x:7, y:6})).toEqual(true);
       expect(node2).toEqual(jasmine.objectContaining({x: 7, y: 6, w: 2, h: 3}));
     });
   });

src/gridstack-dd.ts

@@ -8,7 +8,7 @@
 /* eslint-disable @typescript-eslint/no-unused-vars */
 
 import { GridStackDDI } from './gridstack-ddi';
-import { GridItemHTMLElement, GridStackNode, GridStackElement, DDUIData, DDDragInOpt } from './types';
+import { GridItemHTMLElement, GridStackNode, GridStackElement, DDUIData, DDDragInOpt, GridStackPosition } from './types';
 import { GridStack } from './gridstack';
 import { Utils } from './utils';
 
@@ -115,7 +115,7 @@ GridStack.prototype._setupAcceptWidget = function(): GridStack {
       node.el = this.placeholder; // dom we update while dragging...
 
       this._updateContainerHeight();
-    } else if (this.engine.moveNodeCheck(node, x, y)) {
+    } else if (this.engine.moveNodeCheck(node, {x, y})) {
       this._updateContainerHeight();
     }
   };
@@ -381,6 +381,7 @@ GridStack.prototype._prepareDragDropByNode = function(node: GridStackNode): Grid
     // set the min/max resize info
     cellWidth = this.cellWidth();
     cellHeight = this.getCellHeight(true); // force pixels for calculations
+    this.engine.cacheRects(cellWidth, cellHeight, this.opts.marginTop, this.opts.marginRight, this.opts.marginBottom, this.opts.marginLeft);
     let dd = GridStackDD.get()
       .resizable(el, 'option', 'minWidth', cellWidth * (node.minW || 1))
       .resizable(el, 'option', 'minHeight', cellHeight * (node.minH || 1));
@@ -395,8 +396,8 @@ GridStack.prototype._prepareDragDropByNode = function(node: GridStackNode): Grid
     let top = ui.position.top + (ui.position.top > node._lastUiPosition.top  ? -this.opts.marginBottom : this.opts.marginTop);
     let x = Math.round(left / cellWidth);
     let y = Math.round(top / cellHeight);
-    let w: number;
-    let h: number;
+    let w = node.w;
+    let h = node.h;
     let resizing: boolean;
 
     if (event.type === 'drag') {
@@ -433,7 +434,8 @@ GridStack.prototype._prepareDragDropByNode = function(node: GridStackNode): Grid
         }
       }
       if (node.x === x && node.y === y) return; // skip same
-      if (node._lastTried && node._lastTried.x === x && node._lastTried.y === y) return; // skip one we tried (but failed)
+      // DON'T skip one we tried as we might have failed because of coverage <50% before
+      // if (node._lastTried && node._lastTried.x === x && node._lastTried.y === y) return;
     } else if (event.type === 'resize')  {
       if (x < 0) return;
       // Scrolling page if needed
@@ -446,8 +448,15 @@ GridStack.prototype._prepareDragDropByNode = function(node: GridStackNode): Grid
     }
 
     node._lastTried = {x, y, w, h}; // set as last tried (will nuke if we go there)
-    if (this.engine.moveNodeCheck(node, x, y, w, h)) {
+    let rect: GridStackPosition = { // screen pix of the dragged box
+      x: ui.position.left + this.opts.marginLeft,
+      y: ui.position.top + this.opts.marginTop,
+      w: (ui.size ? ui.size.width : node.w * cellWidth) - this.opts.marginLeft - this.opts.marginRight,
+      h: (ui.size ? ui.size.height : node.h * cellHeight) - this.opts.marginTop - this.opts.marginBottom
+    };
+    if (this.engine.moveNodeCheck(node, {x, y, w, h, cellWidth, cellHeight, rect})) {
       node._lastUiPosition = ui.position;
+      this.engine.cacheRects(cellWidth, cellHeight, this.opts.marginTop, this.opts.marginRight, this.opts.marginBottom, this.opts.marginLeft);
       delete node._skipDown;
       if (resizing && node.subGrid) { (node.subGrid as GridStack).onParentResize(); }
       this._updateContainerHeight();