From 38887486f79d747068377148b1123107889e3d51 Mon Sep 17 00:00:00 2001 From: Alain Dumesny Date: Mon, 3 Feb 2020 15:14:05 -0800 Subject: [PATCH 1/5] 1 column edit propagation fixes * re-order in 1 column no longer affect other layouts (hard to predict). only resize/add/remove carry over. * more fix #37 and part2 of #1120 * column.html demo has been updated to support item delete. --- demo/column.html | 13 ++++++------- doc/CHANGES.md | 1 + src/gridstack.js | 15 ++++++++------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/demo/column.html b/demo/column.html index 40c6f5c2f..66f3cb0d2 100644 --- a/demo/column.html +++ b/demo/column.html @@ -63,22 +63,21 @@

setColumn() grid demo

]; var count = 0; grid.batchUpdate(); - for (count=0; count<4;) { - var n = items[count]; - grid.addWidget($('
' + count++ + (n.text ? n.text : '') + '
'), n); - }; + addWidget(); addWidget(); addWidget(); addWidget(); grid.commit(); - $('#add-widget').click(function() { + function addWidget() { var n = items[count] || { x: Math.round(12 * Math.random()), y: Math.round(5 * Math.random()), width: Math.round(1 + 3 * Math.random()), height: Math.round(1 + 3 * Math.random()) }; - grid.addWidget($('
' + count++ + (n.text ? n.text : '') + '
'), n); - }); + grid.addWidget($('

' + + count++ + (n.text ? n.text : '') + '
'), n); + }; + $('#add-widget').click(function() { addWidget() }); $('#1column').click(function() { delete grid.opts.oneColumnModeDomSort; grid.setColumn(1); $text.text(1);}); $('#1columnDOM').click(function() { grid.opts.oneColumnModeDomSort = true; grid.setColumn(1); $text.text('1 DOM');}); $('#2column').click(function() { grid.setColumn(2); $text.text(2);}); diff --git a/doc/CHANGES.md b/doc/CHANGES.md index 0d27e40f4..8fd0db01c 100644 --- a/doc/CHANGES.md +++ b/doc/CHANGES.md @@ -31,6 +31,7 @@ Change log - add `oneColumnModeDomSort` true|false to let you specify a custom layout (use dom order instead of x,y) for oneColumnMode `setColumn(1)` [#713](https://github.com/gridstack/gridstack.js/issues/713) - fix oneColumnMode to only restore if we auto went to it as window sizes up [#1125](https://github.com/gridstack/gridstack.js/pull/1125) +- re-order in 1 column no longer affect other layouts (hard to predict). only resize/add/remove carry over [#1127](https://github.com/gridstack/gridstack.js/pull/1127) ## v0.6.1 (2020-02-02) diff --git a/src/gridstack.js b/src/gridstack.js index 1e06a8aff..3e6c2bd27 100644 --- a/src/gridstack.js +++ b/src/gridstack.js @@ -1801,20 +1801,21 @@ GridStackEngine.prototype._layoutsNodesChange = function(nodes) { if (!this._layouts || this._ignoreLayoutsNodeChange) return; // remove smaller layouts - we will re-generate those on the fly... larger ones need to update - this._layouts.forEach(function(layout, i) { - if (!layout || i === this.column) return; - if (i < this.column) { - this._layouts[i] = undefined; + this._layouts.forEach(function(layout, column) { + if (!layout || column === this.column) return; + if (column < this.column) { + this._layouts[column] = undefined; } - else { + // move in 1 column don't propagate (hard to do right thing). height are re-used and add/remove carry over + else if (this.column !== 1) { // TODO: save the original x,y,w (h isn't cached) and see what actually changed to propagate correctly ? nodes.forEach(function(node) { var n = layout.find(function(l) { return l._id === node._id }); if (!n) return; - var ratio = i / this.column; + var ratio = column / this.column; n.y = node.y; n.x = Math.round(node.x * ratio); - // width ??? + n.width = Math.round(node.width * ratio); }, this); } }, this); From ddfb38c3b5d11c305eb1492d1cc1e56aa9199673 Mon Sep 17 00:00:00 2001 From: Alain Dumesny Date: Mon, 3 Feb 2020 15:39:48 -0800 Subject: [PATCH 2/5] karm test fixes --- demo/column.html | 6 +++++- spec/gridstack-spec.js | 40 ++++++++++++++++++++-------------------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/demo/column.html b/demo/column.html index 66f3cb0d2..479ee87a5 100644 --- a/demo/column.html +++ b/demo/column.html @@ -53,7 +53,11 @@

setColumn() grid demo

} var items = [ - {x: 0, y: 0, width: 2, height: 2}, + /* match karma testing + {x: 0, y: 0, width: 4, height: 2}, + {x: 4, y: 0, width: 4, height: 4}, + {text: ' auto'}, + */ {x: 2, y: 0, width: 2, height: 1}, {x: 5, y: 1, width: 1, height: 1}, {x: 5, y: 0, width: 2, height: 1}, diff --git a/spec/gridstack-spec.js b/spec/gridstack-spec.js index 19170776f..cf5ea5273 100644 --- a/spec/gridstack-spec.js +++ b/spec/gridstack-spec.js @@ -383,11 +383,12 @@ describe('gridstack', function() { expect(parseInt(el2.attr('data-gs-height'))).toBe(4); expect(parseInt(el3.attr('data-gs-x'))).toBe(0); - expect(parseInt(el3.attr('data-gs-y'))).toBe(6); + expect(parseInt(el3.attr('data-gs-y'))).toBe(6); // ??? kept one column row 6 which is not ideal maybe. expect(parseInt(el3.attr('data-gs-width'))).toBe(1); // ??? could take entire width if it did above expect(parseInt(el3.attr('data-gs-height'))).toBe(1); // back to 1 column, move item2 to beginning to [3][1][2] vertically + // NOTE: MOVE will have NO EFFECT on other layouts (see #1127) grid.setColumn(1); expect(grid.opts.column).toBe(1); grid.move(el3, 0, 0); @@ -406,44 +407,43 @@ describe('gridstack', function() { expect(parseInt(el2.attr('data-gs-width'))).toBe(1); expect(parseInt(el2.attr('data-gs-height'))).toBe(4); - // back to 12 column, el3 to be beginning still, but [1][2] to be in 1 columns still but wide 4x2 and 4x still + // back to 12 column, SAME as before 1 column change (move in 1 column don't effect others) grid.setColumn(12); expect(grid.opts.column).toBe(12); - - expect(parseInt(el3.attr('data-gs-x'))).toBe(0); - expect(parseInt(el3.attr('data-gs-y'))).toBe(0); - expect(parseInt(el3.attr('data-gs-width'))).toBe(1); - expect(parseInt(el3.attr('data-gs-height'))).toBe(1); - expect(parseInt(el1.attr('data-gs-x'))).toBe(0); - expect(parseInt(el1.attr('data-gs-y'))).toBe(1); + expect(parseInt(el1.attr('data-gs-y'))).toBe(0); expect(parseInt(el1.attr('data-gs-width'))).toBe(4); expect(parseInt(el1.attr('data-gs-height'))).toBe(2); - expect(parseInt(el2.attr('data-gs-x'))).toBe(0); - expect(parseInt(el2.attr('data-gs-y'))).toBe(3); + expect(parseInt(el2.attr('data-gs-x'))).toBe(4); + expect(parseInt(el2.attr('data-gs-y'))).toBe(0); expect(parseInt(el2.attr('data-gs-width'))).toBe(4); expect(parseInt(el2.attr('data-gs-height'))).toBe(4); - // 2 column will have item1, item2, item3 in 1 column still but half the width + expect(parseInt(el3.attr('data-gs-x'))).toBe(0); + expect(parseInt(el3.attr('data-gs-y'))).toBe(6); + expect(parseInt(el3.attr('data-gs-width'))).toBe(1); // ??? could take entire width if it did above + expect(parseInt(el3.attr('data-gs-height'))).toBe(1); + + // 2 column will generate from scaled down 12 grid.setColumn(1); // test convert from small, should use 12 layout still grid.setColumn(2); expect(grid.opts.column).toBe(2); - expect(parseInt(el3.attr('data-gs-x'))).toBe(0); - expect(parseInt(el3.attr('data-gs-y'))).toBe(0); - expect(parseInt(el3.attr('data-gs-width'))).toBe(1); // 1 as we scaled from 12 columns - expect(parseInt(el3.attr('data-gs-height'))).toBe(1); - expect(parseInt(el1.attr('data-gs-x'))).toBe(0); - expect(parseInt(el1.attr('data-gs-y'))).toBe(1); + expect(parseInt(el1.attr('data-gs-y'))).toBe(0); expect(parseInt(el1.attr('data-gs-width'))).toBe(1); expect(parseInt(el1.attr('data-gs-height'))).toBe(2); - expect(parseInt(el2.attr('data-gs-x'))).toBe(0); - expect(parseInt(el2.attr('data-gs-y'))).toBe(3); + expect(parseInt(el2.attr('data-gs-x'))).toBe(1); + expect(parseInt(el2.attr('data-gs-y'))).toBe(0); expect(parseInt(el2.attr('data-gs-width'))).toBe(1); expect(parseInt(el2.attr('data-gs-height'))).toBe(4); + + expect(parseInt(el3.attr('data-gs-x'))).toBe(0); + expect(parseInt(el3.attr('data-gs-y'))).toBe(6); + expect(parseInt(el3.attr('data-gs-width'))).toBe(1); + expect(parseInt(el3.attr('data-gs-height'))).toBe(1); }); }); From 0e362f84777d7cd4d030298a6270d92c877b9b96 Mon Sep 17 00:00:00 2001 From: Alain Dumesny Date: Mon, 3 Feb 2020 15:41:24 -0800 Subject: [PATCH 3/5] demo fix --- demo/column.html | 1 + 1 file changed, 1 insertion(+) diff --git a/demo/column.html b/demo/column.html index 479ee87a5..01448f335 100644 --- a/demo/column.html +++ b/demo/column.html @@ -58,6 +58,7 @@

setColumn() grid demo

{x: 4, y: 0, width: 4, height: 4}, {text: ' auto'}, */ + {x: 0, y: 0, width: 2, height: 2}, {x: 2, y: 0, width: 2, height: 1}, {x: 5, y: 1, width: 1, height: 1}, {x: 5, y: 0, width: 2, height: 1}, From 127a6f9f531a46ae61fb6b72e2305c64f1522f95 Mon Sep 17 00:00:00 2001 From: Alain Dumesny Date: Mon, 3 Feb 2020 17:34:28 -0800 Subject: [PATCH 4/5] more 1 column editing tweaks * editing in 1 column (or few columns) does a better job updating higher layout * track before and after and move items accordingly. * Tracking item swap would be even better still. more for fix #37 and part3 #1120 --- demo/column.html | 6 +++--- doc/CHANGES.md | 3 ++- spec/gridstack-spec.js | 37 ++++++++++++++++---------------- src/gridstack.js | 48 ++++++++++++++++++++++++++++++++---------- 4 files changed, 61 insertions(+), 33 deletions(-) diff --git a/demo/column.html b/demo/column.html index 01448f335..ed9b78317 100644 --- a/demo/column.html +++ b/demo/column.html @@ -53,11 +53,11 @@

setColumn() grid demo

} var items = [ - /* match karma testing + /* match karma testing */ {x: 0, y: 0, width: 4, height: 2}, {x: 4, y: 0, width: 4, height: 4}, {text: ' auto'}, - */ + {x: 0, y: 0, width: 2, height: 2}, {x: 2, y: 0, width: 2, height: 1}, {x: 5, y: 1, width: 1, height: 1}, @@ -68,7 +68,7 @@

setColumn() grid demo

]; var count = 0; grid.batchUpdate(); - addWidget(); addWidget(); addWidget(); addWidget(); + addWidget(); addWidget(); //addWidget(); addWidget(); grid.commit(); function addWidget() { diff --git a/doc/CHANGES.md b/doc/CHANGES.md index 8fd0db01c..113707bb3 100644 --- a/doc/CHANGES.md +++ b/doc/CHANGES.md @@ -31,7 +31,8 @@ Change log - add `oneColumnModeDomSort` true|false to let you specify a custom layout (use dom order instead of x,y) for oneColumnMode `setColumn(1)` [#713](https://github.com/gridstack/gridstack.js/issues/713) - fix oneColumnMode to only restore if we auto went to it as window sizes up [#1125](https://github.com/gridstack/gridstack.js/pull/1125) -- re-order in 1 column no longer affect other layouts (hard to predict). only resize/add/remove carry over [#1127](https://github.com/gridstack/gridstack.js/pull/1127) +- editing in 1 column (or few columns) does a better job updating higher layout (track before and after and move items accordingly). +Tracking item swap would be even better still. [#1127](https://github.com/gridstack/gridstack.js/pull/1127) ## v0.6.1 (2020-02-02) diff --git a/spec/gridstack-spec.js b/spec/gridstack-spec.js index cf5ea5273..6f317278e 100644 --- a/spec/gridstack-spec.js +++ b/spec/gridstack-spec.js @@ -383,12 +383,11 @@ describe('gridstack', function() { expect(parseInt(el2.attr('data-gs-height'))).toBe(4); expect(parseInt(el3.attr('data-gs-x'))).toBe(0); - expect(parseInt(el3.attr('data-gs-y'))).toBe(6); // ??? kept one column row 6 which is not ideal maybe. + expect(parseInt(el3.attr('data-gs-y'))).toBe(6); // ??? keep same row, but might more intuitive higher expect(parseInt(el3.attr('data-gs-width'))).toBe(1); // ??? could take entire width if it did above expect(parseInt(el3.attr('data-gs-height'))).toBe(1); // back to 1 column, move item2 to beginning to [3][1][2] vertically - // NOTE: MOVE will have NO EFFECT on other layouts (see #1127) grid.setColumn(1); expect(grid.opts.column).toBe(1); grid.move(el3, 0, 0); @@ -398,6 +397,7 @@ describe('gridstack', function() { expect(parseInt(el3.attr('data-gs-width'))).toBe(1); expect(parseInt(el3.attr('data-gs-height'))).toBe(1); + expect(parseInt(el1.attr('data-gs-x'))).toBe(0); expect(parseInt(el1.attr('data-gs-y'))).toBe(1); expect(parseInt(el1.attr('data-gs-width'))).toBe(1); expect(parseInt(el1.attr('data-gs-height'))).toBe(2); @@ -407,43 +407,44 @@ describe('gridstack', function() { expect(parseInt(el2.attr('data-gs-width'))).toBe(1); expect(parseInt(el2.attr('data-gs-height'))).toBe(4); - // back to 12 column, SAME as before 1 column change (move in 1 column don't effect others) + // back to 12 column, el3 to be beginning still, but [1][2] to be in 1 columns still but wide 4x2 and 4x still grid.setColumn(12); expect(grid.opts.column).toBe(12); + + expect(parseInt(el3.attr('data-gs-x'))).toBe(0); + expect(parseInt(el3.attr('data-gs-y'))).toBe(0); + expect(parseInt(el3.attr('data-gs-width'))).toBe(1); + expect(parseInt(el3.attr('data-gs-height'))).toBe(1); + expect(parseInt(el1.attr('data-gs-x'))).toBe(0); - expect(parseInt(el1.attr('data-gs-y'))).toBe(0); + expect(parseInt(el1.attr('data-gs-y'))).toBe(1); expect(parseInt(el1.attr('data-gs-width'))).toBe(4); expect(parseInt(el1.attr('data-gs-height'))).toBe(2); expect(parseInt(el2.attr('data-gs-x'))).toBe(4); - expect(parseInt(el2.attr('data-gs-y'))).toBe(0); + expect(parseInt(el2.attr('data-gs-y'))).toBe(1); expect(parseInt(el2.attr('data-gs-width'))).toBe(4); expect(parseInt(el2.attr('data-gs-height'))).toBe(4); - expect(parseInt(el3.attr('data-gs-x'))).toBe(0); - expect(parseInt(el3.attr('data-gs-y'))).toBe(6); - expect(parseInt(el3.attr('data-gs-width'))).toBe(1); // ??? could take entire width if it did above - expect(parseInt(el3.attr('data-gs-height'))).toBe(1); - - // 2 column will generate from scaled down 12 + // 2 column will have item1, item2, item3 in 1 column still but half the width grid.setColumn(1); // test convert from small, should use 12 layout still grid.setColumn(2); expect(grid.opts.column).toBe(2); + expect(parseInt(el3.attr('data-gs-x'))).toBe(0); + expect(parseInt(el3.attr('data-gs-y'))).toBe(0); + expect(parseInt(el3.attr('data-gs-width'))).toBe(1); // 1 as we scaled from 12 columns + expect(parseInt(el3.attr('data-gs-height'))).toBe(1); + expect(parseInt(el1.attr('data-gs-x'))).toBe(0); - expect(parseInt(el1.attr('data-gs-y'))).toBe(0); + expect(parseInt(el1.attr('data-gs-y'))).toBe(1); expect(parseInt(el1.attr('data-gs-width'))).toBe(1); expect(parseInt(el1.attr('data-gs-height'))).toBe(2); expect(parseInt(el2.attr('data-gs-x'))).toBe(1); - expect(parseInt(el2.attr('data-gs-y'))).toBe(0); + expect(parseInt(el2.attr('data-gs-y'))).toBe(1); expect(parseInt(el2.attr('data-gs-width'))).toBe(1); expect(parseInt(el2.attr('data-gs-height'))).toBe(4); - - expect(parseInt(el3.attr('data-gs-x'))).toBe(0); - expect(parseInt(el3.attr('data-gs-y'))).toBe(6); - expect(parseInt(el3.attr('data-gs-width'))).toBe(1); - expect(parseInt(el3.attr('data-gs-height'))).toBe(1); }); }); diff --git a/src/gridstack.js b/src/gridstack.js index 3e6c2bd27..88cef7bf1 100644 --- a/src/gridstack.js +++ b/src/gridstack.js @@ -355,12 +355,12 @@ if (this.float) { this.nodes.forEach(function(n, i) { - if (n._updating || n._origY === undefined || n.y === n._origY) { + if (n._updating || n._packY === undefined || n.y === n._packY) { return; } var newY = n.y; - while (newY >= n._origY) { + while (newY >= n._packY) { var collisionNode = this.nodes .slice(0, i) .find(Utils._didCollide, {n: n, newY: newY}); @@ -653,14 +653,14 @@ GridStackEngine.prototype.beginUpdate = function(node) { if (node._updating) return; node._updating = true; - this.nodes.forEach(function(n) { n._origY = n.y; }); + this.nodes.forEach(function(n) { n._packY = n.y; }); }; GridStackEngine.prototype.endUpdate = function() { var n = this.nodes.find(function(n) { return n._updating; }); if (n) { n._updating = false; - this.nodes.forEach(function(n) { delete n._origY; }); + this.nodes.forEach(function(n) { delete n._packY; }); } }; @@ -1806,19 +1806,32 @@ if (column < this.column) { this._layouts[column] = undefined; } - // move in 1 column don't propagate (hard to do right thing). height are re-used and add/remove carry over - else if (this.column !== 1) { - // TODO: save the original x,y,w (h isn't cached) and see what actually changed to propagate correctly ? + else { + // we save the original x,y,w (h isn't cached) to see what actually changed to propagate better. + // Note: we don't need to check against out of bound scaling/moving as that will be done when using those cache values. nodes.forEach(function(node) { var n = layout.find(function(l) { return l._id === node._id }); - if (!n) return; + if (!n) return; // no cache for new nodes. Will use those values. var ratio = column / this.column; - n.y = node.y; - n.x = Math.round(node.x * ratio); - n.width = Math.round(node.width * ratio); + // Y changed, push down same amount + // TODO: detect doing item 'swaps' will help instead of move (especially in 1 column mode) + if (node.y !== node._origY) { + n.y += (node.y - node._origY); + } + // X changed, scale from new position + if (node.x !== node._origX) { + n.x = Math.round(node.x * ratio); + } + // width changed, scale from new width + if (node.width !== node._origW) { + n.width = Math.round(node.width * ratio); + } + // ...height always carries over from cache }, this); } }, this); + + this._saveInitial(); // reset current value now that we diffed. } /** @@ -1907,6 +1920,19 @@ }, this); this.commit(); delete this._ignoreLayoutsNodeChange; + + // save this initial layout so we can see what changed and apply changes to other layouts better (diff) + this._saveInitial(); + } + + /** called to save initial position/size */ + GridStackEngine.prototype._saveInitial = function() { + this.nodes.forEach(function(n) { + n._origX = n.x; + n._origY = n.y; + n._origW = n.width; + n._origH = n.height; + }); } /** From 17d4105b32f8b13074f3a4d4ccb27802caffdfeb Mon Sep 17 00:00:00 2001 From: Alain Dumesny Date: Mon, 3 Feb 2020 17:36:43 -0800 Subject: [PATCH 5/5] demo tweaks --- demo/column.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/demo/column.html b/demo/column.html index ed9b78317..01448f335 100644 --- a/demo/column.html +++ b/demo/column.html @@ -53,11 +53,11 @@

setColumn() grid demo

} var items = [ - /* match karma testing */ + /* match karma testing {x: 0, y: 0, width: 4, height: 2}, {x: 4, y: 0, width: 4, height: 4}, {text: ' auto'}, - + */ {x: 0, y: 0, width: 2, height: 2}, {x: 2, y: 0, width: 2, height: 1}, {x: 5, y: 1, width: 1, height: 1}, @@ -68,7 +68,7 @@

setColumn() grid demo

]; var count = 0; grid.batchUpdate(); - addWidget(); addWidget(); //addWidget(); addWidget(); + addWidget(); addWidget(); addWidget(); addWidget(); grid.commit(); function addWidget() {