more change event fixes

* pass the original nodes for addEvent/removeEvent instead of clones
* _triggerChangeEvent() no longer forced called (with no data, what's the point ?)
* commit() calls remove, added, change in that order
* updated more samples - use batch calls
(the difference in callbacks noise of 0.5.5 vs this code is UDGE for serialize.html as an example)
* fixed error when dragging add item over the trash, and then over second grid

README.md

@@ -63,16 +63,16 @@ Using gridstack.js with jQuery UI
 
 ```html
 <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/gridstack@0.5.5/dist/gridstack.min.css" />
-<script type="text/javascript" src="https://cdn.jsdelivr.net/npm/gridstack@0.5.5/dist/gridstack.all.js"></script>
+<script src="https://cdn.jsdelivr.net/npm/gridstack@0.5.5/dist/gridstack.all.js"></script>
 ```
 
 * Using CDN (debug):
 
 ```html
 <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/gridstack@0.5.5/dist/gridstack.css" />
-<script type="text/javascript" src="https://cdn.jsdelivr.net/npm/gridstack@0.5.5/dist/gridstack.js"></script>
-<script type="text/javascript" src="https://cdn.jsdelivr.net/npm/gridstack@0.5.5/dist/jquery-ui.js"></script>
-<script type="text/javascript" src="https://cdn.jsdelivr.net/npm/gridstack@0.5.5/dist/gridstack.jQueryUI.js"></script>
+<script src="https://cdn.jsdelivr.net/npm/gridstack@0.5.5/dist/gridstack.js"></script>
+<script src="https://cdn.jsdelivr.net/npm/gridstack@0.5.5/dist/jquery-ui.js"></script>
+<script src="https://cdn.jsdelivr.net/npm/gridstack@0.5.5/dist/gridstack.jQueryUI.js"></script>
 ```
 
 * or local:

demo/advance.html

@@ -53,7 +53,7 @@ <h1>Advanced Demo</h1>
       </div>
     </div>
     <div class="col-sm-12 col-md-10">
-      <div class="grid-stack" id="advanced-grid" data-gs-width="12" data-gs-animate="yes">
+      <div class="grid-stack" id="advanced-grid" data-gs-animate="yes">
         <div class="grid-stack-item" data-gs-x="0" data-gs-y="0" data-gs-width="4" data-gs-height="2">
           <div class="grid-stack-item-content">1</div>
         </div>
@@ -105,7 +105,19 @@ <h1>Advanced Demo</h1>
 
   <script type="text/javascript">
     $(function () {
-      $('#advanced-grid').gridstack({
+      var $grid = $('#advanced-grid');
+
+      $grid.on('added', function(e, items) { log(' added ', items) });
+      $grid.on('removed', function(e, items) { log(' removed ', items) });
+      $grid.on('change', function(e, items) { log(' change ', items) });
+      function log(type, items) {
+        if (!items) { return; }
+        var str = '';
+        for (let i=0; i<items.length && items[i]; i++) { str += ' (x,y)=' + items[i].x + ',' + items[i].y; }
+        console.log(type + items.length + ' items.' + str );
+      }
+
+      $grid.gridstack({
         alwaysShowResizeHandle: /Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(
           navigator.userAgent
         ),
@@ -116,6 +128,7 @@ <h1>Advanced Demo</h1>
         removeTimeout: 100,
         acceptWidgets: '.newWidget'
       });
+
       $('.newWidget').draggable({
         revert: 'invalid',
         scroll: false,

demo/responsive.html

@@ -85,11 +85,13 @@ <h1>Responsive grid demo</h1>
         this.grid = $('.grid-stack').data('gridstack');
 
         this.loadGrid = function () {
+          this.grid.batchUpdate();
           this.grid.removeAll();
           var items = GridStackUI.Utils.sort(this.serializedData);
           items.forEach(function (node, i) {
             this.grid.addWidget($('<div><div class="grid-stack-item-content">' + i + '</div></div>'), node);
           }.bind(this));
+          this.grid.commit();
           return false;
         }.bind(this);
 

demo/serialization.html

@@ -27,9 +27,19 @@ <h1>Serialization demo</h1>
 
   <script type="text/javascript">
     $(function () {
-      var options = {
-      };
-      $('.grid-stack').gridstack(options);
+      var $grid = $('.grid-stack');
+
+      $grid.on('added', function(e, items) { log(' added ', items) });
+      $grid.on('removed', function(e, items) { log(' removed ', items) });
+      $grid.on('change', function(e, items) { log(' change ', items) });
+      function log(type, items) {
+        if (!items) { return; }
+        var str = '';
+        for (let i=0; i<items.length && items[i]; i++) { str += ' (x,y)=' + items[i].x + ',' + items[i].y; }
+        console.log(type + items.length + ' items.' + str );
+      }
+
+      $grid.gridstack();
 
       new function () {
         this.serializedData = [
@@ -46,11 +56,13 @@ <h1>Serialization demo</h1>
         this.grid = $('.grid-stack').data('gridstack');
 
         this.loadGrid = function () {
+          this.grid.batchUpdate();
           this.grid.removeAll();
           var items = GridStackUI.Utils.sort(this.serializedData);
           items.forEach(function (node) {
             this.grid.addWidget($('<div><div class="grid-stack-item-content"></div></div>'), node);
           }, this);
+          this.grid.commit();
           return false;
         }.bind(this);
 

demo/two.html

@@ -114,8 +114,9 @@ <h1>Two grids demo</h1>
         $grid.on('removed', function(e, items) { log(id, ' removed ', items) });
         $grid.on('change', function(e, items) { log(id, ' change ', items) });
         function log(id, type, items) {
+          if (!items) { return; }
           var str = '';
-          for (let i=0; i<items.length; i++ ) { str += ' (x,y)=' + items[i].x + ',' + items[i].y /*+ ' ' + items[i].width + 'x' + items[i].height*/ }
+          for (let i=0; i<items.length && items[i]; i++) { str += ' (x,y)=' + items[i].x + ',' + items[i].y; }
           console.log(id + type + items.length + ' items.' + str );
         }
 

doc/CHANGES.md

@@ -31,6 +31,7 @@ Change log
 - Allow percentage as a valid unit for height [#1093](https://github.com/gridstack/gridstack.js/pull/1093)
 - fixed callbacks to get either `added, removed, change` or combination if adding a node require also to change its (x,y) for example.
 Also you can now call `batchUpdate()` before calling a bunch of `addWidget()` and get a single event callback (more efficient).
+[#1096](https://github.com/gridstack/gridstack.js/pull/1096)
 
 ## v0.5.5 (2019-11-27)
 

spec/e2e/html/column.html

@@ -37,9 +37,11 @@ <h1>setColumn() grid demo</h1>
         {x: 2, y: 0, width: 2, height: 1},
         {x: 5, y: 0, width: 1, height: 1},
       ];
+      self.grid.batchUpdate();
       items.forEach(function (node, i) {
         self.grid.addWidget($('<div><div class="grid-stack-item-content">' + i + '</div></div>'), node);
       });
+      self.grid.batchUpdate();
 
       $('#1column').click(function() { self.grid.setColumn(1); });
       $('#2column').click(function() { self.grid.setColumn(2); });

spec/e2e/html/gridstack-with-height.html

@@ -35,6 +35,7 @@ <h1>gridstack.js tests</h1>
         ];
 
         this.grid = $('.grid-stack').data('gridstack');
+        this.grid.batchUpdate();
         this.grid.removeAll();
         items = GridStackUI.Utils.sort(items);
         var id = 0;
@@ -43,6 +44,7 @@ <h1>gridstack.js tests</h1>
           w.attr('id', 'item-' + (++id));
           this.grid.addWidget(w, node);
         }, this);
+        this.grid.commit();
       };
     });
   </script>

src/gridstack.js

@@ -877,7 +877,7 @@
         .on(trashZone, 'dropover', function(event, ui) {
           var el = $(ui.draggable);
           var node = el.data('_gridstack_node');
-          if (node._grid !== self) {
+          if (!node || node._grid !== self) {
             return;
           }
           el.data('inTrashZone', true);
@@ -886,7 +886,7 @@
         .on(trashZone, 'dropout', function(event, ui) {
           var el = $(ui.draggable);
           var node = el.data('_gridstack_node');
-          if (node._grid !== self) {
+          if (!node || node._grid !== self) {
             return;
           }
           el.data('inTrashZone', false);
@@ -1028,36 +1028,28 @@
     }
   };
 
-  GridStack.prototype._triggerChangeEvent = function(forceTrigger) {
+  GridStack.prototype._triggerChangeEvent = function(/*forceTrigger*/) {
     if (this.grid._batchMode) { return; }
     // TODO: compare original X,Y,W,H (or entire node?) instead as _dirty can be a temporary state
     var elements = this.grid.getDirtyNodes();
-    var hasChanges = false;
-
-    var eventParams = []; // eventParams[0] has to be the list of items...
     if (elements && elements.length) {
-      eventParams[0] = elements;
-      hasChanges = true;
-    }
-
-    if (hasChanges || forceTrigger === true) {
-      this.container.trigger('change', eventParams);
+      this.container.trigger('change', [elements]);
       this.grid.cleanNodes(); // clear dirty flags now that we called
     }
   };
 
   GridStack.prototype._triggerAddEvent = function() {
     if (this.grid._batchMode) { return; }
     if (this.grid._addedNodes && this.grid._addedNodes.length > 0) {
-      this.container.trigger('added', [this.grid._addedNodes.map(Utils.clone)]);
+      this.container.trigger('added', [this.grid._addedNodes]);
       this.grid._addedNodes = [];
     }
   };
 
   GridStack.prototype._triggerRemoveEvent = function() {
     if (this.grid._batchMode) { return; }
     if (this.grid._removedNodes && this.grid._removedNodes.length > 0) {
-      this.container.trigger('removed', [this.grid._removedNodes.map(Utils.clone)]);
+      this.container.trigger('removed', [this.grid._removedNodes]);
       this.grid._removedNodes = [];
     }
   };
@@ -1319,13 +1311,13 @@
         return;
       }
 
-      var forceNotify = false;
+      // var forceNotify = false; what is the point of calling 'change' event with no data, when the 'removed' event is already called ?
       self.placeholder.detach();
       node.el = o;
       self.placeholder.hide();
 
       if (node._isAboutToRemove) {
-        forceNotify = true;
+        // forceNotify = true;
         var gridToNotify = el.data('_gridstack_node')._grid;
         gridToNotify._triggerRemoveEvent();
         el.removeData('_gridstack_node');
@@ -1353,7 +1345,7 @@
         }
       }
       self._updateContainerHeight();
-      self._triggerChangeEvent(forceNotify);
+      self._triggerChangeEvent(/*forceNotify*/);
 
       self.grid.endUpdate();
 
@@ -1770,8 +1762,8 @@
   GridStack.prototype.commit = function() {
     this.grid.commit();
     this._updateContainerHeight();
-    this._triggerAddEvent();
     this._triggerRemoveEvent();
+    this._triggerAddEvent();
     this._triggerChangeEvent();
   };