From 099866cfc8e3f0a47f709ec7e60a1d13eb5b46fd Mon Sep 17 00:00:00 2001
From: Walter Leibbrandt <23798+walterl@users.noreply.github.com>
Date: Thu, 22 Aug 2019 01:51:57 +0000
Subject: [PATCH] Filter out style properties with empty values (#10650)

* Filter out style properties with empty values

This caused CSS to break when `{:background-color nil}` was rendered as
`background-color: ;`.

* Use proper nil punning

* Convert `v` to string before passing to `seq`
---
 src/metabase/pulse/render/style.clj       |  3 ++-
 test/metabase/pulse/render/style_test.clj | 12 ++++++++++++
 test/metabase/pulse/render/table_test.clj |  4 ++--
 3 files changed, 16 insertions(+), 3 deletions(-)
 create mode 100644 test/metabase/pulse/render/style_test.clj

diff --git a/src/metabase/pulse/render/style.clj b/src/metabase/pulse/render/style.clj
index 717932042ab..307aa7f5e5f 100644
--- a/src/metabase/pulse/render/style.clj
+++ b/src/metabase/pulse/render/style.clj
@@ -11,7 +11,8 @@
      (style {:font-weight 400, :color \"white\"}) -> \"font-weight: 400; color: white;\""
   [& style-maps]
   (str/join " " (for [[k v] (into {} style-maps)
-                      :let  [v (if (keyword? v) (name v) v)]]
+                      :let  [v (if (keyword? v) (name v) (str v))]
+                      :when (seq v)]
                   (str (name k) ": " v ";"))))
 
 (def ^:const color-brand
diff --git a/test/metabase/pulse/render/style_test.clj b/test/metabase/pulse/render/style_test.clj
new file mode 100644
index 00000000000..d30508ca646
--- /dev/null
+++ b/test/metabase/pulse/render/style_test.clj
@@ -0,0 +1,12 @@
+(ns metabase.pulse.render.style-test
+  (:require [expectations :refer [expect]]
+            [metabase.pulse.render.style :as style]))
+
+;; `style` should filter out nil values
+(expect
+  ""
+  (style/style {:a nil}))
+
+(expect
+  "a: 0; c: 2;"
+  (style/style {:a 0, :b nil, :c 2, :d ""}))
diff --git a/test/metabase/pulse/render/table_test.clj b/test/metabase/pulse/render/table_test.clj
index c23c3671cb2..a8275e6681c 100644
--- a/test/metabase/pulse/render/table_test.clj
+++ b/test/metabase/pulse/render/table_test.clj
@@ -56,8 +56,8 @@
 ;; we should find some similar basic values that can rely on. The goal isn't to test out the javascript choosing in
 ;; the color (that should be done in javascript) but to verify that the pieces are all connecting correctly
 (expect
-  {"1" "",                     "2" "",                     "3" "rgba(0, 255, 0, 0.75)"
-   "4" "",                     "5" "",                     "6" "rgba(0, 128, 128, 0.75)"
+  {"1" nil,                    "2" nil,                    "3" "rgba(0, 255, 0, 0.75)"
+   "4" nil,                    "5" nil,                    "6" "rgba(0, 128, 128, 0.75)"
    "7" "rgba(255, 0, 0, 0.65)" "8" "rgba(255, 0, 0, 0.2)"  "9" "rgba(0, 0, 255, 0.75)"}
   (let [query-results {:cols [{:name "a"} {:name "b"} {:name "c"}]
                        :rows [[1 2 3]
-- 
GitLab