Merge pull request #1331 from adumesny/typescript

TS: drag&drop fixes

Author: Alain Dumesny

README.md

@@ -373,7 +373,7 @@ v2.x is a Typescript rewrite of 1.x, removing all jquery events, using classes a
 'verticalMargin()` to get value --> `getMargin()`
 ```
 
-2. event signatures are generic and not jquery-ui dependent anymore. `gsresizestop` has been removed as `resizestop|dragstop` are now called **after** the DOm attributes have been updated.
+2. event signatures are generic and not jquery-ui dependent anymore. `gsresizestop` has been removed as `resizestop|dragstop` are now called **after** the DOM attributes have been updated.
 
 3. `oneColumnMode` would trigger when `window.width` < 768px by default. We now check for grid width instead (more correct and supports nesting). You might need to adjust grid `minWidth` or `disableOneColumnMode`.
 

demo/advance.html

@@ -31,21 +31,21 @@
   <h1>Advanced Demo</h1>
   <div class="row">
     <div class="col-md-2 d-none d-md-block">
-      <div id="trash" style="padding: 15px; margin-bottom: 15px;" class="text-center">
+      <div id="trash" style="padding: 5px; margin-bottom: 15px;" class="text-center">
         <div>
-          <ion-icon name="trash" style="font-size: 400%"></ion-icon>
+          <ion-icon name="trash" style="font-size: 300%"></ion-icon>
         </div>
         <div>
           <span>Drop here to remove!</span>
         </div>
       </div>
       <div class="newWidget grid-stack-item">
-        <div class="card-body grid-stack-item-content">
+        <div class="grid-stack-item-content" style="padding: 5px;">
           <div>
-            <ion-icon name="add-circle" style="font-size: 400%"></ion-icon>
+            <ion-icon name="add-circle" style="font-size: 300%"></ion-icon>
           </div>
           <div>
-            <span>Drag me in into the dashboard!</span>
+            <span>Drag me in the dashboard!</span>
           </div>
         </div>
       </div>
@@ -109,11 +109,11 @@ <h1>Advanced Demo</h1>
       resizable: {
         handles: 'e, se, s, sw, w'
       },
-      removable: '#trash',
-      removeTimeout: 100,
-      dragIn: '.newWidget',
+      acceptWidgets: true,
+      dragIn: '.newWidget',  // class that can be dragged from outside
       dragInOptions: { revert: 'invalid', scroll: false, appendTo: 'body', helper: 'clone' },
-      acceptWidgets: '.newWidget'
+      removable: '#trash', // drag-out delete class
+      removeTimeout: 100,
     });
 
     grid.on('added removed change', function(e, items) {

demo/events.js

@@ -27,8 +27,12 @@ function addEvents(grid, id) {
   });
 
   grid.on('dropped', function(event, previousWidget, newWidget) {
-    console.log(g + 'dropped - Removed widget that was dragged out of grid:', previousWidget);
-    console.log(g + 'dropped - Added widget in dropped grid:', newWidget);
+    if (previousWidget) {
+      console.log(g + 'dropped - Removed widget from grid:', previousWidget);
+    }
+    if (newWidget) {
+      console.log(g + 'dropped - Added widget in grid:', newWidget);
+    }
   });
 
   grid.on('enable', function(event) {

demo/two.html

@@ -13,15 +13,6 @@
   <script src="../dist/gridstack.all.js"></script>
 
   <style type="text/css">
-    .grid-stack {
-      min-height: 50px;
-    }
-    #grid2 {
-      background: lightcyan;
-    }
-    #grid2 .grid-stack-item-content {
-      background-color: #9caabc;
-    }
     .grid-stack-item-removing {
       opacity: 0.5;
     }
@@ -37,11 +28,11 @@
       text-align: center;
     }
     .sidebar .grid-stack-item {
-      width: 200px;
-      height: 100px;
+      width: 120px;
+      height: 50px;
       border: 2px dashed green;
       text-align: center;
-      line-height: 100px;
+      line-height: 35px;
       z-index: 10;
       background: rgba(0, 255, 0, 0.1);
       cursor: default;
@@ -60,7 +51,7 @@ <h1>Two grids demo</h1>
       <div class="col-md-3">
         <div class="sidebar">
 
-          <!-- will size to fit content -->
+          <!-- will size to match content -->
           <div class="grid-stack-item">
             <div class="grid-stack-item-content">Drag me</div>
           </div>
@@ -94,12 +85,14 @@ <h1>Two grids demo</h1>
   <script type="text/javascript">
     let options = {
       column: 6,
+      minRow: 1, // don't collapse when empty
       cellHeight: 70,
       disableOneColumnMode: true,
       float: false,
-      removable: '.trash',
+      dragIn: '.sidebar .grid-stack-item', // class that can be dragged from outside
+      dragInOptions: { revert: 'invalid', scroll: false, appendTo: 'body', helper: 'clone' }, // clone
+      removable: '.trash', // drag-out delete class
       removeTimeout: 100,
-      dragIn: '.sidebar .grid-stack-item',
       acceptWidgets: function(el) { return true; } // function example, else can be simple: true | false | '.someClass' value
     };
     let grids = GridStack.initAll(options);
@@ -117,8 +110,7 @@ <h1>Two grids demo</h1>
       addEvents(grid, i);
       grid.batchUpdate();
       items.forEach(function (node) {
-        grid.addWidget('<div><div class="grid-stack-item-content">' 
-          + (node.text? node.text : '') + '</div></div>', node);
+        grid.addWidget('<div><div class="grid-stack-item-content">' + (node.text? node.text : '') + '</div></div>', node);
       });
       grid.commit();
     });

doc/CHANGES.md

@@ -44,6 +44,7 @@ Change log
 - add `margin` to replace `verticalMargin` which affects both dimensions in code, rather than one in code the other in CSS.
 You can now have perfect square cells (default) [723](https://github.com/gridstack/gridstack.js/issues/723)
 - fix [1299](https://github.com/gridstack/gridstack.js/pull/1299) many columns round-off error
+- fix [1102](https://github.com/gridstack/gridstack.js/issues/1102) lose functionality when they are moved to a new grid
 
 ## 1.2.0 (2020-08-01)
 

doc/README.md

@@ -127,9 +127,9 @@ Extras:
 options you can pass when calling `addWidget()`
 
 - `autoPosition` - tells to ignore `x` and `y` attributes and to place element to the first available position. Having either one missing will also do that.
-- `x`, `y` - (number) element position in row/column. Note: if one is missing this will `autoPosition` the item
-- `width`, `height` - (number) element size in row/column (default 1x1)
-- `maxWidth`, `minWidth`, `maxHeight`, `minHeight` - element constraints in row/column (default none)
+- `x`, `y` - (number) element position in column/row. Note: if one is missing this will `autoPosition` the item
+- `width`, `height` - (number) element size in column/row (default 1x1)
+- `maxWidth`, `minWidth`, `maxHeight`, `minHeight` - element constraints in column/row (default none)
 - `locked` - means another widget wouldn't be able to move it during dragging or resizing.
 The widget can still be dragged or resized by the user.
 You need to add `noResize` and `noMove` attributes to completely lock the widget.

spec/e2e/html/1102-button-between-grids.html

@@ -0,0 +1,45 @@
+<!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>lose functionality</title>
+
+  <link rel="stylesheet" href="../../../demo/demo.css"/>
+
+  <script src="../../../dist/gridstack.all.js"></script>
+</head>
+<body>
+  <div class="container-fluid">
+    <h1>lose functionality when dragged</h1>
+    <br/>
+    <div class="grid-stack" id="right-grid">
+      <div class="grid-stack-item">
+        <div class="grid-stack-item-content" id="button1">button1</div>
+      </div>
+    </div>
+     <br/>
+    <br/>
+    <div class="grid-stack" id="left-grid">
+      <div class="grid-stack-item">
+        <div class="grid-stack-item-content" id="button2">button2</div>
+      </div>
+    </div>
+  </div>
+
+  <script type="text/javascript">
+    GridStack.initAll({acceptWidgets: true});
+
+    function pressed1(){
+      $('.container-fluid').append("button 1 pressed ");
+    }
+    
+    function pressed2(){
+      $('.container-fluid').append("button 2 pressed ");
+    }
+    $('#button1').click(pressed1);
+    $('#button2').click(pressed2);
+  </script>
+</body>
+</html>

src/gridstack-dd.ts

@@ -23,7 +23,7 @@ export type DDKey = 'minWidth' | 'minHeight' | string;
 export type DDValue = number | string;
 
 /** drag&drop events callbacks */
-export type DDCallback = (event: Event, arg2: GridItemHTMLElement) => void;
+export type DDCallback = (event: Event, arg2: GridItemHTMLElement, helper?: GridItemHTMLElement) => void;
 
 /**
  * Base class for drag'n'drop plugin.

src/gridstack-engine.ts

@@ -320,7 +320,7 @@ export class GridStackEngine {
   }
 
   public removeNode(node: GridStackNode, removeDOM = true, triggerRemoveEvent = false): GridStackEngine {
-    if (triggerRemoveEvent) {
+    if (triggerRemoveEvent) { // we wait until final drop to manually track removed items (rather than during drag)
       this.removedNodes.push(node);
     }
     node._id = null; // hint that node is being removed

src/gridstack.ts

@@ -1322,11 +1322,12 @@ export class GridStack {
 
     /** called when the item stops moving/resizing */
     let onEndMoving = (event: Event) => {
+      if (this.placeholder.parentNode === this.el) { this.el.removeChild(this.placeholder) }
+
+      // if the item has moved to another grid, we're done here
       let target: GridItemHTMLElement = event.target as GridItemHTMLElement;
-      if (!target.gridstackNode) return;
+      if (!target.gridstackNode || target.gridstackNode.grid !== this) return;
 
-      // let forceNotify = false; what is the point of calling 'change' event with no data, when the 'removed' event is already called ?
-      if (this.placeholder.parentNode === this.el) { this.el.removeChild(this.placeholder) }
       node.el = target;
 
       if (node._isAboutToRemove) {
@@ -1623,8 +1624,8 @@ export class GridStack {
         // if not calculate the grid size based on element outer size
         let cellWidth = this.cellWidth();
         let cellHeight = this.getCellHeight();
-        width = node && node.width ? node.width : Math.ceil(el.offsetWidth / cellWidth);
-        height = node && node.height ? node.height : Math.round(el.offsetHeight / cellHeight);
+        width = node && node.width ? node.width : Math.round(el.offsetWidth / cellWidth) || 1;
+        height = node && node.height ? node.height : Math.round(el.offsetHeight / cellHeight) || 1;
 
         let newNode = this.engine.prepareNode({width, height, _added: false, _temporary: true});
         newNode._isOutOfGrid = true;
@@ -1651,43 +1652,55 @@ export class GridStack {
         el.gridstackNode = el._gridstackNodeOrig;
         return false; // prevent parent from receiving msg (which may be grid as well)
       })
-      .on(this.el, 'drop', (event, _el: GridItemHTMLElement) => {
-        if (this.placeholder.parentNode === this.el) {
-          this.el.removeChild(this.placeholder);
+      .on(this.el, 'drop', (event, el: GridItemHTMLElement, helper: GridItemHTMLElement) => {
+        this.placeholder.remove();
+
+        // notify of removal from prev grid...
+        let origNode = el._gridstackNodeOrig;
+        delete el._gridstackNodeOrig;
+        if (origNode && origNode.grid && origNode.grid !== this) {
+          let oGrid = origNode.grid;
+          oGrid.placeholder.remove();
+          origNode.el = el; // was using placeholder, have it point to node we've moved instead
+          oGrid.engine.removedNodes.push(origNode);
+          oGrid._triggerRemoveEvent();
         }
-        let node: GridStackNode = _el.gridstackNode;
-        this.engine.cleanupNode(node);
-        node.grid = this;
-        let originalNode = _el._gridstackNodeOrig;
-        delete _el.gridstackNode;
-        delete _el._gridstackNodeOrig;
-        this.dd
-          .off(_el, 'drag')
-          .draggable(_el, 'destroy')
-          .resizable(_el, 'destroy');
 
-        let el = _el.cloneNode(true) as GridItemHTMLElement;
-
-        el.gridstackNode = node;
-        if (originalNode && originalNode.grid) {
-          originalNode.grid._triggerRemoveEvent();
+        let node: GridStackNode = el.gridstackNode; // use existing placeholder node as it's already in our list with drop location
+        this.engine.cleanupNode(node); // remove all internal _xyz values
+        node.grid = this;
+        this.dd.off(el, 'drag');
+        // if we made a copy ('helper' which is temp) of the original node then insert a copy, else we move the original node (#1102)
+        // as the helper will be nuked by default (by jqueryui and here to make it the same)
+        if (helper !== el) {
+          helper.remove();
+          el.gridstackNode = origNode; // original item (left behind) is re-stored to pre dragging as the node now has drop info
+          el = el.cloneNode(true) as GridItemHTMLElement;
+        } else {
+          el.remove(); // reduce flicker as we change depth here, and size further down
+          this.dd
+            .draggable(el, 'destroy')
+            .resizable(el, 'destroy');
         }
-        _el.remove();
+        el.gridstackNode = node;
         node.el = el;
-        Utils.removePositioningStyles(el);
 
+        Utils.removePositioningStyles(el);
         this._writeAttr(el, node);
         this.el.appendChild(el);
-        this._prepareElementsByNode(el, node);
         this._updateContainerHeight();
         this.engine.addedNodes.push(node);
         this._triggerAddEvent();
         this._triggerChangeEvent();
 
         this.engine.endUpdate();
         if (this._gsEventHandler['dropped']) {
-          this._gsEventHandler['dropped']({type: 'dropped'}, originalNode, node);
+          this._gsEventHandler['dropped']({type: 'dropped'}, origNode && origNode.grid ? origNode : undefined, node);
         }
+
+        // wait till we return out of the drag callback to set the new drag&resize handler or they may get messed up
+        window.setTimeout(() => this._prepareElementsByNode(el, node));
+
         return false; // prevent parent from receiving msg (which may be grid as well)
       });
     return this;