diff --git a/src/commands.test.ts b/src/commands.test.ts index 60fad145..aaaeefb5 100644 --- a/src/commands.test.ts +++ b/src/commands.test.ts @@ -602,6 +602,19 @@ describe("capture", () => { await undo(); expect(editor.getValue()).toBe("Heard: draft line"); + 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([ 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..4f387cfe 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, @@ -23,10 +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; + readonly caret?: { readonly editor: Editor; readonly selections: EditorSelection[] }; } /** Puts a capture where the user writes: the editor they used last, else the episode note. */ @@ -101,11 +103,12 @@ export class CaptureService { cursorLine: editor.getCursor("from").line, }); const before = editor.getValue(); + 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() }; + 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 = @@ -212,11 +215,8 @@ export class CaptureService { } } -/** - * Puts back only the stretch the capture changed, so the caret and the editor's own - * undo history stay. - */ -function revertIn(editor: Editor, { before, after }: 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; @@ -239,6 +239,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 (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 dc5b3bcf..a247194a 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); @@ -20,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 500fb1fe..79bbb859 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, + clickLastInNoteWindow, closePopoutTab, + focusNoteWindow, + noticesInNoteWindow, 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 noticesInNoteWindow(obsidian, draft)).toEqual([ + { message: "Timestamp added to Focused draft", button: "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")) + 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..f21bebe0 100644 --- a/tests/e2e/journey/actions.ts +++ b/tests/e2e/journey/actions.ts @@ -285,6 +285,50 @@ export async function clickInEditor( })()`); } +/** 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)} + 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.`); +} + +export async function clickLastInNoteWindow( + 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} in the window of ${notePath}.`); +} + +export async function noticesInNoteWindow( + 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..14d94cda 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, + clickLastInNoteWindow, + closePopoutTab, + focusNoteWindow, openInEditor, + reloadAfterPopout, runFromPalette, showLibrary, showPodNotes, @@ -366,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"])( @@ -483,6 +491,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.waitFor( + () => + obsidian.dev.evalJson( + `(() => { const { scrollDOM, viewport } = ${editor}.cm; scrollDOM.scrollTop = scrollDOM.scrollHeight; return 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 clickLastInNoteWindow(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 clickLastInNoteWindow(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 }); 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; 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") {