Merge pull request #1646 from adumesny/develop

collision: fix constrain put outside of grid

Author: adumesny

demo/events.js

@@ -28,7 +28,7 @@ function addEvents(grid, id) {
     let node = el.gridstackNode;
     let x = el.getAttribute('gs-x'); // verify node (easiest) and attr are the same
     let y = el.getAttribute('gs-y');
-    console.log(g + 'drag ' + el.textContent + ' pos: (' + node.x + ',' + node.y + ') = (' + x + ',' + y + ')');
+    // console.log(g + 'drag ' + el.textContent + ' pos: (' + node.x + ',' + node.y + ') = (' + x + ',' + y + ')');
   });
 
   grid.on('dragstop', function(event, el) {
@@ -58,7 +58,7 @@ function addEvents(grid, id) {
     let node = el.gridstackNode;
     let w = el.getAttribute('gs-w');  // verify node (easiest) and attr are the same
     let h = el.getAttribute('gs-h');
-    console.log(g + 'resize ' + el.textContent + ' size: (' + node.w + 'x' + node.h + ') = (' + w + 'x' + h + ')');
+    // console.log(g + 'resize ' + el.textContent + ' size: (' + node.w + 'x' + node.h + ') = (' + w + 'x' + h + ')');
   });
 
   grid.on('resizestop', function(event, el) {

spec/e2e/html/141_1534_swap.html

@@ -39,7 +39,7 @@ <h1>Swap collision demo</h1>
     addEvents(grid);
 
     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, h:2, w: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},

src/gridstack-dd.ts

@@ -149,6 +149,9 @@ GridStack.prototype._setupAcceptWidget = function(): GridStack {
         return canAccept;
       }
     })
+    /**
+     * entering our grid area
+     */
     .on(this.el, 'dropover', (event: Event, el: GridItemHTMLElement, helper: GridItemHTMLElement) => {
 
       // ignore drop enter on ourself, and prevent parent from receiving event
@@ -188,6 +191,9 @@ GridStack.prototype._setupAcceptWidget = function(): GridStack {
       GridStackDD.get().on(el, 'drag', onDrag);
       return false; // prevent parent from receiving msg (which may be grid as well)
     })
+    /**
+     * Leaving our grid area...
+     */
     .on(this.el, 'dropout', (event, el: GridItemHTMLElement) => {
       let node = el.gridstackNode;
       if (!node) return;
@@ -211,6 +217,9 @@ GridStack.prototype._setupAcceptWidget = function(): GridStack {
       el.gridstackNode = el._gridstackNodeOrig;
       return false; // prevent parent from receiving msg (which may be grid as well)
     })
+    /**
+     * end - releasing the mouse
+     */
     .on(this.el, 'drop', (event, el: GridItemHTMLElement, helper: GridItemHTMLElement) => {
       let node = el.gridstackNode;
       let wasAdded = !!this.placeholder.parentElement; // skip items not actually added to us because of constrains, but do cleanup #1419
@@ -435,10 +444,10 @@ GridStack.prototype._prepareDragDropByNode = function(node: GridStackNode): Grid
       this._clearRemovingTimeout(el);
       if (!node._temporaryRemoved) {
         Utils.removePositioningStyles(target);
-        this._writePosAttr(target, node.x, node.y, node.w, node.h);
+        this._writePosAttr(target, node);
       } else {
         Utils.removePositioningStyles(target);
-        this._writePosAttr(target, node._beforeDrag.x, node._beforeDrag.y, node.w, node.h);
+        this._writePosAttr(target, {...node._beforeDrag, w: node.w, h: node.h});
         node.x = node._beforeDrag.x;
         node.y = node._beforeDrag.y;
         delete node._temporaryRemoved;
@@ -490,7 +499,7 @@ GridStack.prototype._onStartMoving = function(event: Event, ui: DDUIData, node:
   this.engine.cleanNodes()
     .beginUpdate(node);
 
-  this._writePosAttr(this.placeholder, node.x, node.y, node.w, node.h)
+  this._writePosAttr(this.placeholder, node)
   this.el.append(this.placeholder);
 
   node.el = this.placeholder;
@@ -502,7 +511,7 @@ GridStack.prototype._onStartMoving = function(event: Event, ui: DDUIData, node:
   if (event.type === 'dropover' && !node._added) {
     node._added = true;
     this.engine.addNode(node);
-    this._writePosAttr(this.placeholder, node.x, node.y, node.w, node.h);
+    this._writePosAttr(this.placeholder, node);
     node._moving = true; // lastly mark as moving object
   }
 
@@ -520,7 +529,7 @@ GridStack.prototype._onStartMoving = function(event: Event, ui: DDUIData, node:
 
 /** @internal called when item is being dragged/resized */
 GridStack.prototype._dragOrResize = function(event: Event, ui: DDUIData, node: GridStackNode, cellWidth: number, cellHeight: number): void  {
-  let el = node.el;
+  let el = node.el || event.target as GridItemHTMLElement;
   // calculate the place where we're landing by offsetting margin so actual edge crosses mid point
   let left = ui.position.left + (ui.position.left > node._lastUiPosition.left  ? -this.opts.marginRight : this.opts.marginLeft);
   let top = ui.position.top + (ui.position.top > node._lastUiPosition.top  ? -this.opts.marginBottom : this.opts.marginTop);
@@ -561,10 +570,9 @@ GridStack.prototype._dragOrResize = function(event: Event, ui: DDUIData, node: G
       if (node._removeTimeout) this._clearRemovingTimeout(el);
 
       if (node._temporaryRemoved) {
+        node.el = this.placeholder;
         this.engine.addNode(node);
-        this._writePosAttr(this.placeholder, x, y, w, h);
         this.el.appendChild(this.placeholder);
-        node.el = this.placeholder;
         delete node._temporaryRemoved;
       }
     }
@@ -597,7 +605,7 @@ GridStack.prototype._dragOrResize = function(event: Event, ui: DDUIData, node: G
     this._updateContainerHeight();
 
     let target = event.target as GridItemHTMLElement;
-    this._writePosAttr(target, node.x, node.y, node.w, node.h);
+    this._writePosAttr(target, node);
     if (this._gsEventHandler[event.type]) {
       this._gsEventHandler[event.type](event, target);
     }

src/gridstack-engine.ts

@@ -84,7 +84,7 @@ export class GridStackEngine {
 
     let didMove = false;
     let yOffset = 0;
-    let newOpt: GridStackMoveOpts = {nested: true, pack: false, sanitize: false};
+    let newOpt: GridStackMoveOpts = {nested: true, pack: false};
     while (collide = collide || this.collide(node, nn)) { // could collide with more than 1 item... so repeat for each
       let moved: boolean;
       // if colliding with locked item, OR moving down to a different sized item
@@ -328,16 +328,17 @@ export class GridStackEngine {
     if (isNaN(node.w))  { node.w = defaults.w; }
     if (isNaN(node.h)) { node.h = defaults.h; }
 
-    if (node.maxW) { node.w = Math.min(node.w, node.maxW); }
-    if (node.maxH) { node.h = Math.min(node.h, node.maxH); }
-    if (node.minW) { node.w = Math.max(node.w, node.minW); }
-    if (node.minH) { node.h = Math.max(node.h, node.minH); }
-
     return this.nodeBoundFix(node, resizing);
   }
 
   /** part2 of preparing a node to fit inside our grid - checks  for x,y from grid dimensions */
   public nodeBoundFix(node: GridStackNode, resizing?: boolean): GridStackNode {
+
+    if (node.maxW) { node.w = Math.min(node.w, node.maxW); }
+    if (node.maxH) { node.h = Math.min(node.h, node.maxH); }
+    if (node.minW) { node.w = Math.max(node.w, node.minW); }
+    if (node.minH) { node.h = Math.max(node.h, node.minH); }
+
     if (node.w > this.column) {
       node.w = this.column;
     } else if (node.w < 1) {
@@ -413,7 +414,8 @@ export class GridStackEngine {
 
   /** call to add the given node to our list, fixing collision and re-packing */
   public addNode(node: GridStackNode, triggerAddEvent = false): GridStackNode {
-    if (this.nodes.find(n => n._id === node._id)) return; // prevent inserting twice!
+    let dup: GridStackNode;
+    if (dup = this.nodes.find(n => n._id === node._id)) return dup; // prevent inserting twice! return it instead.
     node = this.prepareNode(node);
 
     if (node.autoPosition) {
@@ -471,7 +473,6 @@ export class GridStackEngine {
     if (node.locked) return false;
     if (!this.changedPosConstrain(node, o)) return false;
     o.pack = true;
-    o.sanitize = false;
 
     // simpler case: move item directly...
     if (!this.maxRow/* && !this.nodes.some(n => n.locked)*/) {
@@ -577,23 +578,19 @@ export class GridStackEngine {
   public moveNode(node: GridStackNode, o: GridStackMoveOpts): boolean {
     if (!node || node.locked || !o) return false;
     if (o.pack === undefined) o.pack = true;
-    if (o.sanitize === undefined) o.sanitize = true;
-    let nn: GridStackNode;
-    if (o.sanitize) {
-      if (typeof o.x !== 'number') { o.x = node.x; }
-      if (typeof o.y !== 'number') { o.y = node.y; }
-      if (typeof o.w !== 'number') { o.w = node.w; }
-      if (typeof o.h !== 'number') { o.h = node.h; }
-
-      // constrain the passed in values and check if we're still changing our node
-      let resizing = (node.w !== o.w || node.h !== o.h);
-      nn = {maxW: node.maxW, maxH: node.maxH, minW: node.minW, minH: node.minH};
-      Utils.copyPos(nn, o);
-      nn = this.prepareNode(nn, resizing);
-      Utils.copyPos(o, nn);
-    }
-    nn = nn || o;
-    if (Utils.samePos(node, nn)) return false;
+
+    // constrain the passed in values and check if we're still changing our node
+    if (typeof o.x !== 'number') { o.x = node.x; }
+    if (typeof o.y !== 'number') { o.y = node.y; }
+    if (typeof o.w !== 'number') { o.w = node.w; }
+    if (typeof o.h !== 'number') { o.h = node.h; }
+    let resizing = (node.w !== o.w || node.h !== o.h);
+    let nn: GridStackNode = {maxW: node.maxW, maxH: node.maxH, minW: node.minW, minH: node.minH};
+    Utils.copyPos(nn, o);
+    nn = this.nodeBoundFix(nn, resizing);
+    Utils.copyPos(o, nn);
+
+    if (Utils.samePos(node, o)) return false;
     let prevPos: GridStackPosition = Utils.copyPos({}, node);
 
     // check if we will need to fix collision at our new location
@@ -628,7 +625,7 @@ export class GridStackEngine {
   public beginUpdate(node: GridStackNode): GridStackEngine {
     if (node._updating) return this;
     node._updating = true;
-    node._beforeDrag = {x: node.x, y: node.y, w: node.w, h: node.h};
+    node._beforeDrag = Utils.copyPos({}, node);
     delete node._skipDown;
     this.nodes.forEach(n => n._packY = n.y);
     return this;

src/gridstack.ts

@@ -9,7 +9,7 @@
 import { GridStackEngine } from './gridstack-engine';
 import { obsoleteOpts, obsoleteAttr, Utils, HeightData } from './utils';
 import { ColumnOptions, GridItemHTMLElement, GridStackElement, GridStackEventHandlerCallback,
-  GridStackNode, GridStackOptions, GridStackWidget, numberOrString, DDUIData, DDDragInOpt } from './types';
+  GridStackNode, GridStackOptions, GridStackWidget, numberOrString, DDUIData, DDDragInOpt, GridStackPosition } from './types';
 import { GridStackDDI } from './gridstack-ddi';
 
 // export all dependent file as well to make it easier for users to just import the main file
@@ -320,7 +320,7 @@ export class GridStack {
           if (removeDOM && n._id === null) {
             if (el && el.parentNode) { el.parentNode.removeChild(el) }
           } else {
-            this._writePosAttr(el, n.x, n.y, n.w, n.h);
+            this._writePosAttr(el, n);
           }
         });
         this._updateStyles(false, maxH); // false = don't recreate, just append if need be
@@ -1241,18 +1241,18 @@ export class GridStack {
   }
 
   /** @internal call to write position x,y,w,h attributes back to element */
-  private _writePosAttr(el: HTMLElement, x?: number, y?: number, w?: number, h?: number): GridStack {
-    if (x !== undefined && x !== null) { el.setAttribute('gs-x', String(x)); }
-    if (y !== undefined && y !== null) { el.setAttribute('gs-y', String(y)); }
-    if (w) { el.setAttribute('gs-w', String(w)); }
-    if (h) { el.setAttribute('gs-h', String(h)); }
+  private _writePosAttr(el: HTMLElement, n: GridStackPosition): GridStack {
+    if (n.x !== undefined && n.x !== null) { el.setAttribute('gs-x', String(n.x)); }
+    if (n.y !== undefined && n.y !== null) { el.setAttribute('gs-y', String(n.y)); }
+    if (n.w) { el.setAttribute('gs-w', String(n.w)); }
+    if (n.h) { el.setAttribute('gs-h', String(n.h)); }
     return this;
   }
 
   /** @internal call to write any default attributes back to element */
   private _writeAttr(el: HTMLElement, node: GridStackWidget): GridStack {
     if (!node) return this;
-    this._writePosAttr(el, node.x, node.y, node.w, node.h);
+    this._writePosAttr(el, node);
 
     let attrs /*: GridStackWidget but strings */ = { // remaining attributes
       autoPosition: 'gs-auto-position',

src/types.ts

@@ -200,8 +200,6 @@ export interface GridStackOptions {
 export interface GridStackMoveOpts extends GridStackPosition {
   /** do we pack (default true) */
   pack?: boolean;
-  /** do we verify for bad or > max/min values (default true) */
-  sanitize?: boolean;
   /** true if we are calling this recursively to prevent simple swap or coverage collision - default false*/
   nested?: boolean;
   /* vars to calculate other cells coordinates */