From e29f13a3d4f12041816c834bb7f844e3468b885b Mon Sep 17 00:00:00 2001 From: Doug MacKenzie Date: Fri, 29 Nov 2024 10:27:35 +1100 Subject: [PATCH 1/3] Use v3 Menu for FilterBar's 'Add filters' --- .../AddFiltersMenu/AddFiltersMenu.tsx | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/packages/components/src/Filter/FilterBar/subcomponents/AddFiltersMenu/AddFiltersMenu.tsx b/packages/components/src/Filter/FilterBar/subcomponents/AddFiltersMenu/AddFiltersMenu.tsx index d084581b848..7ab0cc3f2f2 100644 --- a/packages/components/src/Filter/FilterBar/subcomponents/AddFiltersMenu/AddFiltersMenu.tsx +++ b/packages/components/src/Filter/FilterBar/subcomponents/AddFiltersMenu/AddFiltersMenu.tsx @@ -1,6 +1,7 @@ import React, { useEffect, useRef } from "react" import { useIntl } from "@cultureamp/i18n-react-intl" -import { Menu, MenuList, MenuItem, Button } from "~components/__actions__/v2" +import { Popover } from "react-aria-components" +import { MenuTrigger, Menu, MenuItem, Button } from "~components/__actions__/v3" import { Icon } from "~components/__future__/Icon" import { useFilterBarContext } from "../../context/FilterBarContext" @@ -27,27 +28,26 @@ export const AddFiltersMenu = (): JSX.Element => { }, [focusId]) return ( - } - /> - } - > - - {inactiveFilters.map(({ id, name }) => ( - showFilter(id)} - /> - ))} - - + + + + + {inactiveFilters.map(({ id, name }) => ( + showFilter(id)}> + {name} + + ))} + + + ) } From aaf3117111278baabfe201c0e8b62d98181fbdea Mon Sep 17 00:00:00 2001 From: Doug MacKenzie Date: Fri, 29 Nov 2024 12:12:52 +1100 Subject: [PATCH 2/3] Changeset --- .changeset/violet-snakes-wink.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/violet-snakes-wink.md diff --git a/.changeset/violet-snakes-wink.md b/.changeset/violet-snakes-wink.md new file mode 100644 index 00000000000..2fa8cdba37e --- /dev/null +++ b/.changeset/violet-snakes-wink.md @@ -0,0 +1,5 @@ +--- +"@kaizen/components": patch +--- + +FilterBar: Use new Button+Menu for 'Add filters' button From b01c890d23f78f68553a7b9103ce3a60caea9a5c Mon Sep 17 00:00:00 2001 From: Doug MacKenzie Date: Fri, 29 Nov 2024 12:56:43 +1100 Subject: [PATCH 3/3] Update tests for new menu roles --- .../src/Filter/FilterBar/FilterBar.spec.tsx | 39 +++++++++---------- .../_docs/FilterBar.spec.stories.tsx | 4 +- .../FilterBar/_docs/FilterBar.stories.tsx | 2 +- .../AddFiltersMenu/AddFiltersMenu.spec.tsx | 8 ++-- 4 files changed, 25 insertions(+), 28 deletions(-) diff --git a/packages/components/src/Filter/FilterBar/FilterBar.spec.tsx b/packages/components/src/Filter/FilterBar/FilterBar.spec.tsx index f9ceee3038b..6d5a77e1bcf 100644 --- a/packages/components/src/Filter/FilterBar/FilterBar.spec.tsx +++ b/packages/components/src/Filter/FilterBar/FilterBar.spec.tsx @@ -1,5 +1,5 @@ import React, { useEffect, useState } from "react" -import { screen, waitFor, within, render } from "@testing-library/react" +import { screen, waitFor, render } from "@testing-library/react" import userEvent from "@testing-library/user-event" import { vi } from "vitest" import { FilterMultiSelect } from "../index" @@ -215,8 +215,7 @@ describe("", () => { const addFiltersButton = getByRole("button", { name: "Add Filters" }) await user.click(addFiltersButton) - const list = getByRole("list") - const menuOption = within(list).getByRole("button", { name: "Topping" }) + const menuOption = getByRole("menuitem", { name: "Topping" }) await waitFor(() => { expect(menuOption).toBeVisible() @@ -238,7 +237,7 @@ describe("", () => { }) it("does not show active removable filters in the Add Filters menu", async () => { - const { getByRole } = render( + const { getByRole, queryByRole } = render( filters={filtersRemovable} defaultValues={{ topping: "pearls" }} @@ -249,15 +248,14 @@ describe("", () => { const addFiltersButton = getByRole("button", { name: "Add Filters" }) await user.click(addFiltersButton) - const list = getByRole("list") - const menuOption = within(list).queryByRole("button", { name: "Topping" }) + const menuOption = queryByRole("menuitem", { name: "Topping" }) await waitFor(() => { expect(menuOption).not.toBeInTheDocument() }) }) it("clears the value of a filter if it is removed", async () => { - const { getByRole } = render( + const { getByRole, queryByRole } = render( filters={filtersRemovable} defaultValues={{ topping: "pearls" }} @@ -276,15 +274,14 @@ describe("", () => { const addFiltersButton = getByRole("button", { name: "Add Filters" }) await user.click(addFiltersButton) - const list = getByRole("list") - const menuOption = within(list).getByRole("button", { name: "Topping" }) + const menuOption = getByRole("menuitem", { name: "Topping" }) await waitFor(() => { expect(menuOption).toBeVisible() }) await user.click(menuOption) await waitFor(() => { - expect(list).not.toBeInTheDocument() + expect(queryByRole("menu")).not.toBeInTheDocument() }) expect(getByRole("button", { name: "Topping" })).toBeVisible() }) @@ -303,15 +300,17 @@ describe("", () => { const addFiltersButton = getByRole("button", { name: "Add Filters" }) await user.click(addFiltersButton) - const menuOptionIceLevel = getByRole("button", { name: "Ice Level" }) + const menuOptionIceLevel = getByRole("menuitem", { name: "Ice Level" }) await user.click(menuOptionIceLevel) await user.click(addFiltersButton) - const menuOptionFlavour = getByRole("button", { name: "Flavour" }) + const menuOptionFlavour = getByRole("menuitem", { name: "Flavour" }) await user.click(menuOptionFlavour) await user.click(addFiltersButton) - const menuOptionSugarLevel = getByRole("button", { name: "Sugar Level" }) + const menuOptionSugarLevel = getByRole("menuitem", { + name: "Sugar Level", + }) await user.click(menuOptionSugarLevel) const filters = getAllByTestId(TEST_ID__FILTER) @@ -335,7 +334,7 @@ describe("", () => { const addFiltersButton = getByRole("button", { name: "Add Filters" }) await user.click(addFiltersButton) - const menuOptionIceLevel = getByRole("button", { name: "Ice Level" }) + const menuOptionIceLevel = getByRole("menuitem", { name: "Ice Level" }) await user.click(menuOptionIceLevel) expect(getByRole("button", { name: "Ice Level" })).toHaveFocus() @@ -350,7 +349,7 @@ describe("", () => { const addFiltersButton = getByRole("button", { name: "Add Filters" }) await user.click(addFiltersButton) - const menuOptionOthers = getByRole("button", { name: "Others" }) + const menuOptionOthers = getByRole("menuitem", { name: "Others" }) await user.click(menuOptionOthers) expect(getByRole("button", { name: "Others" })).toHaveFocus() @@ -476,8 +475,7 @@ describe("", () => { await user.click(addFiltersButton) - const list = getByRole("list") - const menuOption = within(list).getByRole("button", { name: "Topping" }) + const menuOption = getByRole("menuitem", { name: "Topping" }) await waitFor(() => { expect(menuOption).toBeVisible() @@ -693,11 +691,10 @@ describe("", () => { await user.click(addFiltersButton) - const list = getByRole("list") - const menuOptionSugar = within(list).getByRole("button", { + const menuOptionSugar = getByRole("menuitem", { name: "Sugar", }) - const menuOptionSyrup = within(list).getByRole("button", { + const menuOptionSyrup = getByRole("menuitem", { name: "Syrup", }) expect(menuOptionSugar).toBeVisible() @@ -706,7 +703,7 @@ describe("", () => { await user.click(menuOptionSugar) await waitFor(() => { - expect(list).not.toBeInTheDocument() + expect(queryByRole("menu")).not.toBeInTheDocument() }) const sugarButton = getByRole("button", { name: "Sugar" }) expect(sugarButton).toBeVisible() diff --git a/packages/components/src/Filter/FilterBar/_docs/FilterBar.spec.stories.tsx b/packages/components/src/Filter/FilterBar/_docs/FilterBar.spec.stories.tsx index dd6db64bb8d..a1b66ef2802 100644 --- a/packages/components/src/Filter/FilterBar/_docs/FilterBar.spec.stories.tsx +++ b/packages/components/src/Filter/FilterBar/_docs/FilterBar.spec.stories.tsx @@ -177,7 +177,7 @@ export const ClearAllFromRemovable: Story = { }) await waitFor(() => { - userEvent.click(canvas.getByRole("button", { name: "Toppings" })) + userEvent.click(canvas.getByRole("menuitem", { name: "Toppings" })) }) }) @@ -226,7 +226,7 @@ export const ClearAllRemovesItself: Story = { await waitFor(() => userEvent.click(canvas.getByRole("button", { name: "Add Filters" })) ) - await userEvent.click(canvas.getByRole("button", { name: "Drank" })) + await userEvent.click(canvas.getByRole("menuitem", { name: "Drank" })) }) await step( diff --git a/packages/components/src/Filter/FilterBar/_docs/FilterBar.stories.tsx b/packages/components/src/Filter/FilterBar/_docs/FilterBar.stories.tsx index e11dacf067a..1b0947cb1fa 100644 --- a/packages/components/src/Filter/FilterBar/_docs/FilterBar.stories.tsx +++ b/packages/components/src/Filter/FilterBar/_docs/FilterBar.stories.tsx @@ -838,7 +838,7 @@ export const UpdatesLabels: Story = { ) }, play: async ({ canvasElement, step }) => { - const canvas = within(canvasElement) + const canvas = within(canvasElement.parentElement!) await step("Initial render complete", async () => { await waitFor(() => canvas.getByRole("button", { name: "Add Filters" })) diff --git a/packages/components/src/Filter/FilterBar/subcomponents/AddFiltersMenu/AddFiltersMenu.spec.tsx b/packages/components/src/Filter/FilterBar/subcomponents/AddFiltersMenu/AddFiltersMenu.spec.tsx index 5b4d76e2683..8f875851588 100644 --- a/packages/components/src/Filter/FilterBar/subcomponents/AddFiltersMenu/AddFiltersMenu.spec.tsx +++ b/packages/components/src/Filter/FilterBar/subcomponents/AddFiltersMenu/AddFiltersMenu.spec.tsx @@ -61,13 +61,13 @@ describe("", () => { await user.click(addFiltersButton) await waitFor(() => { - expect(screen.getByRole("list")).toBeVisible() + expect(screen.getByRole("menu")).toBeVisible() }) - expect(screen.getByRole("button", { name: "Coffee" })).toBeVisible() + expect(screen.getByRole("menuitem", { name: "Coffee" })).toBeVisible() expect( - screen.queryByRole("button", { name: "Tea" }) + screen.queryByRole("menuitem", { name: "Tea" }) ).not.toBeInTheDocument() - expect(screen.getByRole("button", { name: "Milk" })).toBeVisible() + expect(screen.getByRole("menuitem", { name: "Milk" })).toBeVisible() }) it("disables the Add Filters button when there are no inactive filters", async () => {