Skip to content
Snippets Groups Projects
Unverified Commit d31fac77 authored by Howon Lee's avatar Howon Lee Committed by GitHub
Browse files

Static viz funnel fix (more #18676) (#19983)

Still pursuant to #18676

1. Funnel was previously not doing lato font for the bold bit because visx's text doesn't take weights, it takes styles
2. Columns can be nontrivial with respect to funnels, which was an oversight
3. For some reason the first try for fixing 2 also impinged upon values being strings for funnels, which breaks things at SVG level
4. And let's just shove in a nit for the display of trends if the previous value and current value are the same
parent 466f6a4d
No related branches found
No related tags found
No related merge requests found
...@@ -118,10 +118,10 @@ const Funnel = ({ data, settings }: FunnelProps) => { ...@@ -118,10 +118,10 @@ const Funnel = ({ data, settings }: FunnelProps) => {
<> <>
<Text <Text
textAnchor="end" textAnchor="end"
fontWeight={700}
y={firstMeasureTop} y={firstMeasureTop}
fontSize={layout.initialMeasureFontSize} fontSize={layout.initialMeasureFontSize}
fill="black" fill="black"
style={{ fontWeight: 700 }}
> >
{measure} {measure}
</Text> </Text>
......
...@@ -686,7 +686,10 @@ ...@@ -686,7 +686,10 @@
(if (and last-value previous-value unit last-change) (if (and last-value previous-value unit last-change)
(let [value (format-cell timezone-id last-value metric-col viz-settings) (let [value (format-cell timezone-id last-value metric-col viz-settings)
previous (format-cell timezone-id previous-value metric-col viz-settings) previous (format-cell timezone-id previous-value metric-col viz-settings)
adj (if (pos? last-change) (tru "Up") (tru "Down"))] adj (if (pos? last-change) (tru "Up") (tru "Down"))
delta-statement (if (= last-value previous-value)
"No change."
(str adj " " (percentage last-change) "."))]
{:attachments nil {:attachments nil
:content [:div :content [:div
[:div {:style (style/style (style/scalar-style))} [:div {:style (style/style (style/scalar-style))}
...@@ -695,10 +698,10 @@ ...@@ -695,10 +698,10 @@
:font-size :16px :font-size :16px
:font-weight 700 :font-weight 700
:padding-right :16px})} :padding-right :16px})}
adj " " (percentage last-change) "." delta-statement
" Was " previous " last " (format-unit unit)]] " Was " previous " last " (format-unit unit)]]
:render/text (str value "\n" :render/text (str value "\n"
adj " " (percentage last-change) "." delta-statement
" Was " previous " last " (format-unit unit))}) " Was " previous " last " (format-unit unit))})
;; In other words, defaults to plain scalar if we don't have actual changes ;; In other words, defaults to plain scalar if we don't have actual changes
{:attachments nil {:attachments nil
...@@ -796,16 +799,17 @@ ...@@ -796,16 +799,17 @@
(s/defmethod render :funnel :- common/RenderedPulseCard (s/defmethod render :funnel :- common/RenderedPulseCard
[_ render-type timezone-id card _ {:keys [rows cols viz-settings] :as data}] [_ render-type timezone-id card _ {:keys [rows cols viz-settings] :as data}]
;; x-axis-rowfn is always first, y-axis-rowfn is always second (let [[x-axis-rowfn
(let [rows (common/row-preprocess first second rows) y-axis-rowfn] (common/graphing-column-row-fns card data)
[x-col y-col] cols rows (map (juxt x-axis-rowfn y-axis-rowfn)
settings (->js-viz x-col y-col viz-settings) (common/row-preprocess x-axis-rowfn y-axis-rowfn rows))
settings (assoc settings [x-col y-col] cols
:step {:name (:display_name x-col) settings (as-> (->js-viz x-col y-col viz-settings) jsviz-settings
:format (:x settings)} (assoc jsviz-settings :step {:name (:display_name x-col)
:measure {:format (:y settings)}) :format (:x jsviz-settings)}
svg (js-svg/funnel rows settings) :measure {:format (:y jsviz-settings)}))
image-bundle (image-bundle/make-image-bundle render-type svg)] svg (js-svg/funnel rows settings)
image-bundle (image-bundle/make-image-bundle render-type svg)]
{:attachments {:attachments
(image-bundle/image-bundle->attachment image-bundle) (image-bundle/image-bundle->attachment image-bundle)
......
...@@ -308,13 +308,14 @@ ...@@ -308,13 +308,14 @@
:render/text (s/eq "foo")} :render/text (s/eq "foo")}
(body/render :scalar nil pacific-tz nil nil results))))) (body/render :scalar nil pacific-tz nil nil results)))))
(testing "for smartscalars" (testing "for smartscalars"
(let [results {:cols [{:name "value", (let [cols [{:name "value",
:display_name "VALUE", :display_name "VALUE",
:base_type :type/Decimal} :base_type :type/Decimal}
{:name "time", {:name "time",
:display_name "TIME", :display_name "TIME",
:base_type :type/DateTime :base_type :type/DateTime
:effective_type :type/DateTime}] :effective_type :type/DateTime}]
results {:cols cols
:rows [[40.0 :this-month] :rows [[40.0 :this-month]
[30.0 :last-month] [30.0 :last-month]
[20.0 :month-before]] [20.0 :month-before]]
...@@ -323,14 +324,17 @@ ...@@ -323,14 +324,17 @@
:last-change 1.333333 :last-change 1.333333
:col "value" :col "value"
:last-value 40.0}]} :last-value 40.0}]}
sameres {:cols cols
:rows [[40.0 :this-month]
[40.0 :last-month]
[40.0 :month-before]]
:insights [{:previous-value 40.0
:unit :month
:last-change 1.0
:col "value"
:last-value 40.0}]}
;; by "dumb" it is meant "without nonnil insights" ;; by "dumb" it is meant "without nonnil insights"
dumbres {:cols [{:name "value", dumbres {:cols cols
:display_name "VALUE",
:base_type :type/Decimal}
{:name "time",
:display_name "TIME",
:base_type :type/DateTime
:effective_type :type/DateTime}]
:rows [[20.0 :month-before]] :rows [[20.0 :month-before]]
:insights [{:previous-value nil :insights [{:previous-value nil
:unit nil :unit nil
...@@ -339,6 +343,8 @@ ...@@ -339,6 +343,8 @@
:last-value 20.0}]}] :last-value 20.0}]}]
(is (= "40.00\nUp 133.33%. Was 30.00 last month" (is (= "40.00\nUp 133.33%. Was 30.00 last month"
(:render/text (body/render :smartscalar nil pacific-tz nil nil results)))) (:render/text (body/render :smartscalar nil pacific-tz nil nil results))))
(is (= "40.00\nNo change. Was 40.00 last month"
(:render/text (body/render :smartscalar nil pacific-tz nil nil sameres))))
(is (= "20.0\nNothing to compare to." (is (= "20.0\nNothing to compare to."
(:render/text (body/render :smartscalar nil pacific-tz nil nil dumbres)))) (:render/text (body/render :smartscalar nil pacific-tz nil nil dumbres))))
(is (schema= {:attachments (s/eq nil) (is (schema= {:attachments (s/eq nil)
...@@ -418,7 +424,7 @@ ...@@ -418,7 +424,7 @@
:base_type :type/BigInteger :base_type :type/BigInteger
:semantic_type nil}]) :semantic_type nil}])
(def ^:private default-combo-columns (def ^:private default-multi-columns
[{:name "Price", [{:name "Price",
:display_name "Price", :display_name "Price",
:base_type :type/BigInteger :base_type :type/BigInteger
...@@ -465,7 +471,7 @@ ...@@ -465,7 +471,7 @@
(testing "Check multiseries in one card but without explicit combo" (testing "Check multiseries in one card but without explicit combo"
(is (has-inline-image? (is (has-inline-image?
(render-multiseries-bar-graph (render-multiseries-bar-graph
{:cols default-combo-columns {:cols default-multi-columns
:rows [[10.0 1 1231 1] [5.0 10 nil 111] [2.50 20 11 1] [1.25 nil 1231 11]]}))))) :rows [[10.0 1 1231 1] [5.0 10 nil 111] [2.50 20 11 1] [1.25 nil 1231 11]]})))))
(defn- render-area-graph [results] (defn- render-area-graph [results]
...@@ -494,7 +500,7 @@ ...@@ -494,7 +500,7 @@
(testing "Check multiseries in one card but without explicit combo" (testing "Check multiseries in one card but without explicit combo"
(is (has-inline-image? (is (has-inline-image?
(render-multiseries-area-graph (render-multiseries-area-graph
{:cols default-combo-columns {:cols default-multi-columns
:rows [[10.0 1 1231 1] [5.0 10 nil 111] [2.50 20 11 1] [1.25 nil 1231 11]]}))))) :rows [[10.0 1 1231 1] [5.0 10 nil 111] [2.50 20 11 1] [1.25 nil 1231 11]]})))))
(defn- render-waterfall [results] (defn- render-waterfall [results]
...@@ -531,15 +537,15 @@ ...@@ -531,15 +537,15 @@
(deftest render-combo-test (deftest render-combo-test
(testing "Render a combo graph with non-nil values for the x and y axis" (testing "Render a combo graph with non-nil values for the x and y axis"
(is (has-inline-image? (is (has-inline-image?
(render-combo {:cols default-combo-columns (render-combo {:cols default-multi-columns
:rows [[10.0 1 123 111] [5.0 10 12 111] [2.50 20 1337 12312] [1.25 30 -22 123124]]})))) :rows [[10.0 1 123 111] [5.0 10 12 111] [2.50 20 1337 12312] [1.25 30 -22 123124]]}))))
(testing "Render a combo graph with multiple x axes" (testing "Render a combo graph with multiple x axes"
(is (has-inline-image? (is (has-inline-image?
(render-combo-multi-x {:cols default-combo-columns (render-combo-multi-x {:cols default-multi-columns
:rows [[10.0 "Bob" 123 123124] [5.0 "Dobbs" 12 23423] [2.50 "Robbs" 1337 234234] [1.25 "Mobbs" -22 1234123]]})))) :rows [[10.0 "Bob" 123 123124] [5.0 "Dobbs" 12 23423] [2.50 "Robbs" 1337 234234] [1.25 "Mobbs" -22 1234123]]}))))
(testing "Check to make sure we allow nil values for any axis" (testing "Check to make sure we allow nil values for any axis"
(is (has-inline-image? (is (has-inline-image?
(render-combo {:cols default-combo-columns (render-combo {:cols default-multi-columns
:rows [[nil 1 1 23453] [10.0 1 nil nil] [5.0 10 22 1337] [2.50 nil 22 1231] [1.25 nil nil 1231232]]}))))) :rows [[nil 1 1 23453] [10.0 1 nil nil] [5.0 10 22 1337] [2.50 nil 22 1231] [1.25 nil nil 1231232]]})))))
;; Test rendering a sparkline ;; Test rendering a sparkline
...@@ -587,6 +593,11 @@ ...@@ -587,6 +593,11 @@
(render-funnel (render-funnel
{:cols default-columns {:cols default-columns
:rows [[10.0 1] [5.0 10] [2.50 20] [1.25 30]]})))) :rows [[10.0 1] [5.0 10] [2.50 20] [1.25 30]]}))))
(testing "Test that we can render a funnel with extraneous columns and also weird strings stuck in places"
(is (has-inline-image?
(render-funnel
{:cols default-multi-columns
:rows [[10.0 1 2 2] [5.0 10 "11.1" 1] ["2.50" 20 1337 0] [1.25 30 -2 "-2"]]}))))
(testing "Test that we can have some nil values stuck everywhere" (testing "Test that we can have some nil values stuck everywhere"
(is (has-inline-image? (is (has-inline-image?
(render-funnel (render-funnel
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment