Make table column width reasonable (#25436)

* Make table column width reasonable

Truncate column name that is longer than 16 characters

* Add a test to check header truncation of tables

Also adjusted the shape of the header fn's return data a bit -> the `[:th ... ]` vectors aren't wrapped in a seq anymore.

* Fix wrong max column character length 



* Update static viz table style

* Address PR comment, simplifying the code

Co-authored-by: Adam James <adam.vermeer2@gmail.com>

src/metabase/pulse/render/style.clj

@@ -60,13 +60,9 @@
   "Color for dark text."
   "#4C5773")
 
-(def ^:const color-header-row-border
-  "Used as color for the bottom border of table headers for charts with `:table` vizualization."
-  "#EDF0F1")
-
-(def ^:const color-body-row-border
-  "Used as color for the bottom border of table body rows for charts with `:table` vizualization."
-  "#F0F0F04D")
+(def ^:const color-border
+  "Used as color for the border of table, table header, and table body rows for charts with `:table` vizualization."
+  "#F0F0F0")
 
 ;; don't try to improve the code and make this a plain variable, in EE it's customizable which is why it's a function.
 ;; Too much of a hassle to have it be a fn in one version of the code an a constant in another
@@ -95,7 +91,7 @@
   []
   (merge
    (font-style)
-   {:font-size       :14px
+   {:font-size       :18px
     :font-weight     700
     :color           (primary-color)
     :text-decoration :none}))

src/metabase/pulse/render/table.clj

 (ns metabase.pulse.render.table
-  (:require [hiccup.core :refer [h]]
+  (:require [clojure.string :as str]
+            [hiccup.core :refer [h]]
             [medley.core :as m]
             [metabase.pulse.render.color :as color]
             metabase.pulse.render.common
@@ -13,10 +14,9 @@
    (style/font-style)
    {:font-size :12px
     :font-weight     700
-    :color           style/color-text-medium
-    :border-bottom   (str "1px solid " style/color-header-row-border)
-    :padding-top     :20px
-    :padding-bottom  :5px}))
+    :color           style/color-text-dark
+    :border-bottom   (str "2px solid " style/color-border)
+    :border-right    0}))
 
 (def ^:private max-bar-width 106)
 
@@ -24,13 +24,12 @@
   (merge
    (style/font-style)
    {:font-size      :12px
-    :font-weight    700
+    :font-weight    400
     :text-align     :left
     :color          style/color-text-dark
-    :border-bottom  (str "1px solid " style/color-body-row-border)
-    :height         :28px
-    :padding-right  :0.375em
-    :padding-left   :0.375em}))
+    :border-bottom  (str "1px solid " style/color-border)
+    :border-right   (str "1px solid " style/color-border)
+    :padding        "0.75em 1em"}))
 
 (defn- bar-th-style-numeric []
   (merge (style/font-style) (bar-th-style) {:text-align :right}))
@@ -74,14 +73,21 @@
   [score]
   (int (* (/ score 100.0) max-bar-width)))
 
+(def ^:private max-column-character-length 16)
+
+(defn- truncate-text [text]
+  (if (> (count text) max-column-character-length)
+    (str (str/trim (subs text 0 max-column-character-length)) "...")
+    text))
+
 (defn- render-table-head [{:keys [bar-width row]}]
   [:thead
-   [:tr
-    (for [header-cell row]
-      [:th {:style (style/style (row-style-for-type header-cell) (heading-style-for-type header-cell) {:min-width :42px})}
-       (h header-cell)])
-    (when bar-width
-      [:th {:style (style/style (bar-td-style) (bar-th-style) {:width (str bar-width "%")})}])]])
+   (conj (into [:tr]
+               (for [header-cell row]
+                 [:th {:style (style/style (row-style-for-type header-cell) (heading-style-for-type header-cell) {:min-width :42px}) :title header-cell}
+                  (truncate-text (h header-cell))]))
+         (when bar-width
+           [:th {:style (style/style (bar-td-style) (bar-th-style) {:width (str bar-width "%")})}]))])
 
 (defn- render-bar
   [bar-width normalized-zero]
@@ -116,7 +122,11 @@
                       (row-style-for-type cell)
                       {:background-color (get-background-color cell (get column-names col-idx) row-idx)}
                       (when (and bar-width (= col-idx 1))
-                        {:font-weight 700}))}
+                        {:font-weight 700})
+                      (when (= row-idx (dec (count rows)))
+                        {:border-bottom 0})
+                      (when (= col-idx (dec (count row)))
+                        {:border-right 0}))}
          (h cell)])
       (some-> bar-width (render-bar normalized-zero))])])
 
@@ -131,8 +141,8 @@
   ([color-selector normalized-zero column-names [header & rows]]
    [:table {:style (style/style {:max-width "100%"
                                  :white-space :nowrap
-                                 :padding-bottom :8px
-                                 :border-collapse :collapse
+                                 :border  (str "1px solid " style/color-border)
+                                 :border-radius :6px
                                  :width "1%"})
             :cellpadding "0"
             :cellspacing "0"}

test/metabase/pulse/render/table_test.clj

@@ -72,3 +72,14 @@
                (#'table/render-table 0 ["a" "b" "c"] (query-results->header+rows query-results))
                find-table-body
                cell-value->background-color)))))
+
+(deftest header-truncation-test []
+  (let [[normal-heading long-heading :as row] ["Count" (apply str (repeat 120 "A"))]
+        [normal-rendered long-rendered]       (->> (#'table/render-table-head {:row row})
+                                             (tree-seq vector? rest)
+                                             (filter #(= :th (first %)))
+                                             (map last))]
+    (testing "Table Headers are truncated if they are really long."
+      (is (= normal-heading normal-rendered))
+      (is (= "A..." (subs long-rendered (- (count long-rendered) 4) (count long-rendered))))
+      (is (not= long-heading long-rendered)))))