From d90cc4c34bb44dcb957174eb24e611fd26d4f52b Mon Sep 17 00:00:00 2001 From: Cliff Hall Date: Sat, 13 Jun 2026 19:52:27 -0400 Subject: [PATCH] fix(web): filter toggle hover as border, not background (#1460) (#1461) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(web): filter toggle hover as border, not background (#1460) The Logging/History/Network filter toggles used the same --mantine-primary-color-light background for both their hover and their active (toggled-on) state, so clicking a button to turn it off while the cursor was still over it looked like a no-op — the hover background masked the removal of the active background. - Add a shared FilterToggleButton element used by LogControls, NetworkControls, and MessageDirectionFilter, replacing the duplicated UnstyledButton + inline `bg={active ? … : undefined}` in each. - Add a `filterToggle` UnstyledButton theme variant + `.filter-toggle` rules: hover is now a thin border (reserved via a transparent 1px border so it doesn't shift layout); the active state is the filled background, keyed off `aria-pressed="true"`. - This leaves the `.list-item` selectable rows (Prompts/Apps/Tools/ Resources lists, section headers) untouched — they keep their hover-fill since they aren't toggles. - Add `aria-pressed` to the Logging level toggles (previously missing). Tests + stories cover the active/hover distinction via aria-pressed for FilterToggleButton and all three controls. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(web): move filter-toggle border to CSS so :hover actually shows (#1460) The previous attempt set the reserved `1px solid transparent` border via the theme's `styles` callback, which Mantine emits as an *inline* style. Inline styles outrank stylesheet `:hover` rules, so `.filter-toggle:hover { border-color }` never applied — hovering showed no border at all (verified in a real browser via Playwright). Move the border (and border-radius) out of the inline theme styles into the `.filter-toggle` class in App.css, so `:hover` can override the transparent border-color. The active fill (`[aria-pressed="true"]`) was already correct. Verified with real Playwright page.hover: inactive+hover now shows a 1px --inspector-brand-primary border, active shows the fill, active+hover shows both, all distinct. Story play functions assert the active fill and the reserved transparent border (the hover border can't be asserted in the synthetic-event test runner, so it's documented as Playwright-verified). Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- clients/web/src/App.css | 24 +++++++ .../FilterToggleButton.stories.tsx | 67 +++++++++++++++++++ .../FilterToggleButton.test.tsx | 54 +++++++++++++++ .../FilterToggleButton/FilterToggleButton.tsx | 44 ++++++++++++ .../groups/LogControls/LogControls.test.tsx | 14 ++++ .../groups/LogControls/LogControls.tsx | 30 +++------ .../MessageDirectionFilter.tsx | 41 ++++-------- .../NetworkControls/NetworkControls.test.tsx | 14 ++++ .../NetworkControls/NetworkControls.tsx | 30 +++------ clients/web/src/theme/UnstyledButton.ts | 6 ++ 10 files changed, 256 insertions(+), 68 deletions(-) create mode 100644 clients/web/src/components/elements/FilterToggleButton/FilterToggleButton.stories.tsx create mode 100644 clients/web/src/components/elements/FilterToggleButton/FilterToggleButton.test.tsx create mode 100644 clients/web/src/components/elements/FilterToggleButton/FilterToggleButton.tsx diff --git a/clients/web/src/App.css b/clients/web/src/App.css index 2ce7625d..f0435662 100644 --- a/clients/web/src/App.css +++ b/clients/web/src/App.css @@ -192,6 +192,30 @@ background-color: var(--mantine-primary-color-light); } +/* ── Filter toggle buttons (Logging / History / Network) ───────── + * + * Shared by the FilterToggleButton element. Unlike `.list-item`, hover is a + * thin border rather than a background fill, and the active (on) state is the + * filled background. Keeping the two states distinct means toggling a button + * off while the cursor is still over it is immediately visible, instead of the + * hover background masking the removal of the active background (issue #1460). + * The 1px transparent border is reserved here (not as an inline style in the + * theme variant — inline styles outrank stylesheet `:hover` rules) so the hover + * border doesn't shift layout. */ + +.filter-toggle { + border: 1px solid transparent; + border-radius: var(--mantine-radius-md); +} + +.filter-toggle:hover { + border-color: var(--inspector-brand-primary); +} + +.filter-toggle[aria-pressed="true"] { + background-color: var(--mantine-primary-color-light); +} + /* ── Select / Autocomplete dropdown / option (dark mode) ───────── * * Select and Autocomplete each render with their own static selector diff --git a/clients/web/src/components/elements/FilterToggleButton/FilterToggleButton.stories.tsx b/clients/web/src/components/elements/FilterToggleButton/FilterToggleButton.stories.tsx new file mode 100644 index 00000000..93693b6f --- /dev/null +++ b/clients/web/src/components/elements/FilterToggleButton/FilterToggleButton.stories.tsx @@ -0,0 +1,67 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { expect, fn, userEvent, within } from "storybook/test"; +import { FilterToggleButton } from "./FilterToggleButton"; + +const meta: Meta = { + title: "Elements/FilterToggleButton", + component: FilterToggleButton, + args: { + label: "debug", + color: "blue", + onToggle: fn(), + }, +}; + +export default meta; +type Story = StoryObj; + +// Active (on): filled background, aria-pressed=true. +export const Active: Story = { + args: { + active: true, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const button = await canvas.findByRole("button", { name: "debug" }); + await expect(button).toHaveAttribute("aria-pressed", "true"); + // Active renders a filled background (driven by `[aria-pressed="true"]`). + await expect(getComputedStyle(button).backgroundColor).not.toBe( + "rgba(0, 0, 0, 0)", + ); + }, +}; + +const TRANSPARENT = "rgba(0, 0, 0, 0)"; + +// Inactive (off): no fill, and the reserved border is transparent. The visible +// hover border is intentionally NOT asserted here — the Storybook/vitest runner +// dispatches synthetic pointer events that don't engage real CSS `:hover`. The +// border-on-hover (and that it isn't an inline style outranking the `:hover` +// rule — the #1460 regression) was verified with real Playwright `page.hover`. +export const Inactive: Story = { + args: { + active: false, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const button = await canvas.findByRole("button", { name: "debug" }); + await expect(button).toHaveAttribute("aria-pressed", "false"); + + // The reserved border is transparent by default and there's no active fill, + // so the inactive button reads as empty until hovered or toggled on. + await expect(getComputedStyle(button).borderTopColor).toBe(TRANSPARENT); + await expect(getComputedStyle(button).backgroundColor).toBe(TRANSPARENT); + }, +}; + +// Clicking an active button invokes onToggle(false). +export const TogglesOff: Story = { + args: { + active: true, + }, + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + await userEvent.click(await canvas.findByRole("button", { name: "debug" })); + await expect(args.onToggle).toHaveBeenCalledWith(false); + }, +}; diff --git a/clients/web/src/components/elements/FilterToggleButton/FilterToggleButton.test.tsx b/clients/web/src/components/elements/FilterToggleButton/FilterToggleButton.test.tsx new file mode 100644 index 00000000..fdcc108f --- /dev/null +++ b/clients/web/src/components/elements/FilterToggleButton/FilterToggleButton.test.tsx @@ -0,0 +1,54 @@ +import { describe, it, expect, vi } from "vitest"; +import userEvent from "@testing-library/user-event"; +import { renderWithMantine, screen } from "../../../test/renderWithMantine"; +import { FilterToggleButton } from "./FilterToggleButton"; + +const baseProps = { + label: "debug", + color: "blue", + active: false, + onToggle: vi.fn(), +}; + +describe("FilterToggleButton", () => { + it("renders the label as the accessible name", () => { + renderWithMantine(); + expect(screen.getByRole("button", { name: "debug" })).toBeInTheDocument(); + }); + + it("reflects the active state via aria-pressed=true", () => { + renderWithMantine(); + expect(screen.getByRole("button", { name: "debug" })).toHaveAttribute( + "aria-pressed", + "true", + ); + }); + + it("reflects the inactive state via aria-pressed=false", () => { + renderWithMantine(); + expect(screen.getByRole("button", { name: "debug" })).toHaveAttribute( + "aria-pressed", + "false", + ); + }); + + it("toggles to true when an inactive button is clicked", async () => { + const user = userEvent.setup(); + const onToggle = vi.fn(); + renderWithMantine( + , + ); + await user.click(screen.getByRole("button", { name: "debug" })); + expect(onToggle).toHaveBeenCalledWith(true); + }); + + it("toggles to false when an active button is clicked", async () => { + const user = userEvent.setup(); + const onToggle = vi.fn(); + renderWithMantine( + , + ); + await user.click(screen.getByRole("button", { name: "debug" })); + expect(onToggle).toHaveBeenCalledWith(false); + }); +}); diff --git a/clients/web/src/components/elements/FilterToggleButton/FilterToggleButton.tsx b/clients/web/src/components/elements/FilterToggleButton/FilterToggleButton.tsx new file mode 100644 index 00000000..324613df --- /dev/null +++ b/clients/web/src/components/elements/FilterToggleButton/FilterToggleButton.tsx @@ -0,0 +1,44 @@ +import { Text, UnstyledButton } from "@mantine/core"; + +export interface FilterToggleButtonProps { + /** Visible label and accessible name for the toggle. */ + label: string; + /** Mantine text color for the label (e.g. "blue", "red", "dimmed"). */ + color: string; + /** Whether the filter is currently on (rendered as a filled background). */ + active: boolean; + /** Receives the next desired active state when the button is clicked. */ + onToggle: (active: boolean) => void; +} + +const ToggleLabel = Text.withProps({ + ta: "center", + fw: 500, +}); + +/** + * A single full-width filter toggle used by the Logging, History, and Network + * controls. The `filterToggle` theme variant + `.filter-toggle` rules own the + * styling: hover shows a thin border, the active (`aria-pressed`) state shows a + * filled background. Keeping hover as a border (not a fill) means toggling a + * button off while the cursor is still over it is visibly distinct from hover, + * instead of the two states sharing the same background. See issue #1460. + */ +export function FilterToggleButton({ + label, + color, + active, + onToggle, +}: FilterToggleButtonProps) { + return ( + onToggle(!active)} + > + {label} + + ); +} diff --git a/clients/web/src/components/groups/LogControls/LogControls.test.tsx b/clients/web/src/components/groups/LogControls/LogControls.test.tsx index 530f5ca2..dc0356dd 100644 --- a/clients/web/src/components/groups/LogControls/LogControls.test.tsx +++ b/clients/web/src/components/groups/LogControls/LogControls.test.tsx @@ -132,6 +132,20 @@ describe("LogControls", () => { expect(onToggleAllLevels).toHaveBeenCalledTimes(1); }); + it("reflects level visibility via aria-pressed", () => { + renderWithMantine( + , + ); + expect(screen.getByRole("button", { name: "debug" })).toHaveAttribute( + "aria-pressed", + "false", + ); + expect(screen.getByRole("button", { name: "warning" })).toHaveAttribute( + "aria-pressed", + "true", + ); + }); + it("invokes onToggleLevel when a level button is clicked", async () => { const user = userEvent.setup(); const onToggleLevel = vi.fn(); diff --git a/clients/web/src/components/groups/LogControls/LogControls.tsx b/clients/web/src/components/groups/LogControls/LogControls.tsx index a189dea9..caeedaf1 100644 --- a/clients/web/src/components/groups/LogControls/LogControls.tsx +++ b/clients/web/src/components/groups/LogControls/LogControls.tsx @@ -4,12 +4,11 @@ import { Group, Select, Stack, - Text, TextInput, Title, - UnstyledButton, } from "@mantine/core"; import type { LoggingLevel } from "@modelcontextprotocol/sdk/types.js"; +import { FilterToggleButton } from "../../elements/FilterToggleButton/FilterToggleButton"; const LOG_LEVELS: LoggingLevel[] = [ "debug", @@ -102,24 +101,15 @@ export function LogControls({ - {LOG_LEVELS.map((level) => { - const style = LEVEL_COLORS[level]; - const active = visibleLevels[level]; - return ( - onToggleLevel(level, !active)} - > - - {level} - - - ); - })} + {LOG_LEVELS.map((level) => ( + onToggleLevel(level, visible)} + /> + ))} ); diff --git a/clients/web/src/components/groups/MessageDirectionFilter/MessageDirectionFilter.tsx b/clients/web/src/components/groups/MessageDirectionFilter/MessageDirectionFilter.tsx index 5a81a086..b85f554d 100644 --- a/clients/web/src/components/groups/MessageDirectionFilter/MessageDirectionFilter.tsx +++ b/clients/web/src/components/groups/MessageDirectionFilter/MessageDirectionFilter.tsx @@ -1,12 +1,6 @@ -import { - Button, - Group, - Stack, - Text, - Title, - UnstyledButton, -} from "@mantine/core"; +import { Button, Group, Stack, Title } from "@mantine/core"; import type { MessageOrigin } from "@inspector/core/mcp/types.js"; +import { FilterToggleButton } from "../../elements/FilterToggleButton/FilterToggleButton"; const SubtleButton = Button.withProps({ variant: "subtle", @@ -33,8 +27,8 @@ export interface MessageDirectionFilterProps { /** * "Filter by Message Direction" section — a Select/Deselect All control plus a - * listItem toggle per direction (client → server / client ← server). Used by the - * History controls. (Kept as its own component so the section is testable in + * FilterToggleButton per direction (client → server / client ← server). Used by + * the History controls. (Kept as its own component so the section is testable in * isolation and reusable if another screen ever needs a direction filter.) */ export function MessageDirectionFilter({ @@ -53,24 +47,15 @@ export function MessageDirectionFilter({ - {MESSAGE_DIRECTIONS.map(({ origin, label, color }) => { - const active = visibleDirections[origin]; - return ( - onToggleDirection(origin, !active)} - > - - {label} - - - ); - })} + {MESSAGE_DIRECTIONS.map(({ origin, label, color }) => ( + onToggleDirection(origin, visible)} + /> + ))} ); diff --git a/clients/web/src/components/groups/NetworkControls/NetworkControls.test.tsx b/clients/web/src/components/groups/NetworkControls/NetworkControls.test.tsx index 71397a21..0608328b 100644 --- a/clients/web/src/components/groups/NetworkControls/NetworkControls.test.tsx +++ b/clients/web/src/components/groups/NetworkControls/NetworkControls.test.tsx @@ -32,6 +32,20 @@ describe("NetworkControls", () => { expect(onFilterChange).toHaveBeenLastCalledWith("x"); }); + it("clears the filter text when the clear button is clicked", async () => { + const user = userEvent.setup(); + const onFilterChange = vi.fn(); + renderWithMantine( + , + ); + await user.click(screen.getByRole("button", { name: "Clear" })); + expect(onFilterChange).toHaveBeenCalledWith(""); + }); + it("reflects category visibility via aria-pressed", () => { renderWithMantine( - {NETWORK_CATEGORIES.map((category) => { - const active = visibleCategories[category]; - return ( - onToggleCategory(category, !active)} - > - - {category} - - - ); - })} + {NETWORK_CATEGORIES.map((category) => ( + onToggleCategory(category, visible)} + /> + ))} ); diff --git a/clients/web/src/theme/UnstyledButton.ts b/clients/web/src/theme/UnstyledButton.ts index fd37cf5b..e6d8788e 100644 --- a/clients/web/src/theme/UnstyledButton.ts +++ b/clients/web/src/theme/UnstyledButton.ts @@ -3,12 +3,18 @@ import { UnstyledButton } from "@mantine/core"; export const ThemeUnstyledButton = UnstyledButton.extend({ classNames: (_theme, props) => { if (props.variant === "listItem") return { root: "list-item" }; + if (props.variant === "filterToggle") return { root: "filter-toggle" }; return {}; }, styles: (_theme, props) => { if (props.variant === "listItem") { return { root: { borderRadius: "var(--mantine-radius-md)" } }; } + // `filterToggle` is styled entirely via the `.filter-toggle` rules in + // App.css (border-radius, the reserved transparent border, the :hover + // border, and the active background). Crucially the border must NOT be set + // here as an inline style — inline styles outrank stylesheet `:hover` + // rules, which would stop the hover border from ever showing (see #1460). return { root: {} }; }, });