Skip to content

fix(web): add dismiss control for error toasts #897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
binbandit wants to merge 2 commits into
base: main
Choose a base branch
from
+159 −69
Open

fix(web): add dismiss control for error toasts #897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a205539
fix(web): add dismiss control for error toasts
binbandit Mar 11, 2026
c73c714
test(web): stabilize keybindings toast browser coverage
binbandit Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 63 additions & 1 deletion apps/web/src/components/KeybindingsToast.browser.tsx
View file
Open in desktop
Copy link

@macroscopeapp macroscopeapp bot Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low

Issue on line in apps/web/src/components/KeybindingsToast.browser.tsx:419:

In the catch block at line 421, remounted.cleanup() is never called when an error occurs between lines 408–419. The cleanup only calls mounted.cleanup(), but mounted was already cleaned at line 407, leaving remounted resources leaked. Ensure remounted.cleanup() is called in the catch block before re-throwing.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent: 
In file apps/web/src/components/KeybindingsToast.browser.tsx around line 419:

In the catch block at line 421, `remounted.cleanup()` is never called when an error occurs between lines 408–419. The cleanup only calls `mounted.cleanup()`, but `mounted` was already cleaned at line 407, leaving `remounted` resources leaked. Ensure `remounted.cleanup()` is called in the catch block before re-throwing.

Evidence trail:
apps/web/src/components/KeybindingsToast.browser.tsx lines 400-426 at REVIEWED_COMMIT: Line 407 calls `await mounted.cleanup()`, line 408 creates `const remounted = await mountApp()`, line 420 calls `await remounted.cleanup()` in happy path, but catch block at lines 421-423 only calls `await mounted.cleanup().catch(() => {})` - missing `remounted.cleanup()` for errors between lines 408-419.

Copy link
Contributor Author

@binbandit binbandit Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c73c714. The catch path now cleans up remounted before rethrowing, and I isolated this browser suite from the diff worker pool so CI no longer fails on unrelated worker initialization during repeated mount/unmount cycles.

Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,14 @@ import {
import { RouterProvider, createMemoryHistory } from "@tanstack/react-router";
import { ws, http, HttpResponse } from "msw";
import { setupWorker } from "msw/browser";
import type { ReactNode } from "react";
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import { render } from "vitest-browser-react";

vi.mock("../components/DiffWorkerPoolProvider", () => ({
DiffWorkerPoolProvider: ({ children }: { children?: ReactNode }) => children ?? null,
}));

import { useComposerDraftStore } from "../composerDraftStore";
import { getRouter } from "../router";
import { useStore } from "../store";
Expand All @@ -26,6 +31,7 @@ const PROJECT_ID = "project-1" as ProjectId;
const NOW_ISO = "2026-03-04T12:00:00.000Z";

interface TestFixture {
openInEditorErrorMessage: string | null;
snapshot: OrchestrationReadModel;
serverConfig: ServerConfig;
welcome: WsWelcomePayload;
Expand Down Expand Up @@ -116,6 +122,7 @@ function createMinimalSnapshot(): OrchestrationReadModel {

function buildFixture(): TestFixture {
return {
openInEditorErrorMessage: null,
snapshot: createMinimalSnapshot(),
serverConfig: createBaseServerConfig(),
welcome: {
Expand Down Expand Up @@ -181,6 +188,17 @@ const worker = setupWorker(
}
const method = request.body?._tag;
if (typeof method !== "string") return;
if (method === WS_METHODS.shellOpenInEditor && fixture.openInEditorErrorMessage) {
client.send(
JSON.stringify({
id: request.id,
error: {
message: fixture.openInEditorErrorMessage,
},
}),
);
return;
}
client.send(
JSON.stringify({
id: request.id,
Expand Down Expand Up @@ -214,6 +232,10 @@ function queryToastTitles(): string[] {
);
}

function queryDismissButtons(): HTMLButtonElement[] {
return Array.from(document.querySelectorAll<HTMLButtonElement>('[data-slot="toast-close"]'));
}

async function waitForElement<T extends Element>(
query: () => T | null,
errorMessage: string,
Expand Down Expand Up @@ -293,6 +315,7 @@ describe("Keybindings update toast", () => {
});

beforeEach(() => {
fixture = buildFixture();
localStorage.clear();
document.body.innerHTML = "";
pushSequence = 1;
Expand Down Expand Up @@ -340,8 +363,45 @@ describe("Keybindings update toast", () => {
}
});

it("lets users dismiss the follow-up error toast from the keybindings warning action", async () => {
fixture.openInEditorErrorMessage = "Editor unavailable";
const mounted = await mountApp();

try {
sendServerConfigUpdatedPush([
{ kind: "keybindings.malformed-config", message: "Expected JSON array" },
]);
await waitForToast("Invalid keybindings configuration");
expect(queryDismissButtons()).toHaveLength(0);

const openButton = await waitForElement(
() =>
Array.from(
document.querySelectorAll<HTMLButtonElement>('[data-slot="toast-action"]'),
).find((element) => element.textContent === "Open keybindings.json") ?? null,
"Warning toast should render its action button",
);
openButton.click();

await waitForToast("Unable to open keybindings file");
await vi.waitFor(
() => {
expect(queryDismissButtons()).toHaveLength(1);
},
{ timeout: 4_000, interval: 16 },
);

queryDismissButtons()[0]?.click();
await waitForNoToast("Unable to open keybindings file");
} finally {
fixture.openInEditorErrorMessage = null;
await mounted.cleanup();
}
});

it("does not show a toast from the replayed cached value on subscribe", async () => {
const mounted = await mountApp();
let remounted: Awaited<ReturnType<typeof mountApp>> | null = null;

try {
sendServerConfigUpdatedPush([]);
Expand All @@ -351,7 +411,7 @@ describe("Keybindings update toast", () => {
// Remount the app — onServerConfigUpdated replays the cached value
// synchronously on subscribe. This should NOT produce a toast.
await mounted.cleanup();
const remounted = await mountApp();
remounted = await mountApp();

// Give it a moment to process the replayed value
await new Promise((resolve) => setTimeout(resolve, 500));
Expand All @@ -363,7 +423,9 @@ describe("Keybindings update toast", () => {
).toBe(0);

await remounted.cleanup();
remounted = null;
} catch (error) {
await remounted?.cleanup().catch(() => {});
await mounted.cleanup().catch(() => {});
throw error;
}
Expand Down
164 changes: 96 additions & 68 deletions apps/web/src/components/ui/toast.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"use client";

import { Toast } from "@base-ui/react/toast";
import { useEffect, type CSSProperties } from "react";
import { useEffect, type CSSProperties, type ReactNode } from "react";
import { useParams } from "@tanstack/react-router";
import { ThreadId } from "@t3tools/contracts";
import {
Expand All @@ -10,10 +10,12 @@ import {
InfoIcon,
LoaderCircleIcon,
TriangleAlertIcon,
type LucideIcon,
XIcon,
} from "lucide-react";

import { cn } from "~/lib/utils";
import { buttonVariants } from "~/components/ui/button";
import { Button, buttonVariants } from "~/components/ui/button";
import { buildVisibleToastLayout, shouldHideCollapsedToastContent } from "./toast.logic";

type ThreadToastData = {
Expand Down Expand Up @@ -142,6 +144,82 @@ function ThreadToastVisibleAutoDismiss({
return null;
}

function ErrorToastDismissButton() {
return (
<Toast.Close
aria-label="Dismiss error toast"
className="-mt-1 -mr-1 text-muted-foreground/80 hover:text-foreground"
data-slot="toast-close"
render={<Button size="icon-xs" variant="ghost" />}
>
<XIcon className="size-3.5" />
</Toast.Close>
);
}

function ToastCardContent({
actionLabel,
className,
hideCollapsedContent = false,
icon: Icon,
showDismissButton = false,
}: {
actionLabel?: ReactNode | undefined;
className?: string | undefined;
hideCollapsedContent?: boolean | undefined;
icon?: LucideIcon | null | undefined;
showDismissButton?: boolean | undefined;
}) {
const actionButton = actionLabel ? (
<Toast.Action
className={cn(buttonVariants({ size: "xs" }), "shrink-0")}
data-slot="toast-action"
>
{actionLabel}
</Toast.Action>
) : null;

return (
<Toast.Content
className={cn(
"pointer-events-auto overflow-hidden px-3.5 py-3 text-sm",
showDismissButton
? "flex items-start justify-between gap-2"
: "flex items-center justify-between gap-1.5",
hideCollapsedContent && "not-data-expanded:pointer-events-none not-data-expanded:opacity-0",
className,
)}
>
<div className="flex min-w-0 flex-1 gap-2">
{Icon && (
<div
className="[&>svg]:h-lh [&>svg]:w-4 [&_svg]:pointer-events-none [&_svg]:shrink-0"
data-slot="toast-icon"
>
<Icon className="in-data-[type=loading]:animate-spin in-data-[type=error]:text-destructive in-data-[type=info]:text-info in-data-[type=success]:text-success in-data-[type=warning]:text-warning in-data-[type=loading]:opacity-80" />
</div>
)}

<div className="flex min-w-0 flex-1 flex-col gap-0.5">
<Toast.Title className="min-w-0 break-words font-medium" data-slot="toast-title" />
<Toast.Description
className="min-w-0 break-words text-muted-foreground"
data-slot="toast-description"
/>
</div>
</div>
{showDismissButton ? (
<div className="flex shrink-0 flex-col items-end gap-2">
<ErrorToastDismissButton />
{actionButton}
</div>
) : (
actionButton
)}
</Toast.Content>
);
}

function ToastProvider({ children, position = "top-right", ...props }: ToastProviderProps) {
return (
<Toast.Provider toastManager={toastManager} {...props}>
Expand Down Expand Up @@ -196,6 +274,7 @@ function Toasts({ position = "top-right" }: { position: ToastPosition }) {
visibleIndex,
visibleToastLayout.items.length,
);
const showDismissButton = toast.type === "error";

return (
<Toast.Root
Expand Down Expand Up @@ -266,43 +345,16 @@ function Toasts({ position = "top-right" }: { position: ToastPosition }) {
dismissAfterVisibleMs={toast.data?.dismissAfterVisibleMs}
toastId={toast.id}
/>
<Toast.Content
<ToastCardContent
actionLabel={toast.actionProps?.children}
className={cn(
"pointer-events-auto flex items-center justify-between gap-1.5 overflow-hidden px-3.5 py-3 text-sm transition-opacity duration-250 data-expanded:opacity-100",
hideCollapsedContent &&
"not-data-expanded:pointer-events-none not-data-expanded:opacity-0",
"transition-opacity duration-250 data-expanded:opacity-100",
showDismissButton && "pr-2",
)}
>
<div className="flex min-w-0 flex-1 gap-2">
{Icon && (
<div
className="[&>svg]:h-lh [&>svg]:w-4 [&_svg]:pointer-events-none [&_svg]:shrink-0"
data-slot="toast-icon"
>
<Icon className="in-data-[type=loading]:animate-spin in-data-[type=error]:text-destructive in-data-[type=info]:text-info in-data-[type=success]:text-success in-data-[type=warning]:text-warning in-data-[type=loading]:opacity-80" />
</div>
)}

<div className="flex min-w-0 flex-1 flex-col gap-0.5">
<Toast.Title
className="min-w-0 break-words font-medium"
data-slot="toast-title"
/>
<Toast.Description
className="min-w-0 break-words text-muted-foreground"
data-slot="toast-description"
/>
</div>
</div>
{toast.actionProps && (
<Toast.Action
className={cn(buttonVariants({ size: "xs" }), "shrink-0")}
data-slot="toast-action"
>
{toast.actionProps.children}
</Toast.Action>
)}
</Toast.Content>
hideCollapsedContent={hideCollapsedContent}
icon={Icon}
showDismissButton={showDismissButton}
/>
</Toast.Root>
);
})}
Expand Down Expand Up @@ -333,6 +385,7 @@ function AnchoredToasts() {
const Icon = toast.type ? TOAST_ICONS[toast.type as keyof typeof TOAST_ICONS] : null;
const tooltipStyle = toast.data?.tooltipStyle ?? false;
const positionerProps = toast.positionerProps;
const showDismissButton = toast.type === "error";

if (!positionerProps?.anchor) {
return null;
Expand Down Expand Up @@ -361,37 +414,12 @@ function AnchoredToasts() {
<Toast.Title data-slot="toast-title" />
</Toast.Content>
) : (
<Toast.Content className="pointer-events-auto flex items-center justify-between gap-1.5 overflow-hidden px-3.5 py-3 text-sm">
<div className="flex min-w-0 flex-1 gap-2">
{Icon && (
<div
className="[&>svg]:h-lh [&>svg]:w-4 [&_svg]:pointer-events-none [&_svg]:shrink-0"
data-slot="toast-icon"
>
<Icon className="in-data-[type=loading]:animate-spin in-data-[type=error]:text-destructive in-data-[type=info]:text-info in-data-[type=success]:text-success in-data-[type=warning]:text-warning in-data-[type=loading]:opacity-80" />
</div>
)}

<div className="flex min-w-0 flex-1 flex-col gap-0.5">
<Toast.Title
className="min-w-0 break-words font-medium"
data-slot="toast-title"
/>
<Toast.Description
className="min-w-0 break-words text-muted-foreground"
data-slot="toast-description"
/>
</div>
</div>
{toast.actionProps && (
<Toast.Action
className={cn(buttonVariants({ size: "xs" }), "shrink-0")}
data-slot="toast-action"
>
{toast.actionProps.children}
</Toast.Action>
)}
</Toast.Content>
<ToastCardContent
actionLabel={toast.actionProps?.children}
className={showDismissButton ? "pr-2" : undefined}
icon={Icon}
showDismissButton={showDismissButton}
/>
)}
</Toast.Root>
</Toast.Positioner>
Expand Down
Loading