🤖 backported "Large category picker in bulk filter modal"

frontend/src/metabase/query_builder/components/filters/modals/InlineCategoryPicker/InlineCategoryPicker.styled.tsx

@@ -1,6 +1,9 @@
 import styled from "@emotion/styled";
 import { color } from "metabase/lib/colors";
-import { space } from "metabase/styled-components/theme";
+import {
+  space,
+  breakpointMinHeightMedium,
+} from "metabase/styled-components/theme";
 
 import LoadingSpinner from "metabase/components/LoadingSpinner";
 
@@ -19,3 +22,32 @@ export const PickerGrid = styled.div`
   align-items: center;
   gap: ${space(2)};
 `;
+
+export const TokenFieldContainer = styled.div`
+  display: flex;
+  flex-wrap: wrap;
+  padding: ${space(0)};
+  gap: ${space(0)};
+  font-weight: bold;
+  cursor: pointer;
+
+  max-height: 200px;
+  overflow-y: auto;
+
+  border-radius: ${space(1)};
+  border: 1px solid ${color("border-dark")};
+`;
+
+export const AddText = styled.div`
+  min-height: 30px;
+
+  ${breakpointMinHeightMedium} {
+    height: 46px;
+  }
+  margin-left: ${space(1)};
+  border: none;
+  display: flex;
+  align-items: center;
+
+  color: ${color("text-light")};
+`;

frontend/src/metabase/query_builder/components/filters/modals/InlineCategoryPicker/InlineCategoryPicker.tsx

@@ -2,24 +2,23 @@ import React, { useState, useEffect } from "react";
 import { connect } from "react-redux";
 import { t } from "ttag";
 
-import Filter from "metabase-lib/lib/queries/structured/Filter";
+import type Filter from "metabase-lib/lib/queries/structured/Filter";
 import Fields from "metabase/entities/fields";
 import StructuredQuery from "metabase-lib/lib/queries/StructuredQuery";
 import Dimension from "metabase-lib/lib/Dimension";
 import { useSafeAsyncFunction } from "metabase/hooks/use-safe-async-function";
 
 import Warnings from "metabase/query_builder/components/Warnings";
-import Checkbox from "metabase/core/components/CheckBox";
 
-import { BulkFilterSelect } from "../BulkFilterSelect";
 import { InlineValuePicker } from "../InlineValuePicker";
 
 import { MAX_INLINE_CATEGORIES } from "./constants";
-import {
-  PickerContainer,
-  PickerGrid,
-  Loading,
-} from "./InlineCategoryPicker.styled";
+import { isValidOption } from "./utils";
+
+import { SimpleCategoryFilterPicker } from "./SimpleCategoryFilterPicker";
+import { LargeCategoryFilterPicker } from "./LargeCategoryFilterPicker";
+
+import { Loading } from "./InlineCategoryPicker.styled";
 
 const mapStateToProps = (state: any, props: any) => {
   const fieldId = props.dimension?.field?.()?.id;
@@ -83,8 +82,9 @@ export function InlineCategoryPickerComponent({
       });
   }, [dimension, safeFetchFieldValues, shouldFetchFieldValues]);
 
-  const hasCheckboxOperator =
-    !filter || ["=", "!="].includes(filter?.operatorName());
+  const hasCheckboxOperator = ["=", "!="].includes(
+    (filter ?? newFilter)?.operatorName(),
+  );
 
   const hasValidOptions = fieldValues.flat().find(isValidOption);
 
@@ -121,12 +121,12 @@ export function InlineCategoryPickerComponent({
 
   if (showPopoverPicker) {
     return (
-      <BulkFilterSelect
+      <LargeCategoryFilterPicker
         query={query}
-        filter={filter}
+        filter={filter ?? newFilter}
         dimension={dimension}
-        handleChange={onChange}
-        handleClear={onClear}
+        onChange={onChange}
+        onClear={onClear}
       />
     );
   }
@@ -140,45 +140,6 @@ export function InlineCategoryPickerComponent({
   );
 }
 
-interface SimpleCategoryFilterPickerProps {
-  filter: Filter;
-  options: (string | number)[];
-  onChange: (newFilter: Filter) => void;
-}
-
-export function SimpleCategoryFilterPicker({
-  filter,
-  options,
-  onChange,
-}: SimpleCategoryFilterPickerProps) {
-  const filterValues = filter.arguments().filter(isValidOption);
-
-  const handleChange = (option: string | number, checked: boolean) => {
-    const newArgs = checked
-      ? [...filterValues, option]
-      : filterValues.filter(filterValue => filterValue !== option);
-
-    onChange(filter.setArguments(newArgs));
-  };
-
-  return (
-    <PickerContainer data-testid="category-picker">
-      <PickerGrid>
-        {options.map((option: string | number) => (
-          <Checkbox
-            key={option?.toString() ?? "empty"}
-            checked={filterValues.includes(option)}
-            onChange={e => handleChange(option, e.target.checked)}
-            label={option?.toString() ?? t`empty`}
-          />
-        ))}
-      </PickerGrid>
-    </PickerContainer>
-  );
-}
-
-const isValidOption = (option: any) => option !== undefined && option !== null;
-
 export const InlineCategoryPicker = connect(
   mapStateToProps,
   mapDispatchToProps,

frontend/src/metabase/query_builder/components/filters/modals/InlineCategoryPicker/InlineCategoryPicker.unit.spec.tsx

@@ -69,12 +69,29 @@ const emptyCategoryField = new Field({
   metadata,
 });
 
+const nullCategoryField = new Field({
+  database_type: "test",
+  semantic_type: "type/Category",
+  effective_type: "type/Text",
+  base_type: "type/Text",
+  table_id: 8,
+  name: "null_category_field",
+  has_field_values: "list",
+  values: [[null], [undefined]],
+  dimensions: {},
+  dimension_options: [],
+  id: 140,
+  metadata,
+});
+
 // @ts-ignore
 metadata.fields[smallCategoryField.id] = smallCategoryField;
 // @ts-ignore
 metadata.fields[largeCategoryField.id] = largeCategoryField;
 // @ts-ignore
 metadata.fields[emptyCategoryField.id] = emptyCategoryField;
+// @ts-ignore
+metadata.fields[nullCategoryField.id] = nullCategoryField;
 
 const card = {
   dataset_query: {
@@ -93,8 +110,13 @@ const query = question.query() as StructuredQuery;
 const smallDimension = smallCategoryField.dimension();
 const largeDimension = largeCategoryField.dimension();
 const emptyDimension = emptyCategoryField.dimension();
+const nullDimension = nullCategoryField.dimension();
 
 describe("InlineCategoryPicker", () => {
+  beforeEach(() => {
+    console.error = jest.fn();
+    console.warn = jest.fn();
+  });
   it("should render an inline category picker", () => {
     const testFilter = new Filter(
       ["=", ["field", smallCategoryField.id, null], undefined],
@@ -224,8 +246,8 @@ describe("InlineCategoryPicker", () => {
     );
 
     expect(screen.queryByTestId("category-picker")).not.toBeInTheDocument();
-    // should render general purpose picker instead
-    screen.getByTestId("select-button");
+    // should render large option picker
+    expect(screen.getByTestId("large-category-picker")).toBeInTheDocument();
   });
 
   it("should load existing filter selections", () => {
@@ -340,7 +362,7 @@ describe("InlineCategoryPicker", () => {
     expect(fetchSpy).not.toHaveBeenCalled();
   });
 
-  it("should fall back to a bulk (popover) picker if there are many options", () => {
+  it("should render a large category picker if there are many options", () => {
     const testFilter = new Filter(
       ["=", ["field", largeCategoryField.id, null], undefined],
       null,
@@ -363,7 +385,62 @@ describe("InlineCategoryPicker", () => {
     );
 
     expect(screen.queryByTestId("category-picker")).not.toBeInTheDocument();
-    expect(screen.queryByTestId("select-button")).toBeInTheDocument();
+    expect(screen.getByTestId("large-category-picker")).toBeInTheDocument();
+  });
+
+  it("should render a large category picker for no valid options", () => {
+    // the small category picker would just render no checkboxes which looks funny
+    const testFilter = new Filter(
+      ["=", ["field", nullCategoryField.id, null], undefined],
+      null,
+      query,
+    );
+    const changeSpy = jest.fn();
+    const fetchSpy = jest.fn();
+
+    render(
+      <InlineCategoryPickerComponent
+        query={query}
+        filter={testFilter}
+        newFilter={testFilter}
+        onChange={changeSpy}
+        fieldValues={nullCategoryField.values}
+        fetchFieldValues={fetchSpy}
+        dimension={nullDimension}
+        onClear={changeSpy}
+      />,
+    );
+
+    expect(screen.queryByTestId("category-picker")).not.toBeInTheDocument();
+    expect(screen.getByTestId("large-category-picker")).toBeInTheDocument();
+  });
+
+  it("should show field options inline for category fields with many options", () => {
+    const testFilter = new Filter(
+      ["=", ["field", largeCategoryField.id, null], "Raphael 2", "Donatello 3"],
+      null,
+      query,
+    );
+    const changeSpy = jest.fn();
+    const fetchSpy = jest.fn();
+
+    render(
+      <InlineCategoryPickerComponent
+        query={query}
+        filter={testFilter}
+        newFilter={testFilter}
+        onChange={changeSpy}
+        fieldValues={largeCategoryField.values}
+        fetchFieldValues={fetchSpy}
+        dimension={largeDimension}
+        onClear={changeSpy}
+      />,
+    );
+
+    expect(screen.queryByTestId("category-picker")).not.toBeInTheDocument();
+    expect(screen.getByTestId("large-category-picker")).toBeInTheDocument();
+    expect(screen.getByText("Raphael 2")).toBeInTheDocument();
+    expect(screen.getByText("Donatello 3")).toBeInTheDocument();
   });
 
   const fieldSizes = [

frontend/src/metabase/query_builder/components/filters/modals/InlineCategoryPicker/LargeCategoryFilterPicker.tsx

@@ -0,0 +1,80 @@
+import React from "react";
+import { t } from "ttag";
+
+import type Filter from "metabase-lib/lib/queries/structured/Filter";
+import type StructuredQuery from "metabase-lib/lib/queries/StructuredQuery";
+import type Dimension from "metabase-lib/lib/Dimension";
+import { pluralize } from "metabase/lib/formatting";
+
+import Icon from "metabase/components/Icon";
+
+import { BulkFilterSelect } from "../BulkFilterSelect";
+import { TokenFieldContainer, AddText } from "./InlineCategoryPicker.styled";
+
+import {
+  TokenFieldItem,
+  TokenFieldAddon,
+} from "metabase/components/TokenFieldItem";
+
+import { isValidOption } from "./utils";
+
+interface LargeCategoryFilterPickerProps {
+  query: StructuredQuery;
+  filter: Filter;
+  dimension: Dimension;
+  onChange: (newFilter: Filter) => void;
+  onClear: () => void;
+}
+
+export function LargeCategoryFilterPicker({
+  query,
+  filter,
+  dimension,
+  onChange,
+  onClear,
+}: LargeCategoryFilterPickerProps) {
+  const filterValues = filter.arguments().filter(isValidOption);
+
+  const removeValue = (value: string | number) =>
+    onChange(
+      filter.setArguments(
+        filterValues.filter(filterValue => filterValue !== value),
+      ),
+    );
+
+  const preventDefault = (e: React.SyntheticEvent) => e.preventDefault();
+
+  return (
+    <BulkFilterSelect
+      query={query}
+      filter={filter}
+      dimension={dimension}
+      handleChange={onChange}
+      handleClear={onClear}
+      customTrigger={({ onClick }) => (
+        <TokenFieldContainer
+          data-testid="large-category-picker"
+          onClick={onClick}
+        >
+          {filterValues.map(filterValue => (
+            <TokenFieldItem key={filterValue} isValid onClick={preventDefault}>
+              <span>{filterValue}</span>
+              <TokenFieldAddon
+                isValid
+                onClick={e => {
+                  e.stopPropagation();
+                  removeValue(filterValue);
+                }}
+              >
+                <Icon name="close" className="flex align-center" size={12} />
+              </TokenFieldAddon>
+            </TokenFieldItem>
+          ))}
+          <AddText data-testid="select-filter-option">
+            {t`Select ${pluralize(dimension.displayName().toLowerCase())}...`}
+          </AddText>
+        </TokenFieldContainer>
+      )}
+    />
+  );
+}