refactor(session): rewrite session view per W1 visual lock for slice 11b.1 #589
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| /* | ||
| * AttachmentChip — slice 11b.1. | ||
| * | ||
| * One spec across the product (DESIGN.md L444). The composer dock and the | ||
| * user-message bubble both render this primitive; the only difference is | ||
| * the close button, gated by `[data-removable]`. | ||
| * | ||
| * Geometry mirrors the W1 visual preview `.mf-attach` / `.mf-attach-img` | ||
| * stamps in docs/design/preview/message-flow.html. Class names here use | ||
| * the data-component / data-slot convention so they match every other | ||
| * shared primitive in packages/ui. | ||
| */ | ||
|
|
||
| [data-component="attachment-chip"] { | ||
| flex-shrink: 0; | ||
| max-width: 280px; | ||
| position: relative; | ||
| box-sizing: border-box; | ||
| } | ||
|
|
||
| /* File chip — height 64, gap 12, radius-md, 1px hairline, transparent bg | ||
| * so the chip blends with whichever surface owns the row (cream in | ||
| * the bubble, white in the composer dock). */ | ||
| [data-component="attachment-chip"][data-kind="file"] { | ||
| display: inline-flex; | ||
| align-items: center; | ||
| gap: 12px; | ||
| height: 64px; | ||
| padding: 8px 12px; | ||
| border-radius: var(--radius-md); | ||
| border: 1px solid var(--border-weaker); | ||
| background: transparent; | ||
| line-height: 1.4; | ||
| } | ||
|
|
||
| [data-component="attachment-chip"][data-kind="file"] [data-slot="attachment-chip-icon"] { | ||
| width: 48px; | ||
| height: 48px; | ||
| border-radius: var(--radius-sm); | ||
| background: var(--surface-sunken); | ||
| display: inline-flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| flex-shrink: 0; | ||
| color: var(--fg-base); | ||
| } | ||
|
|
||
| [data-component="attachment-chip"][data-kind="file"] [data-slot="attachment-chip-icon"] [data-icon] { | ||
| width: 32px; | ||
| height: 32px; | ||
| } | ||
|
|
||
| [data-component="attachment-chip"][data-kind="file"] [data-slot="attachment-chip-body"] { | ||
| display: flex; | ||
| flex-direction: column; | ||
| min-width: 0; | ||
| gap: 2px; | ||
| flex: 1; | ||
| } | ||
|
|
||
| [data-component="attachment-chip"][data-kind="file"] [data-slot="attachment-chip-name"] { | ||
| font: var(--type-body); | ||
| color: var(--fg-strong); | ||
| line-height: 1.3; | ||
| overflow: hidden; | ||
| text-overflow: ellipsis; | ||
| white-space: nowrap; | ||
| } | ||
|
|
||
| [data-component="attachment-chip"][data-kind="file"] [data-slot="attachment-chip-ext"] { | ||
| font: var(--type-caption); | ||
| color: var(--fg-weak); | ||
| line-height: 1.3; | ||
| text-transform: uppercase; | ||
| letter-spacing: 0.5px; | ||
| } | ||
|
|
||
| /* Image chip — square 64×64 thumbnail, same border + radius as file chip | ||
| * shell so the two read as one family. `object-fit: cover` keeps the | ||
| * thumbnail filling the box regardless of source aspect ratio. */ | ||
| [data-component="attachment-chip"][data-kind="image"] { | ||
| width: 64px; | ||
| height: 64px; | ||
| border-radius: var(--radius-md); | ||
| border: 1px solid var(--border-weaker); | ||
| background: var(--surface-sunken); | ||
| overflow: hidden; | ||
| } | ||
|
|
||
| [data-component="attachment-chip"][data-kind="image"] [data-slot="attachment-chip-image"] { | ||
| width: 100%; | ||
| height: 100%; | ||
| object-fit: cover; | ||
| display: block; | ||
| } | ||
|
|
||
| /* Remove button — 24 circle, fg-strong fill, bg-base glyph, floats at | ||
| * top right -6/-6. Only rendered when `[data-removable]` is present, so | ||
| * the bubble's archived chip never shows it (W1 L1088). */ | ||
| [data-component="attachment-chip"] [data-slot="attachment-chip-remove"] { | ||
| position: absolute; | ||
| top: -6px; | ||
| right: -6px; | ||
| width: 24px; | ||
| height: 24px; | ||
| border-radius: 9999px; | ||
| background: var(--fg-strong); | ||
| color: var(--bg-base); | ||
| border: 0; | ||
| padding: 0; | ||
| cursor: default; | ||
| display: inline-flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| } | ||
|
|
||
| [data-component="attachment-chip"] [data-slot="attachment-chip-remove"] [data-icon] { | ||
| width: 14px; | ||
| height: 14px; | ||
| } | ||
|
|
||
| [data-component="attachment-chip"] [data-slot="attachment-chip-remove"]:focus-visible { | ||
| outline: none; | ||
| box-shadow: 0 0 0 1px var(--brand-primary), 0 0 0 3px rgba(255, 89, 16, 0.2); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| import { expect, test } from "bun:test" | ||
| import { readFileSync } from "node:fs" | ||
|
|
||
| // Source-grep style tests, matching the repo convention used by | ||
| // message-part-stale.test.ts. The full behavioural / removable-vs-read-only | ||
| // matrix is covered by session-turn-user-bubble.test.tsx (Phase 2c) and the | ||
| // dev:desktop D2 manual check. Tests here pin the structural invariants the | ||
| // design doc §3.7 calls out. | ||
|
|
||
| const source = readFileSync(new URL("./attachment-chip.tsx", import.meta.url), "utf8") | ||
| const css = readFileSync(new URL("./attachment-chip.css", import.meta.url), "utf8") | ||
|
|
||
| test("attachment-chip is the shared primitive (one component, file + image)", () => { | ||
| // One component name, two visual kinds via data-kind, gated by removable. | ||
| expect(source).toMatch(/export function AttachmentChip\(/) | ||
| expect(source).toMatch(/data-kind="file"/) | ||
| expect(source).toMatch(/data-kind="image"/) | ||
| }) | ||
|
|
||
| test("attachment-chip × close button is gated by `removable` prop", () => { | ||
| // The Show wrappers around the remove button must use props.removable | ||
| // (DESIGN.md L460, message-flow.html L1088: bubble never renders ×). | ||
| expect(source).toMatch(/<Show when=\{props\.removable\}>/) | ||
| // And the close button glyph must use the existing chrome icon. | ||
| expect(source).toMatch(/<Icon name="close" \/>/) | ||
| }) | ||
|
|
||
| test("attachment-chip file-kind exposes name + ext slots (W1 right column)", () => { | ||
| expect(source).toMatch(/data-slot="attachment-chip-name"/) | ||
| expect(source).toMatch(/data-slot="attachment-chip-ext"/) | ||
| }) | ||
|
|
||
| test("CSS geometry matches W1 lock: file chip h64 / radius-md / max-width 280", () => { | ||
| // Pin the visual contract — these three values are the ones the W1 | ||
| // preview lock-stamp calls out by name. | ||
| expect(css).toMatch(/\[data-component="attachment-chip"\][^{}]*\{[^}]*max-width:\s*280px/) | ||
| expect(css).toMatch(/\[data-kind="file"\][^{}]*\{[^}]*height:\s*64px/) | ||
| expect(css).toMatch(/\[data-kind="file"\][^{}]*\{[^}]*border-radius:\s*var\(--radius-md\)/) | ||
| }) | ||
|
|
||
| test("CSS image-kind: 64×64 square + radius-md + object-fit cover", () => { | ||
| expect(css).toMatch(/\[data-kind="image"\][^{}]*\{[^}]*width:\s*64px[^}]*height:\s*64px/) | ||
| expect(css).toMatch(/\[data-kind="image"\][^}]*\}\s*\n\s*\[data-component="attachment-chip"\]\[data-kind="image"\][^{}]*\[data-slot="attachment-chip-image"\][^{}]*\{[^}]*object-fit:\s*cover/) | ||
| }) | ||
|
|
||
| test("CSS remove button floats top-right with 24-circle + brand-color hairline ring on focus", () => { | ||
| expect(css).toMatch(/\[data-slot="attachment-chip-remove"\][^{}]*\{[^}]*top:\s*-6px[^}]*right:\s*-6px/) | ||
| expect(css).toMatch(/\[data-slot="attachment-chip-remove"\][^{}]*\{[^}]*width:\s*24px[^}]*height:\s*24px/) | ||
| expect(css).toMatch(/\[data-slot="attachment-chip-remove"\]:focus-visible[^{}]*\{[^}]*var\(--brand-primary\)/) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| import { Show, splitProps, type ComponentProps } from "solid-js" | ||
| import { Icon, type IconName } from "./icon" | ||
| import "./attachment-chip.css" | ||
|
|
||
| /** | ||
| * Shared attachment chip for slice 11b.1. Replaces the ad-hoc chip | ||
| * implementations the user-message bubble and (eventually) the composer | ||
| * dock used to maintain side-by-side, ensuring "one attachment spec across | ||
| * the product" per DESIGN.md L444. | ||
| * | ||
| * The single shape difference between bubble (archived state) and dock | ||
| * (editor state) is the close button — controlled by the `removable` prop. | ||
| * All other geometry (height 64 for files, 64×64 for images, radius-md, | ||
| * 1px `--border-weaker`, transparent background, max-width 280) is | ||
| * uniform. | ||
| * | ||
| * The chip is read-only display surface: it does NOT participate in the | ||
| * focus / Tab order (DESIGN.md §a11y for attachments; §6.9 confirms). | ||
| * When `removable` is true, the × button itself does enter the focus | ||
| * order so keyboard users can dismiss attachments. | ||
| */ | ||
| export interface AttachmentChipProps { | ||
| /** Kind of attachment — drives the file vs image visual branch. */ | ||
| kind: "file" | "image" | ||
| /** Display name for files (e.g. `report.pdf`). Ignored for images. */ | ||
| name?: string | ||
| /** Extension label shown under the filename. Defaults to the filename's tail. */ | ||
| ext?: string | ||
| /** Image preview URL for `kind="image"`. */ | ||
| previewUrl?: string | ||
| /** Alt text for the image; falls back to `name` then a default string. */ | ||
| alt?: string | ||
| /** Icon shown inside the 48×48 square slot for files. Defaults to `doc-processing`. */ | ||
| icon?: IconName | ||
| /** When true, render the × close button (composer editor state). */ | ||
| removable?: boolean | ||
| /** Click handler for the × button. Only fires when `removable` is true. */ | ||
| onRemove?: (event: MouseEvent) => void | ||
| /** Accessible label for the remove button (i18n-resolved by caller). */ | ||
| removeLabel?: string | ||
| } | ||
|
|
||
| function defaultExtFromName(name?: string): string | undefined { | ||
| if (!name) return undefined | ||
| const dot = name.lastIndexOf(".") | ||
| if (dot < 0 || dot === name.length - 1) return undefined | ||
| return name.slice(dot + 1) | ||
| } | ||
|
|
||
| export function AttachmentChip(rawProps: AttachmentChipProps & Omit<ComponentProps<"div">, keyof AttachmentChipProps>) { | ||
| const [props, rest] = splitProps(rawProps, [ | ||
| "kind", | ||
| "name", | ||
| "ext", | ||
| "previewUrl", | ||
| "alt", | ||
| "icon", | ||
| "removable", | ||
| "onRemove", | ||
| "removeLabel", | ||
| ]) | ||
|
|
||
| const resolvedExt = () => props.ext ?? defaultExtFromName(props.name) | ||
|
|
||
| return ( | ||
| <Show | ||
| when={props.kind === "image"} | ||
|
Comment on lines
+66
to
+67
Contributor
There was a problem hiding this comment. Guard the image branch when If Suggested patch <Show
- when={props.kind === "image"}
+ when={props.kind === "image" && !!props.previewUrl}
fallback={Also applies to: 107-111 🤖 Prompt for AI Agents |
||
| fallback={ | ||
| <div | ||
| data-component="attachment-chip" | ||
| data-kind="file" | ||
| data-removable={props.removable || undefined} | ||
| {...rest} | ||
| > | ||
| <span data-slot="attachment-chip-icon"> | ||
| <Icon name={props.icon ?? "doc-processing"} /> | ||
| </span> | ||
| <span data-slot="attachment-chip-body"> | ||
| <Show when={props.name}> | ||
| <span data-slot="attachment-chip-name" title={props.name}> | ||
| {props.name} | ||
| </span> | ||
| </Show> | ||
| <Show when={resolvedExt()}> | ||
| <span data-slot="attachment-chip-ext">{resolvedExt()}</span> | ||
| </Show> | ||
| </span> | ||
| <Show when={props.removable}> | ||
| <button | ||
| type="button" | ||
| data-slot="attachment-chip-remove" | ||
| aria-label={props.removeLabel} | ||
| onClick={(event) => props.onRemove?.(event)} | ||
| > | ||
| <Icon name="close" /> | ||
| </button> | ||
| </Show> | ||
|
Comment on lines
+88
to
+97
Contributor
There was a problem hiding this comment. Ensure the remove button always has an accessible name. When Suggested patch export function AttachmentChip(rawProps: AttachmentChipProps & Omit<ComponentProps<"div">, keyof AttachmentChipProps>) {
@@
const resolvedExt = () => props.ext ?? defaultExtFromName(props.name)
+ const resolvedRemoveLabel = () => props.removeLabel ?? "Remove attachment"
@@
<button
type="button"
data-slot="attachment-chip-remove"
- aria-label={props.removeLabel}
+ aria-label={resolvedRemoveLabel()}
onClick={(event) => props.onRemove?.(event)}
>
@@
<button
type="button"
data-slot="attachment-chip-remove"
- aria-label={props.removeLabel}
+ aria-label={resolvedRemoveLabel()}
onClick={(event) => props.onRemove?.(event)}
>Also applies to: 112-121 🤖 Prompt for AI Agents |
||
| </div> | ||
| } | ||
| > | ||
| <div | ||
| data-component="attachment-chip" | ||
| data-kind="image" | ||
| data-removable={props.removable || undefined} | ||
| {...rest} | ||
| > | ||
| <img | ||
| data-slot="attachment-chip-image" | ||
| src={props.previewUrl} | ||
| alt={props.alt ?? props.name ?? "attachment"} | ||
| /> | ||
| <Show when={props.removable}> | ||
| <button | ||
| type="button" | ||
| data-slot="attachment-chip-remove" | ||
| aria-label={props.removeLabel} | ||
| onClick={(event) => props.onRemove?.(event)} | ||
| > | ||
| <Icon name="close" /> | ||
| </button> | ||
| </Show> | ||
| </div> | ||
| </Show> | ||
| ) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Use a pointer cursor for the removable close button.
The remove control is clickable, but
cursor: defaultmakes it appear non-interactive.Suggested patch
[data-component="attachment-chip"] [data-slot="attachment-chip-remove"] { position: absolute; top: -6px; right: -6px; @@ - cursor: default; + cursor: pointer; display: inline-flex; align-items: center; justify-content: center; }📝 Committable suggestion
🤖 Prompt for AI Agents