From 7fa6cf56f2cc402b9bad5cd21f9ce9cc6a8d5228 Mon Sep 17 00:00:00 2001 From: Oisin Coveney <oisin@metabase.com> Date: Wed, 14 Aug 2024 12:59:33 +0300 Subject: [PATCH] fix #46765 - Add dashboard filter popover does not close on click outside (#46771) --- .../AddFilterParameterButton.tsx | 8 +- .../AddFilterParameterButton.unit.spec.tsx | 95 +++++++++++++++++++ .../buttons/AddFilterParameterButton/index.ts | 1 + .../components/ParametersPopover.tsx | 36 ++++--- 4 files changed, 119 insertions(+), 21 deletions(-) rename frontend/src/metabase/dashboard/components/DashboardHeader/buttons/{ => AddFilterParameterButton}/AddFilterParameterButton.tsx (85%) create mode 100644 frontend/src/metabase/dashboard/components/DashboardHeader/buttons/AddFilterParameterButton/AddFilterParameterButton.unit.spec.tsx create mode 100644 frontend/src/metabase/dashboard/components/DashboardHeader/buttons/AddFilterParameterButton/index.ts diff --git a/frontend/src/metabase/dashboard/components/DashboardHeader/buttons/AddFilterParameterButton.tsx b/frontend/src/metabase/dashboard/components/DashboardHeader/buttons/AddFilterParameterButton/AddFilterParameterButton.tsx similarity index 85% rename from frontend/src/metabase/dashboard/components/DashboardHeader/buttons/AddFilterParameterButton.tsx rename to frontend/src/metabase/dashboard/components/DashboardHeader/buttons/AddFilterParameterButton/AddFilterParameterButton.tsx index e6fe6d1458e..33f9cb88add 100644 --- a/frontend/src/metabase/dashboard/components/DashboardHeader/buttons/AddFilterParameterButton.tsx +++ b/frontend/src/metabase/dashboard/components/DashboardHeader/buttons/AddFilterParameterButton/AddFilterParameterButton.tsx @@ -16,7 +16,11 @@ export const AddFilterParameterButton = () => { const isAddParameterPopoverOpen = useSelector(getIsAddParameterPopoverOpen); return ( - <Popover opened={isAddParameterPopoverOpen} position="bottom-end"> + <Popover + opened={isAddParameterPopoverOpen} + onClose={() => dispatch(hideAddParameterPopover())} + position="bottom-end" + > <Popover.Target> <ToolbarButton icon="filter" @@ -29,7 +33,7 @@ export const AddFilterParameterButton = () => { tooltipLabel={t`Add a filter`} /> </Popover.Target> - <Popover.Dropdown> + <Popover.Dropdown data-testid="add-filter-parameter-dropdown"> <ParametersPopover onAddParameter={parameter => dispatch(addParameter(parameter))} onClose={() => dispatch(hideAddParameterPopover())} diff --git a/frontend/src/metabase/dashboard/components/DashboardHeader/buttons/AddFilterParameterButton/AddFilterParameterButton.unit.spec.tsx b/frontend/src/metabase/dashboard/components/DashboardHeader/buttons/AddFilterParameterButton/AddFilterParameterButton.unit.spec.tsx new file mode 100644 index 00000000000..4800dc2168d --- /dev/null +++ b/frontend/src/metabase/dashboard/components/DashboardHeader/buttons/AddFilterParameterButton/AddFilterParameterButton.unit.spec.tsx @@ -0,0 +1,95 @@ +import { screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; + +import { renderWithProviders } from "__support__/ui"; +import { + createMockDashboardState, + createMockState, +} from "metabase-types/store/mocks"; + +import { AddFilterParameterButton } from "./AddFilterParameterButton"; + +const setup = ({ isAddParameterPopoverOpen = false } = {}) => { + const state = createMockState({ + dashboard: createMockDashboardState({ + isAddParameterPopoverOpen, + }), + }); + + return renderWithProviders(<AddFilterParameterButton />, { + storeInitialState: state, + }); +}; + +describe("AddFilterParameterButton", () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + it("should render the button with correct icon and tooltip", () => { + setup(); + const button = screen.getByLabelText("Add a filter"); + expect(button).toBeInTheDocument(); + expect(screen.getByLabelText("filter icon")).toBeInTheDocument(); + }); + + describe("when popover is open based on state", () => { + it("should render the popover when isAddParameterPopoverOpen is true", () => { + setup({ isAddParameterPopoverOpen: true }); + expect( + screen.getByTestId("add-filter-parameter-dropdown"), + ).toBeInTheDocument(); + }); + it("should not render the popover when isAddParameterPopoverOpen is false", () => { + setup({ isAddParameterPopoverOpen: false }); + expect( + screen.queryByTestId("add-filter-parameter-dropdown"), + ).not.toBeInTheDocument(); + }); + }); + + describe("when the popover is closed", () => { + it("should show the popover when the button is clicked", async () => { + setup(); + expect( + screen.queryByTestId("add-filter-parameter-dropdown"), + ).not.toBeInTheDocument(); + await userEvent.click(screen.getByLabelText("Add a filter")); + expect( + screen.getByTestId("add-filter-parameter-dropdown"), + ).toBeInTheDocument(); + }); + }); + + describe("when the popover is open", () => { + it("should close the popover when the button is clicked", async () => { + setup({ isAddParameterPopoverOpen: true }); + expect( + screen.getByTestId("add-filter-parameter-dropdown"), + ).toBeInTheDocument(); + await userEvent.click(screen.getByLabelText("Add a filter")); + expect( + screen.queryByTestId("add-filter-parameter-dropdown"), + ).not.toBeInTheDocument(); + }); + + it("should close the popover when the user clicks outside the popover", async () => { + setup({ isAddParameterPopoverOpen: true }); + expect( + screen.getByTestId("add-filter-parameter-dropdown"), + ).toBeInTheDocument(); + await userEvent.click(document.body); + expect( + screen.queryByTestId("add-filter-parameter-dropdown"), + ).not.toBeInTheDocument(); + }); + }); + + it("should show the popover when the button is clicked", async () => { + setup(); + + const button = screen.getByLabelText("Add a filter"); + await userEvent.click(button); + expect(screen.getByText("What do you want to filter?")).toBeInTheDocument(); + }); +}); diff --git a/frontend/src/metabase/dashboard/components/DashboardHeader/buttons/AddFilterParameterButton/index.ts b/frontend/src/metabase/dashboard/components/DashboardHeader/buttons/AddFilterParameterButton/index.ts new file mode 100644 index 00000000000..867f9f73e36 --- /dev/null +++ b/frontend/src/metabase/dashboard/components/DashboardHeader/buttons/AddFilterParameterButton/index.ts @@ -0,0 +1 @@ +export * from "./AddFilterParameterButton"; diff --git a/frontend/src/metabase/dashboard/components/ParametersPopover.tsx b/frontend/src/metabase/dashboard/components/ParametersPopover.tsx index b6af1b93083..60786210ca4 100644 --- a/frontend/src/metabase/dashboard/components/ParametersPopover.tsx +++ b/frontend/src/metabase/dashboard/components/ParametersPopover.tsx @@ -35,28 +35,26 @@ type ParameterSection = typeof PARAMETER_SECTIONS[number]; export const ParametersPopover = ({ onClose, onAddParameter, -}: ParametersPopoverProps) => { - return ( - <ParameterOptionsSectionsPane - sections={PARAMETER_SECTIONS} - onSelectSection={selectedSection => { - const parameterSection = _.findWhere(PARAMETER_SECTIONS, { - id: selectedSection.id, - }); +}: ParametersPopoverProps) => ( + <ParameterOptionsSectionsPane + sections={PARAMETER_SECTIONS} + onSelectSection={selectedSection => { + const parameterSection = _.findWhere(PARAMETER_SECTIONS, { + id: selectedSection.id, + }); - if (parameterSection) { - const defaultOption = - defaultOptionForParameterSection[parameterSection.id]; + if (parameterSection) { + const defaultOption = + defaultOptionForParameterSection[parameterSection.id]; - if (defaultOption) { - onAddParameter(defaultOption); - } - onClose(); + if (defaultOption) { + onAddParameter(defaultOption); } - }} - /> - ); -}; + onClose(); + } + }} + /> +); const ParameterOptionsSection = ({ section, -- GitLab