From aea2cb22dcfb84c2cfb2c53e3546cde1a748c666 Mon Sep 17 00:00:00 2001
From: "metabase-bot[bot]"
<109303359+metabase-bot[bot]@users.noreply.github.com>
Date: Fri, 16 Feb 2024 22:21:23 +0000
Subject: [PATCH] Add per viz-type card template sizes in x-rays (#38836)
(#38884)
* Add per viz-type card template sizes in x-rays
Previously, any card without a size (width or height) set in their yaml definition was set to a default of `default-card-width` or `default-card-width`, respectively. This PR adds a map of default and min dimensions per viz type (the `:display`) key and fills that in for each template. The template is merged into this result, keeping anything that already existed in the yaml file.
Before merging this we should consider if we want to use the min values and where this data should be stored. It is currently in `metabase.automagic-dashboards.dashboard-templates`, but was lifted from [sizes.ts](https://github.com/metabase/metabase/blob/master/frontend/src/metabase/visualizations/shared/utils/sizes.ts).
* Moving card sizing constants to cljc
The default card sizes were found in `sizes.ts` and were duplicated into `metabase.automagic-dashboards.dashboard-templates`. This PR consolidates those changes into `metabase.shared.automagic-dashboards.constants`.
Note that @kulyk and @JesseSDevaney might want to consider further consolidation of dashboard related constants into this namespace.
* Incorporating Feedback
Changes
- Explicitly defined two constants in the shared file that are clj and js friendly
- Set case to CARD_SIZE_DEFAULTS_JSON
- Updated `metabase.automagic-dashboards.dashboard-templates/set-default-card-dimensions` to use a helper
* Update src/metabase/shared/automagic_dashboards/constants.cljc
* Updating ns name per Jesse
---------
Co-authored-by: Mark Bastian <markbastian@gmail.com>
Co-authored-by: Jesse Devaney <22608765+JesseSDevaney@users.noreply.github.com>
---
.../visualizations/shared/utils/sizes.ts | 94 +------------------
shadow-cljs.edn | 1 +
.../dashboard_templates.clj | 17 ++++
.../automagic_dashboards/populate.clj | 4 +-
src/metabase/shared/dashboards/constants.cljc | 34 +++++++
5 files changed, 57 insertions(+), 93 deletions(-)
create mode 100644 src/metabase/shared/dashboards/constants.cljc
diff --git a/frontend/src/metabase/visualizations/shared/utils/sizes.ts b/frontend/src/metabase/visualizations/shared/utils/sizes.ts
index 8a41326ee2d..c73cb5a8090 100644
--- a/frontend/src/metabase/visualizations/shared/utils/sizes.ts
+++ b/frontend/src/metabase/visualizations/shared/utils/sizes.ts
@@ -1,6 +1,7 @@
import _ from "underscore";
-import { DEFAULT_CARD_SIZE, GRID_WIDTH } from "metabase/lib/dashboard_grid";
+import { DEFAULT_CARD_SIZE } from "metabase/lib/dashboard_grid";
import type { CardDisplayType } from "metabase-types/api";
+import { CARD_SIZE_DEFAULTS_JSON } from "cljs/metabase.shared.dashboards.constants";
type VisualizationSize = { width: number; height: number };
const VISUALIZATION_SIZES: {
@@ -8,96 +9,7 @@ const VISUALIZATION_SIZES: {
min: VisualizationSize;
default: VisualizationSize;
};
-} = {
- line: {
- min: { width: 4, height: 3 },
- default: { width: 12, height: 6 },
- },
- area: {
- min: { width: 4, height: 3 },
- default: { width: 12, height: 6 },
- },
- bar: {
- min: { width: 4, height: 3 },
- default: { width: 12, height: 6 },
- },
- stacked: {
- min: { width: 4, height: 3 },
- default: { width: 12, height: 6 },
- },
- combo: {
- min: { width: 4, height: 3 },
- default: { width: 12, height: 6 },
- },
- row: {
- min: { width: 4, height: 3 },
- default: { width: 12, height: 6 },
- },
- scatter: {
- min: { width: 4, height: 3 },
- default: { width: 12, height: 6 },
- },
- waterfall: {
- min: { width: 4, height: 3 },
- default: { width: 14, height: 6 },
- },
- pie: {
- min: { width: 4, height: 3 },
- default: { width: 12, height: 8 },
- },
- funnel: {
- min: { width: 4, height: 3 },
- default: { width: 12, height: 6 },
- },
- gauge: {
- min: { width: 4, height: 3 },
- default: { width: 12, height: 6 },
- },
- progress: {
- min: { width: 4, height: 3 },
- default: { width: 12, height: 6 },
- },
- map: {
- min: { width: 4, height: 3 },
- default: { width: 12, height: 6 },
- },
- table: {
- min: { width: 4, height: 3 },
- default: { width: 12, height: 9 },
- },
- pivot: {
- min: { width: 4, height: 3 },
- default: { width: 12, height: 9 },
- },
- object: {
- min: { width: 4, height: 3 },
- default: { width: 12, height: 9 },
- },
- scalar: {
- min: { width: 2, height: 2 },
- default: { width: 6, height: 3 },
- },
- smartscalar: {
- min: { width: 2, height: 2 },
- default: { width: 6, height: 3 },
- },
- link: {
- min: { width: 1, height: 1 },
- default: { width: 8, height: 1 },
- },
- action: {
- min: { width: 1, height: 1 },
- default: { width: 4, height: 1 },
- },
- heading: {
- min: { width: 1, height: 1 },
- default: { width: GRID_WIDTH, height: 1 },
- },
- text: {
- min: { width: 1, height: 1 },
- default: { width: 12, height: 3 },
- },
-};
+} = CARD_SIZE_DEFAULTS_JSON;
const getSize = (
visualizationType: CardDisplayType,
diff --git a/shadow-cljs.edn b/shadow-cljs.edn
index 3224ae2be8e..477ec5c0108 100644
--- a/shadow-cljs.edn
+++ b/shadow-cljs.edn
@@ -21,6 +21,7 @@
metabase.lib.limit
metabase.lib.types.isa
metabase.mbql.js
+ metabase.shared.dashboards.constants
metabase.shared.formatting.constants
metabase.shared.formatting.date
metabase.shared.formatting.numbers
diff --git a/src/metabase/automagic_dashboards/dashboard_templates.clj b/src/metabase/automagic_dashboards/dashboard_templates.clj
index 16fa8cd52e5..c5325ffdd5d 100644
--- a/src/metabase/automagic_dashboards/dashboard_templates.clj
+++ b/src/metabase/automagic_dashboards/dashboard_templates.clj
@@ -6,6 +6,7 @@
[clojure.string :as str]
[metabase.automagic-dashboards.populate :as populate]
[metabase.query-processor.util :as qp.util]
+ [metabase.shared.dashboards.constants :as dashboards.constants]
[metabase.util :as u]
[metabase.util.files :as u.files]
[metabase.util.i18n :as i18n :refer [deferred-trs LocalizedString]]
@@ -286,6 +287,21 @@
[dashboard-template]
(transduce (map (comp count ancestors)) + (:applies_to dashboard-template)))
+(defn- ensure-default-card-sizes
+ "Given a card definition from a template, fill in the card template with default width and height
+ values based on the template display type if those dimensions aren't already present."
+ [card-spec]
+ (update-vals
+ card-spec
+ (fn [{:keys [visualization] :as card-spec}]
+ (let [defaults (get-in dashboards.constants/card-size-defaults [(keyword visualization) :default])]
+ (into defaults card-spec)))))
+
+(defn- set-default-card-dimensions
+ "Update the card template dimensions to align with the default FE dimensions."
+ [dashboard-template]
+ (update dashboard-template :cards #(mapv ensure-default-card-sizes %)))
+
(defn- make-dashboard-template
[entity-type {:keys [cards] :as r}]
(-> (cond-> r
@@ -294,6 +310,7 @@
(assoc :dashboard-template-name entity-type
:specificity 0)
(update :applies_to #(or % entity-type))
+ set-default-card-dimensions
dashboard-template-validator
(as-> dashboard-template
(assoc dashboard-template
diff --git a/src/metabase/automagic_dashboards/populate.clj b/src/metabase/automagic_dashboards/populate.clj
index 0472d67235e..b8770bc7ef8 100644
--- a/src/metabase/automagic_dashboards/populate.clj
+++ b/src/metabase/automagic_dashboards/populate.clj
@@ -22,11 +22,11 @@
(def ^Long default-card-width
"Default card width."
- 6)
+ 12)
(def ^Long default-card-height
"Default card height"
- 4)
+ 6)
(defn create-collection!
"Create and return a new collection."
diff --git a/src/metabase/shared/dashboards/constants.cljc b/src/metabase/shared/dashboards/constants.cljc
new file mode 100644
index 00000000000..2453b063f3e
--- /dev/null
+++ b/src/metabase/shared/dashboards/constants.cljc
@@ -0,0 +1,34 @@
+(ns metabase.shared.dashboards.constants)
+
+(def ^:export GRID_WIDTH
+ "Default width of a dashboard"
+ 24)
+
+(def card-size-defaults
+ "Default card sizes per visualization type"
+ {:table {:min {:width 4 :height 3} :default {:width 12 :height 9}}
+ :gauge {:min {:width 4 :height 3} :default {:width 12 :height 6}}
+ :bar {:min {:width 4 :height 3} :default {:width 12 :height 6}}
+ :pie {:min {:width 4 :height 3} :default {:width 12 :height 8}}
+ :scatter {:min {:width 4 :height 3} :default {:width 12 :height 6}}
+ :waterfall {:min {:width 4 :height 3} :default {:width 14 :height 6}}
+ :combo {:min {:width 4 :height 3} :default {:width 12 :height 6}}
+ :stacked {:min {:width 4 :height 3} :default {:width 12 :height 6}}
+ :scalar {:min {:width 2 :height 2} :default {:width 6 :height 3}}
+ :line {:min {:width 4 :height 3} :default {:width 12 :height 6}}
+ :link {:min {:width 1 :height 1} :default {:width 8 :height 1}}
+ :action {:min {:width 1 :height 1} :default {:width 4 :height 1}}
+ :area {:min {:width 4 :height 3} :default {:width 12 :height 6}}
+ :pivot {:min {:width 4 :height 3} :default {:width 12 :height 9}}
+ :funnel {:min {:width 4 :height 3} :default {:width 12 :height 6}}
+ :progress {:min {:width 4 :height 3} :default {:width 12 :height 6}}
+ :smartscalar {:min {:width 2 :height 2} :default {:width 6 :height 3}}
+ :map {:min {:width 4 :height 3} :default {:width 12 :height 6}}
+ :object {:min {:width 4 :height 3} :default {:width 12 :height 9}}
+ :row {:min {:width 4 :height 3} :default {:width 12 :height 6}}
+ :heading {:min {:width 1 :height 1} :default {:width GRID_WIDTH :height 1}}
+ :text {:min {:width 1 :height 1} :default {:width 12 :height 3}}})
+
+#?(:cljs (def ^:export CARD_SIZE_DEFAULTS_JSON
+ "Default card sizes per visualization type as a json object suitable for the FE"
+ (clj->js card-size-defaults)))
--
GitLab