diff --git a/frontend/src/metabase-lib/lib/queries/NativeQuery.ts b/frontend/src/metabase-lib/lib/queries/NativeQuery.ts index 81ab6fcb956a3c5a35339128b111e30cc49ae86b..4f02690a6374974f68567a9ef548888b779248c7 100644 --- a/frontend/src/metabase-lib/lib/queries/NativeQuery.ts +++ b/frontend/src/metabase-lib/lib/queries/NativeQuery.ts @@ -302,6 +302,9 @@ export default class NativeQuery extends AtomicQuery { validateTemplateTags() { return this.templateTags() .map(tag => { + if (!tag["display-name"]) { + return new ValidationError(t`Missing wiget label: ${tag.name}`); + } const dimension = new TemplateTagDimension( tag.name, this.metadata(), diff --git a/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.jsx b/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.jsx index 26203c5789347f7cde6d6549a7068ddc3f1e1fda..55a834cf09019f53f6667ad381f1893b0a70ece7 100644 --- a/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.jsx +++ b/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.jsx @@ -8,9 +8,7 @@ import { Link } from "react-router"; import Schemas from "metabase/entities/schemas"; import Toggle from "metabase/core/components/Toggle"; -import InputBlurChange from "metabase/components/InputBlurChange"; import Select, { Option } from "metabase/core/components/Select"; -import ParameterValueWidget from "metabase/parameters/components/ParameterValueWidget"; import { getParameterOptionsForField } from "metabase/parameters/utils/template-tag-options"; @@ -19,6 +17,16 @@ import { getMetadata } from "metabase/selectors/metadata"; import { SchemaTableAndFieldDataSelector } from "metabase/query_builder/components/DataSelector"; import MetabaseSettings from "metabase/lib/settings"; +import { + ErrorSpan, + TagName, + TagContainer, + ContainerLabel, + InputContainer, + WidgetLabelInput, + DefaultParameterValueWidget, +} from "./TagEditorParam.styled"; + const propTypes = { tag: PropTypes.object.isRequired, parameter: PropTypes.object, @@ -141,16 +149,16 @@ export class TagEditorParam extends Component { const hasWidgetOptions = widgetOptions && widgetOptions.length > 0; const hasNoWidgetType = tag["widget-type"] === "none" || !tag["widget-type"]; + const hasNoWidgetLabel = !tag["display-name"]; return ( - <div className="px3 pt3 mb1 border-top"> - <h4 className="text-medium py1">{t`Variable name`}</h4> - <h3 className="text-heavy text-brand align-self-end mb4">{tag.name}</h3> + <TagContainer> + <ContainerLabel paddingTop>{t`Variable name`}</ContainerLabel> + <TagName>{tag.name}</TagName> - <div className="pb4"> - <h4 className="text-medium pb1">{t`Variable type`}</h4> + <InputContainer> + <ContainerLabel>{t`Variable type`}</ContainerLabel> <Select - className="block" value={tag.type} onChange={e => this.setType(e.target.value)} isInitiallyOpen={!tag.type} @@ -162,16 +170,14 @@ export class TagEditorParam extends Component { <Option value="date">{t`Date`}</Option> <Option value="dimension">{t`Field Filter`}</Option> </Select> - </div> + </InputContainer> {tag.type === "dimension" && ( - <div className="pb4"> - <h4 className="text-medium pb1"> + <InputContainer> + <ContainerLabel> {t`Field to map to`} - {tag.dimension == null && ( - <span className="text-error mx1">{t`(required)`}</span> - )} - </h4> + {tag.dimension == null && <ErrorSpan>{t`(required)`}</ErrorSpan>} + </ContainerLabel> {(!hasSelectedDimensionField || (hasSelectedDimensionField && fieldMetadataLoaded)) && ( @@ -196,17 +202,15 @@ export class TagEditorParam extends Component { )} </Schemas.Loader> )} - </div> + </InputContainer> )} {hasSelectedDimensionField && ( - <div className="pb4"> - <h4 className="text-medium pb1"> + <InputContainer> + <ContainerLabel> {t`Filter widget type`} - {hasNoWidgetType && ( - <span className="text-error mx1">{t`(required)`}</span> - )} - </h4> + {hasNoWidgetType && <ErrorSpan>{t`(required)`}</ErrorSpan>} + </ContainerLabel> <Select className="block" value={this.getFilterWidgetTypeValue(tag, widgetOptions)} @@ -242,38 +246,39 @@ export class TagEditorParam extends Component { </Link> </p> )} - </div> + </InputContainer> )} {(hasWidgetOptions || !isDimension) && ( - <div className="pb4"> - <h4 className="text-medium pb1">{t`Filter widget label`}</h4> - <InputBlurChange + <InputContainer> + <ContainerLabel> + {t`Filter widget label`} + {hasNoWidgetLabel && <ErrorSpan>{t`(required)`}</ErrorSpan>} + </ContainerLabel> + <WidgetLabelInput type="text" value={tag["display-name"]} - className="AdminSelect p1 text-bold text-dark bordered border-medium rounded full" - style={{ fontSize: "14px" }} onBlurChange={e => this.setParameterAttribute("display-name", e.target.value) } /> - </div> + </InputContainer> )} - <div className="pb3"> - <h4 className="text-medium pb1">{t`Required?`}</h4> + <InputContainer lessBottomPadding> + <ContainerLabel>{t`Required?`}</ContainerLabel> <Toggle value={tag.required} onChange={value => this.setRequired(value)} /> - </div> + </InputContainer> {((tag.type !== "dimension" && tag.required) || tag.type === "dimension" || tag["widget-type"]) && ( - <div className="pb3"> - <h4 className="text-medium pb1">{t`Default filter widget value`}</h4> - <ParameterValueWidget + <InputContainer lessBottomPadding> + <ContainerLabel>{t`Default filter widget value`}</ContainerLabel> + <DefaultParameterValueWidget parameter={ tag.type === "dimension" ? parameter || { @@ -293,13 +298,12 @@ export class TagEditorParam extends Component { } value={tag.default} setValue={value => this.setParameterAttribute("default", value)} - className="AdminSelect p1 text-bold text-medium bordered border-medium rounded bg-white" isEditing commitImmediately /> - </div> + </InputContainer> )} - </div> + </TagContainer> ); } } diff --git a/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.styled.tsx b/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.styled.tsx new file mode 100644 index 0000000000000000000000000000000000000000..5fb33db7c5e9e30bc5888c27a333f453dabd2daa --- /dev/null +++ b/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.styled.tsx @@ -0,0 +1,56 @@ +import styled from "@emotion/styled"; +import InputBlurChange from "metabase/components/InputBlurChange"; +import { color } from "metabase/lib/colors"; +import ParameterValueWidget from "metabase/parameters/components/ParameterValueWidget"; + +export const TagContainer = styled.div` + padding: 1.5rem 1.5rem 0 1.5rem; + margin-bottom: 1.5rem; + border-top: 1px solid ${color("border")}; +`; +export const TagName = styled.h3` + font-weight: 900; + margin-bottom: 2rem; + align-self: flex-end; + color: ${color("brand")}; +`; + +interface ContainerLabelProps { + paddingTop: boolean; +} +export const ContainerLabel = styled.h4<ContainerLabelProps>` + padding-bottom: 0.5rem; + color: ${color("text-medium")}; + padding-top: ${props => (props.paddingTop ? "0.5rem" : "0")}; +`; + +export const ErrorSpan = styled.span` + margin: 0 0.5rem; + color: ${color("error")}; +`; + +interface InputContainerProps { + lessBottomPadding: boolean; +} +export const InputContainer = styled.div<InputContainerProps>` + padding-bottom: ${props => (props.lessBottomPadding ? "1.5rem" : "2rem")}; +`; + +export const WidgetLabelInput = styled(InputBlurChange)` + font-weight: 700; + padding: 0.5rem; + border: 1px solid ${color("border-dark")}; + border-radius: 0.5rem; + width: 100%; + color: ${color("text-dark")}; + font-size: 0.875rem; +`; + +export const DefaultParameterValueWidget = styled(ParameterValueWidget)` + padding: 0.5rem; + font-weight: 700; + color: ${color("text-medium")}; + border-radius: 0.5rem; + background-color: ${color("white")}; + border: 2px solid ${color("border")}; +`; diff --git a/frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js b/frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js index d39f0b8213f05dbd004b32a25609efe76cb66d53..8ebad7fd1553fc2bb7e5dc7681df2286f812027e 100644 --- a/frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js +++ b/frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js @@ -194,11 +194,33 @@ describe("NativeQuery", () => { expect(q.canRun()).toBe(true); }); + it("requires a display name", () => { + q = q.setDatasetQuery( + assocIn(q.datasetQuery(), ["native", "template-tags", "foo"], { + name: "foo", + type: "text", + }), + ); + + expect(q.canRun()).toBe(false); + + q = q.setDatasetQuery( + assocIn(q.datasetQuery(), ["native", "template-tags", "foo"], { + name: "foo", + type: "text", + "display-name": "Foo", + }), + ); + + expect(q.canRun()).toBe(true); + }); + it("dimension type without a dimension", () => { q = q.setDatasetQuery( assocIn(q.datasetQuery(), ["native", "template-tags", "foo"], { type: "dimension", "widget-type": "category", + "display-name": "bar", }), ); @@ -210,6 +232,7 @@ describe("NativeQuery", () => { type: "dimension", "widget-type": "category", dimension: ["field", 123, null], + "display-name": "bar", }), ); diff --git a/frontend/test/metabase/scenarios/native-filters/reproductions/11580.cy.spec.js b/frontend/test/metabase/scenarios/native-filters/reproductions/11580.cy.spec.js index 727710b78e41e6e7004de2b4d9d393d45f0f2d98..99a52432c736404e280747ad6609b0d3fd14f14f 100644 --- a/frontend/test/metabase/scenarios/native-filters/reproductions/11580.cy.spec.js +++ b/frontend/test/metabase/scenarios/native-filters/reproductions/11580.cy.spec.js @@ -12,7 +12,7 @@ describe("issue 11580", () => { openNativeEditor(); SQLFilter.enterParameterizedQuery("{{foo}} {{bar}}"); - cy.findByTestId("sidebar-right").find(".text-brand").as("variableLabels"); + cy.findAllByText("Variable name").next().as("variableLabels"); // ensure they're in the right order to start assertVariablesOrder();