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/); +});