* 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
92da6c1675
commit
d90cc4c34b
10 changed files with 256 additions and 68 deletions
|
|
@ -192,6 +192,30 @@
|
||||||
background-color: var(--mantine-primary-color-light);
|
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 / Autocomplete dropdown / option (dark mode) ─────────
|
||||||
*
|
*
|
||||||
* Select and Autocomplete each render with their own static selector
|
* Select and Autocomplete each render with their own static selector
|
||||||
|
|
|
||||||
|
|
@ -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<typeof FilterToggleButton> = {
|
||||||
|
title: "Elements/FilterToggleButton",
|
||||||
|
component: FilterToggleButton,
|
||||||
|
args: {
|
||||||
|
label: "debug",
|
||||||
|
color: "blue",
|
||||||
|
onToggle: fn(),
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
export default meta;
|
||||||
|
type Story = StoryObj<typeof FilterToggleButton>;
|
||||||
|
|
||||||
|
// 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);
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
@ -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(<FilterToggleButton {...baseProps} />);
|
||||||
|
expect(screen.getByRole("button", { name: "debug" })).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("reflects the active state via aria-pressed=true", () => {
|
||||||
|
renderWithMantine(<FilterToggleButton {...baseProps} active={true} />);
|
||||||
|
expect(screen.getByRole("button", { name: "debug" })).toHaveAttribute(
|
||||||
|
"aria-pressed",
|
||||||
|
"true",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("reflects the inactive state via aria-pressed=false", () => {
|
||||||
|
renderWithMantine(<FilterToggleButton {...baseProps} active={false} />);
|
||||||
|
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(
|
||||||
|
<FilterToggleButton {...baseProps} active={false} onToggle={onToggle} />,
|
||||||
|
);
|
||||||
|
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(
|
||||||
|
<FilterToggleButton {...baseProps} active={true} onToggle={onToggle} />,
|
||||||
|
);
|
||||||
|
await user.click(screen.getByRole("button", { name: "debug" }));
|
||||||
|
expect(onToggle).toHaveBeenCalledWith(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
@ -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 (
|
||||||
|
<UnstyledButton
|
||||||
|
w="100%"
|
||||||
|
p="sm"
|
||||||
|
variant="filterToggle"
|
||||||
|
aria-pressed={active}
|
||||||
|
onClick={() => onToggle(!active)}
|
||||||
|
>
|
||||||
|
<ToggleLabel c={color}>{label}</ToggleLabel>
|
||||||
|
</UnstyledButton>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
@ -132,6 +132,20 @@ describe("LogControls", () => {
|
||||||
expect(onToggleAllLevels).toHaveBeenCalledTimes(1);
|
expect(onToggleAllLevels).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("reflects level visibility via aria-pressed", () => {
|
||||||
|
renderWithMantine(
|
||||||
|
<LogControls {...baseProps} visibleLevels={someVisible} />,
|
||||||
|
);
|
||||||
|
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 () => {
|
it("invokes onToggleLevel when a level button is clicked", async () => {
|
||||||
const user = userEvent.setup();
|
const user = userEvent.setup();
|
||||||
const onToggleLevel = vi.fn();
|
const onToggleLevel = vi.fn();
|
||||||
|
|
|
||||||
|
|
@ -4,12 +4,11 @@ import {
|
||||||
Group,
|
Group,
|
||||||
Select,
|
Select,
|
||||||
Stack,
|
Stack,
|
||||||
Text,
|
|
||||||
TextInput,
|
TextInput,
|
||||||
Title,
|
Title,
|
||||||
UnstyledButton,
|
|
||||||
} from "@mantine/core";
|
} from "@mantine/core";
|
||||||
import type { LoggingLevel } from "@modelcontextprotocol/sdk/types.js";
|
import type { LoggingLevel } from "@modelcontextprotocol/sdk/types.js";
|
||||||
|
import { FilterToggleButton } from "../../elements/FilterToggleButton/FilterToggleButton";
|
||||||
|
|
||||||
const LOG_LEVELS: LoggingLevel[] = [
|
const LOG_LEVELS: LoggingLevel[] = [
|
||||||
"debug",
|
"debug",
|
||||||
|
|
@ -102,24 +101,15 @@ export function LogControls({
|
||||||
</SubtleButton>
|
</SubtleButton>
|
||||||
</Group>
|
</Group>
|
||||||
<Stack gap="xs">
|
<Stack gap="xs">
|
||||||
{LOG_LEVELS.map((level) => {
|
{LOG_LEVELS.map((level) => (
|
||||||
const style = LEVEL_COLORS[level];
|
<FilterToggleButton
|
||||||
const active = visibleLevels[level];
|
key={level}
|
||||||
return (
|
label={level}
|
||||||
<UnstyledButton
|
color={LEVEL_COLORS[level].c}
|
||||||
key={level}
|
active={visibleLevels[level]}
|
||||||
w="100%"
|
onToggle={(visible) => onToggleLevel(level, visible)}
|
||||||
p="sm"
|
/>
|
||||||
variant="listItem"
|
))}
|
||||||
bg={active ? "var(--mantine-primary-color-light)" : undefined}
|
|
||||||
onClick={() => onToggleLevel(level, !active)}
|
|
||||||
>
|
|
||||||
<Text c={style.c} ta="center" fw={500}>
|
|
||||||
{level}
|
|
||||||
</Text>
|
|
||||||
</UnstyledButton>
|
|
||||||
);
|
|
||||||
})}
|
|
||||||
</Stack>
|
</Stack>
|
||||||
</Stack>
|
</Stack>
|
||||||
);
|
);
|
||||||
|
|
|
||||||
|
|
@ -1,12 +1,6 @@
|
||||||
import {
|
import { Button, Group, Stack, Title } from "@mantine/core";
|
||||||
Button,
|
|
||||||
Group,
|
|
||||||
Stack,
|
|
||||||
Text,
|
|
||||||
Title,
|
|
||||||
UnstyledButton,
|
|
||||||
} from "@mantine/core";
|
|
||||||
import type { MessageOrigin } from "@inspector/core/mcp/types.js";
|
import type { MessageOrigin } from "@inspector/core/mcp/types.js";
|
||||||
|
import { FilterToggleButton } from "../../elements/FilterToggleButton/FilterToggleButton";
|
||||||
|
|
||||||
const SubtleButton = Button.withProps({
|
const SubtleButton = Button.withProps({
|
||||||
variant: "subtle",
|
variant: "subtle",
|
||||||
|
|
@ -33,8 +27,8 @@ export interface MessageDirectionFilterProps {
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* "Filter by Message Direction" section — a Select/Deselect All control plus a
|
* "Filter by Message Direction" section — a Select/Deselect All control plus a
|
||||||
* listItem toggle per direction (client → server / client ← server). Used by the
|
* FilterToggleButton per direction (client → server / client ← server). Used by
|
||||||
* History controls. (Kept as its own component so the section is testable in
|
* 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.)
|
* isolation and reusable if another screen ever needs a direction filter.)
|
||||||
*/
|
*/
|
||||||
export function MessageDirectionFilter({
|
export function MessageDirectionFilter({
|
||||||
|
|
@ -53,24 +47,15 @@ export function MessageDirectionFilter({
|
||||||
</SubtleButton>
|
</SubtleButton>
|
||||||
</Group>
|
</Group>
|
||||||
<Stack gap="xs">
|
<Stack gap="xs">
|
||||||
{MESSAGE_DIRECTIONS.map(({ origin, label, color }) => {
|
{MESSAGE_DIRECTIONS.map(({ origin, label, color }) => (
|
||||||
const active = visibleDirections[origin];
|
<FilterToggleButton
|
||||||
return (
|
key={origin}
|
||||||
<UnstyledButton
|
label={label}
|
||||||
key={origin}
|
color={color}
|
||||||
w="100%"
|
active={visibleDirections[origin]}
|
||||||
p="sm"
|
onToggle={(visible) => onToggleDirection(origin, visible)}
|
||||||
variant="listItem"
|
/>
|
||||||
aria-pressed={active}
|
))}
|
||||||
bg={active ? "var(--mantine-primary-color-light)" : undefined}
|
|
||||||
onClick={() => onToggleDirection(origin, !active)}
|
|
||||||
>
|
|
||||||
<Text c={color} ta="center" fw={500}>
|
|
||||||
{label}
|
|
||||||
</Text>
|
|
||||||
</UnstyledButton>
|
|
||||||
);
|
|
||||||
})}
|
|
||||||
</Stack>
|
</Stack>
|
||||||
</>
|
</>
|
||||||
);
|
);
|
||||||
|
|
|
||||||
|
|
@ -32,6 +32,20 @@ describe("NetworkControls", () => {
|
||||||
expect(onFilterChange).toHaveBeenLastCalledWith("x");
|
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(
|
||||||
|
<NetworkControls
|
||||||
|
{...baseProps}
|
||||||
|
filterText="auth"
|
||||||
|
onFilterChange={onFilterChange}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
await user.click(screen.getByRole("button", { name: "Clear" }));
|
||||||
|
expect(onFilterChange).toHaveBeenCalledWith("");
|
||||||
|
});
|
||||||
|
|
||||||
it("reflects category visibility via aria-pressed", () => {
|
it("reflects category visibility via aria-pressed", () => {
|
||||||
renderWithMantine(
|
renderWithMantine(
|
||||||
<NetworkControls
|
<NetworkControls
|
||||||
|
|
|
||||||
|
|
@ -3,12 +3,11 @@ import {
|
||||||
CloseButton,
|
CloseButton,
|
||||||
Group,
|
Group,
|
||||||
Stack,
|
Stack,
|
||||||
Text,
|
|
||||||
TextInput,
|
TextInput,
|
||||||
Title,
|
Title,
|
||||||
UnstyledButton,
|
|
||||||
} from "@mantine/core";
|
} from "@mantine/core";
|
||||||
import type { FetchRequestCategory } from "@inspector/core/mcp/types.js";
|
import type { FetchRequestCategory } from "@inspector/core/mcp/types.js";
|
||||||
|
import { FilterToggleButton } from "../../elements/FilterToggleButton/FilterToggleButton";
|
||||||
|
|
||||||
const NETWORK_CATEGORIES: FetchRequestCategory[] = ["auth", "transport"];
|
const NETWORK_CATEGORIES: FetchRequestCategory[] = ["auth", "transport"];
|
||||||
|
|
||||||
|
|
@ -64,24 +63,15 @@ export function NetworkControls({
|
||||||
</SubtleButton>
|
</SubtleButton>
|
||||||
</Group>
|
</Group>
|
||||||
<Stack gap="xs">
|
<Stack gap="xs">
|
||||||
{NETWORK_CATEGORIES.map((category) => {
|
{NETWORK_CATEGORIES.map((category) => (
|
||||||
const active = visibleCategories[category];
|
<FilterToggleButton
|
||||||
return (
|
key={category}
|
||||||
<UnstyledButton
|
label={category}
|
||||||
key={category}
|
color={CATEGORY_COLORS[category]}
|
||||||
w="100%"
|
active={visibleCategories[category]}
|
||||||
p="sm"
|
onToggle={(visible) => onToggleCategory(category, visible)}
|
||||||
variant="listItem"
|
/>
|
||||||
aria-pressed={active}
|
))}
|
||||||
bg={active ? "var(--mantine-primary-color-light)" : undefined}
|
|
||||||
onClick={() => onToggleCategory(category, !active)}
|
|
||||||
>
|
|
||||||
<Text c={CATEGORY_COLORS[category]} ta="center" fw={500}>
|
|
||||||
{category}
|
|
||||||
</Text>
|
|
||||||
</UnstyledButton>
|
|
||||||
);
|
|
||||||
})}
|
|
||||||
</Stack>
|
</Stack>
|
||||||
</Stack>
|
</Stack>
|
||||||
);
|
);
|
||||||
|
|
|
||||||
|
|
@ -3,12 +3,18 @@ import { UnstyledButton } from "@mantine/core";
|
||||||
export const ThemeUnstyledButton = UnstyledButton.extend({
|
export const ThemeUnstyledButton = UnstyledButton.extend({
|
||||||
classNames: (_theme, props) => {
|
classNames: (_theme, props) => {
|
||||||
if (props.variant === "listItem") return { root: "list-item" };
|
if (props.variant === "listItem") return { root: "list-item" };
|
||||||
|
if (props.variant === "filterToggle") return { root: "filter-toggle" };
|
||||||
return {};
|
return {};
|
||||||
},
|
},
|
||||||
styles: (_theme, props) => {
|
styles: (_theme, props) => {
|
||||||
if (props.variant === "listItem") {
|
if (props.variant === "listItem") {
|
||||||
return { root: { borderRadius: "var(--mantine-radius-md)" } };
|
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: {} };
|
return { root: {} };
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue