load() now creates in order

* changed code to create widgets in sorted order instead of reverse order (fixed collision code to handle that case to push down items during collision)
* updated two.html to force collision loading to test

Author: adumesny

demo/two.html

@@ -73,11 +73,13 @@ <h1>Two grids demo</h1>
 
     let items = [
       {x: 0, y: 0, w: 2, h: 2},
-      {x: 3, y: 1, h: 2},
-      {x: 4, y: 1},
-      {x: 2, y: 3, w: 3, maxW: 3, id: 'special', content: 'has maxW=3'},
+      {x: 1, y: 1, h: 2}, // intentional overlap to test collision on load
+      {x: 1, y: 1}, // intentional overlap to test collision on load
+      {x: 3, y: 1},
+      {x: 2, y: 3, w: 3, maxW: 3, content: 'has maxW=3'},
       {x: 2, y: 5}
     ];
+    items.forEach((item, i) => item.content = item.content || String(i));
 
     grids.forEach(function (grid, i) {
       addEvents(grid, i);

doc/CHANGES.md

@@ -5,6 +5,7 @@ Change log
 <!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
 **Table of Contents**  *generated with [DocToc](http://doctoc.herokuapp.com/)*
 
+- [10.2.1-dev (TBD)](#1021-dev-tbd)
 - [10.2.1 (2024-06-23)](#1021-2024-06-23)
 - [10.2.0 (2024-06-02)](#1020-2024-06-02)
 - [10.1.2 (2024-03-30)](#1012-2024-03-30)
@@ -111,6 +112,9 @@ Change log
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
 
+## 10.2.1-dev (TBD)
+* fix: [#2717](https://github.com/gridstack/gridstack.js/pull/2717) load() now creates in order
+
 ## 10.2.1 (2024-06-23)
 * fix: [#2683](https://github.com/gridstack/gridstack.js/issues/2683) check for fixed grid maxRow during resize
 * fix: [#2694](https://github.com/gridstack/gridstack.js/issues/2694) prevent 'r' rotation to items that can't resize (locked, noResize, fixed sizes)

src/gridstack-engine.ts

@@ -39,6 +39,8 @@ export class GridStackEngine {
   protected _prevFloat: boolean;
   /** @internal cached layouts of difference column count so we can restore back (eg 12 -> 1 -> 12) */
   protected _layouts?: GridStackNode[][]; // maps column # to array of values nodes
+  /** @internal set during loading (which is sorted) so item gets added AFTER collision nodes */
+  public _loading?: boolean
   /** @internal true while we are resizing widgets during column resize to skip certain parts */
   protected _inColumnResize?: boolean;
   /** @internal true if we have some items locked */
@@ -91,7 +93,7 @@ export class GridStackEngine {
 
     // during while() collisions MAKE SURE to check entire row so larger items don't leap frog small ones (push them all down starting last in grid)
     let area = nn;
-    if (this._useEntireRowArea(node, nn)) {
+    if (!this._loading && this._useEntireRowArea(node, nn)) {
       area = {x: 0, w: this.column, y: nn.y, h: nn.h};
       collide = this.collide(node, area, opt.skip); // force new hit
     }
@@ -100,14 +102,14 @@ export class GridStackEngine {
     let newOpt: GridStackMoveOpts = {nested: true, pack: false};
     while (collide = collide || this.collide(node, area, opt.skip)) { // could collide with more than 1 item... so repeat for each
       let moved: boolean;
-      // if colliding with a locked item OR moving down with top gravity (and collide could move up) -> skip past the collide,
+      // if colliding with a locked item OR loading (move after) OR moving down with top gravity (and collide could move up) -> skip past the collide,
       // but remember that skip down so we only do this once (and push others otherwise).
-      if (collide.locked || node._moving && !node._skipDown && nn.y > node.y && !this.float &&
+      if (collide.locked || this._loading || node._moving && !node._skipDown && nn.y > node.y && !this.float &&
         // can take space we had, or before where we're going
         (!this.collide(collide, {...collide, y: node.y}, node) || !this.collide(collide, {...collide, y: nn.y - collide.h}, node))) {
         node._skipDown = (node._skipDown || nn.y > node.y);
         moved = this.moveNode(node, {...nn, y: collide.y + collide.h, ...newOpt});
-        if (collide.locked && moved) {
+        if ((collide.locked || this._loading) && moved) {
           Utils.copyPos(nn, node); // moving after lock become our new desired location
         } else if (!collide.locked && moved && opt.pack) {
           // we moved after and will pack: do it now and keep the original drop location, but past the old collide to see what else we might push way

src/gridstack.ts

@@ -252,8 +252,6 @@ export class GridStack {
   protected _sizeThrottle: () => void;
   /** @internal limit auto cell resizing method */
   protected prevWidth: number;
-  /** @internal true when loading items to insert first rather than append */
-  protected _insertNotAppend: boolean;
   /** @internal extra row added when dragging at the bottom of the grid */
   protected _extraDragRow = 0;
   /** @internal true if nested grid should get column count from our width */
@@ -421,7 +419,7 @@ export class GridStack {
 
     // load any passed in children as well, which overrides any DOM layout done above
     if (opts.children) {
-      let children = opts.children;
+      const children = opts.children;
       delete opts.children;
       if (children.length) this.load(children); // don't load empty
     }
@@ -495,11 +493,7 @@ export class GridStack {
     node = this.engine.prepareNode(options);
     this._writeAttr(el, options);
 
-    if (this._insertNotAppend) {
-      this.el.prepend(el);
-    } else {
-      this.el.appendChild(el);
-    }
+    this.el.appendChild(el);
 
     this.makeWidget(el, options);
 
@@ -703,22 +697,16 @@ export class GridStack {
     // make sure size 1x1 (default) is present as it may need to override current sizes
     items.forEach(n => { n.w = n.w || 1; n.h = n.h || 1 });
 
-    // if we have a mix of new items without coordinates and existing items, separate them out so they can be added after #2639
-    let addAfter = items.filter(n => (n.x === undefined || n.y === undefined) && !Utils.find(this.engine.nodes, n.id));
-    if (addAfter.length && addAfter.length !== items.length) {
-      items = items.filter(n => !Utils.find(addAfter, n.id));
-    } else addAfter = [];
-
-    // if passed list has coordinates, use them (insert from end to beginning for conflict resolution) else keep widget order
-    const haveCoord = items.some(w => w.x !== undefined || w.y !== undefined);
-    if (haveCoord) items = Utils.sort(items, -1);
-    this._insertNotAppend = haveCoord; // if we create in reverse order...
+    // sort items. those without coord will be appended last
+    items = Utils.sort(items);
 
     // if we're loading a layout into for example 1 column and items don't fit, make sure to save
     // the original wanted layout so we can scale back up correctly #1471
-    if (items.some(n => ((n.x || 0) + (n.w || 1)) > column)) {
+    let maxColumn = 0;
+    items.forEach(n => { maxColumn = Math.max(maxColumn, (n.x || 0) + n.w) });
+    if (maxColumn > column) {
       this._ignoreLayoutsNodeChange = true; // skip layout update
-      this.engine.cacheLayout(items, 12, true); // TODO: 12 is arbitrary. use max value in layout ?
+      this.engine.cacheLayout(items, maxColumn, true);
     }
 
     // if given a different callback, temporally set it as global option so creating will use it
@@ -728,19 +716,18 @@ export class GridStack {
     let removed: GridStackNode[] = [];
     this.batchUpdate();
 
-    // if we are blank (loading into empty like startup) temp remove animation
-    const noAnim = !this.engine.nodes.length;
-    if (noAnim) this.setAnimation(false);
+    // if we are loading from empty temporarily remove animation
+    const blank = !this.engine.nodes.length;
+    if (blank) this.setAnimation(false);
 
     // see if any items are missing from new layout and need to be removed first
-    if (addRemove) {
+    if (!blank && addRemove) {
       let copyNodes = [...this.engine.nodes]; // don't loop through array you modify
       copyNodes.forEach(n => {
         if (!n.id) return;
         let item = Utils.find(items, n.id);
         if (!item) {
-          if (GridStack.addRemoveCB)
-            GridStack.addRemoveCB(this.el, n, false, false);
+          if (GridStack.addRemoveCB) GridStack.addRemoveCB(this.el, n, false, false);
           removed.push(n); // batch keep track
           this.removeWidget(n.el, true, false);
         }
@@ -749,6 +736,7 @@ export class GridStack {
 
     // now add/update the widgets - starting with removing items in the new layout we will reposition
     // to reduce collision and add no-coord ones at next available spot
+    this.engine._loading = true; // help with collision
     let updateNodes: GridStackWidget[] = [];
     this.engine.nodes = this.engine.nodes.filter(n => {
       if (Utils.find(items, n.id)) { updateNodes.push(n); return false; } // remove if found from list
@@ -778,28 +766,22 @@ export class GridStack {
           let sub = item.el.querySelector('.grid-stack') as GridHTMLElement;
           if (sub && sub.gridstack) {
             sub.gridstack.load(w.subGridOpts.children); // TODO: support updating grid options ?
-            this._insertNotAppend = true; // got reset by above call
           }
         }
       } else if (addRemove) {
         this.addWidget(w);
       }
     });
 
-    // finally append any separate ones that didn't have explicit coordinates last so they can find next empty spot
-    if (addRemove) {
-      addAfter.forEach(w => this.addWidget(w))
-    }
-
+    delete this.engine._loading; // done loading
     this.engine.removedNodes = removed;
     this.batchUpdate(false);
 
     // after commit, clear that flag
     delete this._ignoreLayoutsNodeChange;
-    delete this._insertNotAppend;
     prevCB ? GridStack.addRemoveCB = prevCB : delete GridStack.addRemoveCB;
     // delay adding animation back
-    if (noAnim && this.opts?.animate) this.setAnimation(this.opts.animate, true);
+    if (blank && this.opts?.animate) this.setAnimation(this.opts.animate, true);
     return this;
   }