backported "Use list picker when searching a single value" (#49455)

* Use list picker when searching a single value (#49323)

* WIP: use list picker when searching a single value

* Add test case for multi vs non-multi search fields

* Update loki snapshots

* Add special case to test for multiautocomplete filter

* Use single select value in test

* Add test for using list picker

* Explicitly make sure elements are visible

* Do not use list selector when search is disabled

* Add margin to multi autocomplete dropdown

* Update loki snapshots

* Update loki snapshots

* Update Loki Snapshots

---------

Co-authored-by: Romeo Van Snick <romeo@romeovansnick.be>

e2e/test/scenarios/dashboard-filters/dashboard-chained-filters.cy.spec.js

@@ -173,7 +173,12 @@ describe("scenarios > dashboard > chained filter", () => {
       popover()
         .first()
         .within(() => {
-          multiAutocompleteInput().type("An");
+          if (has_field_values === "search") {
+            multiAutocompleteInput().type("An");
+          }
+          if (has_field_values === "list") {
+            cy.findByRole("textbox").type("An");
+          }
         });
 
       popover()

e2e/test/scenarios/dashboard-filters/dashboard-filters-text-category.cy.spec.js

@@ -13,6 +13,7 @@ import {
   saveDashboard,
   selectDashboardFilter,
   setFilter,
+  sidebar,
   toggleFilterWidgetValues,
   toggleRequiredParameter,
   visitDashboard,
@@ -232,7 +233,47 @@ describe("scenarios > dashboard > filters > text/category", () => {
     toggleFilterWidgetValues(["Twitter"], {
       buttonLabel: "Set to default",
     });
-    filterWidget().findByText("Twitter");
+    filterWidget().findByText("Twitter").should("be.visible");
+  });
+
+  it("should use the list value picker for single-value category filters (metabase#49323)", () => {
+    setFilter("Text or Category", "Is");
+
+    selectDashboardFilter(cy.findByTestId("dashcard"), "Title");
+
+    sidebar().findByText("A single value").click();
+    saveDashboard();
+
+    waitDashboardCardQuery();
+
+    filterWidget().contains("Text").click();
+    popover().within(() => {
+      cy.findByRole("combobox").should("not.exist");
+      cy.findByText("Aerodynamic Concrete Bench").should("be.visible").click();
+      cy.findByText("Aerodynamic Bronze Hat").should("be.visible").click();
+      cy.button("Add filter").click();
+    });
+    filterWidget().findByText("Aerodynamic Bronze Hat").should("be.visible");
+  });
+
+  it("should use the list value picker for multi-value category filters (metabase#49323)", () => {
+    setFilter("Text or Category", "Is");
+
+    selectDashboardFilter(cy.findByTestId("dashcard"), "Title");
+
+    sidebar().findByText("Multiple values").click();
+    saveDashboard();
+
+    waitDashboardCardQuery();
+
+    filterWidget().contains("Text").click();
+    popover().within(() => {
+      cy.findByRole("combobox").should("not.exist");
+      cy.findByText("Aerodynamic Concrete Bench").should("be.visible").click();
+      cy.findByText("Aerodynamic Bronze Hat").should("be.visible").click();
+      cy.button("Add filter").click();
+    });
+    filterWidget().findByText("2 selections").should("be.visible");
   });
 });
 

e2e/test/scenarios/dashboard-filters/parameters.cy.spec.js

@@ -88,14 +88,14 @@ describe("scenarios > dashboard > parameters", () => {
     filterWidget().contains("Text").click();
 
     // After typing "Ga", you should see this name!
-    popover().within(() => multiAutocompleteInput().type("Ga"));
+    popover().within(() => cy.findByPlaceholderText("Search").type("Ga"));
     cy.wait("@dashboard");
     popover().last().contains("Gabrielle Considine");
 
     // Continue typing a "d" and you see "Gadget"
     popover()
       .first()
-      .within(() => multiAutocompleteInput().type("d"));
+      .within(() => cy.findByPlaceholderText("Search").type("d"));
     cy.wait("@dashboard");
 
     popover()

e2e/test/scenarios/filters-reproductions/dashboard-filters-reproductions.cy.spec.js

@@ -192,7 +192,7 @@ describe("issue 8030 + 32444", () => {
             popover().within(() => {
               // the filter is connected only to the first card
               multiAutocompleteInput().type("1{enter}");
-              cy.findByText("Add filter").click();
+              cy.button("Add filter").click();
             });
             cy.wait("@getCardQuery1");
             cy.get("@getCardQuery1.all").should("have.length", 2);
@@ -587,7 +587,6 @@ 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");

frontend/src/metabase/components/FieldValuesWidget/FieldValuesWidget.tsx

@@ -163,10 +163,16 @@ export function FieldValuesWidgetInner({
   const [isExpanded, setIsExpanded] = useState(false);
   const dispatch = useDispatch();
 
+  const isSingleValueSearch =
+    valuesMode === "search" && !multi && !disableSearch;
+  const isListMode =
+    !disableList &&
+    (shouldList({ parameter, fields, disableSearch }) || isSingleValueSearch);
+
   const previousWidth = usePrevious(width);
 
   useMount(() => {
-    if (shouldList({ parameter, fields, disableSearch })) {
+    if (isListMode) {
       fetchValues();
     }
   });
@@ -479,17 +485,15 @@ export function FieldValuesWidgetInner({
     valuesMode,
   });
 
-  const isListMode =
-    !disableList &&
-    shouldList({ parameter, fields, disableSearch }) &&
-    valuesMode === "list";
   const isLoading = loadingState !== "LOADED";
-  const hasListValues = hasList({
-    parameter,
-    fields,
-    disableSearch,
-    options,
-  });
+  const hasListValues =
+    hasList({
+      parameter,
+      fields,
+      disableSearch,
+      options,
+    }) ||
+    (isSingleValueSearch && options.length > 0);
 
   const valueForLabel = (label: string | number) => {
     const value = fieldValues.byLabel.get(label?.toString());
@@ -605,20 +609,22 @@ export function FieldValuesWidgetInner({
             checkedColor={checkedColor}
           />
         ) : !isSimpleInput ? (
-          <MultiAutocomplete
-            data-testid="field-values-multi-autocomplete"
-            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.concat(valueOptions).map(renderStringOption)}
-            placeholder={tokenFieldPlaceholder}
-            shouldCreate={shouldCreate}
-            autoFocus={autoFocus}
-            icon={prefix && <span data-testid="input-prefix">{prefix}</span>}
-            itemComponent={CustomItemComponent}
-          />
+          <Box pr="1rem">
+            <MultiAutocomplete
+              data-testid="field-values-multi-autocomplete"
+              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.concat(valueOptions).map(renderStringOption)}
+              placeholder={tokenFieldPlaceholder}
+              shouldCreate={shouldCreate}
+              autoFocus={autoFocus}
+              icon={prefix && <span data-testid="input-prefix">{prefix}</span>}
+              itemComponent={CustomItemComponent}
+            />
+          </Box>
         ) : (
           <TokenField
             prefix={prefix}

frontend/src/metabase/components/FieldValuesWidget/FieldValuesWidget.unit.spec.tsx

@@ -132,11 +132,24 @@ describe("FieldValuesWidget", () => {
     describe("has_field_values = search", () => {
       const field = metadata.field(PEOPLE.EMAIL);
 
-      it("should not call fetchFieldValues", async () => {
-        const { fetchFieldValues } = await setup({
-          fields: [field],
+      describe("multi = true", () => {
+        it("should not call fetchFieldValues", async () => {
+          const { fetchFieldValues } = await setup({
+            fields: [field],
+            multi: true,
+          });
+          expect(fetchFieldValues).not.toHaveBeenCalled();
+        });
+      });
+
+      describe("multi = false", () => {
+        it("should call fetchFieldValues", async () => {
+          const { fetchFieldValues } = await setup({
+            fields: [field],
+            multi: false,
+          });
+          expect(fetchFieldValues).toHaveBeenCalledWith(field);
         });
-        expect(fetchFieldValues).not.toHaveBeenCalled();
       });
 
       it("should have 'Search by Vendor' as the placeholder text", async () => {