From 3ad2b3161e184b9f27ac49eecfe4cd5f4e8d11e3 Mon Sep 17 00:00:00 2001
From: Alain Dumesny <alaind831+github@gmail.com>
Date: Thu, 5 Nov 2020 08:28:19 -0800
Subject: [PATCH] fix setting marginTop to break resizing

* better fix than #1441 - thanks for suggestion!
* when reisizng we incorrectly use getMargin() which is correctly 'undefined' if all 4 sides don't match
(should had bee using top+bottom but actually  doesn't need any margin, like horizontal)
* fiuxed test cases to check for these scenarios
---
 doc/CHANGES.md         |  3 ++-
 spec/gridstack-spec.ts | 48 +++++++++++++++++++++++++++++++++---------
 src/gridstack.ts       |  7 ++++--
 3 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/doc/CHANGES.md b/doc/CHANGES.md
index 9d402fe3b..5200a0f2a 100644
--- a/doc/CHANGES.md
+++ b/doc/CHANGES.md
@@ -43,8 +43,9 @@ Change log
 ## 2.1.0-dev
 
 - fix `class="ui-draggable-disabled ui-resizable-disabled"` have been added back to static grid items, so existing CSS rule to style continue working [1435](https://github.com/gridstack/gridstack.js/issues/1435)
-- add `data-gs-staticGrid` attribute
+- add `data-gs-static-grid` attribute
 - fix getting DOM element by id with number works (api that uses `GridStackElement` handle more string formats)
+- fix setting `marginTop` (or any 4 sides) to cause resize to break. Thanks [@deadivan](https://github.com/deadivan) for suggested fix.
 
 ## 2.1.0 (2020-10-28)
 
diff --git a/spec/gridstack-spec.ts b/spec/gridstack-spec.ts
index bbcca38eb..bd609b713 100644
--- a/spec/gridstack-spec.ts
+++ b/spec/gridstack-spec.ts
@@ -1172,11 +1172,10 @@ describe('gridstack', function() {
     it('should return margin', function() {
       let options = {
         cellHeight: 80,
-        margin: 10
+        margin: 12
       };
       let grid = GridStack.init(options);
-      let vm = grid.getMargin();
-      expect(vm).toBe(10);
+      expect(grid.getMargin()).toBe(12);
     });
     it('should return update margin', function() {
       let options = {
@@ -1184,31 +1183,60 @@ describe('gridstack', function() {
         margin: 5
       };
       let grid = GridStack.init(options);
-      grid.margin(11);
+      grid.margin('11rem');
       expect(grid.getMargin()).toBe(11);
     });
-    it('should do nothing', function() {
+    it('should change unit', function() {
       let options = {
         cellHeight: 80,
         margin: 10,
       };
       let grid = GridStack.init(options);
       expect(grid.getMargin()).toBe(10);
-      grid.margin(10);
+      grid.margin('10rem');
       expect(grid.getMargin()).toBe(10);
     });
-    /*
-    it('should not update styles', function() {
+    it('should not update styles, with same value', function() {
       let options = {
         cellHeight: 80,
         margin: 5
       };
       let grid: any = GridStack.init(options);
+      expect(grid.getMargin()).toBe(5);
       spyOn(grid, '_updateStyles');
-      grid.margin(11, false);
+      grid.margin('5px', false);
       expect(grid._updateStyles).not.toHaveBeenCalled();
+      expect(grid.getMargin()).toBe(5);
+    });
+    it('should set top/bot/left value directly', function() {
+      let options = {
+        cellHeight: 80,
+        marginTop: 5,
+        marginBottom: 0,
+        marginLeft: 1,
+      };
+      let grid: any = GridStack.init(options);
+      expect(grid.getMargin()).toBe(undefined);
+      expect(grid.opts.marginTop).toBe(5);
+      expect(grid.opts.marginBottom).toBe(0);
+      expect(grid.opts.marginLeft).toBe(1);
+      expect(grid.opts.marginRight).toBe(10); // default value
+    });
+    it('should set all 4 sides, and overall margin', function() {
+      let options = {
+        cellHeight: 80,
+        marginTop: 5,
+        marginBottom: 5,
+        marginLeft: 5,
+        marginRight: 5,
+      };
+      let grid: any = GridStack.init(options);
+      expect(grid.getMargin()).toBe(5);
+      expect(grid.opts.marginTop).toBe(5);
+      expect(grid.opts.marginBottom).toBe(5);
+      expect(grid.opts.marginLeft).toBe(5);
+      expect(grid.opts.marginRight).toBe(5);
     });
-    */
   });
 
   describe('grid.opts.rtl', function() {
diff --git a/src/gridstack.ts b/src/gridstack.ts
index c4237b1f0..860abecdf 100644
--- a/src/gridstack.ts
+++ b/src/gridstack.ts
@@ -1041,7 +1041,7 @@ export class GridStack {
     return this;
   }
 
-  /** returns current vertical margin value */
+  /** returns current margin value (undefined if all 4 sides don't match) */
   public getMargin(): number { return this.opts.margin as number; }
 
   /**
@@ -1324,7 +1324,7 @@ export class GridStack {
       } else if (event.type === 'resize')  {
         if (x < 0) return;
         width = Math.round(ui.size.width / cellWidth);
-        height = Math.round((ui.size.height + this.getMargin()) / cellHeight);
+        height = Math.round(ui.size.height / cellHeight);
       }
       // width and height are undefined if not resizing
       let _lastTriedWidth = (width || node._lastTriedWidth);
@@ -1855,6 +1855,9 @@ export class GridStack {
       delete this.opts.margin;
     }
     this.opts.marginUnit = data.unit; // in case side were spelled out, use those units instead...
+    if (this.opts.marginTop === this.opts.marginBottom && this.opts.marginLeft === this.opts.marginRight && this.opts.marginTop === this.opts.marginRight) {
+      this.opts.margin = this.opts.marginTop; // makes it easier to check for no-ops in setMargin()
+    }
     return this;
   }