From 5e3da93ebcc71d256ef2a25474e115c4c72883b5 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Mon, 12 Sep 2022 22:13:47 +0300
Subject: [PATCH] Make it clearer how to connect a SQL question to a dashboard
filter (#25324)
---
.../DashCardCardParameterMapper.jsx | 24 +++++++++++++--
.../DashCardCardParameterMapper.styled.jsx | 29 ++++++++++++++++++-
frontend/src/metabase/dashboard/utils.js | 23 +++++++++++++++
.../parameters/utils/parameter-type.ts | 5 ++++
.../dashboard-filters/parameters.cy.spec.js | 6 ++--
5 files changed, 82 insertions(+), 5 deletions(-)
diff --git a/frontend/src/metabase/dashboard/components/DashCardCardParameterMapper.jsx b/frontend/src/metabase/dashboard/components/DashCardCardParameterMapper.jsx
index 5457fc15816..95b81bc6ebf 100644
--- a/frontend/src/metabase/dashboard/components/DashCardCardParameterMapper.jsx
+++ b/frontend/src/metabase/dashboard/components/DashCardCardParameterMapper.jsx
@@ -4,6 +4,7 @@ import { connect } from "react-redux";
import _ from "underscore";
import { t } from "ttag";
+import MetabaseSettings from "metabase/lib/settings";
import Icon from "metabase/components/Icon";
import Tooltip from "metabase/components/Tooltip";
import TippyPopover from "metabase/components/Popover/TippyPopover";
@@ -12,6 +13,8 @@ import { isVariableTarget } from "metabase/parameters/utils/targets";
import { isDateParameter } from "metabase/parameters/utils/parameter-type";
import { getMetadata } from "metabase/selectors/metadata";
import {
+ getNativeDashCardEmptyMappingText,
+ isNativeDashCard,
isVirtualDashCard,
showVirtualDashCardInfoText,
} from "metabase/dashboard/utils";
@@ -34,6 +37,10 @@ import {
ChevrondownIcon,
KeyIcon,
Warning,
+ NativeCardDefault,
+ NativeCardIcon,
+ NativeCardText,
+ NativeCardLink,
} from "./DashCardCardParameterMapper.styled";
function formatSelected({ name, sectionName }) {
@@ -94,6 +101,7 @@ function DashCardCardParameterMapper({
);
const isVirtual = isVirtualDashCard(dashcard);
+ const isNative = isNativeDashCard(dashcard);
const hasPermissionsToMap = useMemo(() => {
if (isVirtual) {
@@ -155,14 +163,14 @@ function DashCardCardParameterMapper({
]);
const headerContent = useMemo(() => {
- if (!isVirtual) {
+ if (!isVirtual && !(isNative && isDisabled)) {
return t`Column to filter on`;
} else if (dashcard.size_y !== 1 || isMobile) {
return t`Variable to map to`;
} else {
return null;
}
- }, [dashcard, isVirtual, isMobile]);
+ }, [dashcard, isVirtual, isNative, isDisabled, isMobile]);
const mappingInfoText = t`You can connect widgets to {{variables}} in text cards.`;
@@ -185,6 +193,18 @@ function DashCardCardParameterMapper({
/>
</TextCardDefault>
)
+ ) : isNative && isDisabled ? (
+ <NativeCardDefault>
+ <NativeCardIcon name="info" />
+ <NativeCardText>
+ {getNativeDashCardEmptyMappingText(editingParameter)}
+ </NativeCardText>
+ <NativeCardLink
+ href={MetabaseSettings.docsUrl(
+ "questions/native-editor/sql-parameters",
+ )}
+ >{t`Learn how`}</NativeCardLink>
+ </NativeCardDefault>
) : (
<>
{headerContent && <Header>{headerContent}</Header>}
diff --git a/frontend/src/metabase/dashboard/components/DashCardCardParameterMapper.styled.jsx b/frontend/src/metabase/dashboard/components/DashCardCardParameterMapper.styled.jsx
index f5e7b1fb801..c3ce7ba1d07 100644
--- a/frontend/src/metabase/dashboard/components/DashCardCardParameterMapper.styled.jsx
+++ b/frontend/src/metabase/dashboard/components/DashCardCardParameterMapper.styled.jsx
@@ -5,6 +5,7 @@ import { space } from "metabase/styled-components/theme";
import { color, lighten } from "metabase/lib/colors";
import Icon from "metabase/components/Icon";
import Button from "metabase/core/components/Button";
+import ExternalLink from "metabase/core/components/ExternalLink";
export const Container = styled.div`
margin: ${space(1)} 0;
@@ -14,7 +15,7 @@ export const Container = styled.div`
`;
export const TextCardDefault = styled.div`
- color: ${color("text-medium")};
+ color: ${color("text-dark")};
margin: ${space(1)} 0;
display: flex;
flex-direction: row;
@@ -22,6 +23,32 @@ export const TextCardDefault = styled.div`
line-height: 1.5rem;
`;
+export const NativeCardDefault = styled.div`
+ display: flex;
+ flex-direction: column;
+ align-items: center;
+`;
+
+export const NativeCardIcon = styled(Icon)`
+ color: ${color("text-medium")};
+ margin-bottom: 0.5rem;
+ width: 1.25rem;
+ height: 1.25rem;
+`;
+
+export const NativeCardText = styled.div`
+ color: ${color("text-dark")};
+ max-width: 15rem;
+ text-align: center;
+ line-height: 1.5rem;
+`;
+
+export const NativeCardLink = styled(ExternalLink)`
+ color: ${color("brand")};
+ font-weight: bold;
+ margin-top: 0.5rem;
+`;
+
export const CardLabel = styled.div`
font-size: 0.83em;
margin-bottom: ${space(1)};
diff --git a/frontend/src/metabase/dashboard/utils.js b/frontend/src/metabase/dashboard/utils.js
index a4b23ad6219..bb59522c603 100644
--- a/frontend/src/metabase/dashboard/utils.js
+++ b/frontend/src/metabase/dashboard/utils.js
@@ -1,5 +1,12 @@
import _ from "underscore";
import Utils from "metabase/lib/utils";
+import { isNative } from "metabase/lib/query";
+import { t } from "ttag";
+import {
+ isDateParameter,
+ isNumberParameter,
+ isStringParameter,
+} from "metabase/parameters/utils/parameter-type";
export function syncParametersAndEmbeddingParams(before, after) {
if (after.parameters && before.embedding_params) {
@@ -51,6 +58,10 @@ export function isVirtualDashCard(dashcard) {
return _.isObject(dashcard.visualization_settings.virtual_card);
}
+export function isNativeDashCard(dashcard) {
+ return isNative(dashcard.card?.dataset_query);
+}
+
// For a virtual (text) dashcard without any parameters, returns a boolean indicating whether we should display the
// info text about parameter mapping in the card itself or as a tooltip.
export function showVirtualDashCardInfoText(dashcard, isMobile) {
@@ -61,6 +72,18 @@ export function showVirtualDashCardInfoText(dashcard, isMobile) {
}
}
+export function getNativeDashCardEmptyMappingText(parameter) {
+ if (isDateParameter(parameter)) {
+ return t`Add a date variable to this question to connect it to a dashboard filter.`;
+ } else if (isNumberParameter(parameter)) {
+ return t`Add a number variable to this question to connect it to a dashboard filter.`;
+ } else if (isStringParameter(parameter)) {
+ return t`Add a string variable to this question to connect it to a dashboard filter.`;
+ } else {
+ return t`Add a variable to this question to connect it to a dashboard filter.`;
+ }
+}
+
export function getAllDashboardCards(dashboard) {
const results = [];
if (dashboard) {
diff --git a/frontend/src/metabase/parameters/utils/parameter-type.ts b/frontend/src/metabase/parameters/utils/parameter-type.ts
index 5100ee88129..372f754505b 100644
--- a/frontend/src/metabase/parameters/utils/parameter-type.ts
+++ b/frontend/src/metabase/parameters/utils/parameter-type.ts
@@ -33,6 +33,11 @@ export function isNumberParameter(parameter: Parameter) {
return type === "number";
}
+export function isStringParameter(parameter: Parameter) {
+ const type = getParameterType(parameter);
+ return type === "string";
+}
+
export function isFieldFilterParameter(
parameter: Parameter,
): parameter is FieldFilterUiParameter {
diff --git a/frontend/test/metabase/scenarios/dashboard-filters/parameters.cy.spec.js b/frontend/test/metabase/scenarios/dashboard-filters/parameters.cy.spec.js
index 272ef2a8873..b9ea2cf2df2 100644
--- a/frontend/test/metabase/scenarios/dashboard-filters/parameters.cy.spec.js
+++ b/frontend/test/metabase/scenarios/dashboard-filters/parameters.cy.spec.js
@@ -271,7 +271,7 @@ describe("scenarios > dashboard > parameters", () => {
cy.icon("pencil").click();
cy.icon("filter").click();
cy.findByText("ID").click();
- cy.findByText("No valid fields");
+ cy.findByText(/Add a variable to this question/).should("be.visible");
// Confirm that the correct parameter type is connected to the native question's field filter
cy.findByText(matchingFilterType.name).find(".Icon-gear").click();
@@ -301,7 +301,9 @@ describe("scenarios > dashboard > parameters", () => {
// Confirm that it is not possible to connect filter to the updated question anymore (metabase#9299)
cy.icon("pencil").click();
cy.findByText(matchingFilterType.name).find(".Icon-gear").click();
- cy.findByText("No valid fields");
+ cy.findByText(/Add a string variable to this question/).should(
+ "be.visible",
+ );
});
});
--
GitLab