fix autoPlacement from DOM reading

adumesny · adumesny · commit 624968d02eb7 · 2023-03-26T10:15:39.000-07:00
fix #2232 * reading from DOM will now place missing x,y items first available spot again (broke recently) * added support to save original info including autoPlacement so if you laod into 1 column then go back to 12 it's the same as loading into 12 directly. * added sample showing issue.
diff --git a/doc/CHANGES.md b/doc/CHANGES.md
@@ -83,7 +83,9 @@ Change log
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
 
 ## 7.2.3-dev (TBD)
+* feat [#2229](https://github.com/gridstack/gridstack.js/pull/2229) support nonce for CSP
 * fix [#2206](https://github.com/gridstack/gridstack.js/issues/2206) `load()` with collision fix
+* fix [#2232](https://github.com/gridstack/gridstack.js/issues/2232) `autoPosition` bug loading from DOM
 
 ## 7.2.3 (2023-02-02)
 * fix `addWidget()` to handle passing just {el} which was needed for Angular HMTL template demo
diff --git a/spec/e2e/html/2232_dom_auto_placement_mix.html b/spec/e2e/html/2232_dom_auto_placement_mix.html
@@ -0,0 +1,35 @@
+<!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>DOM reading</title>
+
+  <link rel="stylesheet" href="../../../demo.css"/>
+  <script src="../../../dist/gridstack-all.js"></script>
+
+</head>
+<body>
+  <div class="container-fluid">
+    <h1>DOM reading, with auto placement mix (1&2 set, others not)</h1>
+    <div class="grid-stack">
+      <div class="grid-stack-item" gs-w="12" gs-x="0" gs-y="0">
+        <div class="grid-stack-item-content">1</div>
+      </div>
+      <div class="grid-stack-item" gs-w="4" gs-x="0" gs-y="1">
+        <div class="grid-stack-item-content">2</div>
+      </div>
+      <div class="grid-stack-item" gs-w="4">
+        <div class="grid-stack-item-content">3</div>
+      </div>
+      <div class="grid-stack-item">
+        <div class="grid-stack-item-content">4</div>
+      </div>
+    </div>
+      </div>
+  <script type="text/javascript">
+    let grid = GridStack.init({cellHeight: 70});
+  </script>
+</body>
+</html>
diff --git a/src/gridstack-engine.ts b/src/gridstack-engine.ts
@@ -376,9 +376,10 @@ export class GridStackEngine {
     // remember it's position & width so we can restore back (1 -> 12 column) #1655 #1985
     // IFF we're not in the middle of column resizing!
     const saveOrig = this.column === 1 || node.x + node.w > this.column;
-    if (saveOrig && this.column < 12 && !this._inColumnResize && !node.autoPosition && node._id && this.findCacheLayout(node, 12) === -1) {
+    if (saveOrig && this.column < 12 && !this._inColumnResize && node._id && this.findCacheLayout(node, 12) === -1) {
       let copy = {...node}; // need _id + positions
-      copy.x = Math.min(11, copy.x);
+      if (copy.autoPosition) { delete copy.x; delete copy.y; }
+      else copy.x = Math.min(11, copy.x);
       copy.w = Math.min(12, copy.w);
       this.cacheOneLayout(copy, 12);
     }
@@ -474,20 +475,23 @@ export class GridStackEngine {
     return this;
   }
 
-  /** find the first available empty spot for the given node width/height, updating the x,y attributes. return true if found */
-  public findEmptyPosition(node: GridStackNode): boolean {
-    this.sortNodes();
+  /** find the first available empty spot for the given node width/height, updating the x,y attributes. return true if found.
+   * optionally you can pass your own existing node list and column count, otherwise defaults to that engine data.
+   */
+  public findEmptyPosition(node: GridStackNode, nodeList = this.nodes, column = this.column): boolean {
+    nodeList = Utils.sort(nodeList, -1, column);
     let found = false;
     for (let i = 0; !found; ++i) {
-      let x = i % this.column;
-      let y = Math.floor(i / this.column);
-      if (x + node.w > this.column) {
+      let x = i % column;
+      let y = Math.floor(i / column);
+      if (x + node.w > column) {
         continue;
       }
       let box = {x, y, w: node.w, h: node.h};
-      if (!this.nodes.find(n => Utils.isIntercepted(box, n))) {
+      if (!nodeList.find(n => Utils.isIntercepted(box, n))) {
         node.x = x;
         node.y = y;
+        delete node.autoPosition;
         found = true;
       }
     }
@@ -829,10 +833,15 @@ export class GridStackEngine {
       let j = nodes.findIndex(n => n._id === cacheNode._id);
       if (j !== -1) {
         // still current, use cache info positions
-        nodes[j].x = cacheNode.x;
-        nodes[j].y = cacheNode.y;
-        nodes[j].w = cacheNode.w;
-        newNodes.push(nodes[j]);
+        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);
       }
     });
@@ -892,14 +901,15 @@ export class GridStackEngine {
    */
   public cacheOneLayout(n: GridStackNode, column: number): GridStackEngine {
     n._id = n._id || GridStackEngine._idSeq++;
-    let layout: GridStackNode = {x: n.x, y: n.y, w: n.w, _id: n._id}
+    let l: GridStackNode = {x: n.x, y: n.y, w: n.w, _id: n._id}
+    if (n.autoPosition) { delete l.x; delete l.y; l.autoPosition = true; }
     this._layouts = this._layouts || [];
     this._layouts[column] = this._layouts[column] || [];
     let index = this.findCacheLayout(n, column);
     if (index === -1)
-      this._layouts[column].push(layout);
+      this._layouts[column].push(l);
     else
-      this._layouts[column][index] = layout;
+      this._layouts[column][index] = l;
     return this;
   }
 
diff --git a/src/gridstack.ts b/src/gridstack.ts
@@ -346,19 +346,7 @@ export class GridStack {
 
     if (this.opts.auto) {
       this.batchUpdate(); // prevent in between re-layout #1535 TODO: this only set float=true, need to prevent collision check...
-      let elements: {el: HTMLElement; i: number}[] = [];
-      let column = this.getColumn();
-      if (column === 1 && this._prevColumn) column = this._prevColumn; // do 12 column when reading into 1 column mode
-      this.getGridItems().forEach(el => { // get dom elements (not nodes yet)
-        let x = parseInt(el.getAttribute('gs-x'));
-        let y = parseInt(el.getAttribute('gs-y'));
-        elements.push({
-          el,
-          // if x,y are missing (autoPosition) add them to end of list - but keep their respective DOM order
-          i: (Number.isNaN(x) ? 1000 : x) + (Number.isNaN(y) ? 1000 : y) * column
-        });
-      });
-      elements.sort((a, b) => b.i - a.i).forEach(e => this._prepareElement(e.el)); // revert sort so lowest item wins
+      this.getGridItems().forEach(el => this._prepareElement(el));
       this.batchUpdate(false);
     }
 
