Merge pull request #2341 from adumesny/master

`removeNode()` uses internal _id

Author: Alain Dumesny

angular/projects/demo/src/app/app.component.ts

@@ -35,12 +35,13 @@ export class AppComponent implements OnInit {
   ];
   public gridOptions: GridStackOptions = {
     margin: 5,
-    float: true,
+    // float: true,
     minRow: 1,
   }
+  private sub0: NgGridStackWidget[] = [{x:0, y:0, selector:'app-a'}, {x:1, y:0, content:'plain html'}, {x:0, y:1, selector:'app-b'} ];
   public gridOptionsFull: NgGridStackOptions = {
     ...this.gridOptions,
-    children: [{x:0, y:0, selector:'app-a'}, {x:1, y:0, selector:'app-b'}, {x:2, y:0, content:'plain html'}],
+    children: this.sub0,
   }
 
   // nested grid options
@@ -70,8 +71,8 @@ export class AppComponent implements OnInit {
 
   constructor() {
     // give them content and unique id to make sure we track them during changes below...
-    [...this.items, ...this.subChildren, ...this.sub1, ...this.sub2].forEach((w: NgGridStackWidget) => {
-      if (!w.selector && !w.subGridOpts) w.content = `item ${ids}`;
+    [...this.items, ...this.subChildren, ...this.sub1, ...this.sub2, ...this.sub0].forEach((w: NgGridStackWidget) => {
+      if (!w.selector && !w.content && !w.subGridOpts) w.content = `item ${ids}`;
       w.id = String(ids++);
     });
   }

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/)*
 
+- [8.2.0-dev (TBD)](#820-dev-tbd)
 - [8.2.0 (2023-05-24)](#820-2023-05-24)
 - [8.1.2 (2023-05-22)](#812-2023-05-22)
 - [8.1.1 (2023-05-13)](#811-2023-05-13)
@@ -88,6 +89,9 @@ Change log
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
 
+## 8.2.0-dev (TBD)
+* fix: make sure `removeNode()` uses internal _id (unique) and not node itself (since we clone those often)
+
 ## 8.2.0 (2023-05-24)
 * feat: `makeWidget()` now take optional `GridStackWidget` for sizing
 * fix: make sure `GridStack.saveCB` is call in `removeWidget()`

src/gridstack-engine.ts

@@ -126,10 +126,14 @@ export class GridStackEngine {
 
   /** return the nodes that intercept the given node. Optionally a different area can be used, as well as a second node to skip */
   public collide(skip: GridStackNode, area = skip, skip2?: GridStackNode): GridStackNode {
-    return this.nodes.find(n => n !== skip && n !== skip2 && Utils.isIntercepted(n, area));
+    const skipId = skip._id;
+    const skip2Id = skip2?._id;
+    return this.nodes.find(n => n._id !== skipId && n._id !== skip2Id && Utils.isIntercepted(n, area));
   }
   public collideAll(skip: GridStackNode, area = skip, skip2?: GridStackNode): GridStackNode[] {
-    return this.nodes.filter(n => n !== skip && n !== skip2 && Utils.isIntercepted(n, area));
+    const skipId = skip._id;
+    const skip2Id = skip2?._id;
+    return this.nodes.filter(n => n._id !== skipId && n._id !== skip2Id && Utils.isIntercepted(n, area));
   }
 
   /** does a pixel coverage collision based on where we started, returning the node that has the most coverage that is >50% mid line */
@@ -522,7 +526,7 @@ export class GridStackEngine {
   }
 
   public removeNode(node: GridStackNode, removeDOM = true, triggerEvent = false): GridStackEngine {
-    if (!this.nodes.find(n => n === node)) {
+    if (!this.nodes.find(n => n._id === node._id)) {
       // TEST console.log(`Error: GridStackEngine.removeNode() node._id=${node._id} not found!`)
       return this;
     }
@@ -531,7 +535,7 @@ export class GridStackEngine {
     }
     if (removeDOM) node._removeDOM = true; // let CB remove actual HTML (used to set _id to null, but then we loose layout info)
     // don't use 'faster' .splice(findIndex(),1) in case node isn't in our list, or in multiple times.
-    this.nodes = this.nodes.filter(n => n !== node);
+    this.nodes = this.nodes.filter(n => n._id !== node._id);
     return this._packNodes()
       ._notify([node]);
   }
@@ -564,7 +568,7 @@ export class GridStackEngine {
       column: this.column,
       float: this.float,
       nodes: this.nodes.map(n => {
-        if (n === node) {
+        if (n._id === node._id) {
           clonedNode = {...n};
           return clonedNode;
         }