From 030bd6f18e426acab543635bec9a16a76fa8c8b2 Mon Sep 17 00:00:00 2001 From: Drumee Dev Date: Sat, 19 Sep 2026 09:33:02 -0700 Subject: [PATCH] feat(tasks): a task description accepts dropped files, and shows them arriving MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../builtins/window/tasks/drop-zones.js | 30 +- src/drumee/builtins/window/tasks/index.js | 333 ++++++- .../builtins/window/tasks/skeleton/index.js | 10 + .../builtins/window/tasks/skin/index.scss | 134 +++ tests/helpers/render-skeleton.js | 34 + tests/task-desc-drop.test.js | 835 ++++++++++++++++++ 6 files changed, 1359 insertions(+), 17 deletions(-) create mode 100644 tests/task-desc-drop.test.js diff --git a/src/drumee/builtins/window/tasks/drop-zones.js b/src/drumee/builtins/window/tasks/drop-zones.js index 5d21772d9..c099ee571 100644 --- a/src/drumee/builtins/window/tasks/drop-zones.js +++ b/src/drumee/builtins/window/tasks/drop-zones.js @@ -17,7 +17,17 @@ // __attachments are SIBLING subtrees in __modal-main, so they never contend — // the ordering matters only for __comment-replybox, which sits beside rows in a // thread group. +// +// __desc-editor is first because it is the innermost of them all: it is the +// only zone that can sit INSIDE another one. The two description editors do +// not (they are their own rows, beside __attachments / __create-files rather +// than within them), but the three comment editors are nested in the composer, +// the reply box and a row — and they reach this table too, because the scope +// check that excludes them lives here rather than in the selector. First is +// therefore where "an inner zone wins" puts it, and where it has to be for +// that check to be the thing that decides. const ZONES = [ + { sel: "__desc-editor[data-desc-scope]", scope: "desc" }, { sel: "__comment-replybox", scope: "comment-reply" }, { sel: "__comment-row[data-comment-id]", scope: "comment-row" }, { sel: "__comment-composer", scope: "comment" }, @@ -25,6 +35,12 @@ const ZONES = [ { sel: "__create-files", scope: "create" }, ]; +// The description editors a drop may inline into. Only the two TASK +// descriptions: the comment editors pass their own `editorClass` and so never +// carry __desc-editor at all, but this is the check that decides it rather +// than an accident of the markup — see the fall-through in resolveZone. +const DESC_SCOPES = ["detail", "create"]; + /** * Resolve a pointer's element to a drop-zone descriptor, or null to refuse. * @@ -42,6 +58,18 @@ function resolveZone(pfx, el, ctx) { for (const z of ZONES) { const n = el.closest(`.${pfx}${z.sel}`); if (!n || !ctx.contains(n)) continue; + if (z.scope === "desc") { + const s = n.getAttribute("data-desc-scope"); + // FALLS THROUGH, unlike the comment-row refusal below, and the difference + // is which surface owns the drop. A comment row that is not yours sits in + // __modal-main, where nothing else would claim the file — so stopping is + // the only way to avoid a silent attach somewhere else. A mention editor + // sits inside the composer / reply box / row that DOES own it, and those + // zones are the next entries in this table. Refusing here would take a + // drop the composer has always accepted. + if (!DESC_SCOPES.includes(s)) continue; + return { scope: "desc", key: `desc:${s}`, el: n, descScope: s }; + } if (z.scope === "comment-row") { const id = n.getAttribute("data-comment-id"); // Author-only server-side (_ownComment). Refuse rather than fall through @@ -60,4 +88,4 @@ function resolveZone(pfx, el, ctx) { return null; } -module.exports = { ZONES, resolveZone }; +module.exports = { ZONES, DESC_SCOPES, resolveZone }; diff --git a/src/drumee/builtins/window/tasks/index.js b/src/drumee/builtins/window/tasks/index.js index f1290af3a..a6a7d85c6 100644 --- a/src/drumee/builtins/window/tasks/index.js +++ b/src/drumee/builtins/window/tasks/index.js @@ -393,6 +393,15 @@ class __tasks_panel extends LetcBox { this.el.removeEventListener("mouseleave", this._pointerExit); this._pointerExit = null; } + // Inline-image placeholders hold an object URL each while they upload. + // They live in the editor's DOM rather than on a draft, so the loop below + // cannot see them — a panel closed mid-upload would leak one per image. + for (const url of this._inlinePreviews || []) { + try { + URL.revokeObjectURL(url); + } catch (_) {} + } + this._inlinePreviews = null; // Release pending-file image-preview blob URLs — the two task forms and // the three comment drafts, which carry their own queued files. for (const draft of [ @@ -5248,8 +5257,16 @@ class __tasks_panel extends LetcBox { * a retained node would either leak or silently miss after a re-feed. */ _rememberDropScope(zone) { + // `desc` is excluded for exactly the reason detail/create are: it is a task + // surface, recoverable from the pointer, so remembering it would let a + // stale hover write into a description with no overlay ever shown. + const task = + zone && + (zone.scope === "detail" || + zone.scope === "create" || + zone.scope === "desc"); this._lastDropScope = - zone && zone.scope !== "detail" && zone.scope !== "create" + zone && !task ? { scope: zone.scope, key: zone.key, commentId: zone.commentId } : null; } @@ -5712,10 +5729,16 @@ class __tasks_panel extends LetcBox { }); if (!zone) return null; // A zone only accepts while the surface that owns it is actually open. - if (zone.scope === "detail" && !this._detailDraft) return null; - if (zone.scope === "create" && !this._createDefaults) return null; + // + // A `desc` zone is the same surface as its form's attachment zone, one row + // up — the detail panel's description and its __attachments both live or + // die with _detailDraft — so it answers to the same guard rather than a + // second one that could drift from it. + const surface = zone.scope === "desc" ? zone.descScope : zone.scope; + if (surface === "detail" && !this._detailDraft) return null; + if (surface === "create" && !this._createDefaults) return null; if ( - (zone.scope === "comment" || zone.scope === "comment-reply") && + (surface === "comment" || surface === "comment-reply") && !this._detailId ) { return null; @@ -5927,7 +5950,17 @@ class __tasks_panel extends LetcBox { // Only the one under the cursor may claim it. if (!this._dropPointEl(at)) return null; const zone = this._activeUploadScope(at); - if (zone) return zone; + // A desc zone is a DROP target, not a paste target. + // + // Pasting INTO a description is the editor's own path: the caret is in a + // contenteditable, so _onPasteAttach never runs (_isTextEntry) and + // _onEditorPaste inlines at the caret. This branch is the opposite case — + // the caret is somewhere else entirely and only the POINTER happens to be + // over the editor. Claiming it here would drop an image into a body the + // user is not typing in, from a keystroke that gave no hint it would go + // there. Falls through to the composer default below, exactly as it did + // before this zone existed. + if (zone && zone.scope !== "desc") return zone; // Inside the panel but over no zone — including over another author's // comment, which resolveZone refuses rather than passing through. The // composer is where a paste belongs by default; its draft is allocated on @@ -6048,9 +6081,45 @@ class __tasks_panel extends LetcBox { } const files = Array.from((e.dataTransfer && e.dataTransfer.files) || []); if (!files.length) return; + // WHERE in the description the file landed. The event is the only thing + // that knows, and the upload that follows is async — by the time it + // resolves the drag is long over and there is no pointer left to ask. Same + // reason _onEditorPaste clones the caret range before awaiting. + if (scope.scope === "desc") { + scope.range = this._caretRangeFromPoint(e.clientX, e.clientY); + } return this._attachFilesToZone(scope, files); } + /** + * A collapsed range at a viewport point, or null. + * + * Two vendor spellings and no agreement between them: Chromium and WebKit + * expose caretRangeFromPoint, Gecko caretPositionFromPoint. Neither is + * guaranteed, and null is a perfectly good answer — _insertPastedImage + * appends to the editor when it has no usable range, which is what a drop + * onto the editor's padding should do anyway. + */ + _caretRangeFromPoint(x, y) { + if (typeof document === "undefined" || x == null || y == null) return null; + try { + if (document.caretRangeFromPoint) { + return document.caretRangeFromPoint(x, y); + } + if (document.caretPositionFromPoint) { + const pos = document.caretPositionFromPoint(x, y); + if (!pos || !pos.offsetNode) return null; + const range = document.createRange(); + range.setStart(pos.offsetNode, pos.offset); + range.collapse(true); + return range; + } + } catch (_) { + /* a detached or cross-document node — fall through to appending */ + } + return null; + } + /** * Send files to whatever a resolved zone means, and nothing else. * @@ -6061,6 +6130,10 @@ class __tasks_panel extends LetcBox { */ async _attachFilesToZone(zone, files) { if (!zone || !files || !files.length) return; + // A description takes an image INTO the body and everything else beside it. + if (zone.scope === "desc") { + return this._dropOnDescEditor(zone, files); + } // A comment row has no submit, so arriving IS the commit. if (zone.scope === "comment-row") { return this._dropOnCommentRow(zone.commentId, files); @@ -6071,6 +6144,67 @@ class __tasks_panel extends LetcBox { this._refreshPendingList(this._scopeKey(zone)); } + /** + * A drop on a task description. + * + * Splits the files the way _onEditorPaste splits a paste, and for the same + * reason: the body's marker grammar holds mentions, links and inline images, + * and nothing else. An image goes IN, at the point it was dropped. A PDF, a + * video or a spreadsheet has no marker it could become, so it attaches to the + * task instead — which is where that editor's own paperclip already puts it, + * and where a pasted video already goes. + * + * Sequential, not Promise.all: _insertPastedImage moves the range past the + * node it just inserted, so three images dropped together land in the order + * they were dropped rather than in whatever order their uploads finish. + */ + async _dropOnDescEditor(zone, files) { + const editorEl = zone.el; + const scope = zone.descScope; + if (!editorEl || !scope) return; + const images = []; + const rest = []; + for (const f of files) (this._isDroppableImage(f) ? images : rest).push(f); + // Attachments first. Queuing them is synchronous and touches only the + // draft, so the strip is already showing them while the first image is + // still uploading — rather than both landing at once, several seconds in. + if (rest.length) { + await this._attachFilesToZone({ scope, key: scope }, rest); + } + // Every placeholder goes in FIRST, in one synchronous pass, so a drop of + // three images shows three spinners at once and in the order they were + // dropped. Settling them inside the same loop would mean the second + // placeholder only appeared once the first upload had finished — the + // spinner would then be describing the wait it was added to explain away. + const placed = images.map((file) => ({ + file, + ph: this._beginInlineImage(file, scope, editorEl, zone.range), + })); + for (const { file, ph } of placed) { + // The panel can be closed, or the task switched, mid-upload. + if (!editorEl.isConnected) return; + await this._settleInlineImage(ph, file, scope, editorEl); + } + } + + /** + * Is this dropped file an image, for the purposes of going inline? + * + * By MIME type first, exactly as the paste path tests a clipboard item. The + * extension is the fallback for a file the OS handed over with no type at + * all — a drag out of an archive, off a network share, or from an app that + * simply does not set one. A file that DOES declare a type is taken at its + * word, so a mislabelled .png attaches rather than rendering as a broken + * inline image. + */ + _isDroppableImage(file) { + if (!file) return false; + if (/^image\//.test(file.type || "")) return true; + if (file.type) return false; + const { extension } = this._splitFilename(file.name || ""); + return this._isImageExt(extension); + } + // Queues File objects onto a draft's pending list (picker + drag-drop), // caching an object URL so a picture or a video shows before it lands // (_attachLocalPreview). Names are provisional here; the @@ -6929,12 +7063,27 @@ class __tasks_panel extends LetcBox { // Every one of those paths applies the same editing guard, so a drop can // never land on the task while a comment owns the surface. attachExistingNodes(files, resolved) { - const scope = + let scope = resolved || this._lastDropScope || this._pointerScope() || this._positionlessScope(); if (!scope) return false; + // A workspace node dragged onto a DESCRIPTION attaches; it does not inline. + // Inlining uploads a File and this route has none — it carries a node that + // already exists — so the zone is normalised to the form behind it. + // + // Not cosmetic: without this, _draftForScope below is asked for a + // "desc:detail" draft, _draftForKey does not know that key, and the drop + // returns false having done nothing — while canAttachExisting() has + // already told the folder window not to insert the file into its own body. + // The file would land nowhere at all. + // + // The affordance still lights the description the pointer is actually + // over, which is one row above where the file lands. + if (scope.scope === "desc") { + scope = { scope: scope.descScope, key: scope.descScope }; + } // A comment row has no submit, so the drop IS the commit — _stageRowItems // applies the same dedupes and the same cross-hub placeholder path this // function does for the staged scopes. @@ -7817,16 +7966,15 @@ class __tasks_panel extends LetcBox { } } - async _insertPastedImage(file, scope, editorEl, range) { - let res; - try { - res = await this._uploadInlineImage(file); - } catch (err) { - console.error("[tasks_panel] inline image upload failed:", err); - return; - } - if (!editorEl.isConnected) return; - const node = this._makeInlineImage(res.nid, res.hub, null, true); + /** + * Put a node at a caret range, or at the end of the editor. + * + * Extracted from _insertPastedImage so a placeholder and the image that + * replaces it land by the same rule — and so the range ADVANCES past what it + * just inserted, which is what lets several images dropped together keep the + * order they were dropped in. + */ + _insertInlineNode(node, editorEl, range) { if (range && editorEl.contains(range.startContainer)) { range.deleteContents(); range.insertNode(node); @@ -7838,6 +7986,119 @@ class __tasks_panel extends LetcBox { } else { editorEl.appendChild(node); } + return node; + } + + /** + * Show that an image is on its way, at the point it was dropped or pasted. + * + * SYNCHRONOUS, and that is the whole point: the upload behind it takes + * seconds, and until now nothing at all appeared during them — an image + * dropped on a description read as a drop that had been ignored. + * + * NOTHING HERE CAN REACH THE SAVED BODY, by three separate properties, because + * _onDescInput serializes the editor on every keystroke and a placeholder is + * not something the marker grammar can express: + * + * - the class is __inline-img-pending, and _serializeEditor tests + * `classList.contains(__inline-img)` — a WHOLE-TOKEN match, so this is + * not one, and no image marker is emitted for it; + * - every child is an element, so the serializer's fallback (walk into + * anything it does not recognise and keep the text) finds no text nodes + * and emits the empty string; + * - the retry and discard glyphs are CSS ::after content, which is + * generated content — never in childNodes, never in textContent. + * + * So a failed placeholder can sit in the editor indefinitely, and a save + * while it is there stores the description exactly as if it were not. + */ + _beginInlineImage(file, scope, editorEl, range) { + const pfx = this.fig.family; + const ph = document.createElement("span"); + ph.className = `${pfx}__inline-img-pending`; + ph.setAttribute("contenteditable", "false"); + ph.dataset.status = "uploading"; + // The file is already in the browser, so the real picture can be shown + // while it uploads — the same trick _attachLocalPreview plays for a queued + // attachment, and the reason this reads as "this image, arriving" rather + // than as an anonymous spinner. + let url = null; + try { + url = URL.createObjectURL(file); + } catch (_) { + /* an engine that refuses is fine — the spinner alone still says enough */ + } + if (url) { + (this._inlinePreviews = this._inlinePreviews || new Set()).add(url); + const img = document.createElement("img"); + img.src = url; + img.alt = ""; + img.setAttribute("draggable", "false"); + ph.appendChild(img); + ph.__previewUrl = url; + } + for (const part of ["spinner", "retry", "discard"]) { + const s = document.createElement("span"); + s.className = `${pfx}__inline-img-${part}`; + ph.appendChild(s); + } + return this._insertInlineNode(ph, editorEl, range); + } + + // Drop a placeholder's object URL. Safe to call twice. + _releaseInlinePreview(ph) { + const url = ph && ph.__previewUrl; + if (!url) return; + ph.__previewUrl = null; + if (this._inlinePreviews) this._inlinePreviews.delete(url); + try { + URL.revokeObjectURL(url); + } catch (_) {} + } + + /** + * Upload the file behind a placeholder and put the real image in its place. + * + * On failure the placeholder STAYS, in its error state, offering a retry — + * the alternative is an image that silently never arrives, which is what + * this path did before (it logged to the console and returned). + * + * The placeholder can also be gone by the time the upload lands: + * _renderEditorContent rebuilds the editor body from the draft's markers on + * every render, and a placeholder is deliberately not a marker. That is not + * an error — the image is simply appended, which is exactly what this method + * did in that situation before there were placeholders at all. + */ + async _settleInlineImage(ph, file, scope, editorEl) { + if (ph && ph.isConnected) ph.dataset.status = "uploading"; + let res; + try { + res = await this._uploadInlineImage(file); + } catch (err) { + console.error("[tasks_panel] inline image upload failed:", err); + if (ph && ph.isConnected) { + ph.dataset.status = "error"; + this._wireInlineImageRecovery(ph, file, scope, editorEl); + } else if (typeof Butler !== "undefined" && Butler.say) { + // No placeholder left to carry the failure, so say it out loud rather + // than let the image vanish without a word. + Butler.say(LOCALE.ERROR_NETWORK); + } + return; + } + if (!editorEl.isConnected) { + this._releaseInlinePreview(ph); + return; + } + const node = this._makeInlineImage(res.nid, res.hub, null, true); + if (ph && ph.isConnected) { + ph.replaceWith(node); + } else { + // Wiped by a render while it was uploading — fall back to the end of the + // editor, the same place a stale range has always put it. + this._insertInlineNode(node, editorEl, null); + } + this._releaseInlinePreview(ph); // Pasted images default to a small size (still resizable up via the handle). // Cap at the image's natural width so a small image isn't upscaled, then // re-sync so the width is stored in the draft marker. @@ -7856,6 +8117,46 @@ class __tasks_panel extends LetcBox { this._onDescInput(scope, editorEl); } + /** + * Wire a failed placeholder's two controls. + * + * Native listeners on the node itself, not services: this is raw DOM that + * skeleton feed() never rebuilds, so there is no re-render to survive and + * nothing for onUiEvent to route. They are attached once — a retry that + * fails again comes back through here and would otherwise stack a second + * listener on every attempt. + */ + _wireInlineImageRecovery(ph, file, scope, editorEl) { + if (ph.__wired) return; + ph.__wired = 1; + const pfx = this.fig.family; + ph.addEventListener("click", (e) => { + const hit = e.target && e.target.closest && e.target.closest("span"); + if (!hit) return; + if (hit.classList.contains(`${pfx}__inline-img-discard`)) { + e.preventDefault(); + e.stopPropagation(); + this._releaseInlinePreview(ph); + ph.remove(); + // The placeholder was never in the draft, so nothing needs saving — + // but the editor may now be empty, and _onDescInput is what notices + // (it strips the stray
that defeats the :empty placeholder). + this._onDescInput(scope, editorEl); + return; + } + if (hit.classList.contains(`${pfx}__inline-img-retry`)) { + e.preventDefault(); + e.stopPropagation(); + this._settleInlineImage(ph, file, scope, editorEl); + } + }); + } + + async _insertPastedImage(file, scope, editorEl, range) { + const ph = this._beginInlineImage(file, scope, editorEl, range); + return this._settleInlineImage(ph, file, scope, editorEl); + } + // Promise-wrapped upload for a raw clipboard image File. Tags scope so the // global onUploadResponse skips it (resolved here via the readystate listener). _uploadInlineImage(file) { diff --git a/src/drumee/builtins/window/tasks/skeleton/index.js b/src/drumee/builtins/window/tasks/skeleton/index.js index 5b90e529a..b46d5bf2b 100644 --- a/src/drumee/builtins/window/tasks/skeleton/index.js +++ b/src/drumee/builtins/window/tasks/skeleton/index.js @@ -2497,6 +2497,16 @@ function mentionField(ui, scope, opt = {}) { contenteditable: "true", "data-placeholder": opt.placeholder || LOCALE.TASK_DESCRIPTION_PLACEHOLDER, + // Which editor this is, for the drop zone (../drop-zones.js). It has + // to be an ATTRIBUTE: sys_pn carries the same thing one line above, + // but that is a model field read with mget and never reaches the DOM, + // and a drop resolves from the element under the pointer. + // + // Stamped for all five scopes, not just the two that can be dropped + // into. The zone table is what decides which scopes accept a file + // (DESC_SCOPES), so leaving the other three unlabelled would move + // that decision into whether an attribute happens to be present. + "data-desc-scope": scope, }, }), mentionDropdown(ui, scope), diff --git a/src/drumee/builtins/window/tasks/skin/index.scss b/src/drumee/builtins/window/tasks/skin/index.scss index 921e5f93f..ebb22188e 100644 --- a/src/drumee/builtins/window/tasks/skin/index.scss +++ b/src/drumee/builtins/window/tasks/skin/index.scss @@ -167,6 +167,25 @@ $theme-colors: ( position: relative; } + // The description's drop affordance is the ELEMENT ITSELF — no overlay child. + // + // Every other zone above reveals a `> __drop-overlay` node. This one cannot: + // the editor is contenteditable, so anything inside it is editable content. + // _serializeEditor walks the subtree to build the stored body and would carry + // the overlay's text into it, and _onDescInput inspects the same subtree to + // decide whether the field is empty — an overlay would make an empty + // description look filled and defeat the :empty placeholder. + // + // outline rather than border or box-shadow: a border would reflow the text + // inside a padded box the moment a drag entered it, and the focus ring below + // is an inset shadow, which cannot be dashed. The negative offset draws the + // ring inside the box — the same clipping reason that ring is inset. + &__desc-editor[data-drop-active="1"] { + outline: 2px dashed var(--active-border, rgba(67, 60, 197, 0.4)); + outline-offset: -2px; + background: var(--hover-bg-40, rgba(67, 60, 197, 0.06)); + } + // Instant feedback for heavy view switches (Project Health links, viewbar // tabs): data-view-loading is set by _renderDeferred the moment the link is // clicked and painted BEFORE the full skeleton rebuild runs — a translucent @@ -3101,6 +3120,121 @@ $theme-colors: ( } } + // ── Inline image being uploaded ─────────────────────────────────── + // + // The placeholder _beginInlineImage puts at the drop/paste point. Sized and + // spaced like __inline-img above so the swap to the real image, when the + // upload lands, does not move the text around it. + // + // NOT resizable, unlike the committed image: there is no marker behind it + // yet, so a width dragged here would be thrown away by the swap. + // + // One attribute drives both states, exactly as __attachment-row[data-status] + // does for the strip — see _settleInlineImage. + &__inline-img-pending { + position: relative; + display: inline-block; + max-width: 100%; + min-width: 60px; + width: 220px; + margin: 4px 2px; + vertical-align: bottom; + border-radius: 8px; + line-height: 0; + // A placeholder with no preview (an engine that refused the object URL) + // would otherwise be zero-high and its spinner would sit on the text. + min-height: 60px; + background: var(--overlay-bg-05, rgba(0, 0, 0, 0.05)); + + img { + width: 100%; + height: auto; + display: block; + pointer-events: none; + border-radius: 8px; + } + } + + // Same dimming the attachment strip uses while a file is in flight. + &__inline-img-pending[data-status="uploading"] img { + opacity: 0.45; + } + + &__inline-img-pending[data-status="error"] { + outline: 1px solid var(--signal-error, #d74e49); + outline-offset: 0; + + img { + opacity: 0.3; + } + } + + // Indeterminate, for the same reason the strip's is: the upload xhr's + // progress events are not plumbed through, so this says "working", not + // "how far". Identical geometry and animation to __file-pending-spinner. + &__inline-img-spinner { + position: absolute; + top: 50%; + left: 50%; + width: 18px; + height: 18px; + margin: -9px 0 0 -9px; + border: 2px solid var(--overlay-bg-05, rgba(0, 0, 0, 0.15)); + border-top-color: var(--accent, #433cc5); + border-radius: 50%; + animation: tasks-panel-spin 0.7s linear infinite; + } + + // The two recovery controls. Their glyphs are ::after CONTENT, never text + // nodes — generated content is not in the DOM, so _serializeEditor cannot + // walk it into the saved description. That is what lets a failed + // placeholder sit in the editor without corrupting the body. + &__inline-img-retry, + &__inline-img-discard { + position: absolute; + display: none; + width: 22px; + height: 22px; + line-height: 22px; + text-align: center; + cursor: pointer; + border-radius: 50%; + background: var(--normal-bg-10, #fff); + color: var(--signal-error, #d74e49); + font-size: 13px; + } + + &__inline-img-retry { + top: 50%; + left: 50%; + margin: -11px 0 0 -11px; + + &::after { + content: "\21bb"; // ↻ + } + } + + &__inline-img-discard { + top: 2px; + right: 2px; + + &::after { + content: "\2715"; // ✕ + } + } + + // Only a FAILED placeholder offers them. While it is uploading there is + // nothing to retry and no way to un-upload, so a control there would lie — + // the same rule the strip's ✕ follows. + &__inline-img-pending[data-status="error"] &__inline-img-spinner { + display: none; + } + + &__inline-img-pending[data-status="error"] &__inline-img-retry, + &__inline-img-pending[data-status="error"] &__inline-img-discard { + display: block; + } + // Read-only inline image (comment bodies) — not resizable. &__inline-img-static { max-width: 100%; diff --git a/tests/helpers/render-skeleton.js b/tests/helpers/render-skeleton.js index 6401293e3..cf8f0f825 100644 --- a/tests/helpers/render-skeleton.js +++ b/tests/helpers/render-skeleton.js @@ -248,6 +248,40 @@ function makeUi(over = {}) { // modal reads this whenever it draws, so a fixture without it cannot render // that modal at all. getPendingSubtasks: () => [], + // The rest of the reader surface the skeleton calls. Every one of these was + // absent, so `render()` threw on the FIRST column it drew (cardWindow) and + // nothing could use it — the two tests that render a skeleton today both go + // through renderModule with their own stub, which is what hid it. + // + // Defaults are the "nothing here yet" answer in each case, so a test that + // cares about one overrides it and a test that does not is unaffected. + cardWindow: () => 60, + getDefaultStatus: () => "todo", + getFilteredTasks: () => [], + getTopLevelTasks: () => [], + getTaskById: () => null, + getSubtasks: () => [], + getSubtaskDraft: () => null, + getSubtaskCount: () => ({ done: 0, total: 0 }), + isSubtask: () => false, + isSubtasksOpen: () => false, + getActivity: () => [], + isCommentRowBusy: () => false, + // Per-section in-flight flags for the detail card (attachments / comments / + // history). False = "fetched, and there are none", which is what an + // already-settled fixture should look like. + isLoading: () => false, + // Which overlays have already played their entrance. 0 keeps the fixture + // in the state a freshly-opened overlay is in. + hasPainted: () => 0, + // Mirrors tasks_panel.pickerService — the assignee/reporter scopes' service + // names, which the pickers stamp on their rows. + pickerService: (scope) => { + if (scope === "create") return "create-assignee"; + if (scope === "create-reporter") return "create-reporter"; + if (scope === "detail-reporter") return "set-reporter"; + return "set-assignee"; + }, }; return { ...base, ...over }; } diff --git a/tests/task-desc-drop.test.js b/tests/task-desc-drop.test.js new file mode 100644 index 000000000..b39348242 --- /dev/null +++ b/tests/task-desc-drop.test.js @@ -0,0 +1,835 @@ +// Dropping a file on a task DESCRIPTION. +// +// Until now the description refused every drop: resolveZone had no entry for +// it, so the pointer resolved to nothing and window/tasks/index.js stamped +// `dropEffect = "none"` — a deliberate refusal, documented in the dragover +// handler. This adds the zone, and the whole of the new decision lives in +// drop-zones.js, which is pure: an element, a prefix and a two-method context. +// So it is tested directly, against a DOM small enough to read. +// +// Two things make this worth a test rather than a glance: +// +// 1. THE FALL-THROUGH. The three comment editors are mention editors too. A +// zone that matched them would steal a drop from the composer and the +// reply box, which own it today. What keeps them out is that they pass +// their own `editorClass` and so never carry `__desc-editor` — and that +// is a fact about skeleton/index.js, not about drop-zones.js, which is +// why the second half of this file renders the REAL skeleton to check it. +// The unknown-scope case is tested anyway: it must fall through to the +// zone that encloses it, NOT refuse like a foreign comment row does. +// +// 2. THE SCOPE. `detail` and `create` are one selector apart from each +// other — both are `__desc-editor` — so the scope has to ride an +// attribute. sys_pn never reaches the DOM (it is read with mget), hence +// data-desc-scope. +const test = require("node:test"); +const assert = require("node:assert"); +const { readFileSync } = require("node:fs"); +const { resolve } = require("node:path"); +const { resolveZone } = require("../src/drumee/builtins/window/tasks/drop-zones.js"); +const { render, walk } = require("./helpers/render-skeleton.js"); + +const PFX = "tasks-panel"; + +// ── A DOM with exactly as much as resolveZone touches ────────────────── +// +// `closest` over a parent chain, matching the two selector shapes the ZONES +// table uses: ".cls" and ".cls[attr]". Anything else is a typo in the table +// and should fail loudly rather than silently match nothing. +const SEL = /^\.([A-Za-z0-9_-]+)(?:\[([A-Za-z0-9_-]+)\])?$/; + +function el(className, attrs = {}, kids = []) { + const node = { + className, + attrs, + parentNode: null, + getAttribute: (k) => (k in attrs ? attrs[k] : null), + closest(sel) { + const m = SEL.exec(sel); + assert.ok(m, `unsupported selector in ZONES: ${sel}`); + const [, cls, attr] = m; + let n = this; + while (n) { + const classes = String(n.className || "").split(/\s+/); + if (classes.includes(cls) && (!attr || n.getAttribute(attr) != null)) { + return n; + } + n = n.parentNode; + } + return null; + }, + }; + for (const k of kids) k.parentNode = node; + return node; +} + +// Everything is inside the panel, and `me` owns every comment, unless a test +// says otherwise. +const ctx = (over = {}) => ({ + contains: () => true, + isOwnComment: () => true, + ...over, +}); + +const descEditor = (scope) => + el(`${PFX}__desc-editor`, scope == null ? {} : { "data-desc-scope": scope }); + +test("a drop on the detail description resolves the detail desc zone", () => { + const editor = descEditor("detail"); + el(`${PFX}__detail-row`, {}, [editor]); + + const zone = resolveZone(PFX, editor, ctx()); + assert.ok(zone, "the description must now accept a drop"); + assert.equal(zone.scope, "desc"); + assert.equal(zone.descScope, "detail"); + assert.equal(zone.key, "desc:detail"); + // The zone carries its own element so the lit affordance and the resolved + // scope cannot drift apart — the reason resolveZone returns `el` at all. + assert.equal(zone.el, editor); +}); + +test("a drop on the create-modal description resolves the create desc zone", () => { + const editor = descEditor("create"); + el(`${PFX}__create-field-grow`, {}, [editor]); + + const zone = resolveZone(PFX, editor, ctx()); + assert.equal(zone.scope, "desc"); + assert.equal(zone.descScope, "create"); + assert.equal(zone.key, "desc:create"); +}); + +test("a drop on a CHILD of the description still resolves the editor", () => { + // An inline image or a mention chip is a real element inside the editor, and + // a drop lands on whichever one is under the pointer. + const chip = el(`${PFX}__mention-chip`); + const editor = el( + `${PFX}__desc-editor`, + { "data-desc-scope": "detail" }, + [chip], + ); + el(`${PFX}__detail-row`, {}, [editor]); + + const zone = resolveZone(PFX, chip, ctx()); + assert.equal(zone.descScope, "detail"); + assert.equal(zone.el, editor, "the zone element is the editor, not the chip"); +}); + +test("a desc editor with an unknown scope FALLS THROUGH to its enclosing zone", () => { + // The refusal rule is not the comment-row rule. A foreign comment row refuses + // outright, because nothing above it legitimately owns the drop. A mention + // editor is different: it sits inside a composer that does own it, so an + // unrecognised scope must keep looking rather than swallow the drop. + const editor = descEditor("something-new"); + const composer = el(`${PFX}__comment-composer`, {}, [editor]); + assert.ok(composer); + + const zone = resolveZone(PFX, editor, ctx()); + assert.ok(zone, "must not refuse — the composer owns this drop"); + assert.equal(zone.scope, "comment"); +}); + +test("a desc editor with NO scope attribute falls through too", () => { + const editor = descEditor(null); + el(`${PFX}__comment-composer`, {}, [editor]); + + const zone = resolveZone(PFX, editor, ctx()); + assert.equal(zone && zone.scope, "comment"); +}); + +// ── The zones that already existed keep working ──────────────────────── + +test("the existing zones are unchanged", () => { + const cases = [ + [`${PFX}__comment-replybox`, {}, "comment-reply", "comment-reply"], + [`${PFX}__comment-composer`, {}, "comment", "comment"], + [`${PFX}__attachments`, {}, "detail", "detail"], + [`${PFX}__create-files`, {}, "create", "create"], + ]; + for (const [cls, attrs, scope, key] of cases) { + const n = el(cls, attrs); + const zone = resolveZone(PFX, n, ctx()); + assert.equal(zone && zone.scope, scope, `${cls} -> ${scope}`); + assert.equal(zone && zone.key, key); + } + + const row = el(`${PFX}__comment-row`, { "data-comment-id": "c1" }); + const rowZone = resolveZone(PFX, row, ctx()); + assert.equal(rowZone.scope, "comment-row"); + assert.equal(rowZone.key, "comment-row:c1"); + assert.equal(rowZone.commentId, "c1"); +}); + +test("someone else's comment row is still refused outright", () => { + const row = el(`${PFX}__comment-row`, { "data-comment-id": "c1" }); + const zone = resolveZone(PFX, row, ctx({ isOwnComment: () => false })); + assert.equal(zone, null); +}); + +test("a pointer on panel chrome still refuses", () => { + assert.equal(resolveZone(PFX, el(`${PFX}__viewbar`), ctx()), null); +}); + +test("an element outside the panel is refused even if it matches", () => { + const editor = descEditor("detail"); + const zone = resolveZone(PFX, editor, ctx({ contains: () => false })); + assert.equal(zone, null); +}); + +// ── The skeleton half: which editors carry the hook ──────────────────── +// +// resolveZone can only be right about scope if the markup agrees, and the +// markup is what decides that the three comment editors are out of scope: +// they pass their own editorClass, so they never carry __desc-editor at all. +// A fixture cannot see that — this renders the shipped skeleton. + +const DRAFT = { + title: "t", + description: "", + mention_uids: [], + due_date: "", + start_date: "", + duration_on: false, + status: "todo", + priority: "medium", + reporter_uid: "me", + assignees: [], + labels: [], + pending_files: [], +}; + +const editors = (tree) => { + const out = []; + for (const n of walk(tree)) { + const cls = String(n.className || ""); + if (/__desc-editor|__comment-input|__comment-reply-input|__comment-edit-input/.test(cls)) { + out.push({ cls, scope: (n.attrOpt || {})["data-desc-scope"] }); + } + } + return out; +}; + +test("the detail description editor carries its scope", () => { + const tree = render({ + getDetailTask: () => ({ id: "t1", title: "t", status: "todo", created_by: "me" }), + getDetailDraft: () => DRAFT, + }); + const desc = editors(tree).filter((e) => /__desc-editor/.test(e.cls)); + assert.equal(desc.length, 1, "the detail panel draws one description editor"); + assert.equal(desc[0].scope, "detail"); +}); + +test("the create-modal description editor carries its scope", () => { + const tree = render({ + isCreating: () => true, + getCreateDraft: () => ({ ...DRAFT, subtasks: [] }), + }); + const desc = editors(tree).filter((e) => /__desc-editor/.test(e.cls)); + assert.equal(desc.length, 1); + assert.equal(desc[0].scope, "create"); +}); + +test("the comment editors do NOT carry __desc-editor", () => { + // This is what keeps them out of the new zone. If a future edit drops the + // custom editorClass, the composer's drop silently changes meaning — from + // "ride the comment draft, commit on Send" to "inline into the body". + const tree = render({ + getDetailTask: () => ({ id: "t1", title: "t", status: "todo", created_by: "me" }), + getDetailDraft: () => DRAFT, + }); + const found = editors(tree); + const comment = found.filter((e) => !/__desc-editor/.test(e.cls)); + assert.ok(comment.length >= 1, "the detail panel draws a comment composer"); + for (const c of comment) { + assert.ok( + !/__desc-editor/.test(c.cls), + `${c.cls} must not carry __desc-editor — it would join the desc zone`, + ); + } +}); + +// ── The panel half: what the resolved zone is then USED for ──────────── +// +// The panel is a 10 000-line class that needs the whole runtime to +// instantiate, so — as tests/call-tile-drag.test.js and +// tests/workspace-delete-admin-only.test.js do — the methods are cut out of +// the SOURCE FILE and run against a fake `this`. They therefore test the +// shipped text: rename one of these or change the split and this fails. +const PANEL = resolve( + __dirname, + "../src/drumee/builtins/window/tasks/index.js", +); +const panelSrc = readFileSync(PANEL, "utf8"); + +// One method, by name. Methods sit at two-space indent and close with a " }" +// on its own line, which is what bounds the slice. +function method(name) { + const head = new RegExp(`\\n (?:async )?${name}\\(`).exec(panelSrc); + assert.ok(head, `method ${name} not found in ${PANEL}`); + const from = head.index + 1; + const end = panelSrc.indexOf("\n }\n", from); + assert.ok(end > from, `method ${name} is not closed as expected`); + return panelSrc.slice(from, end + "\n }\n".length); +} + +const Panel = new Function( + `return class { ${[ + "_dropOnDescEditor", + "_isDroppableImage", + "_splitFilename", + "_isImageExt", + "_rememberDropScope", + "_pasteZone", + ] + .map(method) + .join("\n")} }`, +)(); + +const file = (name, type) => ({ name, type, __file: 1 }); + +// A panel with the calls _dropOnDescEditor makes recorded rather than run. +// +// The two image seams are recorded separately because the split between them +// is load-bearing: `placed` is what the user sees IMMEDIATELY (synchronous), +// `settled` is the upload behind it. +function panel(over = {}) { + const p = new Panel(); + p.calls = { attached: [], placed: [], settled: [] }; + p._attachFilesToZone = async (zone, files) => { + p.calls.attached.push({ zone, files }); + }; + p._beginInlineImage = (f, scope, el, range) => { + p.calls.placed.push({ file: f, scope, el, range }); + return { __placeholderFor: f }; + }; + p._settleInlineImage = async (ph, f, scope, el) => { + p.calls.settled.push({ ph, file: f, scope, el }); + }; + return Object.assign(p, over); +} + +const EDITOR = { isConnected: true }; +const descZone = (scope = "detail", range = { r: 1 }) => ({ + scope: "desc", + key: `desc:${scope}`, + descScope: scope, + el: EDITOR, + range, +}); + +test("an image dropped on the description goes INTO it, at the drop point", async () => { + const p = panel(); + const png = file("shot.png", "image/png"); + await p._dropOnDescEditor(descZone(), [png]); + + assert.equal(p.calls.attached.length, 0, "an image must not be attached"); + assert.equal(p.calls.placed.length, 1); + const ins = p.calls.placed[0]; + assert.equal(ins.file, png); + assert.equal(ins.scope, "detail", "inlines against the editor's own scope"); + assert.equal(ins.el, EDITOR); + assert.deepEqual(ins.range, { r: 1 }, "the drop point is carried through"); + assert.equal(p.calls.settled.length, 1, "and its upload is then run"); +}); + +test("a non-image dropped on the description attaches to the task instead", async () => { + const p = panel(); + const pdf = file("report.pdf", "application/pdf"); + await p._dropOnDescEditor(descZone(), [pdf]); + + assert.equal(p.calls.placed.length, 0, "nothing goes into the body"); + assert.equal(p.calls.attached.length, 1); + // The zone it attaches with is the FORM's, not the desc zone — otherwise + // _draftForKey would be handed "desc:detail" and find no draft. + assert.deepEqual(p.calls.attached[0].zone, { scope: "detail", key: "detail" }); + assert.deepEqual(p.calls.attached[0].files, [pdf]); +}); + +test("a video attaches, exactly as a pasted one does", async () => { + const p = panel(); + await p._dropOnDescEditor(descZone(), [file("clip.mp4", "video/mp4")]); + assert.equal(p.calls.placed.length, 0); + assert.equal(p.calls.attached.length, 1); +}); + +test("a mixed drop splits: images in, the rest beside", async () => { + const p = panel(); + const png = file("a.png", "image/png"); + const pdf = file("b.pdf", "application/pdf"); + const jpg = file("c.jpg", "image/jpeg"); + await p._dropOnDescEditor(descZone("create"), [png, pdf, jpg]); + + assert.deepEqual(p.calls.attached[0].files, [pdf], "only the non-images"); + assert.deepEqual( + p.calls.placed.map((i) => i.file), + [png, jpg], + "images land in the order they were dropped", + ); + for (const i of p.calls.placed) assert.equal(i.scope, "create"); +}); + +test("an editor torn out mid-upload stops the rest of the batch", async () => { + const p = panel(); + const el = { isConnected: true }; + p._settleInlineImage = async () => { + el.isConnected = false; // the task was switched while this one uploaded + p.calls.settled.push({}); + }; + await p._dropOnDescEditor( + { scope: "desc", descScope: "detail", el, range: null }, + [file("a.png", "image/png"), file("b.png", "image/png")], + ); + assert.equal(p.calls.settled.length, 1, "the second upload is abandoned"); +}); + +test("a zone with no element or no scope does nothing at all", async () => { + const p = panel(); + await p._dropOnDescEditor({ scope: "desc", descScope: "detail" }, [file("a.png", "image/png")]); + await p._dropOnDescEditor({ scope: "desc", el: EDITOR }, [file("a.png", "image/png")]); + assert.equal(p.calls.placed.length, 0); + assert.equal(p.calls.attached.length, 0); +}); + +test("an image is recognised by type first, by extension only when there is none", () => { + const p = panel(); + assert.equal(p._isDroppableImage(file("a.png", "image/png")), true); + assert.equal(p._isDroppableImage(file("a.webp", "image/webp")), true); + // No type at all — a drag out of an archive or off a share. + assert.equal(p._isDroppableImage(file("a.png", "")), true); + assert.equal(p._isDroppableImage(file("a.PNG", undefined)), true); + assert.equal(p._isDroppableImage(file("a.pdf", "")), false); + // A DECLARED type is taken at its word, so a mislabelled file attaches + // rather than rendering as a broken inline image. + assert.equal(p._isDroppableImage(file("a.png", "application/pdf")), false); + assert.equal(p._isDroppableImage(null), false); +}); + +test("a desc zone is never REMEMBERED for the positionless route", () => { + // Same rule as detail/create: a task surface is recoverable from the + // pointer, and remembering it would let a stale hover write with no + // overlay ever shown. + const p = panel(); + p._rememberDropScope(descZone()); + assert.equal(p._lastDropScope, null); + + p._rememberDropScope({ scope: "comment-row", key: "comment-row:c1", commentId: "c1" }); + assert.deepEqual(p._lastDropScope, { + scope: "comment-row", + key: "comment-row:c1", + commentId: "c1", + }); + + p._rememberDropScope(descZone("create")); + assert.equal(p._lastDropScope, null, "and it clears a remembered one"); +}); + +test("a PASTE over the description is left to the composer, as before", () => { + // Pasting INTO the description never reaches here — the caret is in a + // contenteditable, so _onPasteAttach returns early and _onEditorPaste + // inlines at the caret. This is the other case: the caret is elsewhere and + // only the pointer is over the editor. + const p = panel({ + _lastPointer: { x: 10, y: 10 }, + _dropPointEl: () => ({}), + _activeUploadScope: () => descZone(), + _detailId: "t1", + _mayWriteTasks: () => true, + }); + assert.deepEqual(p._pasteZone(), { scope: "comment", key: "comment" }); +}); + +test("a paste over any OTHER zone still claims it", () => { + const zone = { scope: "comment-row", key: "comment-row:c1", commentId: "c1" }; + const p = panel({ + _lastPointer: { x: 10, y: 10 }, + _dropPointEl: () => ({}), + _activeUploadScope: () => zone, + _detailId: "t1", + _mayWriteTasks: () => true, + }); + assert.equal(p._pasteZone(), zone); +}); + +// ── Loading state for an inline image ───────────────────────────────── +// +// An image dropped or pasted into a description uploads before it can be +// shown, and until now NOTHING appeared during those seconds — the drop read +// as one that had been ignored. _beginInlineImage puts a placeholder in at +// once and _settleInlineImage swaps it for the real image, or turns it red +// with a retry. +// +// The thing that most needs proving is not the spinner. It is that the +// placeholder CANNOT REACH THE SAVED BODY: _onDescInput serializes the editor +// on every keystroke, and a placeholder is not something the marker grammar +// can express. So the real _serializeEditor is run over an editor holding one. + +// A DOM with what these methods touch, and nothing else. +function fakeDom() { + const revoked = []; + let seq = 0; + const mk = (tag) => { + const n = { + tagName: String(tag).toUpperCase(), + nodeType: 1, + childNodes: [], + parentNode: null, + style: {}, + dataset: {}, + attrs: {}, + className: "", + listeners: [], + get classList() { + const own = () => String(n.className || "").split(/\s+/).filter(Boolean); + return { + contains: (c) => own().includes(c), + }; + }, + get isConnected() { + let p = n; + while (p) { + if (p.__root) return true; + p = p.parentNode; + } + return false; + }, + get textContent() { + return n.childNodes + .map((c) => (c.nodeType === 3 ? c.textContent : c.textContent)) + .join(""); + }, + setAttribute: (k, v) => { + n.attrs[k] = String(v); + }, + getAttribute: (k) => (k in n.attrs ? n.attrs[k] : null), + appendChild: (c) => { + c.parentNode = n; + n.childNodes.push(c); + return c; + }, + contains: (o) => { + let p = o; + while (p) { + if (p === n) return true; + p = p.parentNode; + } + return false; + }, + remove: () => { + const p = n.parentNode; + if (!p) return; + p.childNodes.splice(p.childNodes.indexOf(n), 1); + n.parentNode = null; + }, + replaceWith: (x) => { + const p = n.parentNode; + if (!p) return; + p.childNodes.splice(p.childNodes.indexOf(n), 1, x); + x.parentNode = p; + n.parentNode = null; + }, + querySelector: (sel) => { + const want = sel.toUpperCase(); + const hunt = (m) => { + for (const c of m.childNodes) { + if (c.tagName === want) return c; + const deep = hunt(c); + if (deep) return deep; + } + return null; + }; + return hunt(n); + }, + addEventListener: (ev, fn) => n.listeners.push({ ev, fn }), + // Fire a click as the browser would, with `target` set to a descendant. + __click(target) { + const e = { + target: { + ...target, + classList: target.classList, + closest: () => target, + }, + preventDefault() {}, + stopPropagation() {}, + }; + for (const l of n.listeners) if (l.ev === "click") l.fn(e); + }, + }; + return n; + }; + const text = (s) => ({ nodeType: 3, textContent: s, childNodes: [] }); + return { + mk, + text, + revoked, + document: { + createElement: mk, + createRange: () => null, + }, + URL: { + createObjectURL: () => `blob:fake/${++seq}`, + revokeObjectURL: (u) => revoked.push(u), + }, + window: { + getSelection: () => ({ removeAllRanges() {}, addRange() {} }), + }, + }; +} + +const IMG_METHODS = [ + "_insertInlineNode", + "_beginInlineImage", + "_releaseInlinePreview", + "_settleInlineImage", + "_wireInlineImageRecovery", + "_dropOnDescEditor", + "_isDroppableImage", + "_splitFilename", + "_isImageExt", + "_serializeEditor", +]; + +// A panel whose upload is controllable, on a fake DOM. +function imgPanel({ upload } = {}) { + const dom = fakeDom(); + const markers = require("../src/drumee/builtins/window/tasks/mention-markers.js"); + const Cls = new Function( + "document", + "URL", + "window", + "Butler", + "LOCALE", + "imgMarker", + "linkMarker", + "safeUrl", + `return class { ${IMG_METHODS.map(method).join("\n")} }`, + )( + dom.document, + dom.URL, + dom.window, + { said: [], say(m) { this.said.push(m); } }, + { ERROR_NETWORK: "ERROR_NETWORK" }, + markers.imgMarker, + markers.linkMarker, + markers.safeUrl, + ); + const p = new Cls(); + p.fig = { family: "tasks-panel" }; + p.dom = dom; + p.editor = dom.mk("div"); + p.editor.__root = 1; // everything under it counts as connected + p.synced = 0; + p._onDescInput = () => { + p.synced += 1; + }; + p.attached = []; + p._attachFilesToZone = async (zone, files) => { + p.attached.push({ zone, files }); + }; + p._makeInlineImage = (nid, hub) => { + const wrap = dom.mk("span"); + wrap.className = "tasks-panel__inline-img"; + wrap.dataset.nid = String(nid); + if (hub) wrap.dataset.hub = String(hub); + wrap.appendChild(dom.mk("img")); + return wrap; + }; + p.uploads = 0; + p._uploadInlineImage = async () => { + p.uploads += 1; + if (upload === "fail") throw new Error("http 500"); + if (typeof upload === "function") return upload(p.uploads); + return { nid: "n1", hub: "h1" }; + }; + return p; +} + +const PH = "tasks-panel__inline-img-pending"; +const kidsOf = (n) => n.childNodes.map((c) => c.className || c.tagName); + +test("a placeholder appears the moment the image is dropped, before any upload", () => { + const p = imgPanel(); + const ph = p._beginInlineImage(file("a.png", "image/png"), "detail", p.editor, null); + + assert.equal(p.uploads, 0, "synchronous — nothing has been sent yet"); + assert.equal(ph.className, PH); + assert.equal(ph.dataset.status, "uploading"); + assert.equal(ph.attrs.contenteditable, "false", "the caret must skip it"); + assert.equal(p.editor.childNodes[0], ph, "and it is in the editor"); + // The local file is shown while it uploads, as a queued attachment is. + assert.match(ph.querySelector("img").src, /^blob:/); + assert.deepEqual(kidsOf(ph), [ + "IMG", + "tasks-panel__inline-img-spinner", + "tasks-panel__inline-img-retry", + "tasks-panel__inline-img-discard", + ]); +}); + +test("the placeholder CANNOT reach the saved description", () => { + // The whole safety argument, executed rather than asserted in a comment. + const p = imgPanel(); + p.editor.appendChild(p.dom.text("before ")); + p._beginInlineImage(file("a.png", "image/png"), "detail", p.editor, null); + p.editor.appendChild(p.dom.text(" after")); + + assert.equal( + p._serializeEditor(p.editor), + "before after", + "a placeholder serializes to nothing at all", + ); +}); + +test("...while a COMMITTED inline image still serializes to its marker", () => { + // Positive control: the class test is a whole-token match, so + // __inline-img-pending is not __inline-img — and this proves the real one + // still is, i.e. that the exclusion was not achieved by breaking both. + const p = imgPanel(); + const real = p._makeInlineImage("n9", "h9"); + p.editor.appendChild(real); + assert.equal(p._serializeEditor(p.editor), "![img](file:n9@h9)"); +}); + +test("a successful upload swaps the placeholder for the real image", async () => { + const p = imgPanel(); + const ph = p._beginInlineImage(file("a.png", "image/png"), "detail", p.editor, null); + await p._settleInlineImage(ph, file("a.png", "image/png"), "detail", p.editor); + + assert.equal(p.editor.childNodes.length, 1); + assert.equal(p.editor.childNodes[0].className, "tasks-panel__inline-img"); + assert.equal(p.editor.childNodes[0].dataset.nid, "n1"); + assert.equal(ph.isConnected, false, "the placeholder is gone"); + assert.equal(p.dom.revoked.length, 1, "and its object URL was released"); + assert.ok(p.synced > 0, "the draft is resynced from the editor"); +}); + +test("a failed upload keeps the placeholder, in its error state", async () => { + const p = imgPanel({ upload: "fail" }); + const ph = p._beginInlineImage(file("a.png", "image/png"), "detail", p.editor, null); + await p._settleInlineImage(ph, file("a.png", "image/png"), "detail", p.editor); + + assert.equal(ph.isConnected, true, "it must not vanish silently"); + assert.equal(ph.dataset.status, "error"); + assert.equal(p.dom.revoked.length, 0, "the preview stays — retry still needs it"); + // And it is still invisible to the serializer, which is what makes leaving + // a failed placeholder on screen safe at all. + assert.equal(p._serializeEditor(p.editor), ""); +}); + +test("retry re-runs the upload and the image lands", async () => { + let attempt = 0; + const p = imgPanel({ + upload: () => { + attempt += 1; + if (attempt === 1) throw new Error("http 500"); + return { nid: "n2", hub: "h2" }; + }, + }); + const f = file("a.png", "image/png"); + const ph = p._beginInlineImage(f, "detail", p.editor, null); + await p._settleInlineImage(ph, f, "detail", p.editor); + assert.equal(ph.dataset.status, "error"); + + ph.__click({ className: "tasks-panel__inline-img-retry", classList: { contains: (c) => c === "tasks-panel__inline-img-retry" } }); + await new Promise((r) => setImmediate(r)); + + assert.equal(p.uploads, 2); + assert.equal(p.editor.childNodes[0].dataset.nid, "n2"); +}); + +test("retry wires exactly one listener however often it fails", async () => { + const p = imgPanel({ upload: "fail" }); + const f = file("a.png", "image/png"); + const ph = p._beginInlineImage(f, "detail", p.editor, null); + await p._settleInlineImage(ph, f, "detail", p.editor); + await p._settleInlineImage(ph, f, "detail", p.editor); + await p._settleInlineImage(ph, f, "detail", p.editor); + assert.equal( + ph.listeners.filter((l) => l.ev === "click").length, + 1, + "a stacked listener would fire N uploads on one click", + ); +}); + +test("discard removes a failed placeholder and releases its preview", async () => { + const p = imgPanel({ upload: "fail" }); + const f = file("a.png", "image/png"); + const ph = p._beginInlineImage(f, "detail", p.editor, null); + await p._settleInlineImage(ph, f, "detail", p.editor); + + ph.__click({ className: "tasks-panel__inline-img-discard", classList: { contains: (c) => c === "tasks-panel__inline-img-discard" } }); + + assert.equal(ph.isConnected, false); + assert.equal(p.editor.childNodes.length, 0); + assert.equal(p.dom.revoked.length, 1); +}); + +test("a placeholder wiped by a re-render still lets its image land", async () => { + // _renderEditorContent rebuilds the body from the draft's markers, and a + // placeholder is not a marker — so a render mid-upload takes it. The image + // must still arrive: that is the behaviour this path had before there were + // placeholders at all. + const p = imgPanel(); + const f = file("a.png", "image/png"); + const ph = p._beginInlineImage(f, "detail", p.editor, null); + ph.remove(); // the render + await p._settleInlineImage(ph, f, "detail", p.editor); + + assert.equal(p.editor.childNodes.length, 1); + assert.equal(p.editor.childNodes[0].dataset.nid, "n1"); +}); + +test("a drop of several images shows ALL their spinners at once", async () => { + // The point of the loading state: settling inside the placing loop would + // mean the second spinner only appeared once the first upload had finished. + const p = imgPanel(); + const gate = []; + p._uploadInlineImage = () => + new Promise((resolve) => gate.push(() => resolve({ nid: "n1", hub: "h1" }))); + + const zone = { scope: "desc", descScope: "detail", el: p.editor, range: null }; + const run = p._dropOnDescEditor(zone, [ + file("a.png", "image/png"), + file("b.png", "image/png"), + file("c.png", "image/png"), + ]); + await new Promise((r) => setImmediate(r)); + + assert.equal( + p.editor.childNodes.filter((n) => n.className === PH).length, + 3, + "three placeholders, before a single upload has resolved", + ); + assert.equal(gate.length, 1, "and the uploads themselves are still one at a time"); + + // Pump: each upload only queues its gate entry once the previous one has + // resolved, so a single drain would leave the drop hanging. + let done = false; + run.then(() => { done = true; }); + for (let i = 0; i < 20 && !done; i++) { + while (gate.length) gate.shift()(); + await new Promise((r) => setImmediate(r)); + } + await run; +}); + +test("a mixed drop still attaches the non-images and inlines the rest", async () => { + const p = imgPanel(); + const zone = { scope: "desc", descScope: "detail", el: p.editor, range: null }; + await p._dropOnDescEditor(zone, [ + file("a.png", "image/png"), + file("b.pdf", "application/pdf"), + ]); + + assert.deepEqual(p.attached[0].zone, { scope: "detail", key: "detail" }); + assert.deepEqual(p.attached[0].files.map((f) => f.name), ["b.pdf"]); + assert.equal(p.editor.childNodes[0].dataset.nid, "n1"); +}); + +test("a paste still goes through the same begin+settle path", async () => { + // _insertPastedImage keeps its signature, which is what gives paste the + // loading state for free — the two must not diverge. + const src = readFileSync(PANEL, "utf8"); + const body = /\n async _insertPastedImage\([^)]*\)\s*\{([\s\S]*?)\n \}\n/.exec(src); + assert.ok(body, "_insertPastedImage not found"); + assert.match(body[1], /_beginInlineImage/); + assert.match(body[1], /_settleInlineImage/); +});