From 5bc3a425236e9c44e863186225811d22f38ab283 Mon Sep 17 00:00:00 2001 From: Alain Dumesny Date: Sun, 28 Mar 2021 12:21:39 -0700 Subject: [PATCH] collision: `row / maxRow` drag 2 grids * fix #1687 * broken in 4.x - dragging between 2 grids with maxRow/row set * willItFit() could modify the passed in node (with new algorithm) instead of making a clone copy to check if it fits. * added a new vertical grid demo (which has known issues documented) * added unit test --- demo/index.html | 1 + demo/two_vertical.html | 34 ++++++++++++++++++++++++++++++++++ doc/CHANGES.md | 1 + spec/gridstack-spec.ts | 10 +++++++++- src/gridstack-dd.ts | 4 ++-- src/gridstack-engine.ts | 10 +++++++--- src/utils.ts | 11 ++++++++--- 7 files changed, 62 insertions(+), 9 deletions(-) create mode 100644 demo/two_vertical.html diff --git a/demo/index.html b/demo/index.html index 3346ffc6f..9d1b582bc 100644 --- a/demo/index.html +++ b/demo/index.html @@ -20,6 +20,7 @@

Demos

  • Serialization
  • Static
  • Two grids
    • +
  • Two grids Vertical
  • Mobile touch (JQ)
  • Vue3.js
  • Vue2.js
    • diff --git a/demo/two_vertical.html b/demo/two_vertical.html new file mode 100644 index 000000000..9cad829e7 --- /dev/null +++ b/demo/two_vertical.html @@ -0,0 +1,34 @@ + + + + + + + Two vertical grids demo + + + + + +
    +

    Two vertical grids demo - with maxRow

    +

    special care is needed to prevent top grid from growing and causing shifts while you are dragging (which is a know issue).
    + You can either set a fix row, or have enough padding on a parent div to allow for an extra row to be created as needed), or....

    +
    +
    +
    +
    + + + + diff --git a/doc/CHANGES.md b/doc/CHANGES.md index 09551efb4..1bed7f2be 100644 --- a/doc/CHANGES.md +++ b/doc/CHANGES.md @@ -54,6 +54,7 @@ Change log ## 4.0.2-dev - fix [#1693](https://github.com/gridstack/gridstack.js/issues/1693) `load` after `init()` broken in 4.x +- fix [#1687](https://github.com/gridstack/gridstack.js/issues/1687) drag between 2 grids with `row / maxRow` broken in 4.x ## 4.0.2 (2021-3-27) diff --git a/spec/gridstack-spec.ts b/spec/gridstack-spec.ts index 684a28263..41f5b8ab7 100644 --- a/spec/gridstack-spec.ts +++ b/spec/gridstack-spec.ts @@ -1,4 +1,4 @@ -import { GridStack } from '../src/gridstack'; +import { GridStack, GridStackNode } from '../src/gridstack'; import { GridStackDD } from '../src/gridstack-dd'; // html5 vs Jquery set when including all file above import { Utils } from '../src/utils'; @@ -585,6 +585,14 @@ describe('gridstack', function() { expect(grid.willItFit({x:0, y:0, w:12, h:1})).toBe(true); expect(grid.willItFit({x:0, y:0, w:12, h:2})).toBe(false); }); + it('willItFit() not modifying node #1687', function() { + // default 4x2 and 4x4 so anything pushing more than 1 will fail + let grid = GridStack.init({maxRow: 5}); + let node: GridStackNode = {x:0, y:0, w:1, h:1, _id: 1, _temporaryRemoved: true}; + expect(grid.willItFit(node)).toBe(true); + expect(node._temporaryRemoved).toBe(true); + expect(node._id).toBe(1); + }); }); diff --git a/src/gridstack-dd.ts b/src/gridstack-dd.ts index cc238b5f8..177df68ca 100644 --- 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; // so it can be insert onDrag below + 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 @@ -512,7 +512,7 @@ GridStack.prototype._leave = function(node: GridStackNode, el: GridItemHTMLEleme if (node._temporaryRemoved) return; node._temporaryRemoved = true; - this.engine.removeNode(node); // remove placeholder as well + this.engine.removeNode(node); // remove placeholder as well, otherwise it's a sign node is not in our list, which is a bigger issue node.el = node._isExternal && helper ? helper : el; // point back to real item being dragged // finally if item originally came from another grid, but left us, restore things back to prev info diff --git a/src/gridstack-engine.ts b/src/gridstack-engine.ts index decc0e2d9..cd76f017b 100644 --- a/src/gridstack-engine.ts +++ b/src/gridstack-engine.ts @@ -473,7 +473,10 @@ export class GridStackEngine { } public removeNode(node: GridStackNode, removeDOM = true, triggerEvent = false): GridStackEngine { - if (!this.nodes.find(n => n === node)) return; // not in our list + if (!this.nodes.find(n => n === node)) { + // TEST console.log(`Error: GridStackEngine.removeNode() node._id=${node._id} not found!`) + return this; + } if (triggerEvent) { // we wait until final drop to manually track removed items (rather than during drag) this.removedNodes.push(node); } @@ -557,7 +560,8 @@ export class GridStackEngine { float: this.float, nodes: this.nodes.map(n => {return {...n}}) }); - clone.addNode(node); + let n = Utils.copyPos({}, node, true); // clone node so we don't mod any settings on it! #1687 + clone.addNode(n); return clone.getRow() <= this.maxRow; } @@ -612,7 +616,7 @@ export class GridStackEngine { 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}; + let nn: GridStackNode = Utils.copyPos({}, node, true); // get min/max out first, then opt positions next Utils.copyPos(nn, o); nn = this.nodeBoundFix(nn, resizing); Utils.copyPos(o, nn); diff --git a/src/utils.ts b/src/utils.ts index d3348d3c0..d41b8b449 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -3,7 +3,7 @@ * Copyright (c) 2021 Alain Dumesny - see GridStack root license */ -import { GridStackElement, GridStackNode, GridStackOptions, numberOrString, GridStackPosition } from './types'; +import { GridStackElement, GridStackNode, GridStackOptions, numberOrString, GridStackPosition, GridStackWidget } from './types'; export interface HeightData { h: number; @@ -219,12 +219,17 @@ export class Utils { return true; } - /* copies over b size & position */ - static copyPos(a: GridStackPosition, b: GridStackPosition): GridStackPosition { + /* copies over b size & position (GridStackPosition), and possibly min/max as well */ + static copyPos(a: GridStackWidget, b: GridStackWidget, minMax = false): GridStackWidget { a.x = b.x; a.y = b.y; a.w = b.w; a.h = b.h; + if (!minMax) return a; + if (b.minW) a.minW = b.minW; + if (b.minH) a.minH = b.minH; + if (b.maxW) a.maxW = b.maxW; + if (b.maxH) a.maxH = b.maxH; return a; }