From 179724f828b1814c6787722b30a96a9911ae09ea Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Thu, 15 Oct 2026 15:51:46 +0200 Subject: [PATCH 1/4] test(capture): Undo with a popout focused and after a capture widened a column J-TS-21 opens a draft in a popout, focuses that window and captures. The capture saves, but on master the popout's notice reads "[object DocumentFragment]" and has no Undo button. Two journeys type into a table cell of a two-row table, with the table in view and with it scrolled out of view, capture, Undo and capture again. The capture widens the column, so Obsidian pads the row below too. On master the Undo leaves the caret before the table header, and the second capture lands there and breaks the table. A third journey clicks into a cell of a note in a focused popout, lets the cell go stale in Source mode, changes another row on disk, captures and clicks Undo in the popout. It covers popout cells, which no journey did before. On master the capture keeps the table, and the journey stops at the missing Undo button. --- tests/e2e/journey-notes.test.ts | 45 +++++++++ tests/e2e/journey/actions.ts | 49 ++++++++++ tests/e2e/table-cell-capture.test.ts | 138 +++++++++++++++++++++++++++ 3 files changed, 232 insertions(+) diff --git a/tests/e2e/journey-notes.test.ts b/tests/e2e/journey-notes.test.ts index 500fb1fe..27704ade 100644 --- a/tests/e2e/journey-notes.test.ts +++ b/tests/e2e/journey-notes.test.ts @@ -2,7 +2,10 @@ import { describe, expect, onTestFinished, test } from "vitest"; import { createPodNotesE2EHarness } from "./harness"; import { clickInEditor, + clickInNoteWindow, closePopoutTab, + focusNoteWindow, + noticesBeside, openInEditor, openInReadingView, reloadAfterPopout, @@ -996,4 +999,46 @@ describe("episode note and capture journeys", () => { await reloadAfterPopout(obsidian); } }); + + test("J-TS-21 Capture Timestamp with a popout focused says so there, and its Undo takes the capture back", async () => { + const { obsidian, sandbox } = getContext(); + const journey = await setUp("Journey Popout Focused"); + const draft = sandbox.path("Focused draft.md"); + await writeVaultFile(obsidian, draft, "Popout notes\n"); + await detachMarkdownTabs(); + await playFromList(journey.show, ORIGINAL.title); + const paused = await pauseInView(); + try { + await obsidian.dev.evalJsonAsync(`(async () => { + const leaf = app.workspace.openPopoutLeaf(); + await leaf.openFile(app.vault.getAbstractFileByPath(${jsLiteral(draft)}), { state: { mode: "source" } }); + return true; + })()`); + expect(await clickInEditor(obsidian, draft, 0, 12)).toEqual({ line: 0, ch: 12 }); + await focusNoteWindow(obsidian, draft); + + await obsidian.exec("command", { id: "podnotes:capture-timestamp" }); + + const captured = `Popout notes${capture(journey, ORIGINAL, paused.seconds)}\n`; + await waitForText( + draft, + (text) => text === captured, + "the capture in the popout draft", + ); + expect(await noticesBeside(obsidian, draft)).toEqual([ + { message: "Timestamp added to Focused draft", button: "Undo" }, + ]); + await clickInNoteWindow(obsidian, draft, ".notice button.pn-undo"); + await waitForText(draft, (text) => text === "Popout notes\n", "the draft as it was"); + } finally { + if (await obsidian.dev.evalJson("app.workspace.floatingSplit.children.length")) + await closePopoutTab(obsidian, draft); + await obsidian.dev.evalJsonAsync(`(async () => { + app.workspace.requestSaveLayout.cancel(); + await app.workspace.saveLayout(); + return true; + })()`); + await reloadAfterPopout(obsidian); + } + }); }); diff --git a/tests/e2e/journey/actions.ts b/tests/e2e/journey/actions.ts index 7ff4fdbb..7bd9e47d 100644 --- a/tests/e2e/journey/actions.ts +++ b/tests/e2e/journey/actions.ts @@ -285,6 +285,55 @@ export async function clickInEditor( })()`); } +/** Gives the window holding the open editor of `notePath` the OS focus, as a click into it does. */ +export async function focusNoteWindow(obsidian: ObsidianClient, notePath: string): Promise { + const focused = await obsidian.dev.evalJsonAsync(`(async () => { + ${inNoteWindow(notePath)} + win.focus(); + for (let i = 0; i < 50 && activeDocument !== leaf.view.containerEl.ownerDocument; i++) + await new Promise((resolve) => setTimeout(resolve, 100)); + return activeDocument === leaf.view.containerEl.ownerDocument; + })()`); + if (!focused) throw new Error(`The window showing ${notePath} did not take focus.`); +} + +/** + * Clicks the last element `selector` matches, with `text` when given, in the window holding + * the open editor of `notePath`. + */ +export async function clickInNoteWindow( + obsidian: ObsidianClient, + notePath: string, + selector: string, + text?: string, +): Promise { + const clicked = await obsidian.dev.evalJson(`(() => { + ${inNoteWindow(notePath)} + const target = [...leaf.view.containerEl.ownerDocument.querySelectorAll(${jsLiteral(selector)})] + .filter((each) => ${jsLiteral(text ?? null)} === null || each.textContent === ${jsLiteral(text ?? null)}) + .at(-1); + if (!target) return false; + const box = target.getBoundingClientRect(); + press(box.left + box.width / 2, box.top + box.height / 2); + return true; + })()`); + if (!clicked) throw new Error(`Nothing matches ${selector} beside ${notePath}.`); +} + +/** The text and Undo button of each Notice in the window holding the open editor of `notePath`. */ +export async function noticesBeside( + obsidian: ObsidianClient, + notePath: string, +): Promise> { + return await obsidian.dev.evalJson(`(() => { + ${inNoteWindow(notePath)} + return [...leaf.view.containerEl.ownerDocument.querySelectorAll(".notice")].map((notice) => ({ + message: (notice.querySelector(".pn-undo-message") ?? notice).textContent.trim(), + button: notice.querySelector("button.pn-undo")?.textContent ?? null, + })); + })()`); +} + /** Clicks the close button of the popout tab showing `notePath`, which closes its window. */ export async function closePopoutTab(obsidian: ObsidianClient, notePath: string): Promise { const closed = await obsidian.dev.evalJsonAsync(`(async () => { diff --git a/tests/e2e/table-cell-capture.test.ts b/tests/e2e/table-cell-capture.test.ts index 3eee3b8f..0d1d8157 100644 --- a/tests/e2e/table-cell-capture.test.ts +++ b/tests/e2e/table-cell-capture.test.ts @@ -2,7 +2,11 @@ import { describe, expect, onTestFinished, test } from "vitest"; import { createPodNotesE2EHarness } from "./harness"; import { clickInEditor, + clickInNoteWindow, + closePopoutTab, + focusNoteWindow, openInEditor, + reloadAfterPopout, runFromPalette, showLibrary, showPodNotes, @@ -483,6 +487,140 @@ describe("Live Preview table captures", () => { expect((await obsidian.exec("dev:errors")).stdout.trim()).toBe("No errors captured."); }); + test.each(["in view", "scrolled out of view"])( + "#499 after Undo of a capture that widened its column, with the table %s, the next capture lands in the same cell", + async (where) => { + const { obsidian } = getContext(); + const table = TABLE.replace( + "| Start | anchor |", + "| Start | anchor |\n| Later | other |", + ); + const below = Array.from({ length: 2000 }, (_, line) => `Paragraph ${line}.`).join( + "\n", + ); + const path = await setUp({ content: `${table}${below}\n` }); + const editor = `app.workspace.getLeavesOfType("markdown").find((leaf) => leaf.view.file?.path === ${jsLiteral(path)}).view.editor`; + await typeText(obsidian, "USER_EDIT"); + if (where === "scrolled out of view") { + await obsidian.dev.evalJson( + `(() => { const { scrollDOM } = ${editor}.cm; scrollDOM.scrollTop = scrollDOM.scrollHeight; return true; })()`, + ); + await obsidian.waitFor( + () => obsidian.dev.evalJson(`${editor}.cm.viewport.from > 1000`), + { + timeoutMs: 5_000, + intervalMs: 100, + message: "the table out of the editor's viewport", + }, + ); + } + const before = await obsidian.dev.evalJson(`${editor}.getValue()`); + await obsidian.exec("command", { id: "podnotes:capture-timestamp" }); + const captured = await obsidian.dev.evalJson(`${editor}.getValue()`); + expect(rows(captured).slice(2)).toEqual([ + ["Start", "anchorUSER_EDIT00:00:30 \\| cut"], + ["Later", "other"], + ]); + await click(obsidian, lastUndo); + expect( + await eventually(obsidian, () => readVaultText(obsidian, path), before, 10_000), + ).toBe(before); + + await obsidian.exec("command", { id: "podnotes:capture-timestamp" }); + + const unpadded = (text: string) => text.replace(/ *\| */g, "|").replace(/-+/g, "-"); + expect( + await eventually( + obsidian, + async () => unpadded(await readVaultText(obsidian, path)), + unpadded(captured), + 10_000, + ), + ).toBe(unpadded(captured)); + }, + 90_000, + ); + + test("#473 a capture into a stale cell of a note in a focused popout keeps the table, and Undo there puts it back", async () => { + const { obsidian } = getContext(); + const table = + "# Captures\n\n| Moment | Notes |\n| ------ | ------ |\n| Start | anchor |\n| Later | other |\n\nAfter table\n"; + const path = await setUp({ content: table }); + const view = `app.workspace.getLeavesOfType("markdown").find((leaf) => leaf.view.file?.path === ${jsLiteral(path)}).view`; + try { + await obsidian.dev.evalJsonAsync(`(async () => { + const main = ${view}.leaf; + await app.workspace.openPopoutLeaf().openFile(main.view.file, { state: { mode: "source", source: false } }); + main.detach(); + return true; + })()`); + await focusNoteWindow(obsidian, path); + await clickInNoteWindow(obsidian, path, ".markdown-source-view td", "anchor"); + await obsidian.dev.evalJson(`(() => { + const remote = require("@electron/remote"); + const main = remote.getCurrentWindow(); + const popout = remote.BrowserWindow.getAllWindows().find((win) => win.id !== main.id); + for (const type of ["keyDown", "keyUp"]) popout.webContents.sendInputEvent({ type, keyCode: "End" }); + return true; + })()`); + expect( + await eventually( + obsidian, + () => + obsidian.dev.evalJson( + `${view}.editor.editorComponent.tableCell?.containerEl.ownerDocument === ${view}.containerEl.ownerDocument && ${view}.containerEl.ownerDocument !== document`, + ), + true, + ), + "a cell editor open in the popout", + ).toBe(true); + await obsidian.exec("command", { id: "editor:toggle-source" }); + await obsidian.dev.evalJsonAsync( + `app.vault.process(app.vault.getAbstractFileByPath(${jsLiteral(path)}), (text) => text.replace("| other |", "| other, edited elsewhere |")).then(() => true)`, + ); + const before = table.replace("| other |", "| other, edited elsewhere |"); + expect( + await eventually( + obsidian, + () => obsidian.dev.evalJson(`${view}.editor.getValue()`), + before, + 10_000, + ), + ).toBe(before); + expect( + await obsidian.dev.evalJson( + `${view}.editor.editorComponent.tableCell?.containerEl.isConnected ?? null`, + ), + "Obsidian still holds a cell no longer in the document", + ).toBe(false); + const caret = await obsidian.dev.evalJson( + `${view}.editor.posToOffset(${view}.editor.getCursor())`, + ); + + await obsidian.exec("command", { id: "podnotes:capture-timestamp" }); + + const captured = `${before.slice(0, caret)}00:00:30 \\| cut ${before.slice(caret)}`; + expect(rows(captured)[2]).toEqual(["Start", "anchor00:00:30 \\| cut"]); + expect( + await eventually(obsidian, () => readVaultText(obsidian, path), captured, 10_000), + ).toBe(captured); + await clickInNoteWindow(obsidian, path, ".notice button.pn-undo"); + expect( + await eventually(obsidian, () => readVaultText(obsidian, path), before, 10_000), + ).toBe(before); + expect((await obsidian.exec("dev:errors")).stdout.trim()).toBe("No errors captured."); + } finally { + if (await obsidian.dev.evalJson("app.workspace.floatingSplit.children.length")) + await closePopoutTab(obsidian, path); + await obsidian.dev.evalJsonAsync(`(async () => { + app.workspace.requestSaveLayout.cancel(); + await app.workspace.saveLayout(); + return true; + })()`); + await reloadAfterPopout(obsidian); + } + }, 90_000); + test("#445 Source mode still escapes table captures and preserves Undo", async () => { const { obsidian } = getContext(); const path = await setUp({ source: true }); From fbb67cc9bead97786bc926f5bf55ff69ecfbaf4d Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Thu, 15 Oct 2026 16:04:55 +0200 Subject: [PATCH 2/4] fix(capture): build the Undo notice in the main window With a popout focused, Obsidian shows a Notice in that popout, but reads its message with the main window's helpers. They take a DocumentFragment only from the main window, and show any other message as its String(). The notice was built from activeDocument, the popout's document, so it read "[object DocumentFragment]" and had no Undo. createFragment builds in the main window, and Obsidian moves the fragment into the popout, where Undo works. Fixes #498 --- src/ui/undoNotice.ts | 3 ++- vitest.setup.dom.ts | 13 +++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/ui/undoNotice.ts b/src/ui/undoNotice.ts index dc5b3bcf..e1bdb2ad 100644 --- a/src/ui/undoNotice.ts +++ b/src/ui/undoNotice.ts @@ -5,7 +5,8 @@ const showing = new Set<() => void>(); /** A Notice that ends in an Undo button. Undo runs at most once, and hides the notice. */ export function noticeWithUndo(message: string, undo: () => void): void { - const fragment = activeDocument.createDocumentFragment(); + // Obsidian's Notice takes only a main window fragment, also where it shows in a popout. + const fragment = createFragment(); fragment.createSpan({ cls: "pn-undo-message", text: message }); const button = fragment.createEl("button", { cls: "pn-undo", text: "Undo" }); const notice = new Notice(fragment, 7000); diff --git a/vitest.setup.dom.ts b/vitest.setup.dom.ts index f6233499..f8abcba2 100644 --- a/vitest.setup.dom.ts +++ b/vitest.setup.dom.ts @@ -58,6 +58,19 @@ if (typeof (globalThis as { activeDocument?: unknown }).activeDocument === "unde }); } +// Obsidian's `createFragment` builds in the main window's document. +if (typeof (globalThis as { createFragment?: unknown }).createFragment === "undefined") { + ( + globalThis as { + createFragment?: (build?: (fragment: DocumentFragment) => void) => DocumentFragment; + } + ).createFragment = (build) => { + const fragment = document.createDocumentFragment(); + build?.(fragment); + return fragment; + }; +} + // Obsidian points `activeWindow` at the focused window. Tests that open a second // window set it as Obsidian's focus listeners would. if (typeof (globalThis as { activeWindow?: unknown }).activeWindow === "undefined") { From d790b5211e820549821843881c8ed45f3b29ff6f Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Thu, 15 Oct 2026 16:04:55 +0200 Subject: [PATCH 3/4] fix(capture): put the caret back where the capture found it on Undo A capture into a table cell can widen its column, and Obsidian then pads every row. Undo puts back the stretch the capture changed, which then runs from the table's first changed row past the cell, so it holds the caret, and CodeMirror moves the caret to the stretch's start. The same edit makes Obsidian redraw the table and end the cell's editor. The next capture then went before the table header and broke the table. A capture now keeps the selections of the editor it went into, and Undo restores them after its edit, in that editor while it still edits the note. The note is still put back only while it is exactly as the capture left it. Fixes #499 --- src/commands.test.ts | 1 + src/main.activateView.test.ts | 1 + src/services/CaptureService.ts | 26 ++++++++++++++++++-------- tests/e2e/table-cell-capture.test.ts | 4 ++++ tests/mocks/captureEditor.ts | 7 ++++++- 5 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/commands.test.ts b/src/commands.test.ts index 60fad145..61269022 100644 --- a/src/commands.test.ts +++ b/src/commands.test.ts @@ -602,6 +602,7 @@ describe("capture", () => { await undo(); expect(editor.getValue()).toBe("Heard: draft line"); + expect(editor.getSelection()).toBe("draft line"); }); it.each([ diff --git a/src/main.activateView.test.ts b/src/main.activateView.test.ts index 0d3315c7..f066e7c0 100644 --- a/src/main.activateView.test.ts +++ b/src/main.activateView.test.ts @@ -669,6 +669,7 @@ describe("PodNotes onload wiring (#55)", () => { getLine: () => "", lineCount: () => 1, getValue: () => "", + listSelections: () => [], replaceSelection: vi.fn(), }; Object.assign(plugin.app.workspace, { diff --git a/src/services/CaptureService.ts b/src/services/CaptureService.ts index accae5e7..d24b0fec 100644 --- a/src/services/CaptureService.ts +++ b/src/services/CaptureService.ts @@ -2,6 +2,7 @@ import { MarkdownView, Notice, type Editor, + type EditorSelection, type MarkdownFileInfo, type TFile, type WorkspaceLeaf, @@ -27,6 +28,8 @@ export interface Capture { interface Snapshot { readonly before: string; readonly after: string; + /** The selections of the editor the capture went into, as it found them. */ + readonly selections?: EditorSelection[]; } /** Puts a capture where the user writes: the editor they used last, else the episode note. */ @@ -101,16 +104,17 @@ export class CaptureService { cursorLine: editor.getCursor("from").line, }); const before = editor.getValue(); + const selections = editor.listSelections(); endDetachedTableCell(editor); const cell = tableCellWriter(editor); if (cell) cell(inserted); else editor.replaceSelection(inserted); - const snapshot = { before, after: editor.getValue() }; + const snapshot = { before, after: editor.getValue(), selections }; // Outside a note's own view, as in a canvas card, the editor's text need not be // its file's, so only that editor can take the capture back. const undo = target instanceof MarkdownView && file - ? () => this.revert(file, snapshot) + ? () => this.revert(file, snapshot, editor) : () => target.editor === editor && target.file === file && @@ -164,8 +168,11 @@ export class CaptureService { ); } - /** Puts the note back as the capture found it, while it is exactly as the capture left it. */ - private async revert(file: TFile, snapshot: Snapshot): Promise { + /** + * Puts the note back as the capture found it, while it is exactly as the capture left it, + * in `editor` while it still edits the note. + */ + private async revert(file: TFile, snapshot: Snapshot, editor?: Editor): Promise { // An open editor's buffer is the note: a vault write behind it could lose what it // has not saved yet. Obsidian saves a note as it leaves editing, so once that save // lands, a closed tab or one in Reading view leaves the file as the note. @@ -178,7 +185,7 @@ export class CaptureService { if (first) { return ( editing.every((view) => view.editor.getValue() === snapshot.after) && - revertIn(first.editor, snapshot) + revertIn((editing.find((view) => view.editor === editor) ?? first).editor, snapshot) ); } try { @@ -213,10 +220,10 @@ export class CaptureService { } /** - * Puts back only the stretch the capture changed, so the caret and the editor's own - * undo history stay. + * Puts back only the stretch the capture changed, so the editor's own undo history stays, + * and the selections the capture found. */ -function revertIn(editor: Editor, { before, after }: Snapshot): boolean { +function revertIn(editor: Editor, { before, after, selections }: Snapshot): boolean { if (editor.getValue() !== after) return false; endDetachedTableCell(editor); let from = 0; @@ -239,6 +246,9 @@ function revertIn(editor: Editor, { before, after }: Snapshot): boolean { editor.offsetToPos(from), editor.offsetToPos(after.length - tail), ); + // When Obsidian padded the table around the cell, the stretch holds the caret, and the + // edit moves it to the stretch's start, before the table. + if (selections) editor.setSelections(selections); return true; } diff --git a/tests/e2e/table-cell-capture.test.ts b/tests/e2e/table-cell-capture.test.ts index 0d1d8157..74cda6a9 100644 --- a/tests/e2e/table-cell-capture.test.ts +++ b/tests/e2e/table-cell-capture.test.ts @@ -370,6 +370,10 @@ describe("Live Preview table captures", () => { expect( await obsidian.dev.evalJson(`${views}.map((view) => view.editor.getValue())`), ).toEqual([before, before]); + expect( + await obsidian.dev.evalJson(`(${views})[0].editor.getCursor()`), + "the first pane's caret stays", + ).toEqual({ line: 7, ch: 11 }); }); test.each(["Source mode", "the tab showed another note"])( diff --git a/tests/mocks/captureEditor.ts b/tests/mocks/captureEditor.ts index 63346b9e..1bcfa096 100644 --- a/tests/mocks/captureEditor.ts +++ b/tests/mocks/captureEditor.ts @@ -1,4 +1,4 @@ -import type { EditorPosition } from "obsidian"; +import type { EditorPosition, EditorSelection } from "obsidian"; /** Enough of CodeMirror for a capture and its Undo: text, a selection, offsets and ranges. */ export function fakeEditor(text: string, caret = text.length) { @@ -26,6 +26,11 @@ export function fakeEditor(text: string, caret = text.length) { from = offset(anchor); to = offset(head); }, + listSelections: (): EditorSelection[] => [{ anchor: pos(from), head: pos(to) }], + setSelections([{ anchor, head }]: EditorSelection[]) { + from = offset(anchor); + to = offset(head); + }, replaceSelection: (replacement: string) => { value = value.slice(0, from) + replacement + value.slice(to); from = to = from + replacement.length; From a504835d3a5e2d08c22d86355b2b268945cf8ebe Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Thu, 15 Oct 2026 16:43:46 +0200 Subject: [PATCH 4/4] fix(capture): restore the caret only in its editor, and forget a closed popout's notice Undo restored the selections a capture found into whichever editor it reverted in. With the capturing tab closed and another tab of the note open, that moved the other tab's caret. The snapshot now keeps the caret with the editor it came from, and Undo restores it only there. A popout that closes while its Undo notice shows keeps the notice in its document, so the button stayed connected and the notice was checked every second for good. The check now also ends once the button's document has no window. Unit tests cover both, and that the notice is built in the main window. The table journey scrolled out of view now scrolls until the table leaves the viewport, as CodeMirror can scroll back to the cell first. The new e2e helpers say what they do in their names. --- src/commands.test.ts | 12 ++++++++++ src/services/CaptureService.ts | 29 +++++++++--------------- src/ui/undoNotice.test.ts | 34 +++++++++++++++++++++++++++- src/ui/undoNotice.ts | 5 ++-- tests/e2e/journey-notes.test.ts | 8 +++---- tests/e2e/journey/actions.ts | 13 ++++------- tests/e2e/table-cell-capture.test.ts | 14 ++++++------ 7 files changed, 74 insertions(+), 41 deletions(-) diff --git a/src/commands.test.ts b/src/commands.test.ts index 61269022..aaaeefb5 100644 --- a/src/commands.test.ts +++ b/src/commands.test.ts @@ -605,6 +605,18 @@ describe("capture", () => { expect(editor.getSelection()).toBe("draft line"); }); + it("takes the timestamp out of the note's other tab once the tab it went into closed, leaving that tab's caret", async () => { + const { leaf, editor, close, openNote } = await intoEditor("Notes\nLater", 5); + const other = openNote("Journal.md", editor.getValue(), 0); + close(leaf); + + await undo(); + + expect(other.editor.getValue()).toBe("Notes\nLater"); + expect(other.editor.getCursor()).toEqual({ line: 0, ch: 0 }); + expect(said()).toEqual(["Timestamp added to Journal"]); + }); + it.each([ ["its tab was closed", (tab: Tab) => tab.close()], [ diff --git a/src/services/CaptureService.ts b/src/services/CaptureService.ts index d24b0fec..4f387cfe 100644 --- a/src/services/CaptureService.ts +++ b/src/services/CaptureService.ts @@ -24,12 +24,11 @@ export interface Capture { created: boolean; } -/** A note's text as a capture found it and as it left it. */ +/** A note's text as a capture found it and as it left it, and the caret it found in its editor. */ interface Snapshot { readonly before: string; readonly after: string; - /** The selections of the editor the capture went into, as it found them. */ - readonly selections?: EditorSelection[]; + readonly caret?: { readonly editor: Editor; readonly selections: EditorSelection[] }; } /** Puts a capture where the user writes: the editor they used last, else the episode note. */ @@ -104,17 +103,17 @@ export class CaptureService { cursorLine: editor.getCursor("from").line, }); const before = editor.getValue(); - const selections = editor.listSelections(); + const caret = { editor, selections: editor.listSelections() }; endDetachedTableCell(editor); const cell = tableCellWriter(editor); if (cell) cell(inserted); else editor.replaceSelection(inserted); - const snapshot = { before, after: editor.getValue(), selections }; + const snapshot = { before, after: editor.getValue(), caret }; // Outside a note's own view, as in a canvas card, the editor's text need not be // its file's, so only that editor can take the capture back. const undo = target instanceof MarkdownView && file - ? () => this.revert(file, snapshot, editor) + ? () => this.revert(file, snapshot) : () => target.editor === editor && target.file === file && @@ -168,11 +167,8 @@ export class CaptureService { ); } - /** - * Puts the note back as the capture found it, while it is exactly as the capture left it, - * in `editor` while it still edits the note. - */ - private async revert(file: TFile, snapshot: Snapshot, editor?: Editor): Promise { + /** Puts the note back as the capture found it, while it is exactly as the capture left it. */ + private async revert(file: TFile, snapshot: Snapshot): Promise { // An open editor's buffer is the note: a vault write behind it could lose what it // has not saved yet. Obsidian saves a note as it leaves editing, so once that save // lands, a closed tab or one in Reading view leaves the file as the note. @@ -185,7 +181,7 @@ export class CaptureService { if (first) { return ( editing.every((view) => view.editor.getValue() === snapshot.after) && - revertIn((editing.find((view) => view.editor === editor) ?? first).editor, snapshot) + revertIn(first.editor, snapshot) ); } try { @@ -219,11 +215,8 @@ export class CaptureService { } } -/** - * Puts back only the stretch the capture changed, so the editor's own undo history stays, - * and the selections the capture found. - */ -function revertIn(editor: Editor, { before, after, selections }: Snapshot): boolean { +/** Puts back only the stretch the capture changed, so the editor's own undo history stays. */ +function revertIn(editor: Editor, { before, after, caret }: Snapshot): boolean { if (editor.getValue() !== after) return false; endDetachedTableCell(editor); let from = 0; @@ -248,7 +241,7 @@ function revertIn(editor: Editor, { before, after, selections }: Snapshot): bool ); // When Obsidian padded the table around the cell, the stretch holds the caret, and the // edit moves it to the stretch's start, before the table. - if (selections) editor.setSelections(selections); + if (caret?.editor === editor) editor.setSelections(caret.selections); return true; } diff --git a/src/ui/undoNotice.test.ts b/src/ui/undoNotice.test.ts index f54a4123..89a41ddc 100644 --- a/src/ui/undoNotice.test.ts +++ b/src/ui/undoNotice.test.ts @@ -1,6 +1,6 @@ // @vitest-environment jsdom import * as obsidian from "obsidian"; -import { afterEach, expect, test, vi } from "vitest"; +import { afterEach, expect, onTestFinished, test, vi } from "vitest"; import { dismissUndoNotices, noticeWithUndo } from "./undoNotice"; afterEach(() => { @@ -31,6 +31,38 @@ test("says what happened, with an Undo button, for seven seconds", () => { expect(read().button).toHaveTextContent("Undo"); }); +test("builds its message in the main window, as Obsidian's Notice needs also with a popout focused", () => { + const frame = document.body.appendChild(document.createElement("iframe")); + const popout = frame.contentWindow as Window & typeof globalThis; + // Obsidian gives a popout's nodes its DOM helpers too. + const { createEl, createSpan } = DocumentFragment.prototype; + Object.assign(popout.DocumentFragment.prototype, { createEl, createSpan }); + vi.stubGlobal("activeDocument", popout.document); + onTestFinished(() => { + vi.unstubAllGlobals(); + frame.remove(); + }); + const notice = vi.spyOn(obsidian, "Notice"); + + noticeWithUndo("Timestamp added to Draft", () => {}); + + // Obsidian shows any other message as its String(). + expect(notice.mock.calls[0][0]).toBeInstanceOf(DocumentFragment); +}); + +test("stops checking on a notice left in the document of a popout that closed", () => { + vi.useFakeTimers(); + const notice = vi.spyOn(obsidian, "Notice"); + noticeWithUndo("Timestamp added to Draft", () => {}); + // A closed popout's document has no window, and still holds the notice. + const closed = document.implementation.createHTMLDocument(); + closed.body.append(notice.mock.calls[0][0] as DocumentFragment); + + vi.advanceTimersByTime(9000); + + expect(vi.getTimerCount()).toBe(0); +}); + test("Undo runs once and hides the notice", () => { const read = shown(); const undo = vi.fn(); diff --git a/src/ui/undoNotice.ts b/src/ui/undoNotice.ts index e1bdb2ad..a247194a 100644 --- a/src/ui/undoNotice.ts +++ b/src/ui/undoNotice.ts @@ -21,9 +21,10 @@ export function noticeWithUndo(message: string, undo: () => void): void { }; button.addEventListener("click", run); showing.add(dismiss); - // Obsidian keeps a notice up past its time while the pointer rests on it. + // Obsidian keeps a notice up past its time while the pointer rests on it, and a popout + // that closes keeps it in its document. const forget = () => { - if (button.isConnected) window.setTimeout(forget, 1000); + if (button.isConnected && button.doc.defaultView) window.setTimeout(forget, 1000); else showing.delete(dismiss); }; window.setTimeout(forget, 7000); diff --git a/tests/e2e/journey-notes.test.ts b/tests/e2e/journey-notes.test.ts index 27704ade..79bbb859 100644 --- a/tests/e2e/journey-notes.test.ts +++ b/tests/e2e/journey-notes.test.ts @@ -2,10 +2,10 @@ import { describe, expect, onTestFinished, test } from "vitest"; import { createPodNotesE2EHarness } from "./harness"; import { clickInEditor, - clickInNoteWindow, + clickLastInNoteWindow, closePopoutTab, focusNoteWindow, - noticesBeside, + noticesInNoteWindow, openInEditor, openInReadingView, reloadAfterPopout, @@ -1025,10 +1025,10 @@ describe("episode note and capture journeys", () => { (text) => text === captured, "the capture in the popout draft", ); - expect(await noticesBeside(obsidian, draft)).toEqual([ + expect(await noticesInNoteWindow(obsidian, draft)).toEqual([ { message: "Timestamp added to Focused draft", button: "Undo" }, ]); - await clickInNoteWindow(obsidian, draft, ".notice button.pn-undo"); + await clickLastInNoteWindow(obsidian, draft, ".notice button.pn-undo"); await waitForText(draft, (text) => text === "Popout notes\n", "the draft as it was"); } finally { if (await obsidian.dev.evalJson("app.workspace.floatingSplit.children.length")) diff --git a/tests/e2e/journey/actions.ts b/tests/e2e/journey/actions.ts index 7bd9e47d..f21bebe0 100644 --- a/tests/e2e/journey/actions.ts +++ b/tests/e2e/journey/actions.ts @@ -285,7 +285,7 @@ export async function clickInEditor( })()`); } -/** Gives the window holding the open editor of `notePath` the OS focus, as a click into it does. */ +/** Gives the window holding the open editor of `notePath` the OS focus, which input sent to it does not. */ export async function focusNoteWindow(obsidian: ObsidianClient, notePath: string): Promise { const focused = await obsidian.dev.evalJsonAsync(`(async () => { ${inNoteWindow(notePath)} @@ -297,11 +297,7 @@ export async function focusNoteWindow(obsidian: ObsidianClient, notePath: string if (!focused) throw new Error(`The window showing ${notePath} did not take focus.`); } -/** - * Clicks the last element `selector` matches, with `text` when given, in the window holding - * the open editor of `notePath`. - */ -export async function clickInNoteWindow( +export async function clickLastInNoteWindow( obsidian: ObsidianClient, notePath: string, selector: string, @@ -317,11 +313,10 @@ export async function clickInNoteWindow( press(box.left + box.width / 2, box.top + box.height / 2); return true; })()`); - if (!clicked) throw new Error(`Nothing matches ${selector} beside ${notePath}.`); + if (!clicked) throw new Error(`Nothing matches ${selector} in the window of ${notePath}.`); } -/** The text and Undo button of each Notice in the window holding the open editor of `notePath`. */ -export async function noticesBeside( +export async function noticesInNoteWindow( obsidian: ObsidianClient, notePath: string, ): Promise> { diff --git a/tests/e2e/table-cell-capture.test.ts b/tests/e2e/table-cell-capture.test.ts index 74cda6a9..14d94cda 100644 --- a/tests/e2e/table-cell-capture.test.ts +++ b/tests/e2e/table-cell-capture.test.ts @@ -2,7 +2,7 @@ import { describe, expect, onTestFinished, test } from "vitest"; import { createPodNotesE2EHarness } from "./harness"; import { clickInEditor, - clickInNoteWindow, + clickLastInNoteWindow, closePopoutTab, focusNoteWindow, openInEditor, @@ -506,11 +506,11 @@ describe("Live Preview table captures", () => { const editor = `app.workspace.getLeavesOfType("markdown").find((leaf) => leaf.view.file?.path === ${jsLiteral(path)}).view.editor`; await typeText(obsidian, "USER_EDIT"); if (where === "scrolled out of view") { - await obsidian.dev.evalJson( - `(() => { const { scrollDOM } = ${editor}.cm; scrollDOM.scrollTop = scrollDOM.scrollHeight; return true; })()`, - ); await obsidian.waitFor( - () => obsidian.dev.evalJson(`${editor}.cm.viewport.from > 1000`), + () => + obsidian.dev.evalJson( + `(() => { const { scrollDOM, viewport } = ${editor}.cm; scrollDOM.scrollTop = scrollDOM.scrollHeight; return viewport.from > 1000; })()`, + ), { timeoutMs: 5_000, intervalMs: 100, @@ -559,7 +559,7 @@ describe("Live Preview table captures", () => { return true; })()`); await focusNoteWindow(obsidian, path); - await clickInNoteWindow(obsidian, path, ".markdown-source-view td", "anchor"); + await clickLastInNoteWindow(obsidian, path, ".markdown-source-view td", "anchor"); await obsidian.dev.evalJson(`(() => { const remote = require("@electron/remote"); const main = remote.getCurrentWindow(); @@ -608,7 +608,7 @@ describe("Live Preview table captures", () => { expect( await eventually(obsidian, () => readVaultText(obsidian, path), captured, 10_000), ).toBe(captured); - await clickInNoteWindow(obsidian, path, ".notice button.pn-undo"); + await clickLastInNoteWindow(obsidian, path, ".notice button.pn-undo"); expect( await eventually(obsidian, () => readVaultText(obsidian, path), before, 10_000), ).toBe(before);