From 78327476eb67114fc257deb0348102d9a7bf81e3 Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Thu, 16 May 2019 23:04:53 -0700
Subject: [PATCH] feat: improve margin merging (#159)

---
 .../superset-ui-dimension/src/mergeMargin.ts  | 21 +++++++++-------
 .../test/mergeMargin.test.ts                  | 24 +++++++++++++++++++
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/packages/superset-ui-dimension/src/mergeMargin.ts b/packages/superset-ui-dimension/src/mergeMargin.ts
index 82b51f91b9..9a6d835a46 100644
--- a/packages/superset-ui-dimension/src/mergeMargin.ts
+++ b/packages/superset-ui-dimension/src/mergeMargin.ts
@@ -1,19 +1,24 @@
 import { Margin } from './types';
 
+function mergeOneSide(a: number = 0, b: number = 0, operation: (a: number, b: number) => number) {
+  if (Number.isNaN(a) || a === null) return b;
+  else if (Number.isNaN(b) || b === null) return a;
+
+  return operation(a, b);
+}
+
 export default function mergeMargin(
   margin1: Partial<Margin> = {},
   margin2: Partial<Margin> = {},
   mode: 'expand' | 'shrink' = 'expand',
 ) {
-  const { top = 0, left = 0, bottom = 0, right = 0 } = margin1;
-  const { top: top2 = 0, left: left2 = 0, bottom: bottom2 = 0, right: right2 = 0 } = margin2;
-
-  const func = mode === 'expand' ? Math.max : Math.min;
+  const { top, left, bottom, right } = margin1;
+  const operation = mode === 'expand' ? Math.max : Math.min;
 
   return {
-    bottom: func(bottom, bottom2),
-    left: func(left, left2),
-    right: func(right, right2),
-    top: func(top, top2),
+    bottom: mergeOneSide(bottom, margin2.bottom, operation),
+    left: mergeOneSide(left, margin2.left, operation),
+    right: mergeOneSide(right, margin2.right, operation),
+    top: mergeOneSide(top, margin2.top, operation),
   };
 }
diff --git a/packages/superset-ui-dimension/test/mergeMargin.test.ts b/packages/superset-ui-dimension/test/mergeMargin.test.ts
index 101feb401f..f061cedcd7 100644
--- a/packages/superset-ui-dimension/test/mergeMargin.test.ts
+++ b/packages/superset-ui-dimension/test/mergeMargin.test.ts
@@ -163,4 +163,28 @@ describe('mergeMargin(margin1, margin2, mode?)', () => {
       right: -1,
     });
   });
+  it('if there are NaN or null, use another value', () => {
+    expect(
+      mergeMargin(
+        // @ts-ignore to let us pass `null` for testing
+        {
+          top: 10,
+          left: null,
+          bottom: 20,
+          right: NaN,
+        },
+        {
+          top: NaN,
+          left: 30,
+          bottom: null,
+          right: 40,
+        },
+      ),
+    ).toEqual({
+      top: 10,
+      left: 30,
+      bottom: 20,
+      right: 40,
+    });
+  });
 });