gate the composer's image affordances on the model that will receive them
Reopened B4. Flipping `images: false` for OpenCode models made the metadata honest but changed nothing
on screen, because no code read the capability: the drop zone, the paste handler and the attach menu all
accepted images on every harness. The lie B4 described — drop a screenshot, watch it render in your own
bubble, have it discarded before the model sees it — was still there.
The flag is now load-bearing. Three entry points gated on `supportsImages`:
- the drop zone does not claim the drag at all (no highlight, no preventDefault), so the browser keeps
it rather than the composer swallowing a file it will drop on the floor
- an image paste falls through to the default
- the attach menu's Image entry is absent
`selectedModel || model` mirrors ModelSelector's `displayModel`, so the gate and the model name on screen
can never disagree. An unknown model allows images: a missing capability should not remove a working
control, and the flag is only false where we know it is false. Nothing to undo when images are plumbed
through OpenCodeRunParams later — the gate stops firing once the capability is true.
docs/opencode-phase0-review.md, item 1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -9,7 +9,11 @@ import {
|
|||||||
} from '@/components/ui/dropdown-menu';
|
} from '@/components/ui/dropdown-menu';
|
||||||
|
|
||||||
type AttachButtonProps = {
|
type AttachButtonProps = {
|
||||||
onAttachImage: (file: File) => void;
|
/**
|
||||||
|
* Omit when the selected model takes no images: the menu entry disappears rather than offering an
|
||||||
|
* attachment that would be discarded before the model saw it. See `supportsImages` in InputArea.
|
||||||
|
*/
|
||||||
|
onAttachImage?: (file: File) => void;
|
||||||
onAttachWebpage: () => void;
|
onAttachWebpage: () => void;
|
||||||
/** Text files are inlined into the composer rather than sent as an attachment — see below. */
|
/** Text files are inlined into the composer rather than sent as an attachment — see below. */
|
||||||
onAttachText: (text: string) => void;
|
onAttachText: (text: string) => void;
|
||||||
@@ -71,10 +75,12 @@ export function AttachButton({ onAttachImage, onAttachWebpage, onAttachText, siz
|
|||||||
</button>
|
</button>
|
||||||
</DropdownMenuTrigger>
|
</DropdownMenuTrigger>
|
||||||
<DropdownMenuContent side="top" align="start" className="z-[800]">
|
<DropdownMenuContent side="top" align="start" className="z-[800]">
|
||||||
|
{onAttachImage && (
|
||||||
<DropdownMenuItem className="cursor-pointer" onSelect={() => imageInputRef.current?.click()}>
|
<DropdownMenuItem className="cursor-pointer" onSelect={() => imageInputRef.current?.click()}>
|
||||||
<Image className="mr-2 h-4 w-4" />
|
<Image className="mr-2 h-4 w-4" />
|
||||||
Image
|
Image
|
||||||
</DropdownMenuItem>
|
</DropdownMenuItem>
|
||||||
|
)}
|
||||||
{/* "PDF" used to sit beside this one. Both were inert — no onSelect at all — so the menu offered
|
{/* "PDF" used to sit beside this one. Both were inert — no onSelect at all — so the menu offered
|
||||||
four things and did two. There is no PDF text extraction anywhere in the platform, client or
|
four things and did two. There is no PDF text extraction anywhere in the platform, client or
|
||||||
server, so that entry could not be made honest without building one first. */}
|
server, so that entry could not be made honest without building one first. */}
|
||||||
@@ -95,7 +101,7 @@ export function AttachButton({ onAttachImage, onAttachWebpage, onAttachText, siz
|
|||||||
className="hidden"
|
className="hidden"
|
||||||
onChange={(ev) => {
|
onChange={(ev) => {
|
||||||
const file = ev.target.files?.[0];
|
const file = ev.target.files?.[0];
|
||||||
if (file) onAttachImage(file);
|
if (file) onAttachImage?.(file);
|
||||||
ev.target.value = '';
|
ev.target.value = '';
|
||||||
}}
|
}}
|
||||||
/>
|
/>
|
||||||
|
|||||||
@@ -45,7 +45,21 @@ export const InputArea = ({ manager }: InputAreaProps) => {
|
|||||||
|
|
||||||
const { recording, transcribing, toggleRecording } = useAudioRecording(appendToInput);
|
const { recording, transcribing, toggleRecording } = useAudioRecording(appendToInput);
|
||||||
const [urlDialogOpen, setUrlDialogOpen] = useState(false);
|
const [urlDialogOpen, setUrlDialogOpen] = useState(false);
|
||||||
const drop = useImageDrop(attachImage);
|
|
||||||
|
// Does the model actually accept images?
|
||||||
|
//
|
||||||
|
// `list-models.ts` has always published this per model and NOTHING read it, so every image affordance
|
||||||
|
// was offered on every harness. On OpenCode the result was a lie you could watch happen: drop a
|
||||||
|
// screenshot, see it render in your own bubble, and have it discarded before the model ever saw it —
|
||||||
|
// `handleOpenCodeChat`'s message type has no `images` field.
|
||||||
|
//
|
||||||
|
// `selectedModel || model` mirrors ModelSelector's `displayModel`, so the gate and the visible model
|
||||||
|
// name can never disagree. Unknown model → allow: a missing capability should not silently remove a
|
||||||
|
// control that works, and the flag is only false where we know it is false.
|
||||||
|
const effectiveModelId = selectedModel || model;
|
||||||
|
const supportsImages = availableModels.find((m) => m.id === effectiveModelId)?.images !== false;
|
||||||
|
|
||||||
|
const drop = useImageDrop(attachImage, supportsImages);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div
|
<div
|
||||||
@@ -72,7 +86,7 @@ export const InputArea = ({ manager }: InputAreaProps) => {
|
|||||||
|
|
||||||
<div className="flex items-end gap-1 md:gap-2">
|
<div className="flex items-end gap-1 md:gap-2">
|
||||||
<AttachButton
|
<AttachButton
|
||||||
onAttachImage={attachImage}
|
onAttachImage={supportsImages ? attachImage : undefined}
|
||||||
onAttachWebpage={() => setUrlDialogOpen(true)}
|
onAttachWebpage={() => setUrlDialogOpen(true)}
|
||||||
onAttachText={appendToInput}
|
onAttachText={appendToInput}
|
||||||
/>
|
/>
|
||||||
@@ -84,6 +98,9 @@ export const InputArea = ({ manager }: InputAreaProps) => {
|
|||||||
onChange={(ev) => setInput(ev.target.value)}
|
onChange={(ev) => setInput(ev.target.value)}
|
||||||
onKeyDown={handleKeyDown}
|
onKeyDown={handleKeyDown}
|
||||||
onPaste={(ev) => {
|
onPaste={(ev) => {
|
||||||
|
// Let an image paste fall through to the browser's default (nothing) rather than attaching
|
||||||
|
// something this model will discard.
|
||||||
|
if (!supportsImages) return;
|
||||||
const items = ev.clipboardData?.items;
|
const items = ev.clipboardData?.items;
|
||||||
if (!items) return;
|
if (!items) return;
|
||||||
for (const item of Array.from(items)) {
|
for (const item of Array.from(items)) {
|
||||||
@@ -159,11 +176,14 @@ export const InputArea = ({ manager }: InputAreaProps) => {
|
|||||||
* - **Only claim drags that carry files.** Dragging selected text across the composer would otherwise
|
* - **Only claim drags that carry files.** Dragging selected text across the composer would otherwise
|
||||||
* light it up and then swallow the drop, which is how you lose a text-drag into the input.
|
* light it up and then swallow the drop, which is how you lose a text-drag into the input.
|
||||||
*/
|
*/
|
||||||
function useImageDrop(attachImage: (file: File) => void) {
|
function useImageDrop(attachImage: (file: File) => void, enabled = true) {
|
||||||
const [isTarget, setIsTarget] = useState(false);
|
const [isTarget, setIsTarget] = useState(false);
|
||||||
const depth = useRef(0);
|
const depth = useRef(0);
|
||||||
|
|
||||||
const carriesFiles = (ev: DragEvent<HTMLElement>) => Array.from(ev.dataTransfer.types).includes('Files');
|
// When the model takes no images, do not claim the drag at all: no highlight, no preventDefault, and
|
||||||
|
// the drop is left to the browser. Claiming it and then dropping the file on the floor is the exact
|
||||||
|
// behaviour this gate exists to remove.
|
||||||
|
const carriesFiles = (ev: DragEvent<HTMLElement>) => enabled && Array.from(ev.dataTransfer.types).includes('Files');
|
||||||
|
|
||||||
const reset = () => {
|
const reset = () => {
|
||||||
depth.current = 0;
|
depth.current = 0;
|
||||||
|
|||||||
Reference in New Issue
Block a user