From 7a0510af381d9b4e60865ccb4c1f8310e6e899fa Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Wed, 25 Jun 2025 22:35:19 +0000 Subject: [PATCH 1/6] refactor: show icons for multi-select parameter options --- docs/about/contributing/frontend.md | 30 +-- .../MultiSelectCombobox.stories.tsx | 181 ++++++++++++++++ .../MultiSelectCombobox.tsx | 34 ++- .../DynamicParameter/DynamicParameter.tsx | 204 +++++++++--------- 4 files changed, 325 insertions(+), 124 deletions(-) diff --git a/docs/about/contributing/frontend.md b/docs/about/contributing/frontend.md index b121b01a26c59..ceddc5c2ff819 100644 --- a/docs/about/contributing/frontend.md +++ b/docs/about/contributing/frontend.md @@ -259,18 +259,16 @@ We use [Formik](https://formik.org/docs) for forms along with ## Testing -We use three types of testing in our app: **End-to-end (E2E)**, **Integration** +We use three types of testing in our app: **End-to-end (E2E)**, **Integration/Unit** and **Visual Testing**. -### End-to-End (E2E) +### End-to-End (E2E) – Playwright These are useful for testing complete flows like "Create a user", "Import -template", etc. We use [Playwright](https://playwright.dev/). If you only need -to test if the page is being rendered correctly, you should consider using the -**Visual Testing** approach. +template", etc. We use [Playwright](https://playwright.dev/). These tests run against a full Coder instance, backed by a database, and allows you to make sure that features work properly all the way through the stack. "End to end", so to speak. -For scenarios where you need to be authenticated, you can use -`test.use({ storageState: getStatePath("authState") })`. +For scenarios where you need to be authenticated as a certain user, you can use +`login` helper. Passing it some user credentials will log out of any other user account, and will attempt to login using those credentials. For ease of debugging, it's possible to run a Playwright test in headful mode running a Playwright server on your local machine, and executing the test inside @@ -289,22 +287,14 @@ local machine and forward the necessary ports to your workspace. At the end of the script, you will land _inside_ your workspace with environment variables set so you can simply execute the test (`pnpm run playwright:test`). -### Integration +### Integration/Unit – Jest -Test user interactions like "Click in a button shows a dialog", "Submit the form -sends the correct data", etc. For this, we use [Jest](https://jestjs.io/) and -[react-testing-library](https://testing-library.com/docs/react-testing-library/intro/). -If the test involves routing checks like redirects or maybe checking the info on -another page, you should probably consider using the **E2E** approach. +We use Jest mostly for testing code that does _not_ pertain to React. Functions and classes that contain notable app logic, and which are well abstracted from React should have accompanying tests. If the logic is tightly coupled to a React component, a Storybook test or an E2E test may be a better option depending on the scenario. -### Visual testing +### Visual Testing – Storybook -We use visual tests to test components without user interaction like testing if -a page/component is rendered correctly depending on some parameters, if a button -is showing a spinner, if `loading` props are passed correctly, etc. This should -always be your first option since it is way easier to maintain. For this, we use -[Storybook](https://storybook.js.org/) and -[Chromatic](https://www.chromatic.com/). +We use Storybook for testing all of our React code. For static components, you simply add a story that renders the components with the props that you would like to test, and Storybook will record snapshots of it to ensure that it isn't changed unintentionally. If you would like to test an interaction with the component, then you can add an interaction test by specifying a `play` function for the story. For stories with an interaction test, a snapshot will be recorded of the end state of the component. We use +[Chromatic](https://www.chromatic.com/) to manage and compare snapshots in CI. To learn more about testing components that fetch API data, refer to the [**Where to fetch data**](#where-to-fetch-data) section. diff --git a/site/src/components/MultiSelectCombobox/MultiSelectCombobox.stories.tsx b/site/src/components/MultiSelectCombobox/MultiSelectCombobox.stories.tsx index 109a60e60448d..1378f2289bb4f 100644 --- a/site/src/components/MultiSelectCombobox/MultiSelectCombobox.stories.tsx +++ b/site/src/components/MultiSelectCombobox/MultiSelectCombobox.stories.tsx @@ -98,3 +98,184 @@ export const ClearAllComboboxItems: Story = { ); }, }; + +export const WithIcons: Story = { + args: { + placeholder: "Select technology", + emptyIndicator: ( +

+ All technologies selected +

+ ), + options: [ + { + label: "Docker", + value: "docker", + icon: "/icon/docker.png", + }, + { + label: "Kubernetes", + value: "kubernetes", + icon: "/icon/k8s.png", + }, + { + label: "VS Code", + value: "vscode", + icon: "/icon/code.svg", + }, + { + label: "JetBrains", + value: "jetbrains", + icon: "/icon/intellij.svg", + }, + { + label: "Jupyter", + value: "jupyter", + icon: "/icon/jupyter.svg", + }, + ], + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + // Open the combobox + await userEvent.click(canvas.getByPlaceholderText("Select technology")); + + // Verify that Docker option has an icon + const dockerOption = canvas.getByRole("option", { name: /Docker/i }); + const dockerIcon = dockerOption.querySelector("img"); + await expect(dockerIcon).toBeInTheDocument(); + await expect(dockerIcon).toHaveAttribute("src", "/icon/docker.png"); + + // Select Docker and verify icon appears in badge + await userEvent.click(dockerOption); + + // Find the Docker badge + const dockerBadge = canvas + .getByText("Docker") + .closest('[role="button"]')?.parentElement; + const badgeIcon = dockerBadge?.querySelector("img"); + await expect(badgeIcon).toBeInTheDocument(); + await expect(badgeIcon).toHaveAttribute("src", "/icon/docker.png"); + }, +}; + +export const MixedWithAndWithoutIcons: Story = { + args: { + placeholder: "Select resource", + options: [ + { + label: "CPU", + value: "cpu", + icon: "/icon/memory.svg", + }, + { + label: "Memory", + value: "memory", + icon: "/icon/memory.svg", + }, + { + label: "Storage", + value: "storage", + }, + { + label: "Network", + value: "network", + }, + ], + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + // Open the combobox + await userEvent.click(canvas.getByPlaceholderText("Select resource")); + + // Verify that CPU option has an icon + const cpuOption = canvas.getByRole("option", { name: /CPU/i }); + const cpuIcon = cpuOption.querySelector("img"); + await expect(cpuIcon).toBeInTheDocument(); + + // Verify that Storage option does not have an icon + const storageOption = canvas.getByRole("option", { name: /Storage/i }); + const storageIcon = storageOption.querySelector("img"); + await expect(storageIcon).not.toBeInTheDocument(); + + // Select both and verify badges + await userEvent.click(cpuOption); + await userEvent.click(storageOption); + + // CPU badge should have icon + const cpuBadge = canvas + .getByText("CPU") + .closest('[role="button"]')?.parentElement; + const cpuBadgeIcon = cpuBadge?.querySelector("img"); + await expect(cpuBadgeIcon).toBeInTheDocument(); + + // Storage badge should not have icon + const storageBadge = canvas + .getByText("Storage") + .closest('[role="button"]')?.parentElement; + const storageBadgeIcon = storageBadge?.querySelector("img"); + await expect(storageBadgeIcon).not.toBeInTheDocument(); + }, +}; + +export const WithGroupedIcons: Story = { + args: { + placeholder: "Select tools", + groupBy: "category", + options: [ + { + label: "Docker", + value: "docker", + icon: "/icon/docker.png", + category: "Containers", + }, + { + label: "Kubernetes", + value: "kubernetes", + icon: "/icon/k8s.png", + category: "Containers", + }, + { + label: "VS Code", + value: "vscode", + icon: "/icon/code.svg", + category: "IDEs", + }, + { + label: "JetBrains", + value: "jetbrains", + icon: "/icon/intellij.svg", + category: "IDEs", + }, + { + label: "Zed", + value: "zed", + icon: "/icon/zed.svg", + category: "IDEs", + }, + ], + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + // Open the combobox + await userEvent.click(canvas.getByPlaceholderText("Select tools")); + + // Verify grouped options still have icons + const dockerOption = canvas.getByRole("option", { name: /Docker/i }); + const dockerIcon = dockerOption.querySelector("img"); + await expect(dockerIcon).toBeInTheDocument(); + await expect(dockerIcon).toHaveAttribute("src", "/icon/docker.png"); + + const vscodeOption = canvas.getByRole("option", { name: /VS Code/i }); + const vscodeIcon = vscodeOption.querySelector("img"); + await expect(vscodeIcon).toBeInTheDocument(); + await expect(vscodeIcon).toHaveAttribute("src", "/icon/code.svg"); + + // Verify grouping headers are present + await expect(canvas.getByText("Containers")).toBeInTheDocument(); + await expect(canvas.getByText("IDEs")).toBeInTheDocument(); + }, +}; diff --git a/site/src/components/MultiSelectCombobox/MultiSelectCombobox.tsx b/site/src/components/MultiSelectCombobox/MultiSelectCombobox.tsx index 2cfe69a25df52..a021cfddccf3d 100644 --- a/site/src/components/MultiSelectCombobox/MultiSelectCombobox.tsx +++ b/site/src/components/MultiSelectCombobox/MultiSelectCombobox.tsx @@ -2,6 +2,7 @@ * This component is based on multiple-selector * @see {@link https://shadcnui-expansions.typeart.cc/docs/multiple-selector} */ +import { useTheme } from "@emotion/react"; import { Command as CommandPrimitive, useCommandState } from "cmdk"; import { Badge } from "components/Badge/Badge"; import { @@ -25,11 +26,13 @@ import { useRef, useState, } from "react"; +import { getExternalImageStylesFromUrl } from "theme/externalImages"; import { cn } from "utils/cn"; export interface Option { value: string; label: string; + icon?: string; disable?: boolean; /** fixed option that can't be removed. */ fixed?: boolean; @@ -204,6 +207,7 @@ export const MultiSelectCombobox = forwardRef< const [onScrollbar, setOnScrollbar] = useState(false); const [isLoading, setIsLoading] = useState(false); const dropdownRef = useRef(null); + const theme = useTheme(); const [selected, setSelected] = useState( arrayDefaultOptions ?? [], @@ -487,7 +491,20 @@ export const MultiSelectCombobox = forwardRef< data-fixed={option.fixed} data-disabled={disabled || undefined} > - {option.label} +
+ {option.icon && ( + + )} + {option.label} +