load() in 1 column mode layout fix

adumesny · adumesny · commit d9c224118c4c · 2020-11-28T00:12:44.000-08:00
* fif for 1471
* when `load()` is called into a grid that is 1 column mode due to
small area, we now cache the original 12 column layout to restore correctly
* added sample code. Had to change more code than expected.

* Also optimized loading to not re-do some items (re-writing attr multiple times)
* Also fixed react sample
diff --git a/demo/react.html b/demo/react.html
@@ -5,7 +5,7 @@
     <meta name="viewport" content="width=device-width, initial-scale=1.0" />
     <title>Gridstack.js React integration example</title>
     <link rel="stylesheet" href="demo.css" />
-    <script src="../dist/gridstack.all.js"></script>
+    <script src="../dist/gridstack-h5.js"></script>
 
     <!-- Scripts to use react inside html -->
     <script src="https://unpkg.com/react@16/umd/react.production.min.js"></script>
diff --git a/doc/CHANGES.md b/doc/CHANGES.md
@@ -45,7 +45,8 @@ Change log
 
 - we now have a React example, in addition to Vue - Angular is next!. thanks [@eloparco](https://github.com/eloparco)
 - fix placeholder not having custom `GridStackOptions.itemClass`. thanks [@pablosichert](https://github.com/pablosichert)
-- fix [1484](https://github.com/gridstack/gridstack.js/issues/1484) draging between 2 grids and back (regression in 2.0.1) 
+- fix [1484](https://github.com/gridstack/gridstack.js/issues/1484) dragging between 2 grids and back (regression in 2.0.1) 
+- fix [1471](https://github.com/gridstack/gridstack.js/issues/1471) `load()` into 1 column mode doesn't resize back to 12 correctly
 - del `ddPlugin` grid option as we only have one drag&drop plugin at runtime, which is defined by the include you use (HTML5 vs jquery vs none)
 
 ## 2.2.0 (2020-11-7)
diff --git a/spec/e2e/html/1471-load-column1.html b/spec/e2e/html/1471-load-column1.html
@@ -0,0 +1,55 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="utf-8">
+  <meta http-equiv="X-UA-Compatible" content="IE=edge">
+  <meta name="viewport" content="width=device-width, initial-scale=1">
+  <title>load() 1 column</title>
+
+  <link rel="stylesheet" href="../../../demo/demo.css"/>
+  <script src="../../../dist/gridstack-h5.js"></script>
+
+</head>
+<body>
+  <div class="container-fluid">
+    <h1>load() 1 column</h1>
+    <p>resize to below 768px, reload, then expand up to check 12 column</p>
+    <div>
+      <a class="btn btn-primary" onClick="addNewWidget()" href="#">Add Widget</a>
+      <a class="btn btn-primary" onclick="toggleFloat()" id="float" href="#">float: true</a>
+    </div>
+    <br><br>
+    <div class="grid-stack"></div>
+  </div>
+  <script src="../../../demo/events.js"></script>
+  <script type="text/javascript">
+    let grid = GridStack.init({float: true});
+    addEvents(grid);
+
+    let items = [
+      {x: 1, y: 0, width: 2, height: 1, content: '0'},
+      {x: 3, y: 1, width: 1, height: 2, content: '1'},
+      {x: 4, y: 1, width: 1, height: 1, content: '2'},
+      {x: 2, y: 3, width: 3, height: 1, content: '3'},
+      {x: 0, y: 6, width: 2, height: 2, content: '4'}
+    ];
+    grid.load(items);
+
+    addNewWidget = function() {
+      let 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())
+      };
+      n.content = String(count++);
+      grid.addWidget(n);
+    };
+
+    toggleFloat = function() {
+      grid.float(! grid.getFloat());
+      document.querySelector('#float').innerHTML = 'float: ' + grid.getFloat();
+    };
+  </script>
+</body>
+</html>
diff --git a/src/gridstack-engine.ts b/src/gridstack-engine.ts
@@ -196,17 +196,21 @@ export class GridStackEngine {
 
     // assign defaults for missing required fields
     let defaults = {width: 1, height: 1, x: 0, y: 0};
-    node = Utils.defaults(node, defaults);
+    Utils.defaults(node, defaults);
 
-    node.autoPosition = node.autoPosition || false;
-    node.noResize = node.noResize || false;
-    node.noMove = node.noMove || false;
+    if (!node.autoPosition) { delete node.autoPosition; }
+    if (!node.noResize) { delete node.noResize; }
+    if (!node.noMove) { delete node.noMove; }
 
     // check for NaN (in case messed up strings were passed. can't do parseInt() || defaults.x above as 0 is valid #)
-    if (Number.isNaN(node.x))      { node.x = defaults.x; node.autoPosition = true; }
-    if (Number.isNaN(node.y))      { node.y = defaults.y; node.autoPosition = true; }
-    if (Number.isNaN(node.width))  { node.width = defaults.width; }
-    if (Number.isNaN(node.height)) { node.height = defaults.height; }
+    if (typeof node.x == 'string')      { node.x = Number(node.x); }
+    if (typeof node.y == 'string')      { node.y = Number(node.y); }
+    if (typeof node.width == 'string')  { node.width = Number(node.width); }
+    if (typeof node.height == 'string') { node.height = Number(node.height); }
+    if (isNaN(node.x))      { node.x = defaults.x; node.autoPosition = true; }
+    if (isNaN(node.y))      { node.y = defaults.y; node.autoPosition = true; }
+    if (isNaN(node.width))  { node.width = defaults.width; }
+    if (isNaN(node.height)) { node.height = defaults.height; }
 
     if (node.maxWidth) { node.width = Math.min(node.width, node.maxWidth); }
     if (node.maxHeight) { node.height = Math.min(node.height, node.maxHeight); }
@@ -541,10 +545,7 @@ export class GridStackEngine {
     if (!this.nodes.length || oldColumn === column) { return this }
 
     // cache the current layout in case they want to go back (like 12 -> 1 -> 12) as it requires original data
-    let copy: Layout[] = [];
-    this.nodes.forEach((n, i) => { copy[i] = {x: n.x, y: n.y, width: n.width, _id: n._id} }); // only thing we change is x,y,w and id to find it back
-    this._layouts = this._layouts || []; // use array to find larger quick
-    this._layouts[oldColumn] = copy;
+    this.cacheLayout(this.nodes, oldColumn);
 
     // if we're going to 1 column and using DOM order rather than default sorting, then generate that layout
     if (column === 1 && nodes && nodes.length) {
@@ -638,6 +639,24 @@ export class GridStackEngine {
     return this;
   }
 
+  /**
+   * call to cache the given layout internally to the given location so we can restore back when column changes size
+   * @param nodes list of nodes
+   * @param column corresponding column index to save it under
+   * @param clear if true, will force other caches to be removed (default false)
+   */
+  public cacheLayout(nodes: GridStackNode[], column: number, clear = false): GridStackEngine {
+    let copy: Layout[] = [];
+    nodes.forEach((n, i) => {
+      n._id = n._id || GridStackEngine._idSeq++; // make sure we have an id in case this is new layout, else re-use id already set
+      copy[i] = {x: n.x, y: n.y, width: n.width, _id: n._id} // only thing we change is x,y,w and id to find it back
+    });
+    this._layouts = clear ? [] : this._layouts || []; // use array to find larger quick
+    this._layouts[column] = copy;
+    return this;
+  }
+
+
   /** called to remove all internal values */
   public cleanupNode(node: GridStackNode): GridStackEngine {
     for (let prop in node) {
diff --git a/src/gridstack.ts b/src/gridstack.ts
@@ -128,6 +128,8 @@ export class GridStack {
   /** @internal */
   private _prevColumn: number;
   /** @internal */
+  private _ignoreLayoutsNodeChange: boolean;
+  /** @internal */
   public _gsEventHandler = {};
   /** @internal */
   private _styles: GridCSSStyleSheet;
@@ -287,12 +289,10 @@ export class GridStack {
 
     this._updateContainerHeight();
 
-    this.onParentResize();
-
     this._setupDragIn();
     this._setupRemoveDrop();
     this._setupAcceptWidget();
-    this._updateWindowResizeEvent();
+    this._updateWindowResizeEvent(); // finally this may size us down to 1 column
   }
 
   /**
@@ -307,7 +307,7 @@ export class GridStack {
    * grid.addWidget({width: 3, content: 'hello'});
    * grid.addWidget('<div class="grid-stack-item"><div class="grid-stack-item-content">hello</div></div>', {width: 3});
    *
-   * @param el html element, or string definition, or GridStackWidget (which can have content string as well) to add
+   * @param el  GridStackWidget (which can have content string as well), html element, or string definition to add
    * @param options widget position/size options (optional, and ignore if first param is already option) - see GridStackWidget
    */
   public addWidget(els?: GridStackWidget | GridStackElement, options?: GridStackWidget): GridItemHTMLElement {
@@ -343,17 +343,22 @@ export class GridStack {
 
     // Tempting to initialize the passed in opt with default and valid values, but this break knockout demos
     // as the actual value are filled in when _prepareElement() calls el.getAttribute('data-gs-xyz) before adding the node.
-    if (options) {
-      options = {...options};  // make a copy before we modify in case caller re-uses it
-      // make sure we load any DOM attributes that are not specified in passed in options (which override)
-      let domAttr = this._readAttr(el);
-      Utils.defaults(options, domAttr);
-      this.engine.prepareNode(options);
-      this._writeAttr(el, options);
-    }
+    // So make sure we load any DOM attributes that are not specified in passed in options (which override)
+    let domAttr = this._readAttr(el);
+    options = {...(options || {})};  // make a copy before we modify in case caller re-uses it
+    Utils.defaults(options, domAttr);
+    this.engine.prepareNode(options);
+    this._writeAttr(el, options);
 
     this.el.appendChild(el);
-    return this.makeWidget(el);
+
+    // similar to makeWidget() that doesn't read attr again and worse re-create a new node and loose any _id
+    this._prepareElement(el, true, options);
+    this._updateContainerHeight();
+    this._triggerAddEvent();
+    this._triggerChangeEvent();
+
+    return el;
   }
 
   /** saves the current layout returning a list of widgets for serialization */
@@ -385,6 +390,14 @@ export class GridStack {
    **/
   public load(layout: GridStackWidget[], addAndRemove: boolean | ((w: GridStackWidget, add: boolean) => void)  = true): GridStack {
     let items = GridStack.Utils.sort(layout);
+
+    // if we're loading a layout into 1 column (_prevColumn is set only when going to 1) and items don't fit, make sure to save
+    // the original wanted layout so we can scale back up correctly #1471
+    if (this._prevColumn && this._prevColumn !== this.opts.column && items.some(n => (n.x + n.width) > this.opts.column)) {
+      this._ignoreLayoutsNodeChange = true; // skip layout update
+      this.engine.cacheLayout(items, this._prevColumn, true);
+    }
+
     let removed: GridStackNode[] = [];
     this.batchUpdate();
     // see if any items are missing from new layout and need to be removed first
@@ -404,7 +417,7 @@ export class GridStack {
     }
     // now add/update the widgets
     items.forEach(w => {
-      let item = this.engine.nodes.find(n => n.id === w.id);
+      let item = (w.id || w.id === 0) ? this.engine.nodes.find(n => n.id === w.id) : undefined;
       if (item) {
         this.update(item.el, w.x, w.y, w.width, w.height); // TODO: full update
       } else if (addAndRemove) {
@@ -417,6 +430,10 @@ export class GridStack {
     });
     this.engine.removedNodes = removed;
     this.commit();
+
+    // after commit, clear that flag
+    delete this._ignoreLayoutsNodeChange;
+
     return this;
   }
 
@@ -531,7 +548,10 @@ export class GridStack {
     this.engine.updateNodeWidths(oldColumn, column, domNodes, layout);
 
     // and trigger our event last...
-    this._triggerChangeEvent(true); // skip layout update
+    this._ignoreLayoutsNodeChange = true; // skip layout update
+    this._triggerChangeEvent();
+    delete this._ignoreLayoutsNodeChange;
+
     return this;
   }
 
@@ -1007,11 +1027,11 @@ export class GridStack {
   }
 
   /** @internal */
-  private _triggerChangeEvent(skipLayoutChange?: boolean): GridStack {
+  private _triggerChangeEvent(): GridStack {
     if (this.engine.batchMode) { return this; }
     let elements = this.engine.getDirtyNodes(true); // verify they really changed
     if (elements && elements.length) {
-      if (!skipLayoutChange) {
+      if (!this._ignoreLayoutsNodeChange) {
         this.engine.layoutsNodesChange(elements);
       }
       this._triggerEvent('change', elements);
@@ -1024,7 +1044,9 @@ export class GridStack {
   private _triggerAddEvent(): GridStack {
     if (this.engine.batchMode) { return this }
     if (this.engine.addedNodes && this.engine.addedNodes.length > 0) {
-      this.engine.layoutsNodesChange(this.engine.addedNodes);
+      if (!this._ignoreLayoutsNodeChange) {
+        this.engine.layoutsNodesChange(this.engine.addedNodes);
+      }
       // prevent added nodes from also triggering 'change' event (which is called next)
       this.engine.addedNodes.forEach(n => { delete n._dirty; });
       this._triggerEvent('added', this.engine.addedNodes);
@@ -1157,12 +1179,20 @@ export class GridStack {
 
 
   /** @internal */
-  private _prepareElement(el: GridItemHTMLElement, triggerAddEvent = false): GridStack {
-    el.classList.add(this.opts.itemClass);
-    let node = this._readAttr(el, { el: el, grid: this });
-    node = this.engine.addNode(node, triggerAddEvent);
+  private _prepareElement(el: GridItemHTMLElement, triggerAddEvent = false, node?: GridStackNode): GridStack {
+    if (!node) {
+      el.classList.add(this.opts.itemClass);
+      node = this._readAttr(el);
+    }
     el.gridstackNode = node;
-    this._writeAttr(el, node);
+    node.el = el;
+    node.grid = this;
+    let copy = {...node};
+    node = this.engine.addNode(node, triggerAddEvent);
+    // write node attr back in case there was collision or we have to fix bad values during addNode()
+    if (!Utils.same(node, copy)) {
+      this._writeAttr(el, node);
+    }
     this._prepareDragDropByNode(node);
     return this;
   }
@@ -1231,6 +1261,14 @@ export class GridStack {
     node.resizeHandles = el.getAttribute('data-gs-resize-handles');
     node.id = el.getAttribute('data-gs-id');
 
+    // remove any key not found (null or false which is default)
+    for (const key in node) {
+      if (!node.hasOwnProperty(key)) { return; }
+      if (node[key] === null || node[key] === false) {
+        delete node[key];
+      }
+    }
+
     return node;
   }
 
@@ -1305,6 +1343,7 @@ export class GridStack {
     if (workTodo && !forceRemove && !this.opts._isNested && !this._windowResizeBind) {
       this._windowResizeBind = this.onParentResize.bind(this); // so we can properly remove later
       window.addEventListener('resize', this._windowResizeBind);
+      this.onParentResize(); // initially call it once...
     } else if ((forceRemove || !workTodo) && this._windowResizeBind) {
       window.removeEventListener('resize', this._windowResizeBind);
       delete this._windowResizeBind; // remove link to us so we can free
diff --git a/src/utils.ts b/src/utils.ts
@@ -172,8 +172,8 @@ export class Utils {
     return Boolean(v);
   }
 
-  static toNumber(value: null | string): number | null {
-    return (value === null || value.length === 0) ? null : Number(value);
+  static toNumber(value: null | string): number {
+    return (value === null || value.length === 0) ? undefined : Number(value);
   }
 
   static parseHeight(val: numberOrString): HeightData {
@@ -211,6 +211,18 @@ export class Utils {
     return target;
   }
 
+  /** given 2 objects return true if they have the same values. Checks for Object {} having same fields and values (just 1 level down) */
+  static same(a: unknown, b: unknown): boolean {
+    if (typeof a !== 'object')  { return a == b; }
+    if (typeof a !== typeof b) { return false; }
+    // else we have object, check just 1 level deep for being same things...
+    if (Object.keys(a).length !== Object.keys(b).length) { return false; }
+    for (const key in a) {
+      if (a[key] !== b[key]) { return false; }
+    }
+    return true;
+  }
+
   /** makes a shallow copy of the passed json struct */
   // eslint-disable-next-line
   static clone(target: {}): {} {
