diff --git a/frontend/src/metabase/query_builder/components/view/ViewHeader/ViewTitleHeader.module.css b/frontend/src/metabase/query_builder/components/view/ViewHeader/ViewTitleHeader.module.css index c41649197858c0620abe42bb8da220bac1efc64b..ccec02980f46cdd98fac9646fcfe3d884256b70d 100644 --- a/frontend/src/metabase/query_builder/components/view/ViewHeader/ViewTitleHeader.module.css +++ b/frontend/src/metabase/query_builder/components/view/ViewHeader/ViewTitleHeader.module.css @@ -1,13 +1,57 @@ .FilterButton { - transition: background 300ms linear, border 300ms linear; + transition: background 300ms linear; - &:hover:not([data-css-specifity-hack="🤣"]) { + &:hover:not([data-css-specificity-hack="🤣"]) { color: var(--mb-color-filter); - border-color: color-mix(in srgb, var(--mb-color-filter) 30%, white); background-color: color-mix(in srgb, var(--mb-color-filter) 10%, white); } } +.FilterButtonAttachment { + &:not([data-css-specificity-hack="🤣"]) { + padding: 0.5rem; + } + + transition: background 300ms linear; + + &:hover:not([data-css-specificity-hack="🤣"]) { + color: var(--mb-color-filter); + background-color: color-mix(in srgb, var(--mb-color-filter) 10%, white); + + .FilterCountChip { + background-color: var(--mb-color-filter); + } + } + + &[data-expanded="true"] { + background-color: var(--mb-color-filter); + + .FilterCountChip { + background-color: var(--mb-color-bg-white); + color: var(--mb-color-text-dark); + } + + &:hover { + background-color: color-mix(in srgb, var(--mb-color-filter) 80%, white); + + .FilterCountChip { + background-color: var(--mb-color-bg-white); + color: var(--mb-color-text-dark); + } + } + } +} + +.FilterCountChip { + transition: background-color 300ms linear; + background-color: var(--mb-color-text-dark); + font-size: 0.6875rem; + border-radius: 10px; + line-height: 1rem; + padding-inline: 0.5rem; + color: var(--mb-color-text-white); +} + .SummarizeButton { transition: background 300ms linear, border 300ms linear; diff --git a/frontend/src/metabase/query_builder/components/view/ViewHeader/components/FilterHeaderButton.tsx b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/FilterHeaderButton.tsx index b070362aa52eabcbbc2583529b7025219fa1dacd..e445eb802c4ff8742dc7eeb80e411adac92f191c 100644 --- a/frontend/src/metabase/query_builder/components/view/ViewHeader/components/FilterHeaderButton.tsx +++ b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/FilterHeaderButton.tsx @@ -1,8 +1,10 @@ import cx from "classnames"; +import { useMemo } from "react"; import { t } from "ttag"; import type { QueryModalType } from "metabase/query_builder/constants"; import { MODAL_TYPES } from "metabase/query_builder/constants"; +import { getFilterItems } from "metabase/querying/filters/components/FilterPanel/utils"; import { Button } from "metabase/ui"; import * as Lib from "metabase-lib"; import type Question from "metabase-lib/v1/Question"; @@ -13,21 +15,52 @@ import ViewTitleHeaderS from "../ViewTitleHeader.module.css"; interface FilterHeaderButtonProps { className?: string; onOpenModal: (modalType: QueryModalType) => void; + query?: Lib.Query; + isExpanded?: boolean; + onExpand?: () => void; + onCollapse?: () => void; } export function FilterHeaderButton({ className, onOpenModal, + query, + isExpanded, + onExpand, + onCollapse, }: FilterHeaderButtonProps) { + const label = isExpanded ? t`Hide filters` : t`Show filters`; + const items = useMemo(() => query && getFilterItems(query), [query]); + + const shouldShowFilterPanelExpander = Boolean( + items?.length && onExpand && onCollapse, + ); + return ( - <Button - color="filter" - className={cx(className, ViewTitleHeaderS.FilterButton)} - onClick={() => onOpenModal(MODAL_TYPES.FILTERS)} - data-testid="question-filter-header" - > - {t`Filter`} - </Button> + <Button.Group> + <Button + color="filter" + className={cx(className, ViewTitleHeaderS.FilterButton)} + onClick={() => onOpenModal(MODAL_TYPES.FILTERS)} + data-testid="question-filter-header" + > + {t`Filter`} + </Button> + {shouldShowFilterPanelExpander && ( + <Button + aria-label={label} + className={ViewTitleHeaderS.FilterButtonAttachment} + onClick={isExpanded ? onCollapse : onExpand} + data-testid="filters-visibility-control" + data-expanded={isExpanded} + style={{ borderLeft: "none" }} // mantine puts a double border between buttons in groups + > + <div className={ViewTitleHeaderS.FilterCountChip}> + {items?.length} + </div> + </Button> + )} + </Button.Group> ); } diff --git a/frontend/src/metabase/query_builder/components/view/ViewHeader/components/FilterHeaderButton.unit.spec.tsx b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/FilterHeaderButton.unit.spec.tsx new file mode 100644 index 0000000000000000000000000000000000000000..4dc55df8c5d6e237cd241c7dbd7a4111fc32838b --- /dev/null +++ b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/FilterHeaderButton.unit.spec.tsx @@ -0,0 +1,150 @@ +import { createMockMetadata } from "__support__/metadata"; +import { renderWithProviders, screen } from "__support__/ui"; +import Question from "metabase-lib/v1/Question"; +import { + ORDERS, + ORDERS_ID, + PRODUCTS, + SAMPLE_DB_ID, + createSampleDatabase, +} from "metabase-types/api/mocks/presets"; + +import { FilterHeaderButton } from "./FilterHeaderButton"; + +const metadata = createMockMetadata({ + databases: [createSampleDatabase()], +}); + +const QUERY_WITH_FILTERS = Question.create({ + databaseId: SAMPLE_DB_ID, + tableId: ORDERS_ID, + metadata, +}) + .legacyQuery({ useStructuredQuery: true }) + .aggregate(["count"]) + .filter(["time-interval", ["field", ORDERS.CREATED_AT, null], -30, "day"]) + .filter(["=", ["field", ORDERS.TOTAL, null], 1234]) + .filter([ + "contains", + ["field", PRODUCTS.TITLE, { "source-field": ORDERS.PRODUCT_ID }], + "asdf", + ]) + .question() + .query(); + +const QUERY_WITHOUT_FILTERS = Question.create({ + databaseId: SAMPLE_DB_ID, + tableId: ORDERS_ID, + metadata, +}) + .legacyQuery({ useStructuredQuery: true }) + .aggregate(["count"]) + .question() + .query(); + +describe("FilterHeaderButton", () => { + it("should render filter button", () => { + renderWithProviders(<FilterHeaderButton onOpenModal={jest.fn()} />); + + expect(screen.getByRole("button", { name: "Filter" })).toBeInTheDocument(); + expect(screen.getByTestId("question-filter-header")).toBeInTheDocument(); + }); + + it("should not render filter count without a query", () => { + renderWithProviders(<FilterHeaderButton onOpenModal={jest.fn()} />); + + expect( + screen.queryByTestId("filters-visibility-control"), + ).not.toBeInTheDocument(); + }); + + it("should render filter count when a query has filters", () => { + renderWithProviders( + <FilterHeaderButton + onOpenModal={jest.fn()} + query={QUERY_WITH_FILTERS} + isExpanded={false} + onExpand={jest.fn()} + onCollapse={jest.fn()} + />, + ); + expect(screen.getByTestId("filters-visibility-control")).toHaveTextContent( + "3", + ); + }); + + it("should not render filter count when a query has 0 filters", () => { + renderWithProviders( + <FilterHeaderButton + onOpenModal={jest.fn()} + query={QUERY_WITHOUT_FILTERS} + isExpanded={false} + onExpand={jest.fn()} + onCollapse={jest.fn()} + />, + ); + expect( + screen.queryByTestId("filters-visibility-control"), + ).not.toBeInTheDocument(); + }); + + it("should not render filter count without onCollapse function", () => { + renderWithProviders( + <FilterHeaderButton + onOpenModal={jest.fn()} + query={QUERY_WITH_FILTERS} + isExpanded={false} + onExpand={jest.fn()} + />, + ); + expect( + screen.queryByTestId("filters-visibility-control"), + ).not.toBeInTheDocument(); + }); + + it("should not render filter count without onExpand function", () => { + renderWithProviders( + <FilterHeaderButton + onOpenModal={jest.fn()} + query={QUERY_WITH_FILTERS} + isExpanded={false} + onCollapse={jest.fn()} + />, + ); + expect( + screen.queryByTestId("filters-visibility-control"), + ).not.toBeInTheDocument(); + }); + + it("should populate true data-expanded property", () => { + renderWithProviders( + <FilterHeaderButton + onOpenModal={jest.fn()} + query={QUERY_WITH_FILTERS} + isExpanded={true} + onExpand={jest.fn()} + onCollapse={jest.fn()} + />, + ); + expect(screen.getByTestId("filters-visibility-control")).toHaveAttribute( + "data-expanded", + "true", + ); + }); + + it("should populate false data-expanded property", () => { + renderWithProviders( + <FilterHeaderButton + onOpenModal={jest.fn()} + query={QUERY_WITH_FILTERS} + isExpanded={false} + onExpand={jest.fn()} + onCollapse={jest.fn()} + />, + ); + expect(screen.getByTestId("filters-visibility-control")).toHaveAttribute( + "data-expanded", + "false", + ); + }); +}); diff --git a/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionFiltersHeader/QuestionFiltersHeader.tsx b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionFiltersHeader/QuestionFiltersHeader.tsx index a27f9319602b156b3046f0fd801c819987e85c5d..d91c5ec0f69eeee2423e1dc493db9b7cc4086268 100644 --- a/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionFiltersHeader/QuestionFiltersHeader.tsx +++ b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionFiltersHeader/QuestionFiltersHeader.tsx @@ -1,36 +1,8 @@ import { FilterPanel } from "metabase/querying/filters/components/FilterPanel"; -import { FilterPanelButton } from "metabase/querying/filters/components/FilterPanel/FilterPanelButton"; import * as Lib from "metabase-lib"; import type Question from "metabase-lib/v1/Question"; import type { QueryBuilderMode } from "metabase-types/store"; -interface FilterHeaderToggleProps { - className?: string; - query: Lib.Query; - isExpanded: boolean; - onExpand: () => void; - onCollapse: () => void; -} - -export function QuestionFiltersHeaderToggle({ - className, - query, - isExpanded, - onExpand, - onCollapse, -}: FilterHeaderToggleProps) { - return ( - <div className={className}> - <FilterPanelButton - query={query} - isExpanded={isExpanded} - onExpand={onExpand} - onCollapse={onCollapse} - /> - </div> - ); -} - interface FilterHeaderProps { question: Question; expanded: boolean; @@ -77,4 +49,3 @@ const shouldRender = ({ }; QuestionFiltersHeader.shouldRender = shouldRender; -QuestionFiltersHeaderToggle.shouldRender = shouldRender; diff --git a/frontend/src/metabase/query_builder/components/view/ViewHeader/components/ViewTitleHeaderRightSide/ViewTitleHeaderRightSide.tsx b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/ViewTitleHeaderRightSide/ViewTitleHeaderRightSide.tsx index 9e16ec54c33dec75f5ba1ae1cece985bb0ea828a..3a67ecfcfd253430459757de257e8758e05fbde5 100644 --- a/frontend/src/metabase/query_builder/components/view/ViewHeader/components/ViewTitleHeaderRightSide/ViewTitleHeaderRightSide.tsx +++ b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/ViewTitleHeaderRightSide/ViewTitleHeaderRightSide.tsx @@ -16,7 +16,6 @@ import { ExploreResultsLink, FilterHeaderButton, QuestionActions, - QuestionFiltersHeaderToggle, QuestionNotebookButton, QuestionSummarizeWidget, ToggleNativeQueryPreview, @@ -150,19 +149,6 @@ export function ViewTitleHeaderRightSide({ return ( <ViewHeaderActionPanel data-testid="qb-header-action-panel"> - {QuestionFiltersHeaderToggle.shouldRender({ - question, - queryBuilderMode, - isObjectDetail, - }) && ( - <QuestionFiltersHeaderToggle - className={cx(CS.ml2, CS.mr1)} - query={question.query()} - isExpanded={areFiltersExpanded} - onExpand={onExpandFilters} - onCollapse={onCollapseFilters} - /> - )} {FilterHeaderButton.shouldRender({ question, queryBuilderMode, @@ -172,6 +158,10 @@ export function ViewTitleHeaderRightSide({ <FilterHeaderButton className={cx(CS.hide, CS.smShow)} onOpenModal={onOpenModal} + query={question.query()} + isExpanded={areFiltersExpanded} + onExpand={onExpandFilters} + onCollapse={onCollapseFilters} /> )} {QuestionSummarizeWidget.shouldRender({ diff --git a/frontend/src/metabase/querying/filters/components/FilterPanel/FilterPanelButton/FilterPanelButton.styled.tsx b/frontend/src/metabase/querying/filters/components/FilterPanel/FilterPanelButton/FilterPanelButton.styled.tsx deleted file mode 100644 index 73a97b475163861c3d2f284a5160f91f6dd916f6..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/querying/filters/components/FilterPanel/FilterPanelButton/FilterPanelButton.styled.tsx +++ /dev/null @@ -1,34 +0,0 @@ -import styled from "@emotion/styled"; -import type { ButtonHTMLAttributes } from "react"; - -import { alpha, color } from "metabase/lib/colors"; -import type { ButtonProps } from "metabase/ui"; -import { Button } from "metabase/ui"; - -type FilterButtonProps = ButtonHTMLAttributes<HTMLButtonElement> & - ButtonProps & { - isExpanded: boolean; - }; - -const shouldForwardProp = (propName: string) => { - return propName !== "isExpanded"; -}; - -export const FilterButton = styled(Button, { - shouldForwardProp, -})<FilterButtonProps>` - color: ${({ isExpanded }) => - isExpanded ? color("text-white") : color("filter")}; - background-color: ${({ isExpanded }) => - isExpanded ? alpha("filter", 0.8) : alpha("filter", 0.2)}; - transition: border 300ms linear, background 300ms linear; - - &:hover { - color: var(--mb-color-text-white); - background-color: var(--mb-color-filter); - } - - @media (prefers-reduced-motion) { - transition: none; - } -`; diff --git a/frontend/src/metabase/querying/filters/components/FilterPanel/FilterPanelButton/FilterPanelButton.tsx b/frontend/src/metabase/querying/filters/components/FilterPanel/FilterPanelButton/FilterPanelButton.tsx deleted file mode 100644 index a5ffcab7cc87143f17e1109662f81377de962f67..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/querying/filters/components/FilterPanel/FilterPanelButton/FilterPanelButton.tsx +++ /dev/null @@ -1,45 +0,0 @@ -import { useMemo } from "react"; -import { t } from "ttag"; - -import { Icon, Tooltip } from "metabase/ui"; -import type * as Lib from "metabase-lib"; - -import { getFilterItems } from "../utils"; - -import { FilterButton } from "./FilterPanelButton.styled"; - -interface FilterPanelButtonProps { - query: Lib.Query; - isExpanded: boolean; - onExpand: () => void; - onCollapse: () => void; -} - -export function FilterPanelButton({ - query, - isExpanded, - onExpand, - onCollapse, -}: FilterPanelButtonProps) { - const label = isExpanded ? t`Hide filters` : t`Show filters`; - const items = useMemo(() => getFilterItems(query), [query]); - - if (items.length === 0) { - return null; - } - - return ( - <Tooltip label={label}> - <FilterButton - leftIcon={<Icon name="filter" />} - radius="xl" - isExpanded={isExpanded} - aria-label={label} - data-testid="filters-visibility-control" - onClick={isExpanded ? onCollapse : onExpand} - > - {items.length} - </FilterButton> - </Tooltip> - ); -} diff --git a/frontend/src/metabase/querying/filters/components/FilterPanel/FilterPanelButton/FilterPanelButton.unit.spec.tsx b/frontend/src/metabase/querying/filters/components/FilterPanel/FilterPanelButton/FilterPanelButton.unit.spec.tsx deleted file mode 100644 index f298734ac12a08db473237c330dcdd0dcdc7ef88..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/querying/filters/components/FilterPanel/FilterPanelButton/FilterPanelButton.unit.spec.tsx +++ /dev/null @@ -1,38 +0,0 @@ -import { render, screen } from "__support__/ui"; -import type * as Lib from "metabase-lib"; - -import { createQueryWithStringFilter } from "../../FilterPicker/test-utils"; - -import { FilterPanelButton } from "./FilterPanelButton"; - -const { query: initialQuery } = createQueryWithStringFilter(); - -interface SetupOpts { - query: Lib.Query; - isExpanded?: boolean; - onExpand?: () => void; - onCollapse?: () => void; -} - -const setup = (props: SetupOpts = { query: initialQuery }) => { - const { isExpanded = true } = props; - const onExpand = jest.fn(); - const onCollapse = jest.fn(); - - render( - <FilterPanelButton - isExpanded={isExpanded} - onExpand={onExpand} - onCollapse={onCollapse} - {...props} - />, - ); -}; - -describe("FilterPanelButton", () => { - it("should render icon on the left", () => { - setup(); - - expect(screen.getByLabelText("filter icon")).toBeInTheDocument(); - }); -}); diff --git a/frontend/src/metabase/querying/filters/components/FilterPanel/FilterPanelButton/index.ts b/frontend/src/metabase/querying/filters/components/FilterPanel/FilterPanelButton/index.ts deleted file mode 100644 index e2c940e0bab24da0bc34357e38b36549c0df75cf..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/querying/filters/components/FilterPanel/FilterPanelButton/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from "./FilterPanelButton"; diff --git a/frontend/src/metabase/querying/filters/components/FilterPanel/index.ts b/frontend/src/metabase/querying/filters/components/FilterPanel/index.ts index 9813d56c4f20556fa0fb5b4440d3126d41553324..298376e6e67c84dd926df7b55b9074466e5574ef 100644 --- a/frontend/src/metabase/querying/filters/components/FilterPanel/index.ts +++ b/frontend/src/metabase/querying/filters/components/FilterPanel/index.ts @@ -1,2 +1 @@ export * from "./FilterPanel"; -export * from "./FilterPanelButton";