From 62123c073eff1b13aafd09070252ead7ed0cc2e0 Mon Sep 17 00:00:00 2001 From: Romeo Van Snick <romeo@romeovansnick.be> Date: Fri, 14 Jun 2024 10:04:18 +0200 Subject: [PATCH] Use MultiAutocomplete in dashboard filters (#43293) * Use MultiAutocomplete in dashboard filters * Handle prefix on MultiAutocomplete * Remove unnecessary tests for empty values * Remove input className * Handle numbers in MultiAutocomplete * Accept number values in MultiAutocomplete * Use MultiAutocomplete in NumberInputWidget * Filter out empty items * Do not expect formatted number in values * Do not allow adding multiple values when multi is false * Handle undefined values * Make value optional * Fix MultiAutocomplete types * Make MultiAutoComplete generic in its value type * Refine MultiAutocomplete type in NumberInputWidget * Refine MultiAutocomplete type in FieldValuesWidget * Fix typo * Fix MultiAutocomplete tests * Add helpers to work with MultiSelect and MultiAutocomplete inputs * Handle MultiAutocomplete input in tests * Fix filter helper * Use multiAutocompleteInput helper * Find the value remove button in test * Fix tests for NumberInputWidget * Fix tests for DefaultPicker * Fix type in NumberInput * Fix dasboard test * Fix embedding test * Add helpers to remove values * Fix chained filter test * Fix nested test * Fix embedding tests * Fix filter tests * Use removeMultiAutocompleteValue * Use removeMultiAutocompleteValue * Remove unused variable * Use multiAutocompleteInput helper * Handle comma's in values * Handle multiselect * Fix filter test * Add filter * Format field value * Fix embedding tests * Allow rendering custom label for item * Use renderValue in FieldValuesWidget * Fix data prop type * Be more explicit about broken types in FieldValuesWidget * Fix default behaviour for label * Fix embedding tests * Force click * Fix sharing test * Use removeMultiAutocompleteValue * Handle multiautocomplete inputs * Force click filter * Close the suggestions menu * Fix email test * Fix sharing filter * Set viewport in test * Fix sharing test * Remove debugging logs * Use removeMultiAutocompleteValue * Close the suggestions menu using blur * Remove commented out code * Remove the unused color prop * Move shared types to types file * Rename ValueType to TValue * Add missing forwardRef * Add tests for filter and renderValue * Use TokenField for single value parameters * Fix single value test * Be more specific about when to select a single input * Fix single value e2e test * Handle undefined parameter * Accept all RowValue types in MultiSelect * Accept all RowValue types in ListField * Accept all RowValue types in SingleSelectListField * Be more precise about the FieldValuesWidget value types * Be more specific about when to render normal input * Remove duplicate import * Remove maxSelectedValues from MultiAutocomplete * Parse values in FieldValuesWidget * Remove parsing complexity from MultiAutocomplete * Be more strict when parsing numbers * Do not use multiselect when arity is 1 * Remove spurious changes * Pass prefix from FieldValuesWidget * Remove number MultiAutocomplete story * Remove renderValue from MultiAutocomplete * Add custom component to MultiAutocomplete * Add missing data prop * Remove spurious changes in MultiAutocomplete * Remove SelectItem reference * Use renderValue in FieldValuesWidget --- .../helpers/e2e-ui-elements-helpers.js | 24 +++- .../actions/actions-on-dashboards.cy.spec.js | 5 +- .../custom-column-reproductions.cy.spec.js | 9 +- .../dashboard-cards/click-behavior.cy.spec.js | 3 +- .../dashboard-chained-filters.cy.spec.js | 101 ++++++++++++---- .../dashboard-filters-auto-wiring.cy.spec.js | 5 +- .../dashboard-filters-nested.cy.spec.js | 7 +- ...dashboard-filters-text-category.cy.spec.js | 2 +- .../old-parameters.cy.spec.js | 9 +- .../dashboard-filters/parameters.cy.spec.js | 28 +++-- .../shared/dashboard-filters-text-category.js | 8 +- .../scenarios/dashboard/text-cards.cy.spec.js | 5 +- .../dashboard/title-drill.cy.spec.js | 11 +- .../embedding/embedding-dashboard.cy.spec.js | 46 +++++--- .../embedding-linked-filters.cy.spec.js | 38 +++++- ...dashboard-filters-reproductions.cy.spec.js | 4 +- .../helpers/e2e-field-filter-data-objects.js | 2 +- .../helpers/e2e-field-filter-helpers.js | 2 +- .../native-filters-reproductions.cy.spec.js | 5 +- .../sql-field-filter-types.cy.spec.js | 3 + .../sql-field-filter.cy.spec.js | 6 +- .../sharing/downloads/downloads.cy.spec.js | 3 +- .../sharing/sharing-reproductions.cy.spec.js | 3 +- .../sharing/subscriptions.cy.spec.js | 51 +++++--- .../FilterPopover/FilterPopover.unit.spec.tsx | 2 +- .../pickers/DefaultPicker/DefaultPicker.tsx | 1 - .../DefaultPicker/DefaultPicker.unit.spec.tsx | 19 ++- .../FieldValuesWidget/FieldValuesWidget.tsx | 109 ++++++++++++++++-- .../FieldValuesWidget.unit.spec.tsx | 45 -------- .../components/ListField/ListField.tsx | 7 +- .../metabase/components/ListField/types.ts | 3 +- .../SingleSelectListField.tsx | 7 +- .../components/SingleSelectListField/types.ts | 6 +- .../NumberInputWidget/NumberInputWidget.tsx | 38 +++--- .../NumberInputWidget.unit.spec.tsx | 46 ++++++-- .../StringInputWidget/StringInputWidget.tsx | 2 +- .../MultiAutocomplete/MultiAutocomplete.tsx | 4 +- .../MultiAutocomplete.unit.spec.tsx | 13 +++ .../inputs/MultiAutocomplete/index.ts | 2 + .../inputs/MultiAutocomplete/utils.ts | 4 +- .../inputs/MultiSelect/MultiSelect.styled.tsx | 12 ++ 41 files changed, 496 insertions(+), 204 deletions(-) diff --git a/e2e/support/helpers/e2e-ui-elements-helpers.js b/e2e/support/helpers/e2e-ui-elements-helpers.js index df67ff717d8..0f1cd27ce6b 100644 --- a/e2e/support/helpers/e2e-ui-elements-helpers.js +++ b/e2e/support/helpers/e2e-ui-elements-helpers.js @@ -139,7 +139,7 @@ export function setFilterWidgetValue( ) { filterWidget().eq(0).click(); popover().within(() => { - cy.icon("close").click(); + removeMultiAutocompleteValue(0); if (value) { cy.findByPlaceholderText(targetPlaceholder).type(value).blur(); } @@ -257,6 +257,7 @@ export function tableHeaderClick(headerString) { cy.findByTextEnsureVisible(headerString).trigger("mouseup"); }); } + /** * selects the global new button * @param {*} menuItem optional, if provided, will click the New button and return the menu item with the text provided @@ -270,3 +271,24 @@ export function newButton(menuItem) { return cy.findByTestId("app-bar").button("New"); } + +export function multiSelectInput(filter = ":eq(0)") { + return cy.findByRole("combobox").filter(filter).get("input").last(); +} + +export function multiAutocompleteInput(filter = ":eq(0)") { + return cy.findAllByRole("combobox").filter(filter).get("input").last(); +} + +export function multiAutocompleteValue(index, filter = ":eq(0)") { + return cy + .findAllByRole("combobox") + .filter(filter) + .get(`[value][index=${index}]`); +} + +export function removeMultiAutocompleteValue(index, filter) { + return multiAutocompleteValue(index, filter) + .findByRole("button", { hidden: true }) + .click(); +} diff --git a/e2e/test/scenarios/actions/actions-on-dashboards.cy.spec.js b/e2e/test/scenarios/actions/actions-on-dashboards.cy.spec.js index b7935f3cd89..4a9c99785fc 100644 --- a/e2e/test/scenarios/actions/actions-on-dashboards.cy.spec.js +++ b/e2e/test/scenarios/actions/actions-on-dashboards.cy.spec.js @@ -29,6 +29,7 @@ import { createPublicDashboardLink, openStaticEmbeddingModal, visitIframe, + multiAutocompleteInput, } from "e2e/support/helpers"; import { many_data_types_rows } from "e2e/support/test_tables_data"; import { createMockActionParameter } from "metabase-types/api/mocks"; @@ -1187,7 +1188,9 @@ describe( }); filterWidget().click(); - popover().find("input").first().type("{backspace}10"); + popover().within(() => { + multiAutocompleteInput().type("{backspace}10"); + }); cy.button("Update filter").click(); cy.button(actionName).click(); diff --git a/e2e/test/scenarios/custom-column/custom-column-reproductions.cy.spec.js b/e2e/test/scenarios/custom-column/custom-column-reproductions.cy.spec.js index f5b60a929dc..0bf895a5753 100644 --- a/e2e/test/scenarios/custom-column/custom-column-reproductions.cy.spec.js +++ b/e2e/test/scenarios/custom-column/custom-column-reproductions.cy.spec.js @@ -33,6 +33,7 @@ import { tableHeaderClick, resetTestTable, main, + multiAutocompleteInput, } from "e2e/support/helpers"; import { createSegment } from "e2e/support/helpers/e2e-table-metadata-helpers"; @@ -229,7 +230,7 @@ describe("issue 14843", () => { popover().findByText(CC_NAME).click(); selectFilterOperator("Not equal to"); popover().within(() => { - cy.findByPlaceholderText("Enter a number").type("3"); + multiAutocompleteInput().type("3"); cy.button("Add filter").click(); }); @@ -315,8 +316,10 @@ describe("issue 18747", () => { function addValueToParameterFilter() { filterWidget().click(); - popover().find("input").type("14"); - popover().contains("Add filter").click(); + popover().within(() => { + multiAutocompleteInput().type("14"); + cy.button("Add filter").click(); + }); } beforeEach(() => { diff --git a/e2e/test/scenarios/dashboard-cards/click-behavior.cy.spec.js b/e2e/test/scenarios/dashboard-cards/click-behavior.cy.spec.js index c8091fd9f13..eca135daefe 100644 --- a/e2e/test/scenarios/dashboard-cards/click-behavior.cy.spec.js +++ b/e2e/test/scenarios/dashboard-cards/click-behavior.cy.spec.js @@ -29,6 +29,7 @@ import { entityPickerModal, filterWidget, queryBuilderHeader, + removeMultiAutocompleteValue, } from "e2e/support/helpers"; import { b64hash_to_utf8 } from "metabase/lib/encoding"; import { @@ -1447,7 +1448,7 @@ describe("scenarios > dashboard > dashboard cards > click behavior", () => { cy.button(DASHBOARD_FILTER_TEXT.name).click(); popover().within(() => { - cy.icon("close").click(); + removeMultiAutocompleteValue(0); cy.findByPlaceholderText("Search by Name").type("Dell Adams"); cy.button("Update filter").click(); }); diff --git a/e2e/test/scenarios/dashboard-filters/dashboard-chained-filters.cy.spec.js b/e2e/test/scenarios/dashboard-filters/dashboard-chained-filters.cy.spec.js index 029a3384b31..fe3873ec10e 100644 --- a/e2e/test/scenarios/dashboard-filters/dashboard-chained-filters.cy.spec.js +++ b/e2e/test/scenarios/dashboard-filters/dashboard-chained-filters.cy.spec.js @@ -15,6 +15,7 @@ import { editDashboard, setFilter, sidebar, + multiAutocompleteInput, } from "e2e/support/helpers"; const { PEOPLE } = SAMPLE_DATABASE; @@ -93,45 +94,95 @@ describe("scenarios > dashboard > chained filter", () => { filterWidget().contains("Location 1").click(); popover().within(() => { - cy.findByPlaceholderText( - has_field_values === "search" ? "Search by City" : "Search the list", - ).type("An"); - cy.findByText("Anchorage"); - cy.findByText("Anacoco").should("not.exist"); - - cy.get("input").first().clear(); + if (has_field_values === "search") { + multiAutocompleteInput().type("An"); + } + if (has_field_values === "list") { + cy.findByPlaceholderText("Search the list").type("An"); + } }); + popover() + .last() + .within(() => { + cy.findByText("Anchorage"); + cy.findByText("Anacoco").should("not.exist"); + }); + + popover() + .first() + .within(() => { + if (has_field_values === "search") { + multiAutocompleteInput() + .type("{backspace}{backspace}") + // close the suggestion list + .blur(); + } + if (has_field_values === "list") { + cy.findByPlaceholderText("Search the list").clear(); + } + }); + filterWidget().contains("AK").click(); - popover().within(() => { - cy.findByText("AK").click(); - cy.findByText("GA").click(); + popover() + .last() + .within(() => { + cy.findByText("AK").click(); + cy.findByText("GA").click(); - cy.findByText("Update filter").click(); - }); + cy.findByText("Update filter").click(); + }); // do it again to make sure it isn't cached incorrectly filterWidget().contains("Location 1").click(); popover().within(() => { - cy.get("input").first().type("An"); - cy.findByText("Canton"); - cy.findByText("Anchorage").should("not.exist"); + if (has_field_values === "search") { + multiAutocompleteInput().type("An"); + } + if (has_field_values === "list") { + cy.findByPlaceholderText("Search the list").type("An"); + } }); + popover() + .last() + .within(() => { + cy.findByText("Canton"); + cy.findByText("Anchorage").should("not.exist"); + }); + + if (has_field_values === "search") { + popover() + .first() + .within(() => { + // close the suggestion list + multiAutocompleteInput().blur(); + }); + } + filterWidget().contains("GA").click(); - popover().within(() => { - cy.findByText("GA").click(); - cy.findByText("Update filter").click(); - }); + popover() + .last() + .within(() => { + cy.findByText("GA").click(); + cy.findByText("Update filter").click(); + }); // do it again without a state filter to make sure it isn't cached incorrectly filterWidget().contains("Location 1").click(); - popover().within(() => { - cy.get("input").first().type("An"); - cy.findByText("Adrian"); - cy.findByText("Anchorage"); - cy.findByText("Canton"); - }); + popover() + .first() + .within(() => { + multiAutocompleteInput().type("An"); + }); + + popover() + .last() + .within(() => { + cy.findByText("Adrian"); + cy.findByText("Anchorage"); + cy.findByText("Canton"); + }); }); } diff --git a/e2e/test/scenarios/dashboard-filters/dashboard-filters-auto-wiring.cy.spec.js b/e2e/test/scenarios/dashboard-filters/dashboard-filters-auto-wiring.cy.spec.js index 9b81b5ec0d9..010f0bb9197 100644 --- a/e2e/test/scenarios/dashboard-filters/dashboard-filters-auto-wiring.cy.spec.js +++ b/e2e/test/scenarios/dashboard-filters/dashboard-filters-auto-wiring.cy.spec.js @@ -23,6 +23,7 @@ import { dashboardParametersContainer, openQuestionActions, entityPickerModal, + multiAutocompleteInput, findDashCardAction, removeDashboardCard, sidebar, @@ -526,7 +527,7 @@ describe("dashboard filters auto-wiring", () => { dashboardParametersContainer().findByText("ID").click(); popover().within(() => { - cy.findByRole("textbox").type("1{enter}"); + multiAutocompleteInput().type("1,"); cy.button("Add filter").click(); }); @@ -576,7 +577,7 @@ describe("dashboard filters auto-wiring", () => { dashboardParametersContainer().findByText("ID").click(); popover().within(() => { - cy.findByRole("textbox").type("1{enter}"); + multiAutocompleteInput().type("1,"); cy.button("Add filter").click(); }); diff --git a/e2e/test/scenarios/dashboard-filters/dashboard-filters-nested.cy.spec.js b/e2e/test/scenarios/dashboard-filters/dashboard-filters-nested.cy.spec.js index 42a03ca6c3e..bb8bbc32384 100644 --- a/e2e/test/scenarios/dashboard-filters/dashboard-filters-nested.cy.spec.js +++ b/e2e/test/scenarios/dashboard-filters/dashboard-filters-nested.cy.spec.js @@ -8,6 +8,7 @@ import { visitDashboard, setFilter, getDashboardCard, + multiAutocompleteInput, } from "e2e/support/helpers"; const { PRODUCTS_ID } = SAMPLE_DATABASE; @@ -78,11 +79,7 @@ describe("scenarios > dashboard > filters > nested questions", () => { // Add multiple values (metabase#18113) filterWidget().click(); popover().within(() => { - cy.findByPlaceholderText("Enter some text") - .type("Gizmo") - .blur() - .type("Gadget") - .blur(); + multiAutocompleteInput().type("Gizmo,Gadget").blur(); }); cy.button("Add filter").click(); diff --git a/e2e/test/scenarios/dashboard-filters/dashboard-filters-text-category.cy.spec.js b/e2e/test/scenarios/dashboard-filters/dashboard-filters-text-category.cy.spec.js index e253f6b004e..bc83aac8f45 100644 --- a/e2e/test/scenarios/dashboard-filters/dashboard-filters-text-category.cy.spec.js +++ b/e2e/test/scenarios/dashboard-filters/dashboard-filters-text-category.cy.spec.js @@ -102,7 +102,7 @@ describe("scenarios > dashboard > filters > text/category", () => { waitDashboardCardQuery(); filterWidget() .eq(index) - .contains(single ? value : /\d selections/); + .contains(single ? value.replace(/"/g, "") : /\d selections/); cy.log(`Make sure ${operator} filter returns correct result`); cy.findByTestId("dashcard") diff --git a/e2e/test/scenarios/dashboard-filters/old-parameters.cy.spec.js b/e2e/test/scenarios/dashboard-filters/old-parameters.cy.spec.js index 41f7a509be5..79b66e0d96a 100644 --- a/e2e/test/scenarios/dashboard-filters/old-parameters.cy.spec.js +++ b/e2e/test/scenarios/dashboard-filters/old-parameters.cy.spec.js @@ -1,5 +1,10 @@ import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; -import { restore, popover, visitDashboard } from "e2e/support/helpers"; +import { + restore, + popover, + visitDashboard, + multiAutocompleteInput, +} from "e2e/support/helpers"; const { PEOPLE, PEOPLE_ID, PRODUCTS, PRODUCTS_ID } = SAMPLE_DATABASE; @@ -124,7 +129,7 @@ describe("scenarios > dashboard > OLD parameters", () => { // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.contains("City").click(); popover().within(() => { - cy.get("input").type("Flagstaff{enter}"); + multiAutocompleteInput().type("Flagstaff{enter}"); cy.findByText("Add filter").click(); }); diff --git a/e2e/test/scenarios/dashboard-filters/parameters.cy.spec.js b/e2e/test/scenarios/dashboard-filters/parameters.cy.spec.js index 9ad1472e636..fbb4a8ff662 100644 --- a/e2e/test/scenarios/dashboard-filters/parameters.cy.spec.js +++ b/e2e/test/scenarios/dashboard-filters/parameters.cy.spec.js @@ -18,6 +18,7 @@ import { updateDashboardCards, setFilter, spyRequestFinished, + multiAutocompleteInput, } from "e2e/support/helpers"; import { createMockParameter } from "metabase-types/api/mocks"; @@ -80,18 +81,27 @@ describe("scenarios > dashboard > parameters", () => { filterWidget().contains("Text").click(); // After typing "Ga", you should see this name - popover().find("input").type("Ga"); + popover().within(() => multiAutocompleteInput().type("Ga")); cy.wait("@dashboard"); - popover().contains("Gabrielle Considine"); + popover().last().contains("Gabrielle Considine"); // Continue typing a "d" and you see "Gadget" - popover().find("input").type("d"); + popover() + .first() + .within(() => multiAutocompleteInput().type("d")); cy.wait("@dashboard"); - popover().within(() => { - cy.findByText("Gadget").click(); - cy.button("Add filter").click(); - }); + popover() + .last() + .within(() => { + cy.findByText("Gadget").click(); + }); + + popover() + .first() + .within(() => { + cy.button("Add filter").click(); + }); cy.location("search").should("eq", "?text=Gadget"); cy.findAllByTestId("dashcard-container").first().should("contain", "0"); @@ -720,7 +730,7 @@ describe("scenarios > dashboard > parameters", () => { filterWidget().click(); popover().within(() => { - cy.findByRole("textbox").type("Antwan Fisher"); + multiAutocompleteInput().type("Antwan Fisher"); cy.button("Add filter").click(); }); @@ -745,7 +755,7 @@ describe("scenarios > dashboard > parameters", () => { filterWidget().click(); popover().within(() => { - cy.findByRole("textbox").type("Antwan Fisher"); + multiAutocompleteInput().type("Antwan Fisher"); cy.button("Add filter").click(); }); diff --git a/e2e/test/scenarios/dashboard-filters/shared/dashboard-filters-text-category.js b/e2e/test/scenarios/dashboard-filters/shared/dashboard-filters-text-category.js index b06fd31d6c7..3bb378adc38 100644 --- a/e2e/test/scenarios/dashboard-filters/shared/dashboard-filters-text-category.js +++ b/e2e/test/scenarios/dashboard-filters/shared/dashboard-filters-text-category.js @@ -21,7 +21,7 @@ export const DASHBOARD_TEXT_FILTERS = [ { operator: "Contains", single: true, - value: "oo,aa", + value: '"oo,aa"', representativeResult: "No results!", negativeAssertion: "148.23", }, @@ -34,7 +34,7 @@ export const DASHBOARD_TEXT_FILTERS = [ { operator: "Does not contain", single: true, - value: "oo,tt", + value: '"oo,tt"', representativeResult: "37.65", negativeASsertion: "39.58", }, @@ -47,7 +47,7 @@ export const DASHBOARD_TEXT_FILTERS = [ { operator: "Starts with", single: true, - value: "A,b", + value: '"A,b"', representativeResult: "No results!", negativeASsertion: "85.72", }, @@ -60,7 +60,7 @@ export const DASHBOARD_TEXT_FILTERS = [ { operator: "Ends with", single: true, - value: "e,s", + value: '"e,s"', representativeResult: "No results!", negativeASsertion: "47.68", }, diff --git a/e2e/test/scenarios/dashboard/text-cards.cy.spec.js b/e2e/test/scenarios/dashboard/text-cards.cy.spec.js index ded633f3ccb..bb85dbe4917 100644 --- a/e2e/test/scenarios/dashboard/text-cards.cy.spec.js +++ b/e2e/test/scenarios/dashboard/text-cards.cy.spec.js @@ -18,6 +18,7 @@ import { filterWidget, addTextBoxWhileEditing, addHeadingWhileEditing, + multiAutocompleteInput, } from "e2e/support/helpers"; import { createMockParameter } from "metabase-types/api/mocks"; @@ -280,7 +281,7 @@ describe("scenarios > dashboard > parameters in text and heading cards", () => { saveDashboard(); filterWidget().click(); - cy.findByPlaceholderText("Enter a number").type("1{enter}"); + popover().within(() => multiAutocompleteInput().type("1")); cy.button("Add filter").click(); getDashboardCard(0).findByText("Variable: 1").should("exist"); getDashboardCard(1).findByText("Variable: 1").should("exist"); @@ -289,7 +290,7 @@ describe("scenarios > dashboard > parameters in text and heading cards", () => { .findByText("1") .click(); popover().within(() => { - cy.findByRole("textbox").click().type("2{enter}"); + multiAutocompleteInput().type("2"); cy.button("Update filter").click(); }); getDashboardCard(0).findByText("Variable: 1 and 2").should("exist"); diff --git a/e2e/test/scenarios/dashboard/title-drill.cy.spec.js b/e2e/test/scenarios/dashboard/title-drill.cy.spec.js index 1b5c46e6dfe..1987b9811de 100644 --- a/e2e/test/scenarios/dashboard/title-drill.cy.spec.js +++ b/e2e/test/scenarios/dashboard/title-drill.cy.spec.js @@ -1,13 +1,14 @@ import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; import { - addOrUpdateDashboardCard, - appBar, filterWidget, - getDashboardCard, popover, queryBuilderMain, restore, visitDashboard, + addOrUpdateDashboardCard, + getDashboardCard, + appBar, + multiAutocompleteInput, } from "e2e/support/helpers"; const { ORDERS, ORDERS_ID, PEOPLE, PEOPLE_ID, PRODUCTS, PRODUCTS_ID } = @@ -319,7 +320,7 @@ describe("scenarios > dashboard > title drill", () => { // update the parameter filter to a new value filterWidget().contains("Doohickey").click(); popover().within(() => { - cy.get("input").type("{backspace}Gadget{enter}"); + multiAutocompleteInput().type("{backspace}Gadget,"); cy.findByText("Update filter").click(); }); @@ -339,7 +340,7 @@ describe("scenarios > dashboard > title drill", () => { // make sure the unset id parameter works filterWidget().last().click(); popover().within(() => { - cy.get("input").type("5{enter}"); + multiAutocompleteInput().type("5"); cy.findByText("Add filter").click(); }); diff --git a/e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js b/e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js index 5e01131c9f4..fcf4dc2cece 100644 --- a/e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js +++ b/e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js @@ -26,6 +26,7 @@ import { publishChanges, setEmbeddingParameter, assertEmbeddingParameter, + multiAutocompleteInput, } from "e2e/support/helpers"; import { createMockParameter } from "metabase-types/api/mocks"; @@ -107,9 +108,7 @@ describe("scenarios > embedding > dashboard parameters", () => { }); popover().within(() => { - cy.findByPlaceholderText("Search by Name or enter an ID").type( - "1{enter}3{enter}", - ); + cy.findByPlaceholderText("Search by Name or enter an ID").type("1,3,"); cy.button("Add filter").click(); }); @@ -140,7 +139,7 @@ describe("scenarios > embedding > dashboard parameters", () => { openFilterOptions("Name"); cy.findByPlaceholderText("Search by Name").type("L"); - popover().findByText("Lina Heaney").click(); + popover().last().findByText("Lina Heaney").click(); cy.button("Add filter").click(); @@ -317,20 +316,34 @@ describe("scenarios > embedding > dashboard parameters", () => { openFilterOptions("Id"); popover().within(() => { - cy.findByPlaceholderText("Search by Name or enter an ID").type("Aly"); - - cy.contains("Alycia McCullough - 2016"); + multiAutocompleteInput().type("Aly"); }); + popover().last().contains("Alycia McCullough - 2016"); + + // close the suggestions popover + popover() + .first() + .within(() => { + multiAutocompleteInput().blur(); + }); + cy.log("should allow searching PEOPLE.NAME by PEOPLE.NAME"); openFilterOptions("Name"); popover().within(() => { - cy.findByPlaceholderText("Search by Name").type("Aly"); - - cy.contains("Alycia McCullough"); + multiAutocompleteInput().type("{backspace}Aly"); }); + popover().last().contains("Alycia McCullough"); + + // close the suggestions popover + popover() + .first() + .within(() => { + multiAutocompleteInput().blur(); + }); + cy.log("should show values for PEOPLE.SOURCE"); openFilterOptions("Source"); @@ -340,11 +353,18 @@ describe("scenarios > embedding > dashboard parameters", () => { openFilterOptions("User"); popover().within(() => { - cy.findByPlaceholderText("Search by Name or enter an ID").type("Aly"); - - cy.contains("Alycia McCullough - 2016"); + multiAutocompleteInput().type("Aly"); }); + popover().last().contains("Alycia McCullough - 2016"); + + // close the suggestions popover + popover() + .first() + .within(() => { + multiAutocompleteInput().blur(); + }); + cy.log("should accept url parameters"); cy.location().then(location => diff --git a/e2e/test/scenarios/embedding/embedding-linked-filters.cy.spec.js b/e2e/test/scenarios/embedding/embedding-linked-filters.cy.spec.js index 7073ae62231..e6efb35f1f9 100644 --- a/e2e/test/scenarios/embedding/embedding-linked-filters.cy.spec.js +++ b/e2e/test/scenarios/embedding/embedding-linked-filters.cy.spec.js @@ -7,6 +7,8 @@ import { chartPathWithFillColor, echartsContainer, testPairedTooltipValues, + multiAutocompleteInput, + removeMultiAutocompleteValue, } from "e2e/support/helpers"; import { @@ -97,6 +99,12 @@ describe("scenarios > embedding > dashboard > linked filters (metabase#13639, me cy.findByText("Kiana"); cy.findByText("Anacoco").should("not.exist"); cy.findByText("Anchorage").click(); + }); + + popover() + .filter(":contains('Add filter')") + .within(() => { + multiAutocompleteInput().blur(); cy.button("Add filter").click(); }); @@ -170,6 +178,12 @@ describe("scenarios > embedding > dashboard > linked filters (metabase#13639, me cy.findByText("Kiana"); cy.findByText("Anacoco").should("not.exist"); cy.findByText("Anchorage").click(); + }); + + popover() + .filter(":contains('Add filter')") + .within(() => { + multiAutocompleteInput().blur(); cy.button("Add filter").click(); }); @@ -212,10 +226,16 @@ describe("scenarios > embedding > dashboard > linked filters (metabase#13639, me popover() .last() .within(() => { - cy.findByPlaceholderText("Search by City").type("An"); + multiAutocompleteInput().type("An"); cy.findByText("Kiana"); cy.findByText("Anacoco").should("not.exist"); cy.findByText("Anchorage").click(); + }); + + popover() + .filter(":contains('Add filter')") + .within(() => { + multiAutocompleteInput().blur(); cy.button("Add filter").click(); }); @@ -258,6 +278,12 @@ describe("scenarios > embedding > dashboard > linked filters (metabase#13639, me cy.findByText("Kiana"); cy.findByText("Anacoco").should("not.exist"); cy.findByText("Anchorage").click(); + }); + + popover() + .filter(":contains('Add filter')") + .within(() => { + multiAutocompleteInput().blur(); cy.button("Add filter").click(); }); @@ -297,6 +323,12 @@ describe("scenarios > embedding > dashboard > linked filters (metabase#13639, me cy.findByText("Kiana"); cy.findByText("Anacoco").should("not.exist"); cy.findByText("Anchorage").click(); + }); + + popover() + .filter(":contains('Add filter')") + .within(() => { + multiAutocompleteInput().blur(); cy.button("Add filter").click(); }); @@ -345,7 +377,6 @@ describe("scenarios > embedding > dashboard > linked filters (metabase#13639, me cy.findByText("Doohickey").should("not.exist"); cy.findByText("Gadget").should("not.exist"); cy.findByText("Widget").should("not.exist"); - cy.button("Add filter").click(); }); @@ -412,7 +443,8 @@ describe("scenarios > embedding > dashboard > linked filters (metabase#13639, me cy.findByText("2 selections").click(); // Remove one of the previously set filter values - popover().findByText("29").closest("li").find(".Icon-close").click(); + popover().within(() => removeMultiAutocompleteValue(1)); + cy.button("Update filter").click(); cy.findByTestId("table-row") diff --git a/e2e/test/scenarios/filters-reproductions/dashboard-filters-reproductions.cy.spec.js b/e2e/test/scenarios/filters-reproductions/dashboard-filters-reproductions.cy.spec.js index c1a5828b40d..189a630f483 100644 --- a/e2e/test/scenarios/filters-reproductions/dashboard-filters-reproductions.cy.spec.js +++ b/e2e/test/scenarios/filters-reproductions/dashboard-filters-reproductions.cy.spec.js @@ -32,6 +32,7 @@ import { openQuestionsSidebar, visitEmbeddedPage, createQuestion, + multiAutocompleteInput, assertQueryBuilderRowCount, } from "e2e/support/helpers"; import { @@ -177,7 +178,7 @@ describe("issue 8030 + 32444", () => { cy.findByText(filterDetails.name).click(); popover().within(() => { // the filter is connected only to the first card - cy.get("input").type("1{enter}"); + multiAutocompleteInput().type("1{enter}"); cy.findByText("Add filter").click(); }); cy.wait("@getCardQuery1"); @@ -546,6 +547,7 @@ describe("issues 15119 and 16112", () => { // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText(reviewerFilter.name).click(); popover().contains("adam").click(); + multiAutocompleteInput().blur(); cy.button("Add filter").click(); cy.findByTestId("dashcard-container").should("contain", "adam"); diff --git a/e2e/test/scenarios/native-filters/helpers/e2e-field-filter-data-objects.js b/e2e/test/scenarios/native-filters/helpers/e2e-field-filter-data-objects.js index 63579c793f9..399eb5019c2 100644 --- a/e2e/test/scenarios/native-filters/helpers/e2e-field-filter-data-objects.js +++ b/e2e/test/scenarios/native-filters/helpers/e2e-field-filter-data-objects.js @@ -84,7 +84,7 @@ export const DATE_FILTER_SUBTYPES = { }, "Date Filter": { value: { - timeBucket: "years", + timeBucket: "month", }, representativeResult: "Synergistic Steel Chair", }, 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 f31f4aad924..a322ae2272d 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 @@ -36,7 +36,7 @@ export function clearWidgetValue() { } export function setWidgetStringFilter(value) { - popover().find("input").first().type(`${value}{enter}`); + popover().find("input").not("[type=hidden]").first().type(`${value}{enter}`); } /** 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 2ffe4dd0286..66979af2e30 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 @@ -12,6 +12,7 @@ import { queryBuilderMain, visitDashboard, getDashboardCard, + removeMultiAutocompleteValue, } from "e2e/support/helpers"; import * as FieldFilter from "./helpers/e2e-field-filter-helpers"; @@ -1067,7 +1068,7 @@ describe("issue 31606", { tags: "@external" }, () => { }); popover().within(() => { - cy.icon("close").click(); + removeMultiAutocompleteValue(0); cy.findByText("Update filter").click(); }); cy.findByTestId("sidebar-content").within(() => { @@ -1091,7 +1092,7 @@ describe("issue 31606", { tags: "@external" }, () => { filterWidget().click(); popover().within(() => { - cy.icon("close").click(); + removeMultiAutocompleteValue(0); cy.findByText("Update filter").click(); }); 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 5822f6146cc..b24a53e844d 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 @@ -223,6 +223,9 @@ describe("scenarios > filters > sql filters > field filter > String", () => { }); it("when set as the default value for a required filter", () => { + // Use a bigger viewport to avoid elements being obscured by falling out of the screen. + cy.viewport(1280, 1400); + SQLFilter.toggleRequired(); stringFilters.forEach( 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 f6af3220d3f..71722179b89 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 @@ -5,6 +5,8 @@ import { clearFilterWidget, filterWidget, popover, + removeMultiAutocompleteValue, + multiAutocompleteInput, } from "e2e/support/helpers"; import * as FieldFilter from "./helpers/e2e-field-filter-helpers"; @@ -69,7 +71,7 @@ describe("scenarios > filters > sql filters > field filter", () => { SQLFilter.toggleRequired(); filterWidget().click(); popover().within(() => { - cy.icon("close").click(); + removeMultiAutocompleteValue(0); cy.findByText("Set to default").click(); }); filterWidget() @@ -82,7 +84,7 @@ describe("scenarios > filters > sql filters > field filter", () => { SQLFilter.toggleRequired(); filterWidget().click(); popover().within(() => { - cy.get("input").type("10{enter}"); + multiAutocompleteInput().type("10,"); cy.findByText("Update filter").click(); }); filterWidget().icon("time_history").click(); diff --git a/e2e/test/scenarios/sharing/downloads/downloads.cy.spec.js b/e2e/test/scenarios/sharing/downloads/downloads.cy.spec.js index 614fadad7a6..a4950c91cb0 100644 --- a/e2e/test/scenarios/sharing/downloads/downloads.cy.spec.js +++ b/e2e/test/scenarios/sharing/downloads/downloads.cy.spec.js @@ -29,6 +29,7 @@ import { entityPickerModalTab, showDashboardCardActions, getDashboardCard, + multiAutocompleteInput, } from "e2e/support/helpers"; const { ORDERS, ORDERS_ID } = SAMPLE_DATABASE; @@ -158,7 +159,7 @@ describe("scenarios > question > download", () => { filterWidget().contains("ID").click(); - popover().find("input").type("1"); + popover().within(() => multiAutocompleteInput().type("1")); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Add filter").click(); diff --git a/e2e/test/scenarios/sharing/sharing-reproductions.cy.spec.js b/e2e/test/scenarios/sharing/sharing-reproductions.cy.spec.js index 62a057268a9..14e6f0f34cd 100644 --- a/e2e/test/scenarios/sharing/sharing-reproductions.cy.spec.js +++ b/e2e/test/scenarios/sharing/sharing-reproductions.cy.spec.js @@ -31,6 +31,7 @@ import { dashboardHeader, filterWidget, getFullName, + multiAutocompleteInput, } from "e2e/support/helpers"; const { admin } = USERS; @@ -449,7 +450,7 @@ describeEE("issue 24223", () => { cy.findByText("Select…").click(); popover().findByText("Title").click(); cy.findByText("No default").click(); - popover().find("input").type("Awesome"); + popover().within(() => multiAutocompleteInput().type("Awesome")); popover().button("Add filter").click(); saveDashboard(); diff --git a/e2e/test/scenarios/sharing/subscriptions.cy.spec.js b/e2e/test/scenarios/sharing/subscriptions.cy.spec.js index dd6acf24468..a1ceb17cf6e 100644 --- a/e2e/test/scenarios/sharing/subscriptions.cy.spec.js +++ b/e2e/test/scenarios/sharing/subscriptions.cy.spec.js @@ -26,6 +26,8 @@ import { openEmbedModalFromMenu, getEmbedModalSharingPane, setFilter, + multiAutocompleteInput, + removeMultiAutocompleteValue, } from "e2e/support/helpers"; const { PRODUCTS, PRODUCTS_ID } = SAMPLE_DATABASE; @@ -556,14 +558,20 @@ describe("scenarios > dashboard > subscriptions", () => { .findByText("Text") .click(); cy.get("@subscriptionBar").findByText("Corbin Mertz").click(); + + popover().within(() => { + removeMultiAutocompleteValue(0); + multiAutocompleteInput().type("Sallie"); + }); + popover().last().findByText("Sallie Flatley").click(); popover() - .findByText("Corbin Mertz") - .closest("li") - .icon("close") - .click(); - popover().find("input").type("Sallie"); - popover().findByText("Sallie Flatley").click(); - popover().contains("Update filter").click(); + .first() + .within(() => { + // to close the suggestion menu + multiAutocompleteInput().blur(); + cy.button("Update filter").click(); + }); + cy.button("Save").click(); // verify existing subscription shows new default in UI @@ -652,14 +660,15 @@ describe("scenarios > dashboard > subscriptions", () => { .next("aside") .findByText("Corbin Mertz") .click(); + removeMultiAutocompleteValue(0, ":eq(1)"); + popover().within(() => multiAutocompleteInput().type("Sallie")); + popover().last().findByText("Sallie Flatley").click(); popover() - .findByText("Corbin Mertz") - .closest("li") - .icon("close") - .click(); - popover().find("input").type("Sallie"); - popover().findByText("Sallie Flatley").click(); - popover().contains("Update filter").click(); + .first() + .within(() => { + multiAutocompleteInput().blur(); + cy.button("Update filter").click(); + }); cy.button("Save").click(); // verify existing subscription shows new default in UI @@ -684,8 +693,8 @@ describe("scenarios > dashboard > subscriptions", () => { cy.findByText("Emailed hourly").click(); cy.findAllByText("Corbin Mertz").last().click(); - popover().find("input").type("Bob"); - popover().findByText("Bobby Kessler").click(); + popover().within(() => multiAutocompleteInput().type("Bob")); + popover().last().findByText("Bobby Kessler").click(); popover().contains("Update filter").click(); cy.findAllByText("Text 1").last().click(); @@ -788,9 +797,13 @@ function addParametersToDashboard() { // add default value to the above filter cy.findByText("No default").click(); - popover().find("input").type("Corbin"); - popover().findByText("Corbin Mertz").click(); - popover().contains("Add filter").click(); + popover().within(() => { + multiAutocompleteInput().type("Corbin"); + }); + + popover().last().findByText("Corbin Mertz").click(); + + popover().first().contains("Add filter").click({ force: true }); setFilter("Text or Category", "Is"); diff --git a/frontend/src/metabase/admin/datamodel/components/FilterPopover/FilterPopover.unit.spec.tsx b/frontend/src/metabase/admin/datamodel/components/FilterPopover/FilterPopover.unit.spec.tsx index 57dc5e46084..631ddc5a455 100644 --- a/frontend/src/metabase/admin/datamodel/components/FilterPopover/FilterPopover.unit.spec.tsx +++ b/frontend/src/metabase/admin/datamodel/components/FilterPopover/FilterPopover.unit.spec.tsx @@ -77,7 +77,7 @@ describe("FilterPopover", () => { it("should have an operator selector", () => { setup({ filter: NUMERIC_FILTER }); expect(screen.getByText("Equal to")).toBeInTheDocument(); - expect(screen.getByText("1,234")).toBeInTheDocument(); + expect(screen.getByText("1234")).toBeInTheDocument(); }); }); describe("filter options", () => { diff --git a/frontend/src/metabase/admin/datamodel/components/filters/pickers/DefaultPicker/DefaultPicker.tsx b/frontend/src/metabase/admin/datamodel/components/filters/pickers/DefaultPicker/DefaultPicker.tsx index 83bcc63a1b7..e64ff9bad5b 100644 --- a/frontend/src/metabase/admin/datamodel/components/filters/pickers/DefaultPicker/DefaultPicker.tsx +++ b/frontend/src/metabase/admin/datamodel/components/filters/pickers/DefaultPicker/DefaultPicker.tsx @@ -137,7 +137,6 @@ export function DefaultPicker({ return ( <FieldValuesWidget key={index} - className={CS.input} value={values} onChange={onValuesChange} multi={operator.multi} diff --git a/frontend/src/metabase/admin/datamodel/components/filters/pickers/DefaultPicker/DefaultPicker.unit.spec.tsx b/frontend/src/metabase/admin/datamodel/components/filters/pickers/DefaultPicker/DefaultPicker.unit.spec.tsx index 94e61c47c5d..273984d5b8c 100644 --- a/frontend/src/metabase/admin/datamodel/components/filters/pickers/DefaultPicker/DefaultPicker.unit.spec.tsx +++ b/frontend/src/metabase/admin/datamodel/components/filters/pickers/DefaultPicker/DefaultPicker.unit.spec.tsx @@ -6,6 +6,7 @@ import { renderWithProviders, screen, waitForLoaderToBeRemoved, + getByRole, } from "__support__/ui"; import { checkNotNull } from "metabase/lib/types"; import type StructuredQuery from "metabase-lib/v1/queries/StructuredQuery"; @@ -92,7 +93,7 @@ describe("Filters > DefaultPicker", () => { expect( await screen.findByTestId("field-values-widget"), ).toBeInTheDocument(); - expect(screen.getByRole("textbox")).toBeInTheDocument(); + expect(screen.getByRole("combobox")).toBeInTheDocument(); expect(screen.getByText("42")).toBeInTheDocument(); }); @@ -103,7 +104,7 @@ describe("Filters > DefaultPicker", () => { await screen.findByTestId("field-values-widget"), ).toBeInTheDocument(); - expect(screen.getByRole("textbox")).toBeInTheDocument(); + expect(screen.getByRole("combobox")).toBeInTheDocument(); expect(screen.getByText("Ugly Shoes")).toBeInTheDocument(); }); @@ -113,7 +114,7 @@ describe("Filters > DefaultPicker", () => { expect( await screen.findByTestId("field-values-widget"), ).toBeInTheDocument(); - expect(screen.getByRole("textbox")).toBeInTheDocument(); + expect(screen.getByRole("combobox")).toBeInTheDocument(); const productTitles = [ "Fancy Shoes", "Ugly Shoes", @@ -141,7 +142,8 @@ describe("Filters > DefaultPicker", () => { it("should update values for a multi-value filter", async () => { const { setValuesSpy } = await setup({ filter: stringQuery.filters()[0] }); - const input = screen.getByRole("textbox"); + const combobox = screen.getByRole("combobox"); + const input = getInput(combobox); await userEvent.type(input, "Fancy Sandals"); @@ -156,8 +158,8 @@ describe("Filters > DefaultPicker", () => { const { setValueSpy } = await setup({ filter: query.filters()[0] }); const input = screen.getByRole("textbox"); - await userEvent.type(input, "25"); + // index, value expect(setValueSpy).toHaveBeenLastCalledWith(0, 125); }); @@ -177,3 +179,10 @@ describe("Filters > DefaultPicker", () => { expect(onCommitSpy).toHaveBeenCalled(); }); }); + +function getInput(parent: HTMLElement) { + /* eslint-disable-next-line testing-library/prefer-screen-queries */ + const input = getByRole(parent, "searchbox"); + expect(input).toBeInTheDocument(); + return input; +} diff --git a/frontend/src/metabase/components/FieldValuesWidget/FieldValuesWidget.tsx b/frontend/src/metabase/components/FieldValuesWidget/FieldValuesWidget.tsx index 13fc133b5fd..daeeb2c47fc 100644 --- a/frontend/src/metabase/components/FieldValuesWidget/FieldValuesWidget.tsx +++ b/frontend/src/metabase/components/FieldValuesWidget/FieldValuesWidget.tsx @@ -1,6 +1,13 @@ import cx from "classnames"; import type { StyleHTMLAttributes } from "react"; -import { useState, useRef, useEffect } from "react"; +import { + useState, + useRef, + useEffect, + useCallback, + useMemo, + forwardRef, +} from "react"; import { connect } from "react-redux"; import { useMount, usePrevious, useUnmount } from "react-use"; import { jt, t } from "ttag"; @@ -15,6 +22,7 @@ import type { LayoutRendererArgs } from "metabase/components/TokenField/TokenFie import ValueComponent from "metabase/components/Value"; import CS from "metabase/css/core/index.css"; import Fields from "metabase/entities/fields"; +import { formatValue } from "metabase/lib/formatting"; import { parseNumberValue } from "metabase/lib/number"; import { defer } from "metabase/lib/promise"; import { useDispatch } from "metabase/lib/redux"; @@ -25,6 +33,8 @@ import { fetchParameterValues, } from "metabase/parameters/actions"; import { addRemappings } from "metabase/redux/metadata"; +import type { SelectItemProps } from "metabase/ui"; +import { MultiAutocomplete } from "metabase/ui"; import type Question from "metabase-lib/v1/Question"; import type Field from "metabase-lib/v1/metadata/Field"; import type { @@ -90,8 +100,8 @@ export interface IFieldValuesWidgetProps { dashboard?: Dashboard; question?: Question; - value: string[]; - onChange: (value: string[]) => void; + value: RowValue[]; + onChange: (value: RowValue[]) => void; multi?: boolean; autoFocus?: boolean; @@ -100,7 +110,7 @@ export interface IFieldValuesWidgetProps { placeholder?: string; checkedColor?: string; - valueRenderer?: (value: string | number) => JSX.Element; + valueRenderer?: (value: RowValue) => JSX.Element; optionRenderer?: (option: FieldValue) => JSX.Element; layoutRenderer?: (props: LayoutRendererArgs) => JSX.Element; } @@ -361,7 +371,7 @@ export function FieldValuesWidgetInner({ }; if (!valueRenderer) { - valueRenderer = (value: string | number) => + valueRenderer = (value: RowValue) => renderValue({ fields, formatOptions, @@ -434,6 +444,65 @@ export function FieldValuesWidgetInner({ : parseStringValue(value); }; + const shouldCreate = (value: RowValue) => { + if (typeof value === "string" || typeof value === "number") { + const res = parseFreeformValue(value); + return res !== null; + } + + return true; + }; + + const renderStringOption = useCallback( + function (option: FieldValue): { + label: string; + value: string; + } { + const value = option[0]; + const column = fields[0]; + const label = + formatValue(value, { + ...formatOptions, + column, + remap: showRemapping(fields), + jsx: false, + maximumFractionDigits: 20, + // we know it is string | number because we are passing jsx: false + })?.toString() ?? "<null>"; + + return { value: value?.toString() ?? "", label }; + }, + [fields, formatOptions], + ); + + const CustomItemComponent = useMemo( + () => + forwardRef<HTMLDivElement, SelectItemProps>(function CustomItem( + props, + ref, + ) { + const customLabel = + props.value !== undefined && + renderValue({ + fields, + formatOptions, + value: props.value, + }); + + return ( + <ItemWrapper + ref={ref} + {...props} + label={customLabel ?? (props.label || "")} + /> + ); + }), + [fields, formatOptions], + ); + + const isSimpleInput = + !multi && (!parameter || parameter.values_query_type === "none"); + return ( <ErrorBoundary> <div @@ -450,7 +519,7 @@ export function FieldValuesWidgetInner({ <ListField isDashboardFilter={!!parameter} placeholder={tokenFieldPlaceholder} - value={value?.filter((v: string) => v != null)} + value={value?.filter((v: RowValue) => v != null)} onChange={onChange} options={options} optionRenderer={optionRenderer} @@ -466,6 +535,20 @@ export function FieldValuesWidgetInner({ optionRenderer={optionRenderer} checkedColor={checkedColor} /> + ) : !isSimpleInput ? ( + <MultiAutocomplete + onSearchChange={onInputChange} + onChange={values => onChange(values.map(parseFreeformValue))} + value={value + .map(value => value?.toString()) + .filter((v): v is string => v !== null && v !== undefined)} + data={options.map(renderStringOption)} + placeholder={tokenFieldPlaceholder} + shouldCreate={shouldCreate} + autoFocus={autoFocus} + icon={prefix && <span data-testid="input-prefix">{prefix}</span>} + itemComponent={CustomItemComponent} + /> ) : ( <TokenField prefix={prefix} @@ -617,8 +700,6 @@ function renderValue({ fields, formatOptions, value, - autoLoad, - compact, }: { fields: Field[]; formatOptions: Record<string, any>; @@ -633,8 +714,16 @@ function renderValue({ maximumFractionDigits={20} remap={showRemapping(fields)} {...formatOptions} - autoLoad={autoLoad} - compact={compact} /> ); } + +export const ItemWrapper = forwardRef<HTMLDivElement, SelectItemProps>( + function ItemWrapper({ label, value, ...others }, ref) { + return ( + <div ref={ref} {...others}> + {label || value} + </div> + ); + }, +); diff --git a/frontend/src/metabase/components/FieldValuesWidget/FieldValuesWidget.unit.spec.tsx b/frontend/src/metabase/components/FieldValuesWidget/FieldValuesWidget.unit.spec.tsx index de4ecfcf7b4..398abca1079 100644 --- a/frontend/src/metabase/components/FieldValuesWidget/FieldValuesWidget.unit.spec.tsx +++ b/frontend/src/metabase/components/FieldValuesWidget/FieldValuesWidget.unit.spec.tsx @@ -1,8 +1,5 @@ -import userEvent from "@testing-library/user-event"; - import { setupFieldSearchValuesEndpoints } from "__support__/server-mocks"; import { - getBrokenUpTextMatcher, renderWithProviders, screen, waitForLoaderToBeRemoved, @@ -27,7 +24,6 @@ import { LISTABLE_PK_FIELD_VALUE, SEARCHABLE_FK_FIELD_ID, EXPRESSION_FIELD_ID, - metadataWithSearchValuesField, } from "./testMocks"; async function setup({ @@ -272,45 +268,4 @@ describe("FieldValuesWidget", () => { ); }); }); - - describe("NoMatchState", () => { - it("should display field title when one field passed and there are no matching results", async () => { - const field = metadataWithSearchValuesField.field(PEOPLE.PASSWORD); - const displayName = field?.display_name; // "Password" - const searchValue = "somerandomvalue"; - - await setup({ - fields: [field], - multi: true, - disablePKRemappingForSearch: true, - searchValue, - }); - - await userEvent.type( - screen.getByPlaceholderText(`Search by ${displayName}`), - searchValue, - ); - - expect( - await screen.findByText( - getBrokenUpTextMatcher(`No matching ${displayName} found.`), - ), - ).toBeInTheDocument(); - }); - - it("should not display field title when multiple fields passed and no matching results found", async () => { - const searchValue = "somerandomvalue"; - - await setup({ - fields: [metadata.field(PEOPLE.CITY), metadata.field(PEOPLE.NAME)], - multi: true, - disablePKRemappingForSearch: true, - searchValue, - }); - - await userEvent.type(screen.getByPlaceholderText("Search"), searchValue); - - expect(await screen.findByText(`No matching result`)).toBeInTheDocument(); - }); - }); }); diff --git a/frontend/src/metabase/components/ListField/ListField.tsx b/frontend/src/metabase/components/ListField/ListField.tsx index 4ab556b69f0..ecaeb72919a 100644 --- a/frontend/src/metabase/components/ListField/ListField.tsx +++ b/frontend/src/metabase/components/ListField/ListField.tsx @@ -9,6 +9,7 @@ import type { InputProps } from "metabase/core/components/Input"; import Input from "metabase/core/components/Input"; import { useDebouncedValue } from "metabase/hooks/use-debounced-value"; import { SEARCH_DEBOUNCE_DURATION } from "metabase/lib/constants"; +import type { RowValue } from "metabase-types/api"; import { OptionContainer, @@ -21,11 +22,13 @@ import type { ListFieldProps, Option } from "./types"; import { isValidOptionItem } from "./utils"; function createOptionsFromValuesWithoutOptions( - values: string[], + values: RowValue[], options: Option[], ): Option { const optionsMap = _.indexBy(options, "0"); - return values.filter(value => !optionsMap[value]).map(value => [value]); + return values + .filter(value => typeof value !== "string" || !optionsMap[value]) + .map(value => [value]); } export const ListField = ({ diff --git a/frontend/src/metabase/components/ListField/types.ts b/frontend/src/metabase/components/ListField/types.ts index 1ea607a49ac..8bdbbb5f59d 100644 --- a/frontend/src/metabase/components/ListField/types.ts +++ b/frontend/src/metabase/components/ListField/types.ts @@ -1,8 +1,9 @@ +import type { RowValue } from "metabase-types/api"; export type Option = any[]; export interface ListFieldProps { onChange: (value: string[]) => void; - value: string[]; + value: RowValue[]; options: Option; optionRenderer: (option: any) => JSX.Element; placeholder: string; diff --git a/frontend/src/metabase/components/SingleSelectListField/SingleSelectListField.tsx b/frontend/src/metabase/components/SingleSelectListField/SingleSelectListField.tsx index e7ae27aa243..41fa663810d 100644 --- a/frontend/src/metabase/components/SingleSelectListField/SingleSelectListField.tsx +++ b/frontend/src/metabase/components/SingleSelectListField/SingleSelectListField.tsx @@ -8,6 +8,7 @@ import type { InputProps } from "metabase/core/components/Input"; import Input from "metabase/core/components/Input"; import { useDebouncedValue } from "metabase/hooks/use-debounced-value"; import { SEARCH_DEBOUNCE_DURATION } from "metabase/lib/constants"; +import type { RowValue } from "metabase-types/api"; import { OptionContainer, @@ -20,11 +21,13 @@ import type { SingleSelectListFieldProps, Option } from "./types"; import { isValidOptionItem } from "./utils"; function createOptionsFromValuesWithoutOptions( - values: string[], + values: RowValue[], options: Option[], ): Option { const optionsMap = _.indexBy(options, "0"); - return values.filter(value => !optionsMap[value]).map(value => [value]); + return values + .filter(value => typeof value !== "string" || !optionsMap[value]) + .map(value => [value]); } const SingleSelectListField = ({ diff --git a/frontend/src/metabase/components/SingleSelectListField/types.ts b/frontend/src/metabase/components/SingleSelectListField/types.ts index 8409d251d37..27b530fa85b 100644 --- a/frontend/src/metabase/components/SingleSelectListField/types.ts +++ b/frontend/src/metabase/components/SingleSelectListField/types.ts @@ -1,8 +1,10 @@ +import type { RowValue } from "metabase-types/api"; + export type Option = any[]; export interface SingleSelectListFieldProps { - onChange: (value: string[]) => void; - value: string[]; + onChange: (value: RowValue[]) => void; + value: RowValue[]; options: Option; optionRenderer: (option: any) => JSX.Element; placeholder: string; diff --git a/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.tsx b/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.tsx index 1901b1fbc30..a6579f0b71d 100644 --- a/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.tsx +++ b/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.tsx @@ -1,8 +1,7 @@ -import { useState } from "react"; +import { useState, useMemo } from "react"; import { t } from "ttag"; import _ from "underscore"; -import TokenField from "metabase/components/TokenField"; import NumericInput from "metabase/core/components/NumericInput"; import CS from "metabase/css/core/index.css"; import { parseNumberValue } from "metabase/lib/number"; @@ -13,6 +12,7 @@ import { Footer, TokenFieldWrapper, } from "metabase/parameters/components/widgets/Widget.styled"; +import { MultiAutocomplete } from "metabase/ui"; import type { Parameter } from "metabase-types/api"; export type NumberInputWidgetProps = { @@ -58,22 +58,32 @@ export function NumberInputWidget({ } }; + function shouldCreate(value: string | number) { + const res = parseNumberValue(value); + return res !== null && res.toString() === value; + } + + const filteredUnsavedArrayValue = useMemo( + () => unsavedArrayValue.filter((x): x is number => x !== undefined), + [unsavedArrayValue], + ); + return ( <WidgetRoot className={className}> {label && <WidgetLabel>{label}</WidgetLabel>} - {arity === "n" || arity === 1 ? ( + {arity === "n" ? ( <TokenFieldWrapper> - <TokenField - multi={arity === "n"} - updateOnInputChange - autoFocus={autoFocus} - value={unsavedArrayValue} - parseFreeformValue={parseNumberValue} - onChange={newValue => { - setUnsavedArrayValue(newValue); - }} - options={[]} + <MultiAutocomplete + onChange={(values: string[]) => + setUnsavedArrayValue( + values.map(value => parseNumberValue(value) ?? undefined), + ) + } + value={filteredUnsavedArrayValue.map(value => value?.toString())} placeholder={placeholder} + shouldCreate={shouldCreate} + autoFocus={autoFocus} + data={[]} /> </TokenFieldWrapper> ) : ( @@ -113,7 +123,7 @@ export function NumberInputWidget({ ); } -function normalize(value: number[] | undefined): number[] { +function normalize(value: number[] | undefined): (number | undefined)[] { if (Array.isArray(value)) { return value; } else { diff --git a/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.unit.spec.tsx b/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.unit.spec.tsx index a995e2f3745..7753b6abd5c 100644 --- a/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.unit.spec.tsx +++ b/frontend/src/metabase/parameters/components/widgets/NumberInputWidget/NumberInputWidget.unit.spec.tsx @@ -1,4 +1,4 @@ -import { render, screen } from "@testing-library/react"; +import { render, screen, getByText, getByRole } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { createMockParameter } from "metabase-types/api/mocks"; @@ -170,18 +170,23 @@ describe("NumberInputWidget", () => { }); describe("arity of n", () => { - it("should render a token field input", () => { + it("should render a multi autocomplete input", () => { + const value = [1, 2, 3, 4]; render( <NumberInputWidget arity="n" - value={[1, 2, 3, 4]} + value={value} setValue={mockSetValue} parameter={mockParameter} />, ); - const values = screen.getAllByRole("list")[0]; - expect(values).toHaveTextContent("1234"); + const combobox = screen.getByRole("combobox"); + + for (const item of value) { + const value = getValue(combobox, item); + expect(value).toBeInTheDocument(); + } }); it("should correctly parse number inputs", async () => { @@ -194,15 +199,17 @@ describe("NumberInputWidget", () => { />, ); - const input = screen.getByRole("textbox"); - await userEvent.type(input, "foo{enter}123abc{enter}456{enter}"); + const combobox = screen.getByRole("combobox"); + const input = getInput(combobox); + await userEvent.type(input, "foo,123abc,456,", { + pointerEventsCheck: 0, + }); - const values = screen.getAllByRole("list")[0]; - expect(values).toHaveTextContent("123456"); + expect(getValue(combobox, 456)).toBeInTheDocument(); const button = screen.getByRole("button", { name: "Add filter" }); await userEvent.click(button); - expect(mockSetValue).toHaveBeenCalledWith([123, 456]); + expect(mockSetValue).toHaveBeenCalledWith([456]); }); it("should be unsettable", async () => { @@ -215,8 +222,11 @@ describe("NumberInputWidget", () => { />, ); - const input = screen.getByRole("textbox"); - await userEvent.type(input, "{backspace}{backspace}"); + const combobox = screen.getByRole("combobox"); + const input = getInput(combobox); + await userEvent.type(input, "{backspace}{backspace}", { + pointerEventsCheck: 0, + }); const button = screen.getByRole("button", { name: "Update filter" }); @@ -225,3 +235,15 @@ describe("NumberInputWidget", () => { }); }); }); + +function getValue(parent: HTMLElement, value: number) { + /* eslint-disable-next-line testing-library/prefer-screen-queries */ + return getByText(parent, value.toString()); +} + +function getInput(parent: HTMLElement) { + /* eslint-disable-next-line testing-library/prefer-screen-queries */ + const input = getByRole(parent, "searchbox"); + expect(input).toBeInTheDocument(); + return input; +} diff --git a/frontend/src/metabase/parameters/components/widgets/StringInputWidget/StringInputWidget.tsx b/frontend/src/metabase/parameters/components/widgets/StringInputWidget/StringInputWidget.tsx index cb75e15ffdf..0e79fcd0552 100644 --- a/frontend/src/metabase/parameters/components/widgets/StringInputWidget/StringInputWidget.tsx +++ b/frontend/src/metabase/parameters/components/widgets/StringInputWidget/StringInputWidget.tsx @@ -78,7 +78,7 @@ export function StringInputWidget({ function normalize(value: string[] | undefined): string[] { if (Array.isArray(value)) { - return value; + return value.filter(x => x !== undefined); } else { return []; } diff --git a/frontend/src/metabase/ui/components/inputs/MultiAutocomplete/MultiAutocomplete.tsx b/frontend/src/metabase/ui/components/inputs/MultiAutocomplete/MultiAutocomplete.tsx index ddd5ab3c09f..8893a046476 100644 --- a/frontend/src/metabase/ui/components/inputs/MultiAutocomplete/MultiAutocomplete.tsx +++ b/frontend/src/metabase/ui/components/inputs/MultiAutocomplete/MultiAutocomplete.tsx @@ -116,7 +116,7 @@ export function MultiAutocomplete({ if (newSearchValue !== "") { const values = parseValues(newSearchValue); if (values.length >= 1) { - const value = values[0] ?? newSearchValue; + const value = values[0]; if (isValid(value)) { setSelectedValues(unique([...lastSelectedValues, value])); } @@ -191,7 +191,7 @@ function getSelectItem(item: string | SelectItem): SelectItem { } if (!item.label) { - return { value: item.value, label: item.value }; + return { value: item.value, label: item.value?.toString() ?? "" }; } return item; diff --git a/frontend/src/metabase/ui/components/inputs/MultiAutocomplete/MultiAutocomplete.unit.spec.tsx b/frontend/src/metabase/ui/components/inputs/MultiAutocomplete/MultiAutocomplete.unit.spec.tsx index 6fa694c2bf6..2bcf9c65cb6 100644 --- a/frontend/src/metabase/ui/components/inputs/MultiAutocomplete/MultiAutocomplete.unit.spec.tsx +++ b/frontend/src/metabase/ui/components/inputs/MultiAutocomplete/MultiAutocomplete.unit.spec.tsx @@ -282,4 +282,17 @@ describe("MultiAutocomplete", () => { expect(onChange).toHaveBeenLastCalledWith(["כּטץ", "ףקמ"]); expect(input).toHaveValue(""); }); + + it("should be possible to customize what values get filtered", async () => { + const { input } = setup({ + data: EXAMPLE_DATA, + filter: (_query, _selected, item) => !item.label?.endsWith(")"), + }); + + await userEvent.type(input, "Ba", { + pointerEventsCheck: 0, + }); + + expect(screen.queryByText("Bar (2)")).not.toBeInTheDocument(); + }); }); diff --git a/frontend/src/metabase/ui/components/inputs/MultiAutocomplete/index.ts b/frontend/src/metabase/ui/components/inputs/MultiAutocomplete/index.ts index d55dbb8c3c3..56cd7d16c84 100644 --- a/frontend/src/metabase/ui/components/inputs/MultiAutocomplete/index.ts +++ b/frontend/src/metabase/ui/components/inputs/MultiAutocomplete/index.ts @@ -1 +1,3 @@ +export type { SelectItemProps } from "@mantine/core"; + export * from "./MultiAutocomplete"; diff --git a/frontend/src/metabase/ui/components/inputs/MultiAutocomplete/utils.ts b/frontend/src/metabase/ui/components/inputs/MultiAutocomplete/utils.ts index e5c800d0278..1982484f578 100644 --- a/frontend/src/metabase/ui/components/inputs/MultiAutocomplete/utils.ts +++ b/frontend/src/metabase/ui/components/inputs/MultiAutocomplete/utils.ts @@ -2,7 +2,7 @@ import { parse } from "csv-parse/browser/esm/sync"; export function parseValues(str: string): string[] { try { - return parse(str, { + const strings = parse(str, { delimiter: [",", "\t", "\n"], skip_empty_lines: true, relax_column_count: true, @@ -11,6 +11,8 @@ export function parseValues(str: string): string[] { quote: '"', escape: "\\", }).flat(); + + return strings; } catch (err) { return []; } diff --git a/frontend/src/metabase/ui/components/inputs/MultiSelect/MultiSelect.styled.tsx b/frontend/src/metabase/ui/components/inputs/MultiSelect/MultiSelect.styled.tsx index 0599219a48d..b3ee3ccfaa9 100644 --- a/frontend/src/metabase/ui/components/inputs/MultiSelect/MultiSelect.styled.tsx +++ b/frontend/src/metabase/ui/components/inputs/MultiSelect/MultiSelect.styled.tsx @@ -48,10 +48,19 @@ export const getMultiSelectOverrides = gap: theme.spacing.xs, padding: theme.spacing.xs, alignItems: "center", + "[data-with-icon=true] &": { + paddingLeft: 0, + }, }, input: { padding: 0, boxSizing: "border-box", + "&[data-with-icon]": { + paddingLeft: theme.spacing.lg, + }, + }, + icon: { + width: theme.spacing.lg, }, value: { margin: 0, @@ -68,6 +77,9 @@ export const getMultiSelectOverrides = "&::-webkit-search-cancel-button": { display: "none", }, + "[data-with-icon=true] &:first-child": { + marginLeft: 0, + }, }, defaultValue: { padding: 0, -- GitLab