-
Notifications
You must be signed in to change notification settings - Fork 0
feat(fork): the design send says which elements it could not address #71
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 all 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 |
|---|---|---|
|
|
@@ -9,11 +9,12 @@ import { designModeBridge } from "../designModeBridge"; | |
| import { selectDesignModeTab, useDesignModeStore } from "../designModeStore"; | ||
| import { applyDesignUndoEntry } from "../designUndoApply"; | ||
| import { designUndoHistory } from "../designUndoHistory"; | ||
| import type { | ||
| DesignModeAlignAxis, | ||
| DesignModeAlignValue, | ||
| DesignModeSizeMode, | ||
| DesignModeWritableKey, | ||
| import { | ||
| countUnresolvedDesignElements, | ||
| type DesignModeAlignAxis, | ||
| type DesignModeAlignValue, | ||
| type DesignModeSizeMode, | ||
| type DesignModeWritableKey, | ||
| } from "../protocol"; | ||
| import { CanvasControls } from "./CanvasControls"; | ||
| import { AppearanceSection } from "./sections/AppearanceSection"; | ||
|
|
@@ -222,13 +223,20 @@ export function ForkDesignPanel({ runtimeTabId, threadRef, tabId }: Props) { | |
| // navigation adds one — the drafts behind it are a different page's. See | ||
| // designChangeDraftStore's `add`. | ||
| useDesignChangeDraftStore.getState().add(threadRef, runtimeTabId, result); | ||
| // Precision is part of the receipt: buildSend's ~1.5s native-source grace can expire | ||
| // and downgrade elements to selector/text context. Say so here rather than letting | ||
| // WHEN Send was clicked silently change what the agent gets (the pill repeats it). | ||
| const unresolved = countUnresolvedDesignElements(result); | ||
|
Owner
Author
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. Worth a second look before merge: the framing here ("buildSend's grace can expire") only holds when resolution could have succeeded. The result is a permanent, unactionable nag on a surface the fork deliberately de-nagged:
Generated by Claude Code |
||
| toastManager.add({ | ||
| type: "success", | ||
| title: | ||
| result.elementCount === 1 | ||
| ? "Design change attached" | ||
| : `Design changes for ${result.elementCount} elements attached`, | ||
| description: "It rides along with your next message — add a comment or just press Enter.", | ||
| description: | ||
| unresolved > 0 | ||
| ? `${unresolved === result.elements.length ? (unresolved === 1 ? "The element has" : "All of them have") : `${unresolved} of them ${unresolved === 1 ? "has" : "have"}`} no source location — sent with selector and text context. Rides along with your next message.` | ||
| : "It rides along with your next message — add a comment or just press Enter.", | ||
| }); | ||
| } finally { | ||
| setSending(false); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import { describe, expect, it } from "vite-plus/test"; | ||
|
|
||
| import { countUnresolvedDesignElements } from "./protocol"; | ||
|
|
||
| /** | ||
| * The unresolved-count rule the pill and send toast surface: an element with a null | ||
| * sourceLabel is one the request could not source-address by send time (the | ||
| * SEND_SOURCE_WAIT_MS grace expired, or the page has no source mapping at all). | ||
| */ | ||
|
|
||
| const element = (sourceLabel: string | null) => ({ sourceLabel }); | ||
|
|
||
| describe("countUnresolvedDesignElements", () => { | ||
| it("counts only null sourceLabels", () => { | ||
| expect( | ||
| countUnresolvedDesignElements({ | ||
| elements: [element("App.tsx:12"), element(null), element("Card.tsx:8"), element(null)], | ||
| }), | ||
| ).toBe(2); | ||
| }); | ||
|
|
||
| it("is zero when every element resolved", () => { | ||
| expect(countUnresolvedDesignElements({ elements: [element("App.tsx:12")] })).toBe(0); | ||
| }); | ||
|
|
||
| it("is zero for an empty element list", () => { | ||
| expect(countUnresolvedDesignElements({ elements: [] })).toBe(0); | ||
| }); | ||
|
|
||
| it("an empty-string label is resolved, not unresolved — only null means no source", () => { | ||
| expect(countUnresolvedDesignElements({ elements: [element("")] })).toBe(0); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new note is
shrink-0and sits before the remove button inside a pill that ismax-w-full overflow-hidden. Once the summary has shrunk to zero, every remaining child is unshrinkable, so the overflow is clipped from the right — and the right-most child is the X button.Concrete: preview open (design panel is a fixed 325px, so the chat column is already squeezed), a 12-element change with all sources unresolved. The pill now carries icon +
12 elements+12 without source+ X, roughly 100px more unshrinkable width than before this PR. When that exceeds the composer width the X clips out of the pill and the attachment can no longer be removed — a one-way door on the only exit for a pending design change.Cheapest fixes: give the note
min-w-0 truncateso it yields before the button does, or move it ahead of the summary so the truncating child is the last thing that can be squeezed.Generated by Claude Code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Real — an unremovable attachment is a one-way door. Fixed in ed01494: the note is min-w-0 truncate, so it yields before the X does; the comment now documents that ordering constraint.