fix(web): address review feedback on the monitoring column (#1616)
- ResizeHandle: clear the `resizing-col` body class on unmount so a drag
interrupted by the handle unmounting (narrow past the breakpoint / disconnect)
can't leave page-wide user-select:none / col-resize applied.
- Embedded monitor column now shows the full unfiltered stream instead of
silently mirroring the full-size screen's live filter (the column has no
filter control to explain an empty/partial list).
- MonitoringControls: give the SegmentedControl an aria-label ("Monitoring
screen") so the tab group has an accessible name.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
daf736c6e7
commit
9b76272ad1
10 changed files with 124 additions and 11 deletions
|
|
@ -86,6 +86,28 @@ describe("ResizeHandle", () => {
|
||||||
expect(document.body.classList.contains("resizing-col")).toBe(false);
|
expect(document.body.classList.contains("resizing-col")).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("clears the drag body class if it unmounts mid-drag", () => {
|
||||||
|
const onChange = vi.fn();
|
||||||
|
const onCommit = vi.fn();
|
||||||
|
const { unmount } = renderWithMantine(
|
||||||
|
<ResizeHandle
|
||||||
|
value={400}
|
||||||
|
min={320}
|
||||||
|
max={720}
|
||||||
|
onChange={onChange}
|
||||||
|
onCommit={onCommit}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
fireEvent.pointerDown(screen.getByRole("separator"), {
|
||||||
|
clientX: 500,
|
||||||
|
pointerId: 1,
|
||||||
|
});
|
||||||
|
expect(document.body.classList.contains("resizing-col")).toBe(true);
|
||||||
|
// No pointerup/cancel — the handle is removed from the tree first.
|
||||||
|
unmount();
|
||||||
|
expect(document.body.classList.contains("resizing-col")).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
it("ignores pointer moves when no drag is active", () => {
|
it("ignores pointer moves when no drag is active", () => {
|
||||||
const { handle, onChange, onCommit } = setup();
|
const { handle, onChange, onCommit } = setup();
|
||||||
fireEvent.pointerMove(handle, { clientX: 460, pointerId: 1 });
|
fireEvent.pointerMove(handle, { clientX: 460, pointerId: 1 });
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,9 @@
|
||||||
import { useRef, type KeyboardEvent, type PointerEvent } from "react";
|
import {
|
||||||
|
useEffect,
|
||||||
|
useRef,
|
||||||
|
type KeyboardEvent,
|
||||||
|
type PointerEvent,
|
||||||
|
} from "react";
|
||||||
import { Box } from "@mantine/core";
|
import { Box } from "@mantine/core";
|
||||||
|
|
||||||
export interface ResizeHandleProps {
|
export interface ResizeHandleProps {
|
||||||
|
|
@ -45,6 +50,13 @@ export function ResizeHandle({
|
||||||
// Gesture origin, captured on pointer down. `null` while idle.
|
// Gesture origin, captured on pointer down. `null` while idle.
|
||||||
const drag = useRef<{ startX: number; startWidth: number } | null>(null);
|
const drag = useRef<{ startX: number; startWidth: number } | null>(null);
|
||||||
|
|
||||||
|
// If the handle unmounts mid-drag — the window narrows past the split
|
||||||
|
// breakpoint or the server disconnects, either of which removes the handle
|
||||||
|
// from the tree before a pointerup/cancel fires — the drag body class would
|
||||||
|
// otherwise linger and leave `user-select: none` + `col-resize` applied
|
||||||
|
// page-wide. Clear it on unmount so the class is self-healing.
|
||||||
|
useEffect(() => () => document.body.classList.remove("resizing-col"), []);
|
||||||
|
|
||||||
function handlePointerDown(e: PointerEvent<HTMLDivElement>) {
|
function handlePointerDown(e: PointerEvent<HTMLDivElement>) {
|
||||||
drag.current = { startX: e.clientX, startWidth: value };
|
drag.current = { startX: e.clientX, startWidth: value };
|
||||||
// Optional chaining: happy-dom (unit tests) doesn't implement pointer
|
// Optional chaining: happy-dom (unit tests) doesn't implement pointer
|
||||||
|
|
|
||||||
|
|
@ -215,15 +215,26 @@ export function HistoryListPanel({
|
||||||
const [pinnedOpen, setPinnedOpen] = useState(true);
|
const [pinnedOpen, setPinnedOpen] = useState(true);
|
||||||
const [historyOpen, setHistoryOpen] = useState(true);
|
const [historyOpen, setHistoryOpen] = useState(true);
|
||||||
const filteredEntries = useMemo(() => {
|
const filteredEntries = useMemo(() => {
|
||||||
// `.filter()` returns a fresh array, so sorting in-place is safe.
|
// Embedded column: show the full stream (no filter sidebar to explain a
|
||||||
|
// mirrored filter). See LogStreamPanel (#1616). `.filter()` returns a fresh
|
||||||
|
// array, so sorting in-place is safe.
|
||||||
const sorted = entries
|
const sorted = entries
|
||||||
.filter((e) =>
|
.filter(
|
||||||
matchesFilters(e, searchText, visibleDirections, methodFilter),
|
(e) =>
|
||||||
|
embedded ||
|
||||||
|
matchesFilters(e, searchText, visibleDirections, methodFilter),
|
||||||
)
|
)
|
||||||
.sort((a, b) => a.timestamp.getTime() - b.timestamp.getTime());
|
.sort((a, b) => a.timestamp.getTime() - b.timestamp.getTime());
|
||||||
if (sortDirection === "newest-first") sorted.reverse();
|
if (sortDirection === "newest-first") sorted.reverse();
|
||||||
return sorted;
|
return sorted;
|
||||||
}, [entries, searchText, visibleDirections, methodFilter, sortDirection]);
|
}, [
|
||||||
|
entries,
|
||||||
|
searchText,
|
||||||
|
visibleDirections,
|
||||||
|
methodFilter,
|
||||||
|
sortDirection,
|
||||||
|
embedded,
|
||||||
|
]);
|
||||||
|
|
||||||
const pinnedEntries = useMemo(
|
const pinnedEntries = useMemo(
|
||||||
() => filteredEntries.filter((e) => pinnedIds.has(e.id)),
|
() => filteredEntries.filter((e) => pinnedIds.has(e.id)),
|
||||||
|
|
|
||||||
|
|
@ -80,13 +80,16 @@ export function LogStreamPanel({
|
||||||
}: LogStreamPanelProps) {
|
}: LogStreamPanelProps) {
|
||||||
const viewportRef = useScrollMemory("logs-stream");
|
const viewportRef = useScrollMemory("logs-stream");
|
||||||
const filteredEntries = useMemo(() => {
|
const filteredEntries = useMemo(() => {
|
||||||
// `.filter()` returns a fresh array, so sorting in-place is safe.
|
// The embedded column has no filter sidebar, so it shows the full stream
|
||||||
|
// rather than mirroring the full-size screen's live filter with no visible
|
||||||
|
// control to explain it (#1616). `.filter()` returns a fresh array, so
|
||||||
|
// sorting in-place is safe.
|
||||||
const sorted = entries
|
const sorted = entries
|
||||||
.filter((e) => matchesFilters(e, filterText, visibleLevels))
|
.filter((e) => embedded || matchesFilters(e, filterText, visibleLevels))
|
||||||
.sort((a, b) => a.receivedAt.getTime() - b.receivedAt.getTime());
|
.sort((a, b) => a.receivedAt.getTime() - b.receivedAt.getTime());
|
||||||
if (sortDirection === "newest-first") sorted.reverse();
|
if (sortDirection === "newest-first") sorted.reverse();
|
||||||
return sorted;
|
return sorted;
|
||||||
}, [entries, filterText, visibleLevels, sortDirection]);
|
}, [entries, filterText, visibleLevels, sortDirection, embedded]);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<PanelContainer>
|
<PanelContainer>
|
||||||
|
|
|
||||||
|
|
@ -33,6 +33,20 @@ describe("MonitoringControls", () => {
|
||||||
expect(screen.queryByRole("radio", { name: "Network" })).toBeNull();
|
expect(screen.queryByRole("radio", { name: "Network" })).toBeNull();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("gives the tab switcher an accessible name", () => {
|
||||||
|
const { container } = renderWithMantine(
|
||||||
|
<MonitoringControls
|
||||||
|
tabs={TABS}
|
||||||
|
value="Logs"
|
||||||
|
onChange={vi.fn()}
|
||||||
|
onClose={vi.fn()}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
expect(
|
||||||
|
container.querySelector('[aria-label="Monitoring screen"]'),
|
||||||
|
).not.toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
it("marks the active tab as selected", () => {
|
it("marks the active tab as selected", () => {
|
||||||
renderWithMantine(
|
renderWithMantine(
|
||||||
<MonitoringControls
|
<MonitoringControls
|
||||||
|
|
|
||||||
|
|
@ -39,6 +39,7 @@ export function MonitoringControls({
|
||||||
value={value}
|
value={value}
|
||||||
onChange={onChange}
|
onChange={onChange}
|
||||||
data={tabs}
|
data={tabs}
|
||||||
|
aria-label="Monitoring screen"
|
||||||
/>
|
/>
|
||||||
<CloseButton aria-label="Close monitoring column" onClick={onClose} />
|
<CloseButton aria-label="Close monitoring column" onClick={onClose} />
|
||||||
</ControlsBar>
|
</ControlsBar>
|
||||||
|
|
|
||||||
|
|
@ -98,13 +98,17 @@ export function NetworkStreamPanel({
|
||||||
}: NetworkStreamPanelProps) {
|
}: NetworkStreamPanelProps) {
|
||||||
const viewportRef = useScrollMemory("network-stream");
|
const viewportRef = useScrollMemory("network-stream");
|
||||||
const filteredEntries = useMemo(() => {
|
const filteredEntries = useMemo(() => {
|
||||||
// `.filter()` returns a fresh array, so sorting in-place is safe.
|
// Embedded column: show the full stream (no filter sidebar to explain a
|
||||||
|
// mirrored filter). See LogStreamPanel (#1616). `.filter()` returns a fresh
|
||||||
|
// array, so sorting in-place is safe.
|
||||||
const sorted = entries
|
const sorted = entries
|
||||||
.filter((e) => matchesFilters(e, filterText, visibleCategories))
|
.filter(
|
||||||
|
(e) => embedded || matchesFilters(e, filterText, visibleCategories),
|
||||||
|
)
|
||||||
.sort((a, b) => a.timestamp.getTime() - b.timestamp.getTime());
|
.sort((a, b) => a.timestamp.getTime() - b.timestamp.getTime());
|
||||||
if (sortDirection === "newest-first") sorted.reverse();
|
if (sortDirection === "newest-first") sorted.reverse();
|
||||||
return sorted;
|
return sorted;
|
||||||
}, [entries, filterText, visibleCategories, sortDirection]);
|
}, [entries, filterText, visibleCategories, sortDirection, embedded]);
|
||||||
|
|
||||||
const hasEntries = entries.length > 0;
|
const hasEntries = entries.length > 0;
|
||||||
const hasResults = filteredEntries.length > 0;
|
const hasResults = filteredEntries.length > 0;
|
||||||
|
|
|
||||||
|
|
@ -126,4 +126,17 @@ describe("HistoryScreen", () => {
|
||||||
// The sidebar (HistoryControls, with its Search box) is not rendered.
|
// The sidebar (HistoryControls, with its Search box) is not rendered.
|
||||||
expect(screen.queryByPlaceholderText("Search...")).toBeNull();
|
expect(screen.queryByPlaceholderText("Search...")).toBeNull();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("shows the full stream when embedded, ignoring the live filter", () => {
|
||||||
|
// A search that matches nothing would empty the full-size screen...
|
||||||
|
renderWithMantine(
|
||||||
|
<HistoryScreen
|
||||||
|
{...baseProps}
|
||||||
|
ui={{ ...EMPTY_HISTORY_UI, search: "zzz-no-match" }}
|
||||||
|
embedded
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
// ...but the embedded column shows the full stream (not the empty state).
|
||||||
|
expect(screen.queryByText("No request history")).toBeNull();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -146,4 +146,24 @@ describe("LoggingScreen", () => {
|
||||||
// The sidebar (LogControls, with its Set button) is not rendered.
|
// The sidebar (LogControls, with its Set button) is not rendered.
|
||||||
expect(screen.queryByRole("button", { name: "Set" })).toBeNull();
|
expect(screen.queryByRole("button", { name: "Set" })).toBeNull();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("shows the full stream when embedded, ignoring the live filter", () => {
|
||||||
|
const entries = [
|
||||||
|
{
|
||||||
|
receivedAt: new Date(),
|
||||||
|
params: { level: "info" as const, data: "hello" },
|
||||||
|
},
|
||||||
|
];
|
||||||
|
// A filter that would hide the entry on the full-size screen...
|
||||||
|
renderWithMantine(
|
||||||
|
<LoggingScreen
|
||||||
|
{...baseProps}
|
||||||
|
entries={entries}
|
||||||
|
ui={{ ...EMPTY_LOGS_UI, filterText: "zzz-no-match" }}
|
||||||
|
embedded
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
// ...is ignored in the embedded, sidebar-less column.
|
||||||
|
expect(screen.getByText("hello")).toBeInTheDocument();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -179,4 +179,17 @@ describe("NetworkScreen", () => {
|
||||||
// The stream toolbar (Export) is still present.
|
// The stream toolbar (Export) is still present.
|
||||||
expect(screen.getByRole("button", { name: "Export" })).toBeInTheDocument();
|
expect(screen.getByRole("button", { name: "Export" })).toBeInTheDocument();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("shows the full stream when embedded, ignoring the live filter", () => {
|
||||||
|
// A filter that matches nothing would empty the full-size screen...
|
||||||
|
renderWithMantine(
|
||||||
|
<NetworkScreen
|
||||||
|
{...baseProps}
|
||||||
|
ui={{ ...EMPTY_NETWORK_UI, filterText: "zzz-no-match" }}
|
||||||
|
embedded
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
// ...but the embedded column shows the full stream (not the empty state).
|
||||||
|
expect(screen.queryByText("No network requests")).toBeNull();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue