From f76c41f642b3a2e011b1722ed652c8b5082a4dc1 Mon Sep 17 00:00:00 2001
From: Alain Dumesny <alaind831+github@gmail.com>
Date: Tue, 13 Jun 2023 12:35:05 -0700
Subject: [PATCH] column(n, 'list') handles restore back to 12 size

* more fix #2358
* going 12 -> 1 -> 12 in 'list' mode now correctly restore original item width before doing compact()
* updated demo
* updated API docs
---
 demo/column.html        |  49 +++++++------
 doc/README.md           |  23 +++++--
 src/gridstack-engine.ts | 148 ++++++++++++++++++++++------------------
 src/gridstack.ts        |  18 +++--
 src/types.ts            |   2 +-
 5 files changed, 139 insertions(+), 101 deletions(-)

diff --git a/demo/column.html b/demo/column.html
index 33f7a7053..a40327ef8 100644
--- a/demo/column.html
+++ b/demo/column.html
@@ -27,25 +27,28 @@ <h1>column() grid demo (fix cellHeight)</h1>
       </select>
     </div>
     <div>
-      <a onClick="grid.removeAll().load(layout1)" class="btn btn-primary" href="#">random</a>
-      <a onClick="grid.removeAll().load(layout2)" class="btn btn-primary" href="#">list</a>
+      load:
+      <a onClick="grid.removeAll().load(list)" class="btn btn-primary" href="#">list</a>
+      <a onClick="grid.removeAll().load(test1)" class="btn btn-primary" href="#">case 1</a>
+      <a onClick="random()" class="btn btn-primary" href="#">random</a>
       <a onClick="addWidget()" class="btn btn-primary" href="#">Add Widget</a>
-      <a onClick="setOneColumn(false)" class="btn btn-primary" href="#">1 Column</a>
-      <a onClick="setOneColumn(true)" class="btn btn-primary" href="#">1 Column DOM</a>
-      <a onClick="column(2)" class="btn btn-primary" href="#">2 Column</a>
-      <a onClick="column(3)" class="btn btn-primary" href="#">3 Column</a>
-      <a onClick="column(4)" class="btn btn-primary" href="#">4 Column</a>
-      <a onClick="column(6)" class="btn btn-primary" href="#">6 Column</a>
-      <a onClick="column(8)" class="btn btn-primary" href="#">8 Column</a>
-      <a onClick="column(10)" class="btn btn-primary" href="#">10 Column</a>
-      <a onClick="column(12)" class="btn btn-primary" href="#">12 Column</a>
+      column:
+      <a onClick="setOneColumn(false)" class="btn btn-primary" href="#">1</a>
+      <a onClick="setOneColumn(true)" class="btn btn-primary" href="#">1 DOM</a>
+      <a onClick="column(2)" class="btn btn-primary" href="#">2</a>
+      <a onClick="column(3)" class="btn btn-primary" href="#">3</a>
+      <a onClick="column(4)" class="btn btn-primary" href="#">4</a>
+      <a onClick="column(6)" class="btn btn-primary" href="#">6</a>
+      <a onClick="column(8)" class="btn btn-primary" href="#">8</a>
+      <a onClick="column(10)" class="btn btn-primary" href="#">10</a>
+      <a onClick="column(12)" class="btn btn-primary" href="#">12</a>
     </div>
     <br>
     <div class="grid-stack"></div>
   </div>
 
   <script type="text/javascript">
-    let layout1 = [ // DOM order will be 0,1,2,3,4,5,6 vs column1 = 0,1,4,3,2,5,6
+    let test1 = [ // DOM order will be 0,1,2,3,4,5,6 vs column1 = 0,1,4,3,2,5,6
       /* match karma testing
       {x: 0, y: 0, w: 4, h: 2},
       {x: 4, y: 0, w: 4, h: 4},
@@ -61,12 +64,12 @@ <h1>column() grid demo (fix cellHeight)</h1>
       {x: 5, y: 3, w: 2},
       {x: 0, y: 4, w: 12}
     ];
-    let layout2 = [{h:2},{},{},{},{},{},{},{},{},{w:2},{},{},{},{},{},{}];
-    layout2.forEach((n,i) => {
+    let list = [{h:2},{},{},{},{},{},{},{},{},{w:2},{},{},{},{},{},{}];
+    list.forEach((n,i) => {
       n.content = '<button onClick="grid.removeWidget(this.parentNode.parentNode)">X</button><br>' + ++i;
     });
     let count = 0;
-    layout1.forEach(n => {
+    test1.forEach(n => {
       n.content = '<button onClick="grid.removeWidget(this.parentNode.parentNode)">X</button><br>' + count++ + (n.text ? n.text : '');
     });
 
@@ -74,7 +77,7 @@ <h1>column() grid demo (fix cellHeight)</h1>
       float: true,
       disableOneColumnMode: true, // prevent auto column for this manual demo
       cellHeight: 100 // fixed as default 'auto' (square) makes it hard to test 1-3 column in actual large windows tests
-    }).load(layout2);
+    }).load(list);
     let text = document.querySelector('#column-text');
     let layout = 'list';
 
@@ -85,14 +88,18 @@ <h1>column() grid demo (fix cellHeight)</h1>
     });
 
 
+    function random() {
+      grid.removeAll();
+      count = 0;
+      for (i=0; i<8; i++) addWidget(true);
+    }
+
     function addWidget() {
-      let n = items[count] || {
-        x: Math.round(12 * Math.random()),
-        y: Math.round(5 * Math.random()),
+      let n = {
         w: Math.round(1 + 3 * Math.random()),
-        h: Math.round(1 + 3 * Math.random())
+        h: Math.round(1 + 3 * Math.random()),
+        content: '<button onClick="grid.removeWidget(this.parentNode.parentNode)">X</button><br>' + count++,
       };
-      n.content = '<button onClick="grid.removeWidget(this.parentNode.parentNode)">X</button><br>' + count++ + (n.text ? n.text : '');
       grid.addWidget(n);
     };
 
diff --git a/doc/README.md b/doc/README.md
index 67b90d1d6..d00f963cf 100644
--- a/doc/README.md
+++ b/doc/README.md
@@ -33,7 +33,7 @@ gridstack.js API
 - [API](#api)
   - [`addWidget(el?: GridStackWidget | GridStackElement, options?: GridStackWidget)`](#addwidgetel-gridstackwidget--gridstackelement-options-gridstackwidget)
   - [`batchUpdate(flag = true)`](#batchupdateflag--true)
-  - [`compact()`](#compact)
+  - [`compact(layout: CompactOptions = 'compact', doSort = true)`](#compactlayout-compactoptions--compact-dosort--true)
   - [`cellHeight(val: number, update = true)`](#cellheightval-number-update--true)
   - [`cellWidth()`](#cellwidth)
   - [`column(column: number, layout: ColumnOptions = 'moveScale')`](#columncolumn-number-layout-columnoptions--movescale)
@@ -361,9 +361,14 @@ grid.addWidget('<div class="grid-stack-item"><div class="grid-stack-item-content
 
 use before calling a bunch of `addWidget()` to prevent un-necessary relayouts in between (more efficient) and get a single event callback. You will see no changes until `batchUpdate(false)` is called.
 
-### `compact()`
+### `compact(layout: CompactOptions = 'compact', doSort = true)`
 
-re-layout grid items to reclaim any empty space.
+re-layout grid items to reclaim any empty space. Options are:
+- `'list'` keep the widget left->right order the same, even if that means leaving an empty slot if things don't fit
+- `'compact'` might re-order items to fill any empty space
+
+- `doSort` - `false` to let you do your own sorting ahead in case you need to control a different order. (default to sort)
+ 
 
 ### `cellHeight(val: number, update = true)`
 
@@ -385,10 +390,14 @@ Requires `gridstack-extra.css` (or minimized version) for [2-11],
 else you will need to generate correct CSS (see https://github.com/gridstack/gridstack.js#change-grid-columns)
 
 - `column` - Integer > 0 (default 12)
-- `layout` - specify the type of re-layout that will happen (position, size, etc...).
-Note: items will never be outside of the current column boundaries. default ('moveScale'). Ignored for 1 column.
-Possible values: 'moveScale' | 'move' | 'scale' | 'none' | (column: number, oldColumn: number, nodes: GridStackNode[], oldNodes: GridStackNode[]) => void.
-A custom function option takes new/old column count, and array of new/old positions.
+- `layout` - specify the type of re-layout that will happen (position, size, etc...). Values are: `'list' | 'compact' | 'moveScale' | 'move' | 'scale' | 'none' | ((column: number, oldColumn: number, nodes: GridStackNode[], oldNodes: GridStackNode[]) => void);`
+
+* `'list'` - treat items as sorted list, keeping items (un-sized unless too big for column count) sequentially reflowing them
+* `'compact'` - similar to list, but using compact() method which will possibly re-order items if an empty slots are available due to a larger item needing to be pushed to next row
+* `'moveScale'` - will scale and move items by the ratio new newColumnCount / oldColumnCount
+* `'move'` | `'scale'` - will only size or move items
+* `'none'` will leave items unchanged, unless they don't fit in column count
+* custom function that takes new/old column count, and array of new/old positions
 Note: new list may be partially already filled if we have a partial cache of the layout at that size (items were added later). If complete cache is present this won't get called at all.
 
 ### `destroy([removeDOM])`
diff --git a/src/gridstack-engine.ts b/src/gridstack-engine.ts
index fdecbf80d..7fbfbb0a1 100644
--- a/src/gridstack-engine.ts
+++ b/src/gridstack-engine.ts
@@ -54,7 +54,7 @@ export class GridStackEngine {
     this.onChange = opts.onChange;
   }
 
-  public batchUpdate(flag = true): GridStackEngine {
+  public batchUpdate(flag = true, doPack = true): GridStackEngine {
     if (!!this.batchMode === flag) return this;
     this.batchMode = flag;
     if (flag) {
@@ -64,7 +64,8 @@ export class GridStackEngine {
     } else {
       this._float = this._prevFloat;
       delete this._prevFloat;
-      this._packNodes()._notify();
+      if (doPack) this._packNodes();
+      this._notify();
     }
     return this;
   }
@@ -258,12 +259,14 @@ export class GridStackEngine {
     return !this.collide(nn);
   }
 
-  /** re-layout grid items to reclaim any empty space - optionally keeping the sort order exactly the same (list mode) vs truly finding an empty spaces */
-  public compact(layout: CompactOptions = 'compact', sortBefore = true): GridStackEngine {
+  /** re-layout grid items to reclaim any empty space - optionally keeping the sort order exactly the same ('list' mode) vs truly finding an empty spaces */
+  public compact(layout: CompactOptions = 'compact', doSort = true): GridStackEngine {
     if (this.nodes.length === 0) return this;
-    this.batchUpdate()
-    if (sortBefore) this.sortNodes();
-    this._inColumnResize = true; // faster addNode()
+    if (doSort) this.sortNodes();
+    const wasBatch = this.batchMode;
+    if (!wasBatch) this.batchUpdate();
+    const wasColumnResize = this._inColumnResize;
+    if (!wasColumnResize) this._inColumnResize = true; // faster addNode()
     let copyNodes = this.nodes;
     this.nodes = []; // pretend we have no nodes to conflict layout to start with...
     copyNodes.forEach((n, index, list) => {
@@ -274,8 +277,9 @@ export class GridStackEngine {
       }
       this.addNode(n, false, after); // 'false' for add event trigger
     });
-    delete this._inColumnResize;
-    return this.batchUpdate(false);
+    if (!wasColumnResize) delete this._inColumnResize;
+    if (!wasBatch) this.batchUpdate(false);
+    return this;
   }
 
   /** enable/disable floating widgets (default: `false`) See [example](http://gridstackjs.com/demo/float.html) */
@@ -518,14 +522,16 @@ export class GridStackEngine {
     delete node._temporaryRemoved;
     delete node._removeDOM;
 
+    let skipCollision: boolean;
     if (node.autoPosition && this.findEmptyPosition(node, this.nodes, this.column, after)) {
       delete node.autoPosition; // found our slot
+      skipCollision = true;
     }
 
     this.nodes.push(node);
     if (triggerAddEvent) { this.addedNodes.push(node); }
 
-    this._fixCollisions(node);
+    if (!skipCollision) this._fixCollisions(node);
     if (!this.batchMode) { this._packNodes()._notify(); }
     return node;
   }
@@ -792,23 +798,21 @@ export class GridStackEngine {
    * @param layout specify the type of re-layout that will happen (position, size, etc...).
    * Note: items will never be outside of the current column boundaries. default (moveScale). Ignored for 1 column
    */
-  public updateNodeWidths(prevColumn: number, column: number, nodes: GridStackNode[], layout: ColumnOptions = 'moveScale'): GridStackEngine {
+  public columnChanged(prevColumn: number, column: number, nodes: GridStackNode[], layout: ColumnOptions = 'moveScale'): GridStackEngine {
     if (!this.nodes.length || !column || prevColumn === column) return this;
 
     // simpler shortcuts layouts
     const doCompact = layout === 'compact' || layout === 'list';
     if (doCompact) {
       this.sortNodes(1, prevColumn); // sort with original layout once and only once (new column will affect order otherwise)
-      return this.compact(layout, false);
     }
-    
-    // cache the current layout in case they want to go back (like 12 -> 1 -> 12) as it requires original data
-    this.cacheLayout(this.nodes, prevColumn);
+
+    // cache the current layout in case they want to go back (like 12 -> 1 -> 12) as it requires original data IFF we're sizing down (see below)
+    if (column < prevColumn) this.cacheLayout(this.nodes, prevColumn);
     this.batchUpdate(); // do this EARLY as it will call saveInitial() so we can detect where we started for _dirty and collision
     let newNodes: GridStackNode[] = [];
-
-
-    // if we're going to 1 column and using DOM order rather than default sorting, then generate that layout
+    
+    // if we're going to 1 column and using DOM order (item passed in) rather than default sorting, then generate that layout
     let domOrder = false;
     if (column === 1 && nodes?.length) {
       domOrder = true;
@@ -822,14 +826,13 @@ export class GridStackEngine {
       newNodes = nodes;
       nodes = [];
     } else {
-      nodes = Utils.sort(this.nodes, -1, prevColumn); // current column reverse sorting so we can insert last to front (limit collision)
+      nodes = doCompact ? this.nodes : Utils.sort(this.nodes, -1, prevColumn); // current column reverse sorting so we can insert last to front (limit collision)
     }
 
     // see if we have cached previous layout IFF we are going up in size (restore) otherwise always
     // generate next size down from where we are (looks more natural as you gradually size down).
-    let cacheNodes: GridStackNode[] = [];
-    if (column > prevColumn) {
-      cacheNodes = this._layouts[column] || [];
+    if (column > prevColumn && this._layouts) {
+      const cacheNodes = this._layouts[column] || [];
       // ...if not, start with the largest layout (if not already there) as down-scaling is more accurate
       // by pretending we came from that larger column by assigning those values as starting point
       let lastIndex = this._layouts.length - 1;
@@ -839,59 +842,72 @@ export class GridStackEngine {
           let n = nodes.find(n => n._id === cacheNode._id);
           if (n) {
             // still current, use cache info positions
-            n.x = cacheNode.x;
-            n.y = cacheNode.y;
+            if (!doCompact) {
+              n.x = cacheNode.x;
+              n.y = cacheNode.y;
+            }
             n.w = cacheNode.w;
           }
         });
       }
-    }
 
-    // if we found cache re-use those nodes that are still current
-    cacheNodes.forEach(cacheNode => {
-      let j = nodes.findIndex(n => n._id === cacheNode._id);
-      if (j !== -1) {
-        // still current, use cache info positions
-        if (cacheNode.autoPosition || isNaN(cacheNode.x) || isNaN(cacheNode.y)) {
-          this.findEmptyPosition(cacheNode, newNodes);
+      // if we found cache re-use those nodes that are still current
+      cacheNodes.forEach(cacheNode => {
+        let j = nodes.findIndex(n => n._id === cacheNode._id);
+        if (j !== -1) {
+          // still current, use cache info positions
+          if (doCompact) {
+            nodes[j].w = cacheNode.w; // only w is used, and don't trim the list
+            return;
+          }
+          if (cacheNode.autoPosition || isNaN(cacheNode.x) || isNaN(cacheNode.y)) {
+            this.findEmptyPosition(cacheNode, newNodes);
+          }
+          if (!cacheNode.autoPosition) {
+            nodes[j].x = cacheNode.x;
+            nodes[j].y = cacheNode.y;
+            nodes[j].w = cacheNode.w;
+            newNodes.push(nodes[j]);
+          }
+          nodes.splice(j, 1);
         }
-        if (!cacheNode.autoPosition) {
-          nodes[j].x = cacheNode.x;
-          nodes[j].y = cacheNode.y;
-          nodes[j].w = cacheNode.w;
-          newNodes.push(nodes[j]);
+      });
+    }
+
+    // much simpler layout that just compacts
+    if (doCompact) {
+      this.compact(layout, false);
+    } else {
+      // ...and add any extra non-cached ones
+      if (nodes.length) {
+        if (typeof layout === 'function') {
+          layout(column, prevColumn, newNodes, nodes);
+        } else if (!domOrder) {
+          let ratio = (doCompact || layout === 'none') ? 1 : column / prevColumn;
+          let move = (layout === 'move' || layout === 'moveScale');
+          let scale = (layout === 'scale' || layout === 'moveScale');
+          nodes.forEach(node => {
+            // NOTE: x + w could be outside of the grid, but addNode() below will handle that
+            node.x = (column === 1 ? 0 : (move ? Math.round(node.x * ratio) : Math.min(node.x, column - 1)));
+            node.w = ((column === 1 || prevColumn === 1) ? 1 : scale ? (Math.round(node.w * ratio) || 1) : (Math.min(node.w, column)));
+            newNodes.push(node);
+          });
+          nodes = [];
         }
-        nodes.splice(j, 1);
       }
-    });
-    // ...and add any extra non-cached ones
-    if (nodes.length) {
-      if (typeof layout === 'function') {
-        layout(column, prevColumn, newNodes, nodes);
-      } else if (!domOrder) {
-        let ratio = column / prevColumn;
-        let move = (layout === 'move' || layout === 'moveScale');
-        let scale = (layout === 'scale' || layout === 'moveScale');
-        nodes.forEach(node => {
-          // NOTE: x + w could be outside of the grid, but addNode() below will handle that
-          node.x = (column === 1 ? 0 : (move ? Math.round(node.x * ratio) : Math.min(node.x, column - 1)));
-          node.w = ((column === 1 || prevColumn === 1) ? 1 :
-            scale ? (Math.round(node.w * ratio) || 1) : (Math.min(node.w, column)));
-          newNodes.push(node);
-        });
-        nodes = [];
-      }
-    }
 
-    // finally re-layout them in reverse order (to get correct placement)
-    if (!domOrder) newNodes = Utils.sort(newNodes, -1, column);
-    this._inColumnResize = true; // prevent cache update
-    this.nodes = []; // pretend we have no nodes to start with (add() will use same structures) to simplify layout
-    newNodes.forEach(node => {
-      this.addNode(node, false); // 'false' for add event trigger
-      delete node._orig; // make sure the commit doesn't try to restore things back to original
-    });
-    this.batchUpdate(false);
+      // finally re-layout them in reverse order (to get correct placement)
+      if (!domOrder) newNodes = Utils.sort(newNodes, -1, column);
+      this._inColumnResize = true; // prevent cache update
+      this.nodes = []; // pretend we have no nodes to start with (add() will use same structures) to simplify layout
+      newNodes.forEach(node => {
+        this.addNode(node, false); // 'false' for add event trigger
+        delete node._orig; // make sure the commit doesn't try to restore things back to original
+      });
+    }
+    
+    this.nodes.forEach(n => delete n._orig); // clear _orig before batch=false so it doesn't handle float=true restore
+    this.batchUpdate(false, !doCompact);
     delete this._inColumnResize;
     return this;
   }
diff --git a/src/gridstack.ts b/src/gridstack.ts
index 8fbd48af4..6cfbed9ad 100644
--- a/src/gridstack.ts
+++ b/src/gridstack.ts
@@ -9,7 +9,7 @@ import { GridStackEngine } from './gridstack-engine';
 import { Utils, HeightData, obsolete } from './utils';
 import { gridDefaults, ColumnOptions, GridItemHTMLElement, GridStackElement, GridStackEventHandlerCallback,
   GridStackNode, GridStackWidget, numberOrString, DDUIData, DDDragInOpt, GridStackPosition, GridStackOptions,
-  dragInDefaultOptions, GridStackEventHandler, GridStackNodesHandler, AddRemoveFcn, SaveFcn } from './types';
+  dragInDefaultOptions, GridStackEventHandler, GridStackNodesHandler, AddRemoveFcn, SaveFcn, CompactOptions } from './types';
 
 /*
  * and include D&D by default
@@ -796,9 +796,15 @@ export class GridStack {
     return (this.el.clientWidth || this.el.parentElement.clientWidth || window.innerWidth);
   }
 
-  /** re-layout grid items to reclaim any empty space */
-  public compact(): GridStack {
-    this.engine.compact();
+  /**
+   * re-layout grid items to reclaim any empty space. Options are:
+   * 'list' keep the widget left->right order the same, even if that means leaving an empty slot if things don't fit
+   * 'compact' might re-order items to fill any empty space
+   * 
+   * doSort - 'false' to let you do your own sorting ahead in case you need to control a different order. (default to sort)
+   **/
+  public compact(layout: CompactOptions = 'compact', doSort = true): GridStack {
+    this.engine.compact(layout, doSort);
     this._triggerChangeEvent();
     return this;
   }
@@ -810,7 +816,7 @@ export class GridStack {
    * else you will need to generate correct CSS (see https://github.com/gridstack/gridstack.js#change-grid-columns)
    * @param column - Integer > 0 (default 12).
    * @param layout specify the type of re-layout that will happen (position, size, etc...).
-   * Note: items will never be outside of the current column boundaries. default (moveScale). Ignored for 1 column
+   * Note: items will never be outside of the current column boundaries. default ('moveScale'). Ignored for 1 column
    */
   public column(column: number, layout: ColumnOptions = 'moveScale'): GridStack {
     if (column < 1 || this.opts.column === column) return this;
@@ -837,7 +843,7 @@ export class GridStack {
       });
       if (!domNodes.length) { domNodes = undefined; }
     }
-    this.engine.updateNodeWidths(oldColumn, column, domNodes, layout);
+    this.engine.columnChanged(oldColumn, column, domNodes, layout);
     if (this._isAutoCellHeight) this.cellHeight();
 
     // and trigger our event last...
diff --git a/src/types.ts b/src/types.ts
index 5447df502..097726062 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -53,7 +53,7 @@ export const dragInDefaultOptions: DDDragInOpt = {
  * different layout options when changing # of columns, including a custom function that takes new/old column count, and array of new/old positions
  * Note: new list may be partially already filled if we have a cache of the layout at that size and new items were added later.
  * Options are:
- * 'list' - treat items as sorted list, keeping items (unsized unless too big for column count) sequentially reflowing them
+ * 'list' - treat items as sorted list, keeping items (un-sized unless too big for column count) sequentially reflowing them
  * 'compact' - similar to list, but using compact() method which will possibly re-order items if an empty slots are available due to a larger item needing to be pushed to next row
  * 'moveScale' - will scale and move items by the ratio new newColumnCount / oldColumnCount
  * 'move' | 'scale' - will only size or move items