diff --git a/e2e/test/scenarios/filters-reproductions/filters-reproductions.cy.spec.js b/e2e/test/scenarios/filters-reproductions/filters-reproductions.cy.spec.js index dd7370b6b7b495dc465bf6d39fab90fac03060d0..dfb0fc175228f01613b0d629c665a48c36c40f78 100644 --- a/e2e/test/scenarios/filters-reproductions/filters-reproductions.cy.spec.js +++ b/e2e/test/scenarios/filters-reproductions/filters-reproductions.cy.spec.js @@ -17,6 +17,7 @@ import { filterWidget, getNotebookStep, modal, + openNativeEditor, openNotebook, openOrdersTable, openPeopleTable, @@ -28,6 +29,7 @@ import { restore, resyncDatabase, selectFilterOperator, + sidebar, startNewQuestion, tableHeaderClick, visitQuestionAdhoc, @@ -1546,3 +1548,49 @@ describe("issue 49321", () => { }); }); }); + +describe("issue 44665", () => { + beforeEach(() => { + restore(); + cy.signInAsNormalUser(); + }); + + it("should use the correct widget for the default value picker (metabase#44665)", () => { + openNativeEditor().type("select * from {{param"); + sidebar() + .last() + .within(() => { + cy.findByText("Search box").click(); + cy.findByText("Edit").click(); + }); + + modal().within(() => { + cy.findByText("Custom list").click(); + cy.findByRole("textbox").type("foo\nbar\nbaz"); + cy.button("Done").click(); + }); + + sidebar().last().findByText("Enter a default value…").click(); + popover().within(() => { + cy.findByPlaceholderText("Enter a default value…").should("be.visible"); + cy.findByText("foo").should("be.visible"); + cy.findByText("bar").should("be.visible"); + cy.findByText("baz").should("be.visible"); + }); + + sidebar() + .last() + .within(() => { + cy.findByText("Enter a default value…").click(); + cy.findByText("Dropdown list").click(); + cy.findByText("Enter a default value…").click(); + }); + + popover().within(() => { + cy.findByPlaceholderText("Enter a default value…").should("be.visible"); + cy.findByText("foo").should("be.visible"); + cy.findByText("bar").should("be.visible"); + cy.findByText("baz").should("be.visible"); + }); + }); +}); diff --git a/e2e/test/scenarios/native-filters/helpers/e2e-date-filter-helpers.js b/e2e/test/scenarios/native-filters/helpers/e2e-date-filter-helpers.js index 5c5b28073241b83932651f63b49e7f9047d04675..3b2021d18cc5f72a975153cb48fb9c650402d18d 100644 --- a/e2e/test/scenarios/native-filters/helpers/e2e-date-filter-helpers.js +++ b/e2e/test/scenarios/native-filters/helpers/e2e-date-filter-helpers.js @@ -45,12 +45,10 @@ export function setRelativeDate(term) { cy.findByText(term).click(); } -export function setAdHocFilter({ - condition, - quantity, - timeBucket, - includeCurrent = false, -} = {}) { +export function setAdHocFilter( + { condition, quantity, timeBucket, includeCurrent = false } = {}, + buttonLabel = "Add filter", +) { cy.findByText("Relative dates...").click(); if (condition) { cy.findByText(condition).click({ force: true }); @@ -77,5 +75,5 @@ export function setAdHocFilter({ cy.findByText(/^Include/).click(); } - cy.button("Add filter").click(); + cy.button(buttonLabel).click(); } diff --git a/e2e/test/scenarios/native-filters/helpers/e2e-field-filter-helpers.js b/e2e/test/scenarios/native-filters/helpers/e2e-field-filter-helpers.js index 502c092f2cc8aecf6ee4fdd85ce249e9e3ed7dcb..b2ade6a78b86ddb86e6b3441f7c80649676661ca 100644 --- a/e2e/test/scenarios/native-filters/helpers/e2e-field-filter-helpers.js +++ b/e2e/test/scenarios/native-filters/helpers/e2e-field-filter-helpers.js @@ -82,8 +82,8 @@ export function applyFilterByType( * * @param {string} value */ -export function addDefaultStringFilter(value) { - enterDefaultValue(value, "Add filter"); +export function addDefaultStringFilter(value, buttonLabel = "Update filter") { + enterDefaultValue(value, buttonLabel); } // FIELD FILTER NUMBER FILTERS @@ -108,12 +108,12 @@ export function addWidgetNumberFilter( * @param {array|string} value * @return {function} */ -export function addDefaultNumberFilter(value) { +export function addDefaultNumberFilter(value, buttonLabel = "Add filter") { if (isBetweenFilter(value)) { cy.findByText("Enter a default value…").click(); - addBetweenFilter(value); + addBetweenFilter(value, buttonLabel); } else { - enterDefaultValue(value); + enterDefaultValue(value, buttonLabel); } } @@ -193,7 +193,11 @@ function enterDefaultValue(value, buttonLabel = "Add filter") { * @param {string} searchTerm * @param {string} result */ -export function pickDefaultValue(searchTerm, result) { +export function pickDefaultValue( + searchTerm, + result, + buttonLabel = "Add filter", +) { cy.findByText("Enter a default value…").click(); cy.findByPlaceholderText("Enter a default value…").type(searchTerm); @@ -207,7 +211,7 @@ export function pickDefaultValue(searchTerm, result) { // cy.findByLabelText(result).should("be.visible").click(); - cy.button("Add filter").click(); + cy.button(buttonLabel).click(); } /** diff --git a/e2e/test/scenarios/native-filters/native-filters-reproductions.cy.spec.js b/e2e/test/scenarios/native-filters/native-filters-reproductions.cy.spec.js index 9c933966def153ee813121407f96b4fdabaaeee7..831f46a10d8f0431b877eb777c4d34ba6862d12d 100644 --- a/e2e/test/scenarios/native-filters/native-filters-reproductions.cy.spec.js +++ b/e2e/test/scenarios/native-filters/native-filters-reproductions.cy.spec.js @@ -484,7 +484,7 @@ describe("issue 15444", () => { // This flow tests the ability to pick the filter from a dropdown when there are not too many results (easy to choose from). popover().within(() => { cy.findByText("Doohickey").click(); - cy.button("Add filter").click(); + cy.button("Update filter").click(); }); SQLFilter.runQuery(); @@ -1074,7 +1074,7 @@ describe("issue 31606", { tags: "@external" }, () => { .should("have.value", "ID") .should("be.disabled"); - FieldFilter.addDefaultStringFilter("2"); + FieldFilter.addDefaultStringFilter("2", "Add filter"); cy.findByTestId("sidebar-content").within(() => { cy.findByText("Enter a default value…").should("not.exist"); diff --git a/e2e/test/scenarios/native-filters/sql-field-filter-types.cy.spec.js b/e2e/test/scenarios/native-filters/sql-field-filter-types.cy.spec.js index 65a386b6139df101ab5428bf75fc816fd9283cd6..ce88d52ffbf4c8c9c47e8abc608ef82bb1dc2183 100644 --- a/e2e/test/scenarios/native-filters/sql-field-filter-types.cy.spec.js +++ b/e2e/test/scenarios/native-filters/sql-field-filter-types.cy.spec.js @@ -14,7 +14,7 @@ const dateFilters = Object.entries(DATE_FILTER_SUBTYPES); describe("scenarios > filters > sql filters > field filter > Date", () => { function openDateFilterPicker(isFilterRequired) { const selector = isFilterRequired - ? cy.findByPlaceholderText("Select a default value…") + ? cy.findByText("Select a default value…") : cy.get("fieldset"); return selector.click(); @@ -24,6 +24,7 @@ describe("scenarios > filters > sql filters > field filter > Date", () => { filterType, filterValue, isFilterRequired = false, + buttonLabel = "Add filter", } = {}) { openDateFilterPicker(isFilterRequired); @@ -38,12 +39,12 @@ describe("scenarios > filters > sql filters > field filter > Date", () => { case "Single Date": DateFilter.setSingleDate(filterValue); - cy.findByText("Add filter").click(); + cy.findByText(buttonLabel).click(); break; case "Date Range": DateFilter.setDateRange(filterValue); - cy.findByText("Add filter").click(); + cy.findByText(buttonLabel).click(); break; case "Relative Date": @@ -51,7 +52,7 @@ describe("scenarios > filters > sql filters > field filter > Date", () => { break; case "All Options": - DateFilter.setAdHocFilter(filterValue); + DateFilter.setAdHocFilter(filterValue, buttonLabel); break; default: @@ -107,6 +108,7 @@ describe("scenarios > filters > sql filters > field filter > Date", () => { filterType: subType, filterValue: value, isFilterRequired: true, + buttonLabel: "Update filter", }); SQLFilter.runQuery(); @@ -165,7 +167,7 @@ describe("scenarios > filters > sql filters > field filter > Number", () => { FieldFilter.setWidgetType(subType); - FieldFilter.addDefaultNumberFilter(value); + FieldFilter.addDefaultNumberFilter(value, "Update filter"); SQLFilter.runQuery(); @@ -235,7 +237,7 @@ describe("scenarios > filters > sql filters > field filter > String", () => { FieldFilter.setWidgetType(subType); searchTerm - ? FieldFilter.pickDefaultValue(searchTerm, value) + ? FieldFilter.pickDefaultValue(searchTerm, value, "Update filter") : FieldFilter.addDefaultStringFilter(value); SQLFilter.runQuery(); diff --git a/e2e/test/scenarios/native-filters/sql-field-filter.cy.spec.js b/e2e/test/scenarios/native-filters/sql-field-filter.cy.spec.js index 70b23aacb4f656205026dfee57fe99b4e42b124a..2dda8035aed5589ceaae04ea8ce028802b563ce5 100644 --- a/e2e/test/scenarios/native-filters/sql-field-filter.cy.spec.js +++ b/e2e/test/scenarios/native-filters/sql-field-filter.cy.spec.js @@ -114,7 +114,7 @@ describe("scenarios > filters > sql filters > field filter", () => { it("should work when set initially as default value and then through the filter widget", () => { cy.log("the default value should apply"); - FieldFilter.addDefaultStringFilter("2"); + FieldFilter.addDefaultStringFilter("2", "Add filter"); SQLFilter.runQuery(); cy.findByTestId("query-visualization-root").within(() => { cy.findByText("Small Marble Shoes"); diff --git a/e2e/test/scenarios/native-filters/sql-filters-reset-clear.cy.spec.ts b/e2e/test/scenarios/native-filters/sql-filters-reset-clear.cy.spec.ts index 16c694b8198f5786d5bb1487ae0c700e7f1d538a..59fe765da0778f5a4285e61a05d635334645faa9 100644 --- a/e2e/test/scenarios/native-filters/sql-filters-reset-clear.cy.spec.ts +++ b/e2e/test/scenarios/native-filters/sql-filters-reset-clear.cy.spec.ts @@ -228,6 +228,10 @@ describe("scenarios > filters > sql filters > reset & clear", () => { popover().findByRole("searchbox").clear().type(value).blur(); popover().button("Add filter").click(); }, + updateValue: value => { + popover().findByRole("searchbox").clear().type(value).blur(); + popover().button("Update filter").click(); + }, }); }); @@ -536,7 +540,10 @@ describe("scenarios > filters > sql filters > reset & clear", () => { cy.log(NO_DEFAULT_NON_REQUIRED); filterSection("no_default_non_required").within(() => { filter("Default filter widget value").scrollIntoView(); - filterInput("Default filter widget value").should("have.value", ""); + filter("Default filter widget value").should( + "have.text", + "Select a default value…", + ); checkStatusIcon("Default filter widget value", "chevron"); filter("Default filter widget value").click(); }); @@ -545,34 +552,37 @@ describe("scenarios > filters > sql filters > reset & clear", () => { popover().button("Add filter").click(); filterSection("no_default_non_required").within(() => { - filterInput("Default filter widget value").should( - "have.value", + filter("Default filter widget value").should( + "have.text", otherValueFormatted, ); checkStatusIcon("Default filter widget value", "clear"); clearIcon("Default filter widget value").click(); - filterInput("Default filter widget value").should("have.value", ""); + filter("Default filter widget value").should( + "have.text", + "Select a default value…", + ); checkStatusIcon("Default filter widget value", "chevron"); }); cy.log(NO_DEFAULT_REQUIRED); filterSection("no_default_required").within(() => { filter("Default filter widget value (required)").scrollIntoView(); - filterInput("Default filter widget value (required)").should( - "have.value", - "", + filter("Default filter widget value (required)").should( + "have.text", + "Select a default value…", ); checkStatusIcon("Default filter widget value (required)", "chevron"); filter("Default filter widget value (required)").click(); }); popover().findByRole("textbox").clear().type(otherValue).blur(); - popover().button("Add filter").click(); + popover().button("Update filter").click(); filterSection("no_default_required").within(() => { - filterInput("Default filter widget value").should( - "have.value", + filter("Default filter widget value").should( + "have.text", otherValueFormatted, ); checkStatusIcon("Default filter widget value", "clear"); @@ -584,14 +594,17 @@ describe("scenarios > filters > sql filters > reset & clear", () => { cy.log(DEFAULT_NON_REQUIRED); filterSection("default_non_required").within(() => { filter("Default filter widget value").scrollIntoView(); - filterInput("Default filter widget value").should( - "have.value", + filter("Default filter widget value").should( + "have.text", defaultValueFormatted, ); checkStatusIcon("Default filter widget value", "clear"); clearIcon("Default filter widget value").click(); - filterInput("Default filter widget value").should("have.value", ""); + filter("Default filter widget value").should( + "have.text", + "Select a default value…", + ); checkStatusIcon("Default filter widget value", "chevron"); filter("Default filter widget value").click(); }); @@ -600,8 +613,8 @@ describe("scenarios > filters > sql filters > reset & clear", () => { popover().button("Add filter").click(); filterSection("default_non_required").within(() => { - filterInput("Default filter widget value").should( - "have.value", + filter("Default filter widget value").should( + "have.text", otherValueFormatted, ); checkStatusIcon("Default filter widget value", "clear"); @@ -610,27 +623,27 @@ describe("scenarios > filters > sql filters > reset & clear", () => { cy.log(DEFAULT_REQUIRED); filterSection("default_required").within(() => { filter("Default filter widget value").scrollIntoView(); - filterInput("Default filter widget value").should( - "have.value", + filter("Default filter widget value").should( + "have.text", defaultValueFormatted, ); checkStatusIcon("Default filter widget value", "clear"); clearIcon("Default filter widget value").click(); - filterInput("Default filter widget value (required)").should( - "have.value", - "", + filter("Default filter widget value (required)").should( + "have.text", + "Select a default value…", ); checkStatusIcon("Default filter widget value (required)", "chevron"); filter("Default filter widget value (required)").click(); }); popover().findByRole("textbox").clear().type(otherValue).blur(); - popover().button("Add filter").click(); + popover().button("Update filter").click(); filterSection("default_required").within(() => { - filterInput("Default filter widget value").should( - "have.value", + filter("Default filter widget value").should( + "have.text", otherValueFormatted, ); checkStatusIcon("Default filter widget value", "clear"); @@ -694,7 +707,7 @@ describe("scenarios > filters > sql filters > reset & clear", () => { filter("Default filter widget value (required)").click(); }); - setValue(otherValue); + updateValue(otherValue); filterSection("no_default_required").within(() => { filter("Default filter widget value").should( diff --git a/e2e/test/scenarios/native-filters/sql-filters-source.cy.spec.js b/e2e/test/scenarios/native-filters/sql-filters-source.cy.spec.js index 2131f1e3e9f72c08b8c2def68c0e5e60d982db72..4165abd63a22eb7d93fb47961c7cd7f7ca9a80ee 100644 --- a/e2e/test/scenarios/native-filters/sql-filters-source.cy.spec.js +++ b/e2e/test/scenarios/native-filters/sql-filters-source.cy.spec.js @@ -80,7 +80,7 @@ describe("scenarios > filters > sql filters > values source", () => { SQLFilter.toggleRequired(); FieldFilter.openEntryForm(true); FieldFilter.selectFilterValueFromList("Gadget", { - buttonLabel: "Add filter", + buttonLabel: "Update filter", }); }); @@ -105,9 +105,13 @@ describe("scenarios > filters > sql filters > values source", () => { SQLFilter.toggleRequired(); cy.findByTestId("sidebar-content") - .findByPlaceholderText("Start typing to filter…") + .findByText("Enter a default value…") .click(); - popover().findByText("Gadget").click(); + + popover().within(() => { + cy.findByText("Gadget").click(); + cy.button("Update filter").click(); + }); }); it("should be able to use a structured question source without saving the question", () => { @@ -833,10 +837,13 @@ describe("scenarios > filters > sql filters > values source > number parameter", }); cy.findByTestId("sidebar-content") - .findByPlaceholderText("Select a default value…") + .findByText("Enter a default value…") .click(); - popover().findByText("Twenty").click(); + popover().within(() => { + cy.findByText("Twenty").click(); + cy.button("Add filter").click(); + }); saveQuestion("SQL filter"); diff --git a/e2e/test/scenarios/native-filters/sql-filters.cy.spec.js b/e2e/test/scenarios/native-filters/sql-filters.cy.spec.js index 6d91c42417ce21bd1d4d340c4e54225758c9d8d4..f51b2c8f3bf81731cfa91f1c8b1a36bbe92b1874 100644 --- a/e2e/test/scenarios/native-filters/sql-filters.cy.spec.js +++ b/e2e/test/scenarios/native-filters/sql-filters.cy.spec.js @@ -186,11 +186,11 @@ describe("scenarios > filters > sql filters > basic filter types", () => { SQLFilter.toggleRequired(); cy.findByTestId("sidebar-content") - .findByPlaceholderText("Select a default value…") + .findByText("Select a default value…") .click(); popover().within(() => { cy.findByText("15").click(); - cy.findByText("Add filter").click(); + cy.findByText("Update filter").click(); }); SQLFilter.runQuery(); @@ -202,7 +202,7 @@ describe("scenarios > filters > sql filters > basic filter types", () => { function setDefaultDate(year = "2024", month = "01", day = "22") { cy.findByTestId("sidebar-content") - .findByPlaceholderText("Select a default value…") + .findByText("Select a default value…") .click(); popover().within(() => { DateFilter.setSingleDate(`${month}/${day}/${year}`); diff --git a/frontend/src/metabase/query_builder/components/template_tags/TagEditorParamParts/DefaultRequiredValueControl.tsx b/frontend/src/metabase/query_builder/components/template_tags/TagEditorParamParts/DefaultRequiredValueControl.tsx index 168bf876c9a30709d194165848e6cf67a72bd4b6..fc25c16fb93812ec86feba17728de71c53dde95d 100644 --- a/frontend/src/metabase/query_builder/components/template_tags/TagEditorParamParts/DefaultRequiredValueControl.tsx +++ b/frontend/src/metabase/query_builder/components/template_tags/TagEditorParamParts/DefaultRequiredValueControl.tsx @@ -1,12 +1,15 @@ import { t } from "ttag"; import _ from "underscore"; -import { ParameterValuePicker } from "metabase/parameters/components/ParameterValuePicker"; import { RequiredParamToggle } from "metabase/parameters/components/RequiredParamToggle"; import { Flex, Text } from "metabase/ui"; import type { Parameter, TemplateTag } from "metabase-types/api"; -import { ContainerLabel, ErrorSpan } from "./TagEditorParam.styled"; +import { + ContainerLabel, + DefaultParameterValueWidget, + ErrorSpan, +} from "./TagEditorParam.styled"; export function DefaultRequiredValueControl({ tag, @@ -31,14 +34,18 @@ export function DefaultRequiredValueControl({ </ContainerLabel> <Flex gap="xs" direction="column"> - <div aria-labelledby={`default-value-label-${tag.id}`}> - <ParameterValuePicker - tag={tag} - parameter={parameter} - value={tag.default} - onValueChange={onChangeDefaultValue} - /> - </div> + {parameter && ( + <div aria-labelledby={`default-value-label-${tag.id}`}> + <DefaultParameterValueWidget + parameter={parameter} + value={tag.default} + setValue={onChangeDefaultValue} + isEditing + commitImmediately + mimicMantine + /> + </div> + )} <RequiredParamToggle uniqueId={tag.id} diff --git a/frontend/src/metabase/query_builder/components/template_tags/TagEditorParamParts/TagEditorParam.styled.tsx b/frontend/src/metabase/query_builder/components/template_tags/TagEditorParamParts/TagEditorParam.styled.tsx index c0456fc805b79cd6b0d8aa4c7e169ab0d94b42be..090d2d5370865c3351bf4a7a14f5c03e8da19d0f 100644 --- a/frontend/src/metabase/query_builder/components/template_tags/TagEditorParamParts/TagEditorParam.styled.tsx +++ b/frontend/src/metabase/query_builder/components/template_tags/TagEditorParamParts/TagEditorParam.styled.tsx @@ -39,12 +39,12 @@ export const InputContainer = styled.label<InputContainerProps>` `; export const DefaultParameterValueWidget = styled(ParameterValueWidget)` - padding: 0.5rem; - font-weight: 700; - color: var(--mb-color-text-medium); - border-radius: 0.5rem; + color: var(--mb-color-text-dark); + padding: 0.75rem 0.75rem; + border: 1px solid var(--mb-color-border); + border-radius: 4px; background-color: var(--mb-color-bg-white); - border: 2px solid var(--mb-color-border); + font-weight: normal; `; export const ToggleContainer = styled.div`