-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Polish collapsible Fix with AI error banner #4351
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.
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
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,9 @@ | ||
| - button: | ||
| - img | ||
| - img | ||
| - text: Error Line 6 error | ||
| - img | ||
| - text: "Tip: Check if restarting the app fixes the error." | ||
| - paragraph: Line 6 error | ||
| - button "Collapse error banner" [expanded] | ||
| - button "Dismiss error banner" | ||
| - button "Show details" | ||
| - text: Try restarting the app. | ||
| - button "Copy": | ||
| - img | ||
| - text: "" | ||
| - button "Fix error with AI": | ||
| - img | ||
| - text: "" | ||
| - button "Fix error with AI" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,9 @@ | ||
| - button: | ||
| - img | ||
| - img | ||
| - text: "/Error Line 6 error Stack trace: Index \\(http:\\/\\/localhost:\\d+\\/src\\/pages\\/Index\\.tsx:6:6\\)/" | ||
| - img | ||
| - text: "Tip: Check if restarting the app fixes the error." | ||
| - paragraph: Line 6 error | ||
| - button "Collapse error banner" [expanded] | ||
| - button "Dismiss error banner" | ||
| - button "Hide details" [expanded] | ||
| - text: "/Error Line 6 error Stack trace: Index \\(http:\\/\\/localhost:\\d+\\/src\\/pages\\/Index\\.tsx:6:6\\) Try restarting the app\\./" | ||
| - button "Copy": | ||
| - img | ||
| - text: "" | ||
| - button "Fix error with AI": | ||
| - img | ||
| - text: "" | ||
| - button "Fix error with AI" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| import { fireEvent, render, screen } from "@testing-library/react"; | ||
| import { describe, expect, it, vi } from "vitest"; | ||
| import { PreviewErrorBanner } from "./PreviewErrorBanner"; | ||
|
|
||
| vi.mock("@/hooks/useStreamChat", () => ({ | ||
| useStreamChat: () => ({ isStreaming: false }), | ||
| })); | ||
|
|
||
| vi.mock("@/components/CopyErrorMessage", () => ({ | ||
| CopyErrorMessage: () => <button type="button">Copy</button>, | ||
| })); | ||
|
|
||
| const previewError = { | ||
| message: "Error Line 6 error\nStack trace: Index.tsx:6:6", | ||
| source: "preview-app" as const, | ||
| }; | ||
|
|
||
| describe("PreviewErrorBanner", () => { | ||
| it("collapses to a compact summary and can be expanded again", () => { | ||
| render( | ||
| <PreviewErrorBanner | ||
| error={previewError} | ||
| onDismiss={vi.fn()} | ||
| onAIFix={vi.fn()} | ||
| />, | ||
| ); | ||
|
|
||
| expect(screen.getByText("Try restarting the app.")).toBeTruthy(); | ||
| expect( | ||
| screen.getByRole("button", { name: "Fix error with AI" }), | ||
| ).toBeTruthy(); | ||
|
|
||
| fireEvent.click( | ||
| screen.getByRole("button", { name: "Collapse error banner" }), | ||
| ); | ||
|
|
||
| expect(screen.getByTestId("preview-error-banner")).toBeTruthy(); | ||
| expect(screen.getByText("Line 6 error")).toBeTruthy(); | ||
| expect(screen.queryByText("Try restarting the app.")).toBeNull(); | ||
| expect( | ||
| screen.queryByRole("button", { name: "Fix error with AI" }), | ||
| ).toBeNull(); | ||
|
|
||
| fireEvent.click( | ||
| screen.getByRole("button", { name: "Expand error banner" }), | ||
| ); | ||
|
|
||
| expect(screen.getByText("Try restarting the app.")).toBeTruthy(); | ||
| expect( | ||
| screen.getByRole("button", { name: "Fix error with AI" }), | ||
| ).toBeTruthy(); | ||
| }); | ||
|
|
||
| it("keeps dismissal separate from collapsing", () => { | ||
| const onDismiss = vi.fn(); | ||
| render( | ||
| <PreviewErrorBanner | ||
| error={previewError} | ||
| onDismiss={onDismiss} | ||
| onAIFix={vi.fn()} | ||
| />, | ||
| ); | ||
|
|
||
| fireEvent.click( | ||
| screen.getByRole("button", { name: "Dismiss error banner" }), | ||
| ); | ||
|
|
||
| expect(onDismiss).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it("still reveals the full error message independently", () => { | ||
| render( | ||
| <PreviewErrorBanner | ||
| error={previewError} | ||
| onDismiss={vi.fn()} | ||
| onAIFix={vi.fn()} | ||
| />, | ||
| ); | ||
|
|
||
| expect(screen.queryByText(/Stack trace/)).toBeNull(); | ||
| fireEvent.click(screen.getByRole("button", { name: "Show details" })); | ||
|
|
||
| expect(screen.getByText(/Stack trace/)).toBeTruthy(); | ||
| expect(screen.getByRole("button", { name: "Hide details" })).toBeTruthy(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| import { useState } from "react"; | ||
| import { | ||
| CircleAlert, | ||
| ChevronDown, | ||
| ChevronUp, | ||
| Lightbulb, | ||
| Sparkles, | ||
| X, | ||
| } from "lucide-react"; | ||
| import { CopyErrorMessage } from "@/components/CopyErrorMessage"; | ||
| import { useStreamChat } from "@/hooks/useStreamChat"; | ||
|
|
||
| interface PreviewErrorBannerProps { | ||
| error: | ||
| | { | ||
| message: string; | ||
| source: "preview-app" | "dyad-app" | "dyad-sync"; | ||
| } | ||
| | undefined; | ||
| onDismiss: () => void; | ||
| onAIFix: () => void; | ||
| } | ||
|
|
||
| export function PreviewErrorBanner({ | ||
| error, | ||
| onDismiss, | ||
| onAIFix, | ||
| }: PreviewErrorBannerProps) { | ||
| const [isBannerCollapsed, setIsBannerCollapsed] = useState(false); | ||
|
keppo-bot[bot] marked this conversation as resolved.
keppo-bot[bot] marked this conversation as resolved.
keppo-bot[bot] marked this conversation as resolved.
|
||
| const [areErrorDetailsVisible, setAreErrorDetailsVisible] = useState(false); | ||
| const { isStreaming } = useStreamChat(); | ||
|
|
||
| if (!error) return null; | ||
|
|
||
| const isDockerError = error.message.includes("Cannot connect to the Docker"); | ||
| const isInternalDyadError = error.source === "dyad-app"; | ||
| const isSyncError = error.source === "dyad-sync"; | ||
|
|
||
| const firstLine = error.message.split("\n")[0]; | ||
| const summaryWithoutErrorPrefix = firstLine.replace(/^Error:?\s+/i, ""); | ||
| const errorSummary = summaryWithoutErrorPrefix || firstLine; | ||
|
|
||
| return ( | ||
| <div | ||
| className="absolute top-2 left-2 right-2 z-10 rounded-md border border-red-200 bg-red-50 p-3 shadow-sm dark:border-red-800 dark:bg-red-950" | ||
| data-testid="preview-error-banner" | ||
| > | ||
| <div className="flex items-start gap-2"> | ||
| <CircleAlert | ||
| aria-hidden="true" | ||
| size={16} | ||
| className="mt-0.5 shrink-0 text-red-600 dark:text-red-400" | ||
| /> | ||
|
|
||
| <div className="min-w-0 flex-1"> | ||
| <div className="flex min-w-0 items-center gap-2"> | ||
| <p | ||
| className="truncate text-sm font-medium text-red-800 dark:text-red-200" | ||
| title={error.message} | ||
| > | ||
| {errorSummary} | ||
| </p> | ||
| {(isInternalDyadError || isSyncError) && ( | ||
| <span className="shrink-0 rounded bg-red-100 px-1.5 py-0.5 text-xs font-medium text-red-700 dark:bg-red-900 dark:text-red-300"> | ||
| {isSyncError ? "Cloud sync issue" : "Internal Dyad error"} | ||
| </span> | ||
| )} | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="flex shrink-0 items-center gap-0.5"> | ||
| <button | ||
| type="button" | ||
| onClick={() => setIsBannerCollapsed((collapsed) => !collapsed)} | ||
| aria-label={ | ||
| isBannerCollapsed | ||
| ? "Expand error banner" | ||
| : "Collapse error banner" | ||
| } | ||
| aria-expanded={!isBannerCollapsed} | ||
| aria-controls="preview-error-banner-content" | ||
| className="rounded p-1 text-red-500 transition-colors hover:bg-red-100 hover:text-red-700 dark:text-red-400 dark:hover:bg-red-900 dark:hover:text-red-200" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM New banner controls are missing cursor-pointer The collapse toggle (line 76), the dismiss button (line 89), and the "Show details" toggle (line 100) have no cursor-pointer class, while the "Fix error with AI" button in the same banner explicitly sets it. This project is on Tailwind v4, whose preflight no longer applies cursor: pointer to buttons, and there is no global CSS rule restoring it, so these three new affordances will show the default arrow cursor. Sibling components such as PreviewLoadingScreen's error toggle set cursor-pointer explicitly for exactly this reason, so the banner ends up inconsistent with the rest of the preview panel in a PR whose goal is polish. 💡 Suggestion: Add cursor-pointer to the collapse, dismiss, and Show details button class strings. |
||
| data-testid="preview-error-banner-toggle" | ||
| > | ||
| {isBannerCollapsed ? ( | ||
| <ChevronDown aria-hidden="true" size={14} /> | ||
| ) : ( | ||
| <ChevronUp aria-hidden="true" size={14} /> | ||
| )} | ||
| </button> | ||
| <button | ||
| type="button" | ||
| onClick={onDismiss} | ||
| aria-label="Dismiss error banner" | ||
| className="rounded p-1 text-red-500 transition-colors hover:bg-red-100 hover:text-red-700 dark:text-red-400 dark:hover:bg-red-900 dark:hover:text-red-200" | ||
| > | ||
| <X aria-hidden="true" size={14} /> | ||
| </button> | ||
| </div> | ||
| </div> | ||
|
|
||
| {!isBannerCollapsed && ( | ||
| <div id="preview-error-banner-content" className="mt-2 pl-6"> | ||
| <button | ||
| type="button" | ||
| className="text-xs font-medium text-red-700 underline-offset-2 hover:text-red-900 hover:underline dark:text-red-300 dark:hover:text-red-100" | ||
| onClick={() => | ||
| setAreErrorDetailsVisible((detailsVisible) => !detailsVisible) | ||
| } | ||
| aria-expanded={areErrorDetailsVisible} | ||
| > | ||
| {areErrorDetailsVisible ? "Hide details" : "Show details"} | ||
| </button> | ||
|
|
||
| {areErrorDetailsVisible && ( | ||
| <pre className="mt-1 max-h-40 overflow-auto whitespace-pre-wrap break-words font-mono text-xs text-red-700 dark:text-red-300"> | ||
| {error.message} | ||
| </pre> | ||
| )} | ||
|
|
||
| <div className="mt-2 flex items-start gap-2 text-sm text-red-700 dark:text-red-200"> | ||
| <Lightbulb | ||
| aria-hidden="true" | ||
| size={15} | ||
| className="mt-0.5 shrink-0 text-red-600 dark:text-red-300" | ||
| /> | ||
| <span> | ||
| {isDockerError | ||
| ? "Make sure Docker Desktop is running and try restarting the app." | ||
| : isSyncError | ||
| ? "Dyad could not upload your latest local changes to the cloud sandbox. Check your network connection or wait for sync to recover." | ||
| : isInternalDyadError | ||
| ? "Try restarting the Dyad app or your computer." | ||
| : "Try restarting the app."} | ||
| </span> | ||
| </div> | ||
|
|
||
| {!isDockerError && error.source === "preview-app" && ( | ||
| <div className="mt-3 flex justify-end gap-2"> | ||
| <CopyErrorMessage errorMessage={error.message} /> | ||
| <button | ||
| type="button" | ||
| disabled={isStreaming} | ||
| onClick={onAIFix} | ||
| className="flex cursor-pointer items-center gap-1 rounded bg-red-500 px-2 py-1 text-sm text-white transition-colors hover:bg-red-600 disabled:cursor-not-allowed disabled:opacity-50 dark:bg-red-600 dark:hover:bg-red-700" | ||
| > | ||
| <Sparkles aria-hidden="true" size={14} /> | ||
| <span>Fix error with AI</span> | ||
| </button> | ||
| </div> | ||
| )} | ||
| </div> | ||
| )} | ||
| </div> | ||
| ); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.