feat(tasks): a task description accepts dropped files, and shows them… - #612
Merged
Merged
Conversation
… arriving
Dropping a file on a task description did nothing. resolveZone had no entry
for it, so the pointer resolved to no zone and the dragover handler stamped
`dropEffect = "none"` — a deliberate refusal, and one the code documented.
This makes it a drop target for the two TASK descriptions (detail + create),
splitting a drop exactly as _onEditorPaste already splits a paste: an image
goes INTO the body at the point it was dropped, and anything the marker
grammar cannot express — a PDF, a video — attaches beside it, which is where
that editor's own paperclip already puts it.
The new zone is `__desc-editor[data-desc-scope]`, first in the table. It is
special-cased like `comment-row` but with the opposite refusal rule: an
unrecognised scope CONTINUES rather than returning null. A foreign comment row
refuses because nothing above it would claim the drop; a mention editor sits
inside the composer or reply box that does, and those must keep the drops they
have always taken. The scope rides an attribute because sys_pn is a model
field read with mget and never reaches the DOM.
An inline image also uploads before it can be shown, and until now nothing
appeared during those seconds — the drop read as one that had been ignored.
_insertPastedImage is split into _beginInlineImage (synchronous: the
placeholder, showing the local file dimmed under the strip's own spinner) and
_settleInlineImage (the upload, then the swap — or a red state with retry and
discard). Paste goes through the same pair, so both surfaces gained the
loading state and the two cannot diverge.
Nothing in the placeholder can reach the saved description, by three separate
properties: __inline-img-pending is not __inline-img under classList's
whole-token match, so no image marker is emitted; every child is an element,
so the serializer's fallback walk finds no text; and the retry/discard glyphs
are CSS ::after content, which is never in childNodes. A failed placeholder
can therefore sit in the editor indefinitely and a save stores the body as if
it were not there. tests/task-desc-drop.test.js runs the real _serializeEditor
over one to prove it, with a committed inline image as the positive control.
Three things that would otherwise have broken quietly:
- attachExistingNodes normalises a desc zone to its form. Without it an
internal grid drag resolves no draft and returns false, while
canAttachExisting() has already told the folder not to insert the file —
it would land nowhere at all.
- _rememberDropScope excludes desc for the same reason it excludes
detail/create: a task surface is recoverable from the pointer, and
remembering it would let a stale hover write with no overlay shown.
- _pasteZone excludes desc, so paste behaviour is unchanged. Pasting INTO a
description is the editor's own path; this branch is the case where the
caret is elsewhere and only the pointer is over the editor.
The affordance is CSS-only on the element itself, unlike every other zone's
overlay child: the editor is contenteditable, so an injected node would be
editable content that _serializeEditor carries into the body and _onDescInput
counts when deciding the field is empty.
Also fills in tests/helpers/render-skeleton.js's makeUi, which was missing 15
reader methods the skeleton calls — render() threw on the first column it drew,
hidden because both existing consumers go through renderModule.
Known limit: the placeholder is DOM-only. _renderEditorContent rebuilds the
body from the draft's markers on every render, so a render mid-upload wipes it;
the image still lands via the append fallback, which is what this path did
before there were placeholders. Covered by a test rather than left to chance.
Not verified in a browser: the task schema is not provisioned locally (no hub
instance has a `task` table), so the drag itself needs stage.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.


… arriving
Dropping a file on a task description did nothing. resolveZone had no entry for it, so the pointer resolved to no zone and the dragover handler stamped
dropEffect = "none"— a deliberate refusal, and one the code documented. This makes it a drop target for the two TASK descriptions (detail + create), splitting a drop exactly as _onEditorPaste already splits a paste: an image goes INTO the body at the point it was dropped, and anything the marker grammar cannot express — a PDF, a video — attaches beside it, which is where that editor's own paperclip already puts it.The new zone is
__desc-editor[data-desc-scope], first in the table. It is special-cased likecomment-rowbut with the opposite refusal rule: an unrecognised scope CONTINUES rather than returning null. A foreign comment row refuses because nothing above it would claim the drop; a mention editor sits inside the composer or reply box that does, and those must keep the drops they have always taken. The scope rides an attribute because sys_pn is a model field read with mget and never reaches the DOM.An inline image also uploads before it can be shown, and until now nothing appeared during those seconds — the drop read as one that had been ignored. _insertPastedImage is split into _beginInlineImage (synchronous: the placeholder, showing the local file dimmed under the strip's own spinner) and _settleInlineImage (the upload, then the swap — or a red state with retry and discard). Paste goes through the same pair, so both surfaces gained the loading state and the two cannot diverge.
Nothing in the placeholder can reach the saved description, by three separate properties: __inline-img-pending is not __inline-img under classList's whole-token match, so no image marker is emitted; every child is an element, so the serializer's fallback walk finds no text; and the retry/discard glyphs are CSS ::after content, which is never in childNodes. A failed placeholder can therefore sit in the editor indefinitely and a save stores the body as if it were not there. tests/task-desc-drop.test.js runs the real _serializeEditor over one to prove it, with a committed inline image as the positive control.
Three things that would otherwise have broken quietly:
The affordance is CSS-only on the element itself, unlike every other zone's overlay child: the editor is contenteditable, so an injected node would be editable content that _serializeEditor carries into the body and _onDescInput counts when deciding the field is empty.
Also fills in tests/helpers/render-skeleton.js's makeUi, which was missing 15 reader methods the skeleton calls — render() threw on the first column it drew, hidden because both existing consumers go through renderModule.
Known limit: the placeholder is DOM-only. _renderEditorContent rebuilds the body from the draft's markers on every render, so a render mid-upload wipes it; the image still lands via the append fallback, which is what this path did before there were placeholders. Covered by a test rather than left to chance.
Not verified in a browser: the task schema is not provisioned locally (no hub instance has a
tasktable), so the drag itself needs stage.