From 273079e8278546b4e0a8b56a3a97b454b244200f Mon Sep 17 00:00:00 2001 From: vorotamoroz Date: Thu, 29 Oct 2026 14:58:38 +0000 Subject: [PATCH] Own browser UI sessions and shared navigation --- devs.md | 12 + package.json | 4 +- src/apps/browser/BrowserConfirm.ts | 21 +- src/apps/browser/BrowserMenu.ts | 113 ++++-- .../browser/BrowserSvelteDialogManager.ts | 41 ++- src/apps/browser/BrowserUiScope.ts | 38 +++ src/apps/browser/LiveSyncBrowserUIService.ts | 21 +- .../createLiveSyncBrowserServiceHub.ts | 26 +- src/common/diffNavigation.ts | 25 ++ ...cumentHistoryModal.navigation.unit.spec.ts | 321 ++++++++++++++++++ .../DocumentHistory/DocumentHistoryModal.ts | 30 +- .../ConflictResolveModal.ts | 20 +- .../ConflictResolveModal.unit.spec.ts | 189 ++++++++++- src/modules/services/SvelteDialogSession.ts | 28 +- test/apps/browser/BrowserConfirm.unit.spec.ts | 73 ++++ test/apps/browser/BrowserMenu.unit.spec.ts | 286 ++++++++++++++++ .../BrowserSvelteDialogManager.unit.spec.ts | 199 +++++++++++ test/apps/browser/BrowserUiScope.unit.spec.ts | 56 +++ .../browser/SvelteDialogSession.unit.spec.ts | 61 ++++ .../ui-lifecycle/browser-ui-lifecycle.test.ts | 202 +++++++++++ test/browser-apps/ui-lifecycle/index.html | 15 + .../src/TestLifecycleDialog.svelte | 16 + test/browser-apps/ui-lifecycle/src/main.ts | 154 +++++++++ test/browser-apps/ui-lifecycle/tsconfig.json | 9 + test/browser-apps/ui-lifecycle/vite.config.ts | 25 ++ .../scripts/conflict-dialog-policy.ts | 36 +- .../scripts/document-history-nav.ts | 44 +++ updates.md | 4 + 28 files changed, 1948 insertions(+), 121 deletions(-) create mode 100644 src/apps/browser/BrowserUiScope.ts create mode 100644 src/common/diffNavigation.ts create mode 100644 src/modules/features/DocumentHistory/DocumentHistoryModal.navigation.unit.spec.ts create mode 100644 test/apps/browser/BrowserConfirm.unit.spec.ts create mode 100644 test/apps/browser/BrowserMenu.unit.spec.ts create mode 100644 test/apps/browser/BrowserSvelteDialogManager.unit.spec.ts create mode 100644 test/apps/browser/BrowserUiScope.unit.spec.ts create mode 100644 test/browser-apps/ui-lifecycle/browser-ui-lifecycle.test.ts create mode 100644 test/browser-apps/ui-lifecycle/index.html create mode 100644 test/browser-apps/ui-lifecycle/src/TestLifecycleDialog.svelte create mode 100644 test/browser-apps/ui-lifecycle/src/main.ts create mode 100644 test/browser-apps/ui-lifecycle/tsconfig.json create mode 100644 test/browser-apps/ui-lifecycle/vite.config.ts diff --git a/devs.md b/devs.md index 788d351b..d618e88c 100644 --- a/devs.md +++ b/devs.md @@ -174,6 +174,18 @@ Declare shared UI requests with `declareUICommand` from `@vrtmr The registry belongs to one UIService instance. Reuse the same declaration object at each call site, and keep active UI cleanup in the feature and dialogue owner. `UIService` disposes the registry when the host's context emits the plug-in-unloaded event. +Each browser service hub creates a local `BrowserUiScope` with a `document` and +`signal`, exposed as readonly `hub.browserUi`. Pass overrides through +the service-hub option `ui: { document, signal }`; when `document` is omitted, +the scope snapshots legacy `_activeDocument`. The scope is bound to +its root `ServiceContext` unload event and optional parent `AbortSignal`, and is +shared by browser confirmation and the Svelte dialogue manager. A dialogue +resolves with its result or `undefined` on ordinary close; cancellation rejects +with the abort reason, including `Error('Plug-in unloaded')` on root-context +unload. A `Menu` can receive `hub.browserUi`; closing, replacing, or aborting it +unmounts its Svelte component, and callbacks from a closed session cannot close +the replacement. + ### Key Architectural Components - **LiveSyncLocalDB** (`@vrtmrz/livesync-commonlib/compat/pouchdb/LiveSyncLocalDB`): Local PouchDB database wrapper diff --git a/package.json b/package.json index a9efc5aa..53bab329 100644 --- a/package.json +++ b/package.json @@ -132,7 +132,9 @@ "version": "node version-bump.mjs && node update-workspaces.mjs && npm run pretty:json && git add manifest.json versions.json src/apps/cli/package.json src/apps/webpeer/package.json src/apps/webapp/package.json", "test:e2e:obsidian:startup-integrity": "tsx test/e2e-obsidian/scripts/startup-integrity.ts", "test:e2e:obsidian:third-party-chunks": "tsx test/e2e-obsidian/scripts/third-party-chunks.ts", - "test:e2e:obsidian:upgrade-livesync-batching": "tsx test/e2e-obsidian/scripts/upgrade-livesync-batching.ts" + "test:e2e:obsidian:upgrade-livesync-batching": "tsx test/e2e-obsidian/scripts/upgrade-livesync-batching.ts", + "pretest:browser-ui-lifecycle": "svelte-check --tsconfig test/browser-apps/ui-lifecycle/tsconfig.json --fail-on-warnings && vite build --config test/browser-apps/ui-lifecycle/vite.config.ts", + "test:browser-ui-lifecycle": "deno test -A --no-check --frozen --config test/browser-apps/deno.json --lock test/browser-apps/deno.lock test/browser-apps/ui-lifecycle/browser-ui-lifecycle.test.ts" }, "keywords": [], "author": "vorotamoroz", diff --git a/src/apps/browser/BrowserConfirm.ts b/src/apps/browser/BrowserConfirm.ts index a7cbc5a7..8f875777 100644 --- a/src/apps/browser/BrowserConfirm.ts +++ b/src/apps/browser/BrowserConfirm.ts @@ -1,11 +1,10 @@ import type { ServiceContext } from "@vrtmrz/livesync-commonlib/context"; -import { _activeDocument } from "@vrtmrz/livesync-commonlib/compat/common/coreEnvFunctions"; -import { EVENT_PLUGIN_UNLOADED } from "@vrtmrz/livesync-commonlib/compat/events/coreEvents"; import { BrowserUiNotifications, createBrowserUi } from "@vrtmrz/browser-ui-kit"; import { createNativeElement } from "@/apps/browserDom"; import { renderMessageMarkdownInto } from "./ui/renderMessageMarkdown"; import { UiInteractionsConfirm } from "./UiInteractionsConfirm"; +import { createBrowserUiScope, type BrowserUiScope } from "./BrowserUiScope"; /** * Compatibility facade consumed by Commonlib while browser presentation is @@ -14,26 +13,26 @@ import { UiInteractionsConfirm } from "./UiInteractionsConfirm"; export class BrowserConfirm extends UiInteractionsConfirm { readonly context: T; - constructor(context: T) { - const dialogueController = new AbortController(); + constructor(context: T, ui: BrowserUiScope = createBrowserUiScope(context)) { const notifications = new BrowserUiNotifications({ - document: _activeDocument, + document: ui.document, }); super({ ui: createBrowserUi({ - document: _activeDocument, - signal: dialogueController.signal, + document: ui.document, + signal: ui.signal, renderMarkdown: ({ container, markdown }) => { renderMessageMarkdownInto(container, markdown); }, }), notifications, - createActionAnchor: () => createNativeElement(_activeDocument, "a"), + createActionAnchor: () => createNativeElement(ui.document, "a"), }); this.context = context; - context.events.onceEvent(EVENT_PLUGIN_UNLOADED, () => { - dialogueController.abort(); + if (ui.signal.aborted) { notifications.dispose(); - }); + } else { + ui.signal.addEventListener("abort", () => notifications.dispose(), { once: true }); + } } } diff --git a/src/apps/browser/BrowserMenu.ts b/src/apps/browser/BrowserMenu.ts index 55af8921..7b9bd8db 100644 --- a/src/apps/browser/BrowserMenu.ts +++ b/src/apps/browser/BrowserMenu.ts @@ -1,9 +1,19 @@ -import { promiseWithResolvers, type PromiseWithResolvers } from "octagonal-wheels/promises"; +import { fireAndForget, promiseWithResolvers, type PromiseWithResolvers } from "octagonal-wheels/promises"; import { createNativeElement } from "@/apps/browserDom"; -import { mount } from "svelte"; +import type { BrowserUiOptions } from "./BrowserUiScope"; +import { mount, unmount } from "svelte"; import MenuView from "./ui/MenuView.svelte"; import { _activeDocument } from "@vrtmrz/livesync-commonlib/compat/common/coreEnvFunctions"; +type MountedMenu = ReturnType; +type MenuSession = { + waitingForClose: PromiseWithResolvers; + element: HTMLElement; + component?: MountedMenu; + abortListener?: () => void; + closed: boolean; +}; + export class MenuItem { type = "item"; title = ""; @@ -29,7 +39,15 @@ export class Menu { type = "menu"; items: (MenuItem | MenuSeparator)[] = []; - constructor() {} + private readonly document: Document; + private readonly signal?: AbortSignal; + private activeSession?: MenuSession; + + constructor(options: BrowserUiOptions = {}) { + this.document = options.document ?? _activeDocument; + this.signal = options.signal; + } + addItem(callback: (item: MenuItem) => void) { const item = new MenuItem(); callback(item); @@ -41,32 +59,75 @@ export class Menu { return this; } waitingForClose?: PromiseWithResolvers; - showAtPosition(pos: { x: number; y: number }) { - const el = createNativeElement(_activeDocument, "div"); - if (this.waitingForClose) { - this.waitingForClose.resolve(); + + showAtPosition(pos: { x: number; y: number }): Promise { + if (this.activeSession) { + this.closeSession(this.activeSession); } - this.waitingForClose = promiseWithResolvers(); - mount(MenuView, { - target: el, - props: { - items: this.items, - closeMenu: () => { - this.waitingForClose?.resolve(); - this.waitingForClose = undefined; + if (this.signal?.aborted) { + this.waitingForClose = undefined; + return Promise.resolve(); + } + + const waitingForClose = promiseWithResolvers(); + const element = createNativeElement(this.document, "div"); + const session: MenuSession = { + waitingForClose, + element, + closed: false, + }; + this.activeSession = session; + this.waitingForClose = waitingForClose; + + const closeMenu = () => this.closeSession(session); + session.abortListener = closeMenu; + this.signal?.addEventListener("abort", closeMenu, { once: true }); + + try { + session.component = mount(MenuView, { + target: element, + props: { + items: this.items, + closeMenu, + x: pos.x, + y: pos.y, }, - x: pos.x, - y: pos.y, - }, - }); - _activeDocument.body.appendChild(el); - void this.waitingForClose.promise.finally(() => { - el.remove(); - }); - return this.waitingForClose.promise; + }); + if (!session.closed) { + this.document.body.appendChild(element); + } else if (session.component) { + fireAndForget(() => unmount(session.component!)); + } + } catch (error) { + this.closeSession(session); + throw error; + } + + return waitingForClose.promise; } + + private closeSession(session: MenuSession): void { + if (session.closed) return; + session.closed = true; + if (session.abortListener) { + this.signal?.removeEventListener("abort", session.abortListener); + session.abortListener = undefined; + } + if (this.activeSession === session) { + this.activeSession = undefined; + this.waitingForClose = undefined; + } + + session.element.remove(); + if (session.component) { + fireAndForget(() => unmount(session.component!)); + } + session.waitingForClose.resolve(); + } + hide() { - this.waitingForClose?.resolve(); - this.waitingForClose = undefined; + if (this.activeSession) { + this.closeSession(this.activeSession); + } } } diff --git a/src/apps/browser/BrowserSvelteDialogManager.ts b/src/apps/browser/BrowserSvelteDialogManager.ts index b0e89a0d..17ae4f8d 100644 --- a/src/apps/browser/BrowserSvelteDialogManager.ts +++ b/src/apps/browser/BrowserSvelteDialogManager.ts @@ -8,6 +8,7 @@ import type { SvelteDialogManagerDependencies } from "@vrtmrz/livesync-commonlib import { _activeDocument } from "@vrtmrz/livesync-commonlib/compat/common/coreEnvFunctions"; import DialogHost from "@/modules/services/LiveSyncUI/DialogHost.svelte"; import { SvelteDialogSession } from "@/modules/services/SvelteDialogSession"; +import { createBrowserUiScope, type BrowserUiScope } from "./BrowserUiScope"; export class BrowserModal { contentEl: HTMLElement; @@ -15,14 +16,14 @@ export class BrowserModal { modalEl: HTMLElement; isOpen: boolean = false; baseEl: HTMLElement; - constructor() { - const baseEl = createNativeElement(_activeDocument, "popup"); + constructor(document: Document = _activeDocument) { + const baseEl = createNativeElement(document, "popup"); this.baseEl = baseEl; - this.contentEl = createNativeElement(_activeDocument, "div"); + this.contentEl = createNativeElement(document, "div"); this.contentEl.className = "modal-content"; - this.titleEl = createNativeElement(_activeDocument, "div"); + this.titleEl = createNativeElement(document, "div"); this.titleEl.className = "modal-title"; - this.modalEl = createNativeElement(_activeDocument, "div"); + this.modalEl = createNativeElement(document, "div"); this.modalEl.className = "modal"; this.modalEl.hidden = true; this.modalEl.appendChild(this.titleEl); @@ -33,7 +34,7 @@ export class BrowserModal { this.isOpen = true; this.modalEl.hidden = false; if (!this.baseEl.parentElement) { - _activeDocument.body.appendChild(this.baseEl); + this.baseEl.ownerDocument.body.appendChild(this.baseEl); } this.onOpen(); } @@ -57,9 +58,10 @@ export class LiveSyncBrowserDialog, component: ComponentHasResult, - initialData?: U + initialData?: U, + private readonly ui?: BrowserUiScope ) { - super(); + super(ui?.document ?? _activeDocument); this.session = new SvelteDialogSession({ surface: this, context, @@ -67,9 +69,15 @@ export class LiveSyncBrowserDialog extends SvelteDialogManagerBase { + constructor( + context: T, + dependencies: SvelteDialogManagerDependencies, + private readonly ui: BrowserUiScope = createBrowserUiScope(context) + ) { + super(context, dependencies); + } + override async openSvelteDialog( component: ComponentHasResult, initialData?: TU ): Promise { - const dialog = new LiveSyncBrowserDialog(this.context, this.dependents, component, initialData); + if (this.ui.signal.aborted) throw this.ui.signal.reason ?? new Error("Plug-in unloaded"); + const dialog = new LiveSyncBrowserDialog( + this.context, + this.dependents, + component, + initialData, + this.ui + ); dialog.open(); return await dialog.waitForClose(); } diff --git a/src/apps/browser/BrowserUiScope.ts b/src/apps/browser/BrowserUiScope.ts new file mode 100644 index 00000000..3fbc4439 --- /dev/null +++ b/src/apps/browser/BrowserUiScope.ts @@ -0,0 +1,38 @@ +import type { ServiceContext } from "@vrtmrz/livesync-commonlib/context"; +import { EVENT_PLUGIN_UNLOADED } from "@vrtmrz/livesync-commonlib/compat/events/coreEvents"; +import { _activeDocument } from "@vrtmrz/livesync-commonlib/compat/common/coreEnvFunctions"; + +export interface BrowserUiOptions { + readonly document?: Document; + readonly signal?: AbortSignal; +} + +/** Browser presentation target and lifetime owned by one application context. */ +export interface BrowserUiScope { + readonly document: Document; + readonly signal: AbortSignal; +} + +export function createBrowserUiScope( + context: Pick, + options: BrowserUiOptions = {} +): BrowserUiScope { + const controller = new AbortController(); + const parent = options.signal; + let unregisterUnload: (() => void) | undefined; + const end = (reason: unknown): void => { + if (controller.signal.aborted) return; + unregisterUnload?.(); + unregisterUnload = undefined; + parent?.removeEventListener("abort", onParentAbort); + controller.abort(reason); + }; + const onParentAbort = () => end(parent?.reason); + if (parent?.aborted) { + controller.abort(parent.reason); + } else { + unregisterUnload = context.events.onceEvent(EVENT_PLUGIN_UNLOADED, () => end(new Error("Plug-in unloaded"))); + parent?.addEventListener("abort", onParentAbort, { once: true }); + } + return { document: options.document ?? _activeDocument, signal: controller.signal }; +} diff --git a/src/apps/browser/LiveSyncBrowserUIService.ts b/src/apps/browser/LiveSyncBrowserUIService.ts index c0ab9d00..e8be307d 100644 --- a/src/apps/browser/LiveSyncBrowserUIService.ts +++ b/src/apps/browser/LiveSyncBrowserUIService.ts @@ -7,6 +7,7 @@ import type { InjectableAPIService } from "@vrtmrz/livesync-commonlib/compat/ser import { UIService } from "@vrtmrz/livesync-commonlib/compat/services/implements/base/UIService"; import DialogToCopy from "@/modules/services/LiveSyncUI/dialogues/DialogueToCopy.svelte"; import { BrowserSvelteDialogManager } from "./BrowserSvelteDialogManager"; +import type { BrowserUiScope } from "./BrowserUiScope"; export interface LiveSyncBrowserUIServiceDependencies { API: InjectableAPIService; @@ -20,15 +21,19 @@ export class LiveSyncBrowserUIService extends UIServic override get dialogToCopy() { return DialogToCopy; } - constructor(context: T, dependents: LiveSyncBrowserUIServiceDependencies) { + constructor(context: T, dependents: LiveSyncBrowserUIServiceDependencies, ui?: BrowserUiScope) { const browserConfirm = dependents.API.confirm; - const obsidianSvelteDialogManager = new BrowserSvelteDialogManager(context, { - appLifecycle: dependents.appLifecycle, - config: dependents.config, - replicator: dependents.replicator, - confirm: browserConfirm, - control: dependents.control, - }); + const obsidianSvelteDialogManager = new BrowserSvelteDialogManager( + context, + { + appLifecycle: dependents.appLifecycle, + config: dependents.config, + replicator: dependents.replicator, + confirm: browserConfirm, + control: dependents.control, + }, + ui + ); super(context, { dialogManager: obsidianSvelteDialogManager, APIService: dependents.API, diff --git a/src/apps/browser/createLiveSyncBrowserServiceHub.ts b/src/apps/browser/createLiveSyncBrowserServiceHub.ts index 3197114a..cb0e355d 100644 --- a/src/apps/browser/createLiveSyncBrowserServiceHub.ts +++ b/src/apps/browser/createLiveSyncBrowserServiceHub.ts @@ -22,6 +22,7 @@ import { InjectableVaultServiceCompat } from "@vrtmrz/livesync-commonlib/compat/ import { setLang, translateLiveSyncMessage } from "@/common/translation"; import { BrowserConfirm } from "./BrowserConfirm"; +import { createBrowserUiScope, type BrowserUiOptions, type BrowserUiScope } from "./BrowserUiScope"; import { createBrowserKeyValueDatabaseFactory } from "./BrowserKeyValueDatabase"; import { LiveSyncBrowserAPIService, type LiveSyncBrowserAPIServiceOptions } from "./LiveSyncBrowserAPIService"; import { LiveSyncBrowserUIService } from "./LiveSyncBrowserUIService"; @@ -41,6 +42,7 @@ export interface LiveSyncBrowserRestartPolicy { export interface LiveSyncBrowserServiceHubOptions { deviceValues: ValueStore & ValueDeleter; context?: T; + ui?: BrowserUiOptions; getSystemVaultName?: () => string; settings?: LiveSyncBrowserSettingsPersistence; restart?: LiveSyncBrowserRestartPolicy; @@ -56,15 +58,18 @@ class LiveSyncBrowserKeyValueDBService extends KeyValu /** LiveSync-owned service composition shared by WebApp and WebPeer. */ export class LiveSyncBrowserServiceHub extends InjectableServiceHub { + readonly browserUi: BrowserUiScope; + constructor(options: LiveSyncBrowserServiceHubOptions) { const context = options.context ?? (new ServiceContext({ translate: translateLiveSyncMessage, }) as T); + const browserUi = createBrowserUiScope(context, options.ui); const API = new LiveSyncBrowserAPIService(context, { ...options.API, - confirm: new BrowserConfirm(context), + confirm: new BrowserConfirm(context, browserUi), getSystemVaultName: options.getSystemVaultName ?? (() => "livesync-browser"), }); const conflict = new InjectableConflictService(context); @@ -147,13 +152,17 @@ export class LiveSyncBrowserServiceHub extends Injecta APIService: API, replicatorService: replicator, }); - const ui = new LiveSyncBrowserUIService(context, { - API, - appLifecycle, - config, - control, - replicator, - }); + const ui = new LiveSyncBrowserUIService( + context, + { + API, + appLifecycle, + config, + control, + replicator, + }, + browserUi + ); super(context, { API, @@ -175,6 +184,7 @@ export class LiveSyncBrowserServiceHub extends Injecta ui, vault, }); + this.browserUi = browserUi; } } diff --git a/src/common/diffNavigation.ts b/src/common/diffNavigation.ts new file mode 100644 index 00000000..543e8490 --- /dev/null +++ b/src/common/diffNavigation.ts @@ -0,0 +1,25 @@ +import { stepCyclicIndex } from "octagonal-wheels/collection"; + +export function navigateDiffNodes( + root: ParentNode, + selector: string, + navigation: { + currentDiffIndex: number; + diffNavIndicator: { setText(text: string): void }; + }, + direction: "prev" | "next" +): void { + const diffElements = root.querySelectorAll(selector); + if (diffElements.length === 0) return; + + const previousFocused = root.querySelector(".diff-focused"); + if (previousFocused) { + previousFocused.classList.remove("diff-focused"); + } + + navigation.currentDiffIndex = stepCyclicIndex(navigation.currentDiffIndex, diffElements.length, direction); + const target = diffElements[navigation.currentDiffIndex]; + target.classList.add("diff-focused"); + target.scrollIntoView({ behavior: "smooth", block: "center" }); + navigation.diffNavIndicator.setText(`${navigation.currentDiffIndex + 1}/${diffElements.length}`); +} diff --git a/src/modules/features/DocumentHistory/DocumentHistoryModal.navigation.unit.spec.ts b/src/modules/features/DocumentHistory/DocumentHistoryModal.navigation.unit.spec.ts new file mode 100644 index 00000000..bef11ead --- /dev/null +++ b/src/modules/features/DocumentHistory/DocumentHistoryModal.navigation.unit.spec.ts @@ -0,0 +1,321 @@ +import { describe, expect, it, vi } from "vitest"; + +vi.mock("@/deps.ts", () => ({ + Modal: class Modal {}, + TFile: class TFile {}, +})); + +vi.mock("@/main.ts", () => ({ + default: class ObsidianLiveSyncPlugin {}, +})); + +vi.mock("@/common/utils.ts", () => ({ + getPathFromTFile: (file: { path: string }) => file.path, + isValidPath: vi.fn(() => true), +})); + +vi.mock("octagonal-wheels/concurrency/lock", () => ({ + scheduleOnceIfDuplicated: vi.fn((_key: string, callback: () => unknown) => callback()), +})); + +import { scheduleOnceIfDuplicated } from "octagonal-wheels/concurrency/lock"; +import { DocumentHistoryModal } from "./DocumentHistoryModal.ts"; + +type NavigationTarget = { + classList: { add: ReturnType; remove: ReturnType }; + scrollIntoView: ReturnType; +}; + +function createDiffView(count: number) { + const focused: { current: NavigationTarget | undefined } = { current: undefined }; + const createTargets = (targetCount: number) => + Array.from({ length: targetCount }, () => { + const target: NavigationTarget = { + classList: { + add: vi.fn(() => { + focused.current = target; + }), + remove: vi.fn(() => { + if (focused.current === target) focused.current = undefined; + }), + }, + scrollIntoView: vi.fn(), + }; + return target; + }); + let targets = createTargets(count); + const contentView = { + querySelectorAll: vi.fn(() => targets), + querySelector: vi.fn(() => (targets.includes(focused.current as NavigationTarget) ? focused.current : null)), + }; + return { + contentView, + targets, + replaceTargets(targetCount: number) { + focused.current = undefined; + targets = createTargets(targetCount); + return targets; + }, + }; +} + +function createHistoryModal() { + return Object.create(DocumentHistoryModal.prototype) as DocumentHistoryModal; +} + +describe("DocumentHistoryModal cyclic navigation", () => { + it("commits the selected index before scrolling, including when scrolling fails", () => { + const modal = createHistoryModal(); + const view = createDiffView(2); + const indicator = { setText: vi.fn() }; + const failure = new Error("fixture scroll failed"); + Object.assign(modal, { contentView: view.contentView, currentDiffIndex: -1, diffNavIndicator: indicator }); + view.targets[0].scrollIntoView.mockImplementation(() => { + expect(modal.currentDiffIndex).toBe(0); + throw failure; + }); + expect(() => modal.navigateDiff("next")).toThrow(failure); + expect(modal.currentDiffIndex).toBe(0); + expect(indicator.setText).not.toHaveBeenCalled(); + }); + + it("starts at the first diff for next and at the last diff for previous", () => { + const nextModal = createHistoryModal(); + const nextView = createDiffView(3); + const nextIndicator = { setText: vi.fn() }; + Object.assign(nextModal, { + contentView: nextView.contentView, + currentDiffIndex: -1, + diffNavIndicator: nextIndicator, + }); + + nextModal.navigateDiff("next"); + + expect(nextModal.currentDiffIndex).toBe(0); + expect(nextView.targets[0].classList.add).toHaveBeenCalledWith("diff-focused"); + expect(nextView.targets[0].scrollIntoView).toHaveBeenCalledWith({ behavior: "smooth", block: "center" }); + expect(nextIndicator.setText).toHaveBeenCalledWith("1/3"); + + const previousModal = createHistoryModal(); + const previousView = createDiffView(3); + const previousIndicator = { setText: vi.fn() }; + Object.assign(previousModal, { + contentView: previousView.contentView, + currentDiffIndex: -1, + diffNavIndicator: previousIndicator, + }); + + previousModal.navigateDiff("prev"); + + expect(previousModal.currentDiffIndex).toBe(2); + expect(previousView.targets[2].classList.add).toHaveBeenCalledWith("diff-focused"); + expect(previousView.targets[2].scrollIntoView).toHaveBeenCalledWith({ behavior: "smooth", block: "center" }); + expect(previousIndicator.setText).toHaveBeenCalledWith("3/3"); + }); + + it("wraps in both directions while moving focus, scrolling, and updating the indicator", () => { + const modal = createHistoryModal(); + const { contentView, targets } = createDiffView(3); + const indicator = { setText: vi.fn() }; + Object.assign(modal, { contentView, currentDiffIndex: -1, diffNavIndicator: indicator }); + + modal.navigateDiff("next"); + modal.navigateDiff("next"); + modal.navigateDiff("next"); + modal.navigateDiff("next"); + modal.navigateDiff("prev"); + + expect(modal.currentDiffIndex).toBe(2); + expect(targets.map((target) => target.classList.add.mock.calls.length)).toEqual([2, 1, 2]); + expect(targets.map((target) => target.classList.remove.mock.calls.length)).toEqual([2, 1, 1]); + expect(targets.map((target) => target.scrollIntoView.mock.calls.length)).toEqual([2, 1, 2]); + expect(indicator.setText.mock.calls.map(([text]) => text)).toEqual(["1/3", "2/3", "3/3", "1/3", "3/3"]); + }); + + it("leaves the current navigation index and content view untouched when no diff exists", () => { + const modal = createHistoryModal(); + const { contentView } = createDiffView(0); + const indicator = { setText: vi.fn() }; + Object.assign(modal, { contentView, currentDiffIndex: 1, diffNavIndicator: indicator }); + + modal.navigateDiff("next"); + + expect(modal.currentDiffIndex).toBe(1); + expect(contentView.querySelector).not.toHaveBeenCalled(); + expect(indicator.setText).not.toHaveBeenCalled(); + }); + + it("looks up diff nodes again after the content view is replaced", () => { + const modal = createHistoryModal(); + const view = createDiffView(2); + const indicator = { setText: vi.fn() }; + Object.assign(modal, { contentView: view.contentView, currentDiffIndex: -1, diffNavIndicator: indicator }); + + modal.navigateDiff("next"); + const replacementTargets = view.replaceTargets(3); + modal.navigateDiff("next"); + + expect(modal.currentDiffIndex).toBe(1); + expect(view.contentView.querySelectorAll).toHaveBeenCalledTimes(2); + expect(view.contentView.querySelector).toHaveBeenCalledTimes(2); + expect(replacementTargets[1].classList.add).toHaveBeenCalledWith("diff-focused"); + expect(replacementTargets[1].scrollIntoView).toHaveBeenCalledWith({ behavior: "smooth", block: "center" }); + expect(indicator.setText.mock.calls.map(([text]) => text)).toEqual(["1/2", "2/3"]); + }); + + it("keeps navigation indices and focus independent between modal instances", () => { + const firstModal = createHistoryModal(); + const secondModal = createHistoryModal(); + const firstView = createDiffView(2); + const secondView = createDiffView(2); + Object.assign(firstModal, { + contentView: firstView.contentView, + currentDiffIndex: -1, + diffNavIndicator: { setText: vi.fn() }, + }); + Object.assign(secondModal, { + contentView: secondView.contentView, + currentDiffIndex: -1, + diffNavIndicator: { setText: vi.fn() }, + }); + + firstModal.navigateDiff("next"); + secondModal.navigateDiff("next"); + firstModal.navigateDiff("next"); + + expect(firstModal.currentDiffIndex).toBe(1); + expect(secondModal.currentDiffIndex).toBe(0); + expect(firstView.targets[0].classList.remove).toHaveBeenCalledWith("diff-focused"); + expect(secondView.targets[0].classList.remove).not.toHaveBeenCalled(); + expect(secondView.targets[0].classList.add).toHaveBeenCalledWith("diff-focused"); + }); + + it("resets the indicator and visibility according to showDiff", () => { + const modal = createHistoryModal(); + const view = createDiffView(2); + const indicator = { setText: vi.fn() }; + const diffNavContainer = { setCssStyles: vi.fn() }; + const diffOnlyLabel = { setCssStyles: vi.fn() }; + Object.assign(modal, { + contentView: view.contentView, + currentDiffIndex: 1, + diffNavIndicator: indicator, + diffNavContainer, + diffOnlyLabel, + showDiff: true, + }); + + modal.resetDiffNavigation(); + + expect(modal.currentDiffIndex).toBe(-1); + expect(view.contentView.querySelectorAll).toHaveBeenCalledWith(".history-added, .history-deleted"); + expect(indicator.setText).toHaveBeenLastCalledWith("0/2"); + expect(diffNavContainer.setCssStyles).toHaveBeenLastCalledWith({ display: "flex" }); + expect(diffOnlyLabel.setCssStyles).toHaveBeenLastCalledWith({ display: "inline-block" }); + + view.replaceTargets(0); + modal.resetDiffNavigation(); + expect(indicator.setText).toHaveBeenLastCalledWith("\u2014"); + + Object.assign(modal, { showDiff: false }); + modal.resetDiffNavigation(); + expect(view.contentView.querySelectorAll).toHaveBeenCalledTimes(2); + expect(indicator.setText).toHaveBeenLastCalledWith("\u2014"); + expect(diffNavContainer.setCssStyles).toHaveBeenLastCalledWith({ display: "none" }); + expect(diffOnlyLabel.setCssStyles).toHaveBeenLastCalledWith({ display: "none" }); + }); + + it("updates visibility without an indicator", () => { + const modal = createHistoryModal(); + const { contentView } = createDiffView(2); + const diffNavContainer = { setCssStyles: vi.fn() }; + Object.assign(modal, { contentView, currentDiffIndex: 1, diffNavContainer, showDiff: true }); + + expect(() => modal.resetDiffNavigation()).not.toThrow(); + + expect(modal.currentDiffIndex).toBe(-1); + expect(contentView.querySelectorAll).not.toHaveBeenCalled(); + expect(diffNavContainer.setCssStyles).toHaveBeenCalledWith({ display: "flex" }); + }); + + it("maps selected search results to the revision slider and loads each wrapped selection", () => { + const modal = createHistoryModal(); + const scheduleLoad = vi.mocked(scheduleOnceIfDuplicated); + scheduleLoad.mockClear(); + const indicator = { setText: vi.fn() }; + Object.assign(modal, { + searchResults: [ + { rev: "newest", index: 0, matchType: "Content" }, + { rev: "middle", index: 2, matchType: "Diff" }, + { rev: "oldest", index: 4, matchType: "Content" }, + ], + currentSearchIndex: -1, + revs_info: Array.from({ length: 5 }, (_, index) => ({ rev: `rev-${index}` })), + range: { value: "" }, + loadRevs: vi.fn(), + searchResultIndicator: indicator, + searchPrevBtn: { disabled: true }, + searchNextBtn: { disabled: true }, + searchKeyword: "needle", + showDiff: false, + }); + + modal.navigateSearch("next"); + expect(modal.currentSearchIndex).toBe(0); + expect(modal.range.value).toBe("4"); + expect(modal.loadRevs).toHaveBeenCalledTimes(1); + expect(scheduleLoad).toHaveBeenCalledWith("loadRevs", expect.any(Function)); + expect(indicator.setText).toHaveBeenLastCalledWith("1/3 matches"); + + modal.navigateSearch("next"); + expect(modal.currentSearchIndex).toBe(1); + expect(modal.range.value).toBe("2"); + expect(modal.loadRevs).toHaveBeenCalledTimes(2); + + modal.navigateSearch("next"); + expect(modal.currentSearchIndex).toBe(2); + expect(modal.range.value).toBe("0"); + + modal.navigateSearch("next"); + expect(modal.currentSearchIndex).toBe(0); + expect(modal.range.value).toBe("4"); + + modal.navigateSearch("prev"); + expect(modal.currentSearchIndex).toBe(2); + expect(modal.range.value).toBe("0"); + expect(indicator.setText.mock.calls.map(([text]) => text)).toEqual([ + "1/3 matches", + "2/3 matches", + "3/3 matches", + "1/3 matches", + "3/3 matches", + ]); + }); + + it("leaves search state and controls untouched when there are no results", () => { + const modal = createHistoryModal(); + const indicator = { setText: vi.fn() }; + const searchPrevBtn = { disabled: true }; + const searchNextBtn = { disabled: true }; + Object.assign(modal, { + searchResults: [], + currentSearchIndex: -1, + range: { value: "unchanged" }, + loadRevs: vi.fn(), + searchResultIndicator: indicator, + searchPrevBtn, + searchNextBtn, + searchKeyword: "needle", + showDiff: false, + }); + + modal.navigateSearch("next"); + + expect(modal.currentSearchIndex).toBe(-1); + expect(modal.range.value).toBe("unchanged"); + expect(modal.loadRevs).not.toHaveBeenCalled(); + expect(indicator.setText).not.toHaveBeenCalled(); + expect(searchPrevBtn.disabled).toBe(true); + expect(searchNextBtn.disabled).toBe(true); + }); +}); diff --git a/src/modules/features/DocumentHistory/DocumentHistoryModal.ts b/src/modules/features/DocumentHistory/DocumentHistoryModal.ts index 57b734ba..1a9b106e 100644 --- a/src/modules/features/DocumentHistory/DocumentHistoryModal.ts +++ b/src/modules/features/DocumentHistory/DocumentHistoryModal.ts @@ -15,6 +15,8 @@ import { isErrorOfMissingDoc } from "@vrtmrz/livesync-commonlib/compat/pouchdb/u import { fireAndForget, getDocData } from "@vrtmrz/livesync-commonlib/compat/common/utils"; import { isPlainText, stripPrefix } from "@vrtmrz/livesync-commonlib/compat/string_and_binary/path"; import { scheduleOnceIfDuplicated } from "octagonal-wheels/concurrency/lock"; +import { stepCyclicIndex } from "octagonal-wheels/collection"; +import { navigateDiffNodes } from "@/common/diffNavigation.ts"; import type { LiveSyncBaseCore } from "@/LiveSyncBaseCore.ts"; import { compatGlobal } from "@vrtmrz/livesync-commonlib/compat/common/coreEnvFunctions"; import { @@ -375,26 +377,7 @@ export class DocumentHistoryModal extends Modal { * Only effective when diff highlighting is enabled. */ navigateDiff(direction: "prev" | "next") { - const diffElements = this.contentView.querySelectorAll(".history-added, .history-deleted"); - if (diffElements.length === 0) return; - - // Remove previous focus highlight - const prevFocused = this.contentView.querySelector(".diff-focused"); - if (prevFocused) { - prevFocused.classList.remove("diff-focused"); - } - - if (direction === "next") { - this.currentDiffIndex = (this.currentDiffIndex + 1) % diffElements.length; - } else { - this.currentDiffIndex = this.currentDiffIndex <= 0 ? diffElements.length - 1 : this.currentDiffIndex - 1; - } - - const target = diffElements[this.currentDiffIndex]; - target.classList.add("diff-focused"); - target.scrollIntoView({ behavior: "smooth", block: "center" }); - - this.diffNavIndicator.setText(`${this.currentDiffIndex + 1}/${diffElements.length}`); + navigateDiffNodes(this.contentView, ".history-added, .history-deleted", this, direction); } /** @@ -519,12 +502,7 @@ export class DocumentHistoryModal extends Modal { navigateSearch(direction: "prev" | "next") { if (this.searchResults.length === 0) return; - if (direction === "next") { - this.currentSearchIndex = (this.currentSearchIndex + 1) % this.searchResults.length; - } else { - this.currentSearchIndex = - this.currentSearchIndex <= 0 ? this.searchResults.length - 1 : this.currentSearchIndex - 1; - } + this.currentSearchIndex = stepCyclicIndex(this.currentSearchIndex, this.searchResults.length, direction); const match = this.searchResults[this.currentSearchIndex]; this.range.value = `${this.revs_info.length - 1 - match.index}`; diff --git a/src/modules/features/InteractiveConflictResolving/ConflictResolveModal.ts b/src/modules/features/InteractiveConflictResolving/ConflictResolveModal.ts index a5b22068..bc6674d7 100644 --- a/src/modules/features/InteractiveConflictResolving/ConflictResolveModal.ts +++ b/src/modules/features/InteractiveConflictResolving/ConflictResolveModal.ts @@ -7,6 +7,7 @@ import { type FilePathWithPrefix, } from "@vrtmrz/livesync-commonlib/compat/common/types"; import { EVENT_CONFLICT_CANCELLED, EVENT_PLUGIN_UNLOADED, eventHub } from "@/common/events.ts"; +import { navigateDiffNodes } from "@/common/diffNavigation.ts"; import { promiseWithResolvers } from "octagonal-wheels/promises"; import { POSTPONED, type MergeDialogResult } from "@/serviceFeatures/interactiveConflictResolution/types"; @@ -83,24 +84,7 @@ export class ConflictResolveModal extends Modal { } navigateDiff(direction: "prev" | "next") { - const diffElements = this.diffView.querySelectorAll(".added, .deleted"); - if (diffElements.length === 0) return; - - const prevFocused = this.diffView.querySelector(".diff-focused"); - if (prevFocused) { - prevFocused.classList.remove("diff-focused"); - } - - if (direction === "next") { - this.currentDiffIndex = (this.currentDiffIndex + 1) % diffElements.length; - } else { - this.currentDiffIndex = this.currentDiffIndex <= 0 ? diffElements.length - 1 : this.currentDiffIndex - 1; - } - - const target = diffElements[this.currentDiffIndex]; - target.classList.add("diff-focused"); - target.scrollIntoView({ behavior: "smooth", block: "center" }); - this.diffNavIndicator.setText(`${this.currentDiffIndex + 1}/${diffElements.length}`); + navigateDiffNodes(this.diffView, ".added, .deleted", this, direction); } resetDiffNavigation() { diff --git a/src/modules/features/InteractiveConflictResolving/ConflictResolveModal.unit.spec.ts b/src/modules/features/InteractiveConflictResolving/ConflictResolveModal.unit.spec.ts index 8e4cfd94..b37fb8d1 100644 --- a/src/modules/features/InteractiveConflictResolving/ConflictResolveModal.unit.spec.ts +++ b/src/modules/features/InteractiveConflictResolving/ConflictResolveModal.unit.spec.ts @@ -106,11 +106,7 @@ describe("ConflictResolveModal result lifecycle", () => { }); it("closes and completes its result when the plug-in unloads", async () => { - const modal = new ConflictResolveModal( - {} as never, - "open-during-unload.md" as FilePathWithPrefix, - conflict - ); + const modal = new ConflictResolveModal({} as never, "open-during-unload.md" as FilePathWithPrefix, conflict); modal.onOpen(); eventHub.emitEvent(EVENT_PLUGIN_UNLOADED); @@ -165,3 +161,186 @@ describe("ConflictResolveModal result lifecycle", () => { expect(previousState).toBe("still-open"); }); }); + +type NavigationTarget = { + classList: { add: ReturnType; remove: ReturnType }; + scrollIntoView: ReturnType; +}; + +function createDiffView(count: number) { + const focused: { current: NavigationTarget | undefined } = { current: undefined }; + const createTargets = (targetCount: number) => + Array.from({ length: targetCount }, () => { + const target: NavigationTarget = { + classList: { + add: vi.fn(() => { + focused.current = target; + }), + remove: vi.fn(() => { + if (focused.current === target) focused.current = undefined; + }), + }, + scrollIntoView: vi.fn(), + }; + return target; + }); + let targets = createTargets(count); + const diffView = { + querySelectorAll: vi.fn(() => targets), + querySelector: vi.fn(() => (targets.includes(focused.current as NavigationTarget) ? focused.current : null)), + }; + return { + diffView, + targets, + replaceTargets(targetCount: number) { + focused.current = undefined; + targets = createTargets(targetCount); + return targets; + }, + }; +} + +describe("ConflictResolveModal cyclic navigation", () => { + it("commits the selected index before scrolling, including when scrolling fails", () => { + const modal = Object.create(ConflictResolveModal.prototype) as ConflictResolveModal; + const view = createDiffView(2); + const indicator = { setText: vi.fn() }; + const failure = new Error("fixture scroll failed"); + Object.assign(modal, { diffView: view.diffView, currentDiffIndex: -1, diffNavIndicator: indicator }); + view.targets[0].scrollIntoView.mockImplementation(() => { + expect(modal.currentDiffIndex).toBe(0); + throw failure; + }); + expect(() => modal.navigateDiff("next")).toThrow(failure); + expect(modal.currentDiffIndex).toBe(0); + expect(indicator.setText).not.toHaveBeenCalled(); + }); + + it("starts at the first diff for next and at the last diff for previous", () => { + const nextModal = new ConflictResolveModal({} as never, "next.md" as FilePathWithPrefix, conflict); + const nextView = createDiffView(3); + const nextIndicator = { setText: vi.fn() }; + Object.assign(nextModal, { + diffView: nextView.diffView, + currentDiffIndex: -1, + diffNavIndicator: nextIndicator, + }); + + nextModal.navigateDiff("next"); + + expect(nextModal.currentDiffIndex).toBe(0); + expect(nextView.targets[0].classList.add).toHaveBeenCalledWith("diff-focused"); + expect(nextView.targets[0].scrollIntoView).toHaveBeenCalledWith({ behavior: "smooth", block: "center" }); + expect(nextIndicator.setText).toHaveBeenCalledWith("1/3"); + + const previousModal = new ConflictResolveModal({} as never, "previous.md" as FilePathWithPrefix, conflict); + const previousView = createDiffView(3); + const previousIndicator = { setText: vi.fn() }; + Object.assign(previousModal, { + diffView: previousView.diffView, + currentDiffIndex: -1, + diffNavIndicator: previousIndicator, + }); + + previousModal.navigateDiff("prev"); + + expect(previousModal.currentDiffIndex).toBe(2); + expect(previousView.targets[2].classList.add).toHaveBeenCalledWith("diff-focused"); + expect(previousView.targets[2].scrollIntoView).toHaveBeenCalledWith({ behavior: "smooth", block: "center" }); + expect(previousIndicator.setText).toHaveBeenCalledWith("3/3"); + }); + + it("wraps in both directions while moving focus, scrolling, and updating the indicator", () => { + const modal = new ConflictResolveModal({} as never, "wrap.md" as FilePathWithPrefix, conflict); + const { diffView, targets } = createDiffView(3); + const indicator = { setText: vi.fn() }; + Object.assign(modal, { diffView, currentDiffIndex: -1, diffNavIndicator: indicator }); + + modal.navigateDiff("next"); + modal.navigateDiff("next"); + modal.navigateDiff("next"); + modal.navigateDiff("next"); + modal.navigateDiff("prev"); + + expect(modal.currentDiffIndex).toBe(2); + expect(targets.map((target) => target.classList.add.mock.calls.length)).toEqual([2, 1, 2]); + expect(targets.map((target) => target.classList.remove.mock.calls.length)).toEqual([2, 1, 1]); + expect(targets.map((target) => target.scrollIntoView.mock.calls.length)).toEqual([2, 1, 2]); + expect(indicator.setText.mock.calls.map(([text]) => text)).toEqual(["1/3", "2/3", "3/3", "1/3", "3/3"]); + }); + + it("leaves the current navigation index and diff view untouched when no diff exists", () => { + const modal = new ConflictResolveModal({} as never, "empty.md" as FilePathWithPrefix, conflict); + const { diffView } = createDiffView(0); + const indicator = { setText: vi.fn() }; + Object.assign(modal, { diffView, currentDiffIndex: 1, diffNavIndicator: indicator }); + + modal.navigateDiff("next"); + + expect(modal.currentDiffIndex).toBe(1); + expect(diffView.querySelector).not.toHaveBeenCalled(); + expect(indicator.setText).not.toHaveBeenCalled(); + }); + + it("looks up diff nodes again after the diff view is replaced", () => { + const modal = new ConflictResolveModal({} as never, "replace.md" as FilePathWithPrefix, conflict); + const view = createDiffView(2); + const indicator = { setText: vi.fn() }; + Object.assign(modal, { diffView: view.diffView, currentDiffIndex: -1, diffNavIndicator: indicator }); + + modal.navigateDiff("next"); + const replacementTargets = view.replaceTargets(3); + modal.navigateDiff("next"); + + expect(modal.currentDiffIndex).toBe(1); + expect(view.diffView.querySelectorAll).toHaveBeenCalledTimes(2); + expect(view.diffView.querySelector).toHaveBeenCalledTimes(2); + expect(replacementTargets[1].classList.add).toHaveBeenCalledWith("diff-focused"); + expect(replacementTargets[1].scrollIntoView).toHaveBeenCalledWith({ behavior: "smooth", block: "center" }); + expect(indicator.setText.mock.calls.map(([text]) => text)).toEqual(["1/2", "2/3"]); + }); + + it("keeps navigation indices and focus independent between modal instances", () => { + const firstModal = new ConflictResolveModal({} as never, "first.md" as FilePathWithPrefix, conflict); + const secondModal = new ConflictResolveModal({} as never, "second.md" as FilePathWithPrefix, conflict); + const firstView = createDiffView(2); + const secondView = createDiffView(2); + Object.assign(firstModal, { + diffView: firstView.diffView, + currentDiffIndex: -1, + diffNavIndicator: { setText: vi.fn() }, + }); + Object.assign(secondModal, { + diffView: secondView.diffView, + currentDiffIndex: -1, + diffNavIndicator: { setText: vi.fn() }, + }); + + firstModal.navigateDiff("next"); + secondModal.navigateDiff("next"); + firstModal.navigateDiff("next"); + + expect(firstModal.currentDiffIndex).toBe(1); + expect(secondModal.currentDiffIndex).toBe(0); + expect(firstView.targets[0].classList.remove).toHaveBeenCalledWith("diff-focused"); + expect(secondView.targets[0].classList.remove).not.toHaveBeenCalled(); + expect(secondView.targets[0].classList.add).toHaveBeenCalledWith("diff-focused"); + }); + + it("always resets the indicator using the current diff count", () => { + const modal = new ConflictResolveModal({} as never, "reset.md" as FilePathWithPrefix, conflict); + const view = createDiffView(2); + const indicator = { setText: vi.fn() }; + Object.assign(modal, { diffView: view.diffView, currentDiffIndex: 1, diffNavIndicator: indicator }); + + modal.resetDiffNavigation(); + + expect(modal.currentDiffIndex).toBe(-1); + expect(view.diffView.querySelectorAll).toHaveBeenCalledWith(".added, .deleted"); + expect(indicator.setText).toHaveBeenLastCalledWith("0/2"); + + view.replaceTargets(0); + modal.resetDiffNavigation(); + expect(indicator.setText).toHaveBeenLastCalledWith("\u2014"); + }); +}); diff --git a/src/modules/services/SvelteDialogSession.ts b/src/modules/services/SvelteDialogSession.ts index 3643aae7..07435b50 100644 --- a/src/modules/services/SvelteDialogSession.ts +++ b/src/modules/services/SvelteDialogSession.ts @@ -34,6 +34,7 @@ export interface SvelteDialogSessionOptions; initialData?: TInitial; renderer?: SvelteDialogRenderer; + signal?: AbortSignal; } const svelteDialogRenderer: SvelteDialogRenderer = { @@ -46,11 +47,7 @@ const svelteDialogRenderer: SvelteDialogRenderer = { }; /** Owns one Svelte dialogue result and mount lifecycle for a concrete host surface. */ -export class SvelteDialogSession< - TResult, - TInitial, - TContext extends ServiceContext = ServiceContext, -> { +export class SvelteDialogSession { private readonly surface: SvelteDialogSurface; private readonly context: TContext; private readonly dependencies: SvelteDialogManagerDependencies; @@ -58,11 +55,13 @@ export class SvelteDialogSession< private readonly component: ComponentHasResult; private readonly initialData?: TInitial; private readonly renderer: SvelteDialogRenderer; + private readonly signal?: AbortSignal; private result?: TResult; private pendingResult?: PromiseWithResolvers; private mountedDialog?: MountedDialog; private unregisterUnload?: () => void; + private unregisterAbort?: () => void; constructor(options: SvelteDialogSessionOptions) { this.surface = options.surface; @@ -72,10 +71,14 @@ export class SvelteDialogSession< this.component = options.component; this.initialData = options.initialData; this.renderer = options.renderer ?? svelteDialogRenderer; + this.signal = options.signal; } onOpen(): void { + if (this.signal?.aborted) throw this.signal.reason ?? new Error("Plug-in unloaded"); this.surface.contentEl.replaceChildren(); + this.unregisterAbort?.(); + this.unregisterAbort = undefined; this.unregisterUnload?.(); this.unregisterUnload = undefined; if (this.pendingResult) { @@ -85,14 +88,23 @@ export class SvelteDialogSession< this.result = undefined; const pendingResult = promiseWithResolvers(); this.pendingResult = pendingResult; - this.unregisterUnload = this.context.events.onceEvent(EVENT_PLUGIN_UNLOADED, () => { + const cancel = (reason: unknown) => { if (this.pendingResult !== pendingResult) { return; } this.pendingResult = undefined; - pendingResult.reject(new Error("Plug-in unloaded")); + pendingResult.reject(reason); this.surface.close(); + }; + this.unregisterUnload = this.context.events.onceEvent(EVENT_PLUGIN_UNLOADED, () => { + cancel(new Error("Plug-in unloaded")); }); + const signal = this.signal; + if (signal) { + const onAbort = () => cancel(signal.reason ?? new Error("Plug-in unloaded")); + signal.addEventListener("abort", onAbort, { once: true }); + this.unregisterAbort = () => signal.removeEventListener("abort", onAbort); + } this.mountedDialog = this.renderer.mount(this.dialogHost, this.surface.contentEl, { onSetupContext: (props: DialogSvelteComponentBaseProps) => { @@ -124,6 +136,8 @@ export class SvelteDialogSession< } onClose(): void { + this.unregisterAbort?.(); + this.unregisterAbort = undefined; this.unregisterUnload?.(); this.unregisterUnload = undefined; this.pendingResult?.resolve(this.result); diff --git a/test/apps/browser/BrowserConfirm.unit.spec.ts b/test/apps/browser/BrowserConfirm.unit.spec.ts new file mode 100644 index 00000000..d279eefd --- /dev/null +++ b/test/apps/browser/BrowserConfirm.unit.spec.ts @@ -0,0 +1,73 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { createServiceContext } from "@vrtmrz/livesync-commonlib/context"; +import { EVENT_PLUGIN_UNLOADED } from "@vrtmrz/livesync-commonlib/compat/events/coreEvents"; + +const kit = vi.hoisted(() => ({ + uiOptions: [] as { document: Document; signal: AbortSignal }[], + notices: [] as { document: Document; dispose: ReturnType; show: ReturnType }[], +})); +vi.mock("@vrtmrz/browser-ui-kit", () => ({ + createBrowserUi(options: { document: Document; signal: AbortSignal }) { + kit.uiOptions.push(options); + return { promptText: vi.fn(async () => "selected") }; + }, + BrowserUiNotifications: class { + dispose = vi.fn(); + show = vi.fn(); + constructor(options: { document: Document }) { + kit.notices.push({ document: options.document, dispose: this.dispose, show: this.show }); + } + }, +})); +import { BrowserConfirm } from "@/apps/browser/BrowserConfirm"; +import { createBrowserUiScope } from "@/apps/browser/BrowserUiScope"; + +beforeEach(() => { + kit.uiOptions.length = 0; + kit.notices.length = 0; +}); + +describe("BrowserConfirm UI owner", () => { + it("passes the same document and lifetime to existing Kit capabilities", async () => { + const context = createServiceContext(); + const anchor = { textContent: "", click: vi.fn() }; + const document = { createElement: vi.fn(() => anchor) } as unknown as Document; + const ui = createBrowserUiScope(context, { document }); + const confirm = new BrowserConfirm(context, ui); + expect(kit.uiOptions[0]).toMatchObject({ document, signal: ui.signal }); + expect(kit.notices[0].document).toBe(document); + await expect(confirm.askString("Title", "Value", "Placeholder")).resolves.toBe("selected"); + confirm.askInPopup("key", "Go {HERE}", (link) => { + link.textContent = "Open"; + }); + expect(document.createElement).toHaveBeenCalledWith("a"); + expect(kit.notices[0].show).toHaveBeenCalledWith( + "key", + expect.objectContaining({ action: expect.objectContaining({ label: "Open" }) }) + ); + context.events.emitEvent(EVENT_PLUGIN_UNLOADED); + expect(ui.signal.aborted).toBe(true); + expect(kit.notices[0].dispose).toHaveBeenCalledOnce(); + }); + it("disposes only the notifications belonging to the unloaded context", () => { + const first = createServiceContext(); + const second = createServiceContext(); + new BrowserConfirm(first, createBrowserUiScope(first, { document: {} as Document })); + new BrowserConfirm(second, createBrowserUiScope(second, { document: {} as Document })); + first.events.emitEvent(EVENT_PLUGIN_UNLOADED); + first.events.emitEvent(EVENT_PLUGIN_UNLOADED); + expect(kit.notices[0].dispose).toHaveBeenCalledOnce(); + expect(kit.notices[1].dispose).not.toHaveBeenCalled(); + second.events.emitEvent(EVENT_PLUGIN_UNLOADED); + }); + it("disposes notifications immediately for a pre-aborted owner", () => { + const context = createServiceContext(); + const parent = new AbortController(); + parent.abort(); + new BrowserConfirm(context, createBrowserUiScope(context, { signal: parent.signal })); + expect(kit.uiOptions[0].signal.aborted).toBe(true); + expect(kit.notices[0].dispose).toHaveBeenCalledOnce(); + context.events.emitEvent(EVENT_PLUGIN_UNLOADED); + expect(kit.notices[0].dispose).toHaveBeenCalledOnce(); + }); +}); diff --git a/test/apps/browser/BrowserMenu.unit.spec.ts b/test/apps/browser/BrowserMenu.unit.spec.ts new file mode 100644 index 00000000..b0ef0fb6 --- /dev/null +++ b/test/apps/browser/BrowserMenu.unit.spec.ts @@ -0,0 +1,286 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const browserMenuHarness = vi.hoisted(() => { + const createDocument = () => { + const created: { tag: string; removed: boolean; remove: ReturnType }[] = []; + const appended: unknown[] = []; + const document = { + createElement: vi.fn((tag: string) => { + const element: { tag: string; removed: boolean; remove: ReturnType } = { + tag, + removed: false, + remove: vi.fn(() => { + element.removed = true; + }), + }; + created.push(element); + return element; + }), + body: { + appendChild: vi.fn((element: unknown) => { + appended.push(element); + return element; + }), + }, + }; + return { document, created, appended }; + }; + + return { + documents: { + active: createDocument(), + explicit: createDocument(), + }, + mounts: [] as { + component: unknown; + target: unknown; + props: Record; + handle: { id: number }; + }[], + unmounts: [] as unknown[], + }; +}); + +vi.mock("@vrtmrz/livesync-commonlib/compat/common/coreEnvFunctions", () => ({ + _activeDocument: browserMenuHarness.documents.active.document, +})); +vi.mock("svelte", () => ({ + mount: vi.fn((component: unknown, options: { target: unknown; props: Record }) => { + const handle = { id: browserMenuHarness.mounts.length }; + browserMenuHarness.mounts.push({ component, target: options.target, props: options.props, handle }); + return handle; + }), + unmount: vi.fn((handle: unknown) => { + browserMenuHarness.unmounts.push(handle); + return Promise.resolve(); + }), +})); + +import { Menu, MenuItem, MenuSeparator } from "@/apps/browser/BrowserMenu"; + +type BrowserMenuOptionsFixture = { document?: Document; signal?: AbortSignal }; + +function menuWithOptions(options: BrowserMenuOptionsFixture): Menu { + const MenuWithOptions = Menu as unknown as new (options: BrowserMenuOptionsFixture) => Menu; + return new MenuWithOptions(options); +} + +function explicitDocument(): Document { + return browserMenuHarness.documents.explicit.document as unknown as Document; +} + +function closeMenu(index: number): () => void { + const close = browserMenuHarness.mounts[index]!.props.closeMenu; + if (typeof close !== "function") throw new Error("Menu mount did not receive a close callback"); + return close as () => void; +} + +async function settlesAfterMicrotask(promise: Promise): Promise { + let settled = false; + void promise.then(() => { + settled = true; + }); + await Promise.resolve(); + return settled; +} + +beforeEach(() => { + browserMenuHarness.documents.active.created.length = 0; + browserMenuHarness.documents.active.appended.length = 0; + browserMenuHarness.documents.explicit.created.length = 0; + browserMenuHarness.documents.explicit.appended.length = 0; + browserMenuHarness.mounts.length = 0; + browserMenuHarness.unmounts.length = 0; + vi.clearAllMocks(); +}); + +afterEach(() => { + for (const mount of browserMenuHarness.mounts) { + const close = mount.props.closeMenu; + if (typeof close === "function") (close as () => void)(); + } +}); + +describe("browser Menu lifecycle", () => { + it("preserves item, separator, chained construction, and positioned display APIs", async () => { + const menu = new Menu(); + const handler = vi.fn(async () => {}); + + expect( + menu.addItem((item) => { + item.setTitle("Open").setIcon("folder").onClick(handler); + }) + ).toBe(menu); + expect(menu.addSeparator()).toBe(menu); + expect(menu.items[0]).toBeInstanceOf(MenuItem); + expect(menu.items[0]).toMatchObject({ type: "item", title: "Open", icon: "folder", handler }); + expect(menu.items[1]).toBeInstanceOf(MenuSeparator); + + const displayed = menu.showAtPosition({ x: 12, y: 34 }); + expect(browserMenuHarness.documents.active.document.createElement).toHaveBeenCalledWith("div"); + expect(browserMenuHarness.documents.active.appended).toEqual([browserMenuHarness.mounts[0]!.target]); + expect(browserMenuHarness.mounts[0]!.props).toMatchObject({ items: menu.items, x: 12, y: 34 }); + expect(browserMenuHarness.mounts[0]!.target).toBe(browserMenuHarness.documents.active.created[0]); + closeMenu(0)(); + await expect(displayed).resolves.toBeUndefined(); + }); + + it("keeps the returned close promise pending until a menu close action", async () => { + const menu = new Menu(); + const displayed = menu.showAtPosition({ x: 1, y: 2 }); + + expect(await settlesAfterMicrotask(displayed)).toBe(false); + closeMenu(0)(); + await expect(displayed).resolves.toBeUndefined(); + }); + + it("resolves the active display promise when hidden without a menu action", async () => { + const menu = new Menu(); + const displayed = menu.showAtPosition({ x: 1, y: 2 }); + + menu.hide(); + + await expect(displayed).resolves.toBeUndefined(); + }); + + it("closes a replaced display while keeping the replacement active", async () => { + const menu = new Menu(); + const first = menu.showAtPosition({ x: 1, y: 2 }); + const staleClose = closeMenu(0); + const firstElement = browserMenuHarness.mounts[0]!.target as { removed: boolean }; + const second = menu.showAtPosition({ x: 3, y: 4 }); + const secondElement = browserMenuHarness.mounts[1]!.target as { removed: boolean }; + + await expect(first).resolves.toBeUndefined(); + expect(firstElement.removed).toBe(true); + expect(secondElement.removed).toBe(false); + expect(menu.waitingForClose?.promise).toBe(second); + expect(await settlesAfterMicrotask(second)).toBe(false); + + try { + staleClose(); + expect(menu.waitingForClose?.promise).toBe(second); + expect(secondElement.removed).toBe(false); + expect(await settlesAfterMicrotask(second)).toBe(false); + } finally { + closeMenu(1)(); + } + await expect(second).resolves.toBeUndefined(); + }); + + it("mounts a menu in its explicitly supplied document", async () => { + const menu = menuWithOptions({ document: explicitDocument() }); + + const displayed = menu.showAtPosition({ x: 5, y: 6 }); + + expect(browserMenuHarness.documents.explicit.document.createElement).toHaveBeenCalledWith("div"); + expect(browserMenuHarness.documents.explicit.appended).toEqual([browserMenuHarness.mounts[0]!.target]); + expect(browserMenuHarness.mounts[0]!.target).toBe(browserMenuHarness.documents.explicit.created[0]); + expect(browserMenuHarness.documents.active.document.createElement).not.toHaveBeenCalled(); + closeMenu(0)(); + await expect(displayed).resolves.toBeUndefined(); + }); + + it("aborting one menu closes only that menu", async () => { + const firstOwner = new AbortController(); + const secondOwner = new AbortController(); + const firstMenu = menuWithOptions({ document: explicitDocument(), signal: firstOwner.signal }); + const secondMenu = menuWithOptions({ document: explicitDocument(), signal: secondOwner.signal }); + const first = firstMenu.showAtPosition({ x: 1, y: 2 }); + const second = secondMenu.showAtPosition({ x: 3, y: 4 }); + let firstSettled = false; + void first.then(() => { + firstSettled = true; + }); + firstOwner.abort(); + + await Promise.resolve(); + expect(firstSettled).toBe(true); + expect(secondMenu.waitingForClose?.promise).toBe(second); + expect(await settlesAfterMicrotask(second)).toBe(false); + closeMenu(1)(); + await expect(second).resolves.toBeUndefined(); + }); + + it("does not mount for an already aborted owner", async () => { + const owner = new AbortController(); + owner.abort(); + const menu = menuWithOptions({ document: explicitDocument(), signal: owner.signal }); + + const displayed = menu.showAtPosition({ x: 1, y: 2 }); + + expect(browserMenuHarness.mounts).toHaveLength(0); + expect(await settlesAfterMicrotask(displayed)).toBe(true); + }); + + it("does not mount again after its owner aborts", async () => { + const owner = new AbortController(); + const menu = menuWithOptions({ document: explicitDocument(), signal: owner.signal }); + const first = menu.showAtPosition({ x: 1, y: 2 }); + owner.abort(); + + expect(await settlesAfterMicrotask(first)).toBe(true); + const reopened = menu.showAtPosition({ x: 3, y: 4 }); + expect(await settlesAfterMicrotask(reopened)).toBe(true); + expect(browserMenuHarness.mounts).toHaveLength(1); + }); + + it.each(["hide", "dismissal", "replacement", "abort"] as const)( + "unmounts each Svelte component exactly once for %s", + async (reason) => { + const owner = new AbortController(); + const menu = menuWithOptions({ document: explicitDocument(), signal: owner.signal }); + const first = menu.showAtPosition({ x: 1, y: 2 }); + let second: Promise | undefined; + + if (reason === "hide") { + menu.hide(); + } else if (reason === "dismissal") { + closeMenu(0)(); + } else if (reason === "replacement") { + second = menu.showAtPosition({ x: 3, y: 4 }); + closeMenu(1)(); + } else { + owner.abort(); + } + + expect(await settlesAfterMicrotask(first)).toBe(true); + if (second) expect(await settlesAfterMicrotask(second)).toBe(true); + for (const mount of browserMenuHarness.mounts) { + expect(browserMenuHarness.unmounts.filter((handle) => handle === mount.handle)).toHaveLength(1); + } + } + ); + + it("removes the menu element before resolving the close promise", async () => { + const menu = new Menu(); + const displayed = menu.showAtPosition({ x: 1, y: 2 }); + const element = browserMenuHarness.mounts[0]!.target as { removed: boolean }; + let removedWhenResolved = false; + void displayed.then(() => { + removedWhenResolved = element.removed; + }); + + menu.hide(); + + await displayed; + expect(removedWhenResolved).toBe(true); + }); + + it("removes its abort listener when closed so a later abort does not unmount it again", async () => { + const owner = new AbortController(); + const addListener = vi.spyOn(owner.signal, "addEventListener"); + const removeListener = vi.spyOn(owner.signal, "removeEventListener"); + const menu = menuWithOptions({ document: explicitDocument(), signal: owner.signal }); + const displayed = menu.showAtPosition({ x: 1, y: 2 }); + const handle = browserMenuHarness.mounts[0]!.handle; + + menu.hide(); + await expect(displayed).resolves.toBeUndefined(); + owner.abort(); + + expect(addListener.mock.calls.map(([type]) => type)).toContain("abort"); + expect(removeListener.mock.calls.map(([type]) => type)).toContain("abort"); + expect(browserMenuHarness.unmounts.filter((unmounted) => unmounted === handle)).toHaveLength(1); + }); +}); diff --git a/test/apps/browser/BrowserSvelteDialogManager.unit.spec.ts b/test/apps/browser/BrowserSvelteDialogManager.unit.spec.ts new file mode 100644 index 00000000..71e8ebad --- /dev/null +++ b/test/apps/browser/BrowserSvelteDialogManager.unit.spec.ts @@ -0,0 +1,199 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { createServiceContext } from "@vrtmrz/livesync-commonlib/context"; +import { EVENT_PLUGIN_UNLOADED } from "@vrtmrz/livesync-commonlib/compat/events/coreEvents"; +import type { + ComponentHasResult, + DialogHostProps, + SvelteDialogManagerDependencies, +} from "@vrtmrz/livesync-commonlib/compat/services/implements/base/SvelteDialog"; + +const rendering = vi.hoisted(() => ({ + mount: vi.fn(), + unmount: vi.fn(async (_component: unknown) => {}), + legacy: { document: undefined as Document | undefined }, +})); +vi.mock("svelte", async (importOriginal) => ({ + ...(await importOriginal()), + mount: rendering.mount, + unmount: rendering.unmount, +})); +vi.mock("@vrtmrz/livesync-commonlib/compat/common/coreEnvFunctions", () => ({ + get _activeDocument() { + return rendering.legacy.document; + }, +})); +vi.mock("@/modules/services/LiveSyncUI/DialogHost.svelte", () => ({ default: () => {} })); + +import { + BrowserModal, + BrowserSvelteDialogManager, + LiveSyncBrowserDialog, +} from "@/apps/browser/BrowserSvelteDialogManager"; +import { createBrowserUiScope } from "@/apps/browser/BrowserUiScope"; + +class TestElement { + children: TestElement[] = []; + parentElement: TestElement | null = null; + hidden = false; + className = ""; + textContent = ""; + constructor( + readonly ownerDocument: TestDocument, + readonly tagName: string + ) {} + appendChild(child: TestElement) { + child.remove(); + child.parentElement = this; + this.children.push(child); + return child; + } + remove() { + if (this.parentElement) + this.parentElement.children = this.parentElement.children.filter((child) => child !== this); + this.parentElement = null; + } + replaceChildren() { + for (const child of [...this.children]) child.remove(); + } +} +class TestDocument { + body = new TestElement(this, "body"); + createElement(tag: string) { + return new TestElement(this, tag); + } +} +const asDocument = (document: TestDocument) => document as unknown as Document; +const component = (() => {}) as unknown as ComponentHasResult; +type Context = ReturnType; +type UiOptions = { document?: Document; signal?: AbortSignal }; +function createManager(options?: UiOptions) { + const context = createServiceContext(); + return { + context, + manager: new BrowserSvelteDialogManager( + context, + {} as SvelteDialogManagerDependencies, + options ? createBrowserUiScope(context, options) : undefined + ), + }; +} +function lastMount() { + const call = rendering.mount.mock.calls[rendering.mount.mock.calls.length - 1]; + return { target: call[1].target as TestElement, props: call[1].props as DialogHostProps }; +} + +beforeEach(() => { + vi.clearAllMocks(); + rendering.legacy.document = asDocument(new TestDocument()); + rendering.mount.mockImplementation(() => ({})); +}); +afterEach(() => { + rendering.legacy.document = undefined; +}); + +describe("Browser Svelte dialogue ownership", () => { + it("releases every per-dialogue context subscription on legacy direct close", async () => { + const subscriptions: ReturnType[] = []; + const context = { + events: { + onceEvent: vi.fn(() => { + const unregister = vi.fn(); + subscriptions.push(unregister); + return unregister; + }), + }, + } as unknown as Context; + const dialog = new LiveSyncBrowserDialog(context, {} as SvelteDialogManagerDependencies, component); + dialog.open(); + const result = dialog.waitForClose(); + dialog.close(); + await expect(result).resolves.toBeUndefined(); + expect(subscriptions.length).toBeGreaterThan(0); + for (const unsubscribe of subscriptions) expect(unsubscribe).toHaveBeenCalledOnce(); + }); + + it("preserves the legacy document, normal result, and component cleanup", async () => { + const { manager } = createManager(); + const result = manager.openSvelteDialog(component, "initial"); + const { target, props } = lastMount(); + expect(target.ownerDocument).toBe(rendering.legacy.document); + expect(props.getInitialData?.()).toBe("initial"); + props.setResult("selected"); + props.closeDialog(); + await expect(result).resolves.toBe("selected"); + expect((rendering.legacy.document as unknown as TestDocument).body.children).toHaveLength(0); + expect(rendering.unmount).toHaveBeenCalledOnce(); + }); + it("preserves rejection when the owning context unloads", async () => { + const { context, manager } = createManager(); + const result = manager.openSvelteDialog(component); + const rejected = expect(result).rejects.toThrow("Plug-in unloaded"); + context.events.emitEvent(EVENT_PLUGIN_UNLOADED); + await rejected; + expect(rendering.unmount).toHaveBeenCalledOnce(); + expect((rendering.legacy.document as unknown as TestDocument).body.children).toHaveLength(0); + }); + it("uses its supplied document even when the legacy document changes before opening", async () => { + const targetDocument = new TestDocument(); + const { manager } = createManager({ document: asDocument(targetDocument) }); + rendering.legacy.document = asDocument(new TestDocument()); + const result = manager.openSvelteDialog(component); + const { target, props } = lastMount(); + expect(target.ownerDocument).toBe(targetDocument); + expect(targetDocument.body.children).toHaveLength(1); + expect((rendering.legacy.document as unknown as TestDocument).body.children).toHaveLength(0); + props.closeDialog(); + await expect(result).resolves.toBeUndefined(); + }); + it("aborts one owner without closing another owner's dialogue", async () => { + const firstOwner = new AbortController(); + const firstDocument = new TestDocument(); + const secondDocument = new TestDocument(); + const first = createManager({ document: asDocument(firstDocument), signal: firstOwner.signal }); + const second = createManager({ document: asDocument(secondDocument) }); + const firstResult = first.manager.openSvelteDialog(component); + let observedFirst: unknown = "still pending"; + void firstResult.then( + () => { + observedFirst = "resolved"; + }, + (error) => { + observedFirst = error; + } + ); + const secondResult = second.manager.openSvelteDialog(component); + const secondProps = lastMount().props; + const reason = new Error("fixture owner ended"); + firstOwner.abort(reason); + await vi.waitFor(() => expect(observedFirst).toBe(reason), { timeout: 1000, interval: 20 }); + expect(firstDocument.body.children).toHaveLength(0); + expect(secondDocument.body.children).toHaveLength(1); + secondProps.setResult("second"); + secondProps.closeDialog(); + await expect(secondResult).resolves.toBe("second"); + }); + it("does not render for an already aborted owner or reopen after unload", async () => { + const owner = new AbortController(); + const reason = new Error("fixture owner ended"); + owner.abort(reason); + const aborted = createManager({ signal: owner.signal }); + const rejected = aborted.manager.openSvelteDialog(component).catch((error) => error); + expect(rendering.mount).not.toHaveBeenCalled(); + await expect(rejected).resolves.toBe(reason); + const unloaded = createManager(); + unloaded.context.events.emitEvent(EVENT_PLUGIN_UNLOADED); + const afterUnload = unloaded.manager.openSvelteDialog(component).catch((error) => error); + expect(rendering.mount).not.toHaveBeenCalled(); + await expect(afterUnload).resolves.toBeInstanceOf(Error); + }); + it("keeps a native modal in the document which created its elements", () => { + const document = new TestDocument(); + const modal = new BrowserModal(asDocument(document)); + rendering.legacy.document = asDocument(new TestDocument()); + modal.open(); + expect(document.body.children).toEqual([modal.baseEl]); + expect((modal.contentEl as unknown as TestElement).ownerDocument).toBe(document); + modal.close(); + expect(document.body.children).toHaveLength(0); + }); +}); diff --git a/test/apps/browser/BrowserUiScope.unit.spec.ts b/test/apps/browser/BrowserUiScope.unit.spec.ts new file mode 100644 index 00000000..abf32433 --- /dev/null +++ b/test/apps/browser/BrowserUiScope.unit.spec.ts @@ -0,0 +1,56 @@ +import { createServiceContext } from "@vrtmrz/livesync-commonlib/context"; +import { EVENT_PLUGIN_UNLOADED } from "@vrtmrz/livesync-commonlib/compat/events/coreEvents"; +import { describe, expect, it, vi } from "vitest"; +import { createBrowserUiScope } from "@/apps/browser/BrowserUiScope"; + +describe("browser UI scope", () => { + it("owns its document and cancellation independently from another application", () => { + const firstContext = createServiceContext(); + const secondContext = createServiceContext(); + const firstDocument = {} as Document; + const secondDocument = {} as Document; + const first = createBrowserUiScope(firstContext, { document: firstDocument }); + const second = createBrowserUiScope(secondContext, { document: secondDocument }); + const abort = vi.fn(); + first.signal.addEventListener("abort", abort); + firstContext.events.emitEvent(EVENT_PLUGIN_UNLOADED); + firstContext.events.emitEvent(EVENT_PLUGIN_UNLOADED); + expect(first.document).toBe(firstDocument); + expect(second.document).toBe(secondDocument); + expect(first.signal.aborted).toBe(true); + expect(first.signal.reason).toEqual(new Error("Plug-in unloaded")); + expect(second.signal.aborted).toBe(false); + expect(abort).toHaveBeenCalledOnce(); + secondContext.events.emitEvent(EVENT_PLUGIN_UNLOADED); + }); + it("forwards parent cancellation and unregisters the context subscription", () => { + const context = createServiceContext(); + const parent = new AbortController(); + const unregister = vi.fn(); + const once = vi.spyOn(context.events, "onceEvent").mockReturnValue(unregister); + const scope = createBrowserUiScope(context, { signal: parent.signal }); + const reason = new Error("fixture host ended"); + parent.abort(reason); + expect(scope.signal.aborted).toBe(true); + expect(scope.signal.reason).toBe(reason); + expect(unregister).toHaveBeenCalledOnce(); + once.mockRestore(); + }); + it("removes the parent listener on unload and does not subscribe an already aborted parent", () => { + const context = createServiceContext(); + const parent = new AbortController(); + const remove = vi.spyOn(parent.signal, "removeEventListener"); + const scope = createBrowserUiScope(context, { signal: parent.signal }); + context.events.emitEvent(EVENT_PLUGIN_UNLOADED); + expect(remove).toHaveBeenCalledWith("abort", expect.any(Function)); + const reason = scope.signal.reason; + parent.abort(new Error("later")); + expect(scope.signal.reason).toBe(reason); + const closedContext = createServiceContext(); + const subscribe = vi.spyOn(closedContext.events, "onceEvent"); + const closed = createBrowserUiScope(closedContext, { signal: parent.signal }); + expect(closed.signal.aborted).toBe(true); + expect(closed.signal.reason).toBe(parent.signal.reason); + expect(subscribe).not.toHaveBeenCalled(); + }); +}); diff --git a/test/apps/browser/SvelteDialogSession.unit.spec.ts b/test/apps/browser/SvelteDialogSession.unit.spec.ts index f87fe25f..08b46313 100644 --- a/test/apps/browser/SvelteDialogSession.unit.spec.ts +++ b/test/apps/browser/SvelteDialogSession.unit.spec.ts @@ -86,3 +86,64 @@ describe("SvelteDialogSession", () => { expect(close).toHaveBeenCalledOnce(); }); }); + +describe("SvelteDialogSession owner signal", () => { + it("rejects and releases the mounted dialogue when its owner aborts", async () => { + const context = createServiceContext(); + const controller = new AbortController(); + const lifecycle = { signal: controller.signal }; + const unmount = vi.fn().mockResolvedValue(undefined); + let session: SvelteDialogSession; + const close = vi.fn(() => session.onClose()); + session = new SvelteDialogSession({ + ...lifecycle, + surface: { + contentEl: { replaceChildren: vi.fn() } as unknown as HTMLElement, + close, + setTitle: vi.fn(), + }, + context, + dependencies: {} as SvelteDialogManagerDependencies, + dialogHost: (() => {}) as unknown as Component, + component: (() => {}) as unknown as ComponentHasResult, + renderer: { mount: () => ({}) as never, unmount }, + }); + session.onOpen(); + let observed: unknown = "pending"; + void session.waitForClose().catch((error) => { + observed = error; + }); + const reason = new Error("fixture owner ended"); + controller.abort(reason); + await vi.waitFor(() => expect(observed).toBe(reason), { timeout: 1000, interval: 20 }); + expect(close).toHaveBeenCalledOnce(); + expect(unmount).toHaveBeenCalledOnce(); + context.events.emitEvent(EVENT_PLUGIN_UNLOADED); + expect(close).toHaveBeenCalledOnce(); + }); + + it("detaches an owner signal when normal close has already settled the result", async () => { + const context = createServiceContext(); + const controller = new AbortController(); + const remove = vi.spyOn(controller.signal, "removeEventListener"); + const lifecycle = { signal: controller.signal }; + const close = vi.fn(); + const session = new SvelteDialogSession({ + ...lifecycle, + surface: { contentEl: { replaceChildren: vi.fn() } as unknown as HTMLElement, close, setTitle: vi.fn() }, + context, + dependencies: {} as SvelteDialogManagerDependencies, + dialogHost: (() => {}) as unknown as Component, + component: (() => {}) as unknown as ComponentHasResult, + renderer: { mount: () => ({}) as never, unmount: vi.fn().mockResolvedValue(undefined) }, + }); + session.onOpen(); + const result = session.waitForClose(); + session.onClose(); + await expect(result).resolves.toBeUndefined(); + expect(remove).toHaveBeenCalledWith("abort", expect.any(Function)); + controller.abort(); + context.events.emitEvent(EVENT_PLUGIN_UNLOADED); + expect(close).not.toHaveBeenCalled(); + }); +}); diff --git a/test/browser-apps/ui-lifecycle/browser-ui-lifecycle.test.ts b/test/browser-apps/ui-lifecycle/browser-ui-lifecycle.test.ts new file mode 100644 index 00000000..1376500d --- /dev/null +++ b/test/browser-apps/ui-lifecycle/browser-ui-lifecycle.test.ts @@ -0,0 +1,202 @@ +import { assertEquals } from "@std/assert"; +import { resolve } from "@std/path"; +import { chromium } from "playwright"; + +import { observePageFailures, startStaticServer, waitFor } from "../helpers/browser.ts"; + +type OwnerName = "owner-a" | "owner-b" | "owner-c" | "owner-d"; +type DialogOutcome = + | { readonly kind: "resolved"; readonly value: unknown } + | { readonly kind: "rejected"; readonly message: string }; + +interface BrowserUiLifecycleFixture { + readonly ready: boolean; + startDialog(name: OwnerName, initialData?: string): void; + startMenu(name: OwnerName, title: string): void; + clickSavedBackdrop(name: OwnerName): void; + hideMenu(name: OwnerName): void; + unload(name: OwnerName): void; + abortParent(name: OwnerName): void; + state(name: OwnerName): { + readonly dialogs: readonly DialogOutcome[]; + readonly menus: readonly string[]; + readonly hasSavedBackdrop: boolean; + }; +} + +declare global { + interface Window { + browserUiLifecycle?: BrowserUiLifecycleFixture; + } +} + +const fixtureDist = resolve(import.meta.dirname!, "../../../_testdata/browser-ui-lifecycle"); + +Deno.test({ + name: "Browser UI: dialogue and menu lifecycles remain owned by their document and plug-in scope", + sanitizeOps: false, + sanitizeResources: false, + async fn() { + const server = await startStaticServer(fixtureDist); + const browser = await chromium.launch({ headless: true }); + try { + const page = await browser.newPage(); + const assertNoPageFailures = observePageFailures(page); + await page.goto(server.baseUrl); + await page.waitForFunction(() => window.browserUiLifecycle?.ready === true); + + const startDialog = async (name: OwnerName, initialData?: string) => + await page.evaluate( + ({ name, initialData }) => window.browserUiLifecycle!.startDialog(name, initialData), + { name, initialData } + ); + const startMenu = async (name: OwnerName, title: string) => + await page.evaluate(({ name, title }) => window.browserUiLifecycle!.startMenu(name, title), { + name, + title, + }); + const state = async (name: OwnerName) => + await page.evaluate((ownerName) => window.browserUiLifecycle!.state(ownerName), name); + + const ownerA = page.frameLocator("#owner-a"); + const ownerB = page.frameLocator("#owner-b"); + const ownerC = page.frameLocator("#owner-c"); + const ownerD = page.frameLocator("#owner-d"); + + await startDialog("owner-a", "A-seed"); + await startDialog("owner-b", "B-seed"); + const initial = await page.evaluate(() => ({ + first: window.browserUiLifecycle!.state("owner-a").dialogs, + second: window.browserUiLifecycle!.state("owner-b").dialogs, + })); + assertEquals(initial, { first: [], second: [] }); + assertNoPageFailures(); + await ownerA.getByTestId("lifecycle-dialog").waitFor(); + await ownerB.getByTestId("lifecycle-dialog").waitFor(); + assertEquals(await page.locator(".dialog-host").count(), 0); + assertEquals(await ownerA.locator(".dialog-host").count(), 1); + assertEquals(await ownerB.locator(".dialog-host").count(), 1); + + await ownerA.getByTestId("submit-dialog").click(); + await waitFor(async () => (await state("owner-a")).dialogs.length === 1, "Owner A dialogue did not finish"); + assertEquals((await state("owner-a")).dialogs, [{ kind: "resolved", value: "result:A-seed" }]); + assertEquals(await ownerA.locator(".dialog-host").count(), 0); + assertEquals(await ownerB.locator(".dialog-host").count(), 1); + + await ownerB.getByTestId("close-dialog").click(); + await waitFor(async () => (await state("owner-b")).dialogs.length === 1, "Owner B dialogue did not finish"); + assertEquals((await state("owner-b")).dialogs, [{ kind: "resolved", value: "undefined" }]); + + await startMenu("owner-a", "first menu"); + await ownerA.getByText("first menu", { exact: true }).waitFor(); + assertEquals(await page.locator(".background").count(), 0); + assertEquals(await ownerA.locator(".background").count(), 1); + assertEquals(await ownerB.locator(".background").count(), 0); + + await startMenu("owner-a", "replacement menu"); + await ownerA.getByText("replacement menu", { exact: true }).waitFor(); + await waitFor( + async () => (await state("owner-a")).menus.length === 1, + "Replacing the menu did not close its first promise" + ); + assertEquals((await state("owner-a")).menus, ["resolved"]); + assertEquals((await state("owner-a")).hasSavedBackdrop, true); + + await page.evaluate(() => window.browserUiLifecycle!.clickSavedBackdrop("owner-a")); + assertEquals(await ownerA.locator(".background").count(), 1); + assertEquals((await state("owner-a")).menus, ["resolved"]); + await page.evaluate(() => window.browserUiLifecycle!.hideMenu("owner-a")); + await waitFor( + async () => (await state("owner-a")).menus.length === 2, + "Hiding the replacement menu did not finish" + ); + assertEquals((await state("owner-a")).menus, ["resolved", "resolved"]); + assertEquals(await ownerA.locator(".background").count(), 0); + + await startDialog("owner-a", "unload-A"); + await startMenu("owner-a", "A unload menu"); + await startDialog("owner-b", "still-B"); + await startMenu("owner-b", "B remains open"); + await ownerA.getByTestId("lifecycle-dialog").waitFor(); + await ownerA.getByText("A unload menu", { exact: true }).waitFor(); + await ownerB.getByTestId("lifecycle-dialog").waitFor(); + await ownerB.getByText("B remains open", { exact: true }).waitFor(); + + await page.evaluate(() => window.browserUiLifecycle!.unload("owner-a")); + await waitFor( + async () => + (await state("owner-a")).dialogs.length === 2 && (await state("owner-a")).menus.length === 3, + "Owner A unload did not settle its UI operations" + ); + assertEquals((await state("owner-a")).dialogs[1], { kind: "rejected", message: "Plug-in unloaded" }); + assertEquals(await ownerA.locator(".dialog-host").count(), 0); + assertEquals(await ownerA.locator(".background").count(), 0); + assertEquals(await ownerB.locator(".dialog-host").count(), 1); + assertEquals(await ownerB.locator(".background").count(), 1); + + await startDialog("owner-a", "after-unload"); + await startMenu("owner-a", "after unload menu"); + await waitFor( + async () => + (await state("owner-a")).dialogs.length === 3 && (await state("owner-a")).menus.length === 4, + "Post-unload UI operations did not settle" + ); + assertEquals((await state("owner-a")).dialogs[2], { kind: "rejected", message: "Plug-in unloaded" }); + assertEquals((await state("owner-a")).menus[3], "resolved"); + assertEquals(await ownerA.locator(".dialog-host").count(), 0); + assertEquals(await ownerA.locator(".background").count(), 0); + + await startDialog("owner-c", "parent-abort"); + await startMenu("owner-c", "parent abort menu"); + await ownerC.getByTestId("lifecycle-dialog").waitFor(); + await ownerC.getByText("parent abort menu", { exact: true }).waitFor(); + await page.evaluate(() => window.browserUiLifecycle!.abortParent("owner-c")); + await waitFor( + async () => + (await state("owner-c")).dialogs.length === 1 && (await state("owner-c")).menus.length === 1, + "Parent abort did not settle its UI operations" + ); + assertEquals((await state("owner-c")).dialogs[0], { kind: "rejected", message: "Fixture UI owner ended" }); + assertEquals((await state("owner-c")).menus[0], "resolved"); + assertEquals(await ownerC.locator(".dialog-host").count(), 0); + assertEquals(await ownerC.locator(".background").count(), 0); + assertEquals(await ownerB.locator(".dialog-host").count(), 1); + assertEquals(await ownerB.locator(".background").count(), 1); + + await startDialog("owner-c", "after-parent-abort"); + await startMenu("owner-c", "after parent abort menu"); + await waitFor( + async () => + (await state("owner-c")).dialogs.length === 2 && (await state("owner-c")).menus.length === 2, + "Post-abort UI operations did not settle" + ); + assertEquals((await state("owner-c")).dialogs[1], { kind: "rejected", message: "Fixture UI owner ended" }); + assertEquals((await state("owner-c")).menus[1], "resolved"); + assertEquals(await ownerC.locator(".dialog-host").count(), 0); + assertEquals(await ownerC.locator(".background").count(), 0); + + await startDialog("owner-d", "pre-aborted"); + await startMenu("owner-d", "pre-aborted menu"); + await waitFor( + async () => + (await state("owner-d")).dialogs.length === 1 && (await state("owner-d")).menus.length === 1, + "Pre-aborted UI operations did not settle" + ); + assertEquals((await state("owner-d")).dialogs[0], { kind: "rejected", message: "Fixture UI owner ended" }); + assertEquals((await state("owner-d")).menus[0], "resolved"); + assertEquals(await ownerD.locator(".dialog-host").count(), 0); + assertEquals(await ownerD.locator(".background").count(), 0); + + await page.evaluate(() => window.browserUiLifecycle!.hideMenu("owner-b")); + await waitFor(async () => (await state("owner-b")).menus.length === 1, "Owner B menu did not close"); + await ownerB.getByTestId("close-dialog").click(); + await waitFor(async () => (await state("owner-b")).dialogs.length === 2, "Owner B dialogue did not close"); + assertEquals(await ownerB.locator(".background").count(), 0); + assertEquals(await ownerB.locator(".dialog-host").count(), 0); + assertNoPageFailures(); + } finally { + await browser.close(); + await server.close(); + } + }, +}); diff --git a/test/browser-apps/ui-lifecycle/index.html b/test/browser-apps/ui-lifecycle/index.html new file mode 100644 index 00000000..bbee4dc7 --- /dev/null +++ b/test/browser-apps/ui-lifecycle/index.html @@ -0,0 +1,15 @@ + + + + + + Browser UI lifecycle fixture + + + + + + + + + diff --git a/test/browser-apps/ui-lifecycle/src/TestLifecycleDialog.svelte b/test/browser-apps/ui-lifecycle/src/TestLifecycleDialog.svelte new file mode 100644 index 00000000..22858aaf --- /dev/null +++ b/test/browser-apps/ui-lifecycle/src/TestLifecycleDialog.svelte @@ -0,0 +1,16 @@ + + +
+ + +
diff --git a/test/browser-apps/ui-lifecycle/src/main.ts b/test/browser-apps/ui-lifecycle/src/main.ts new file mode 100644 index 00000000..1f8c3d49 --- /dev/null +++ b/test/browser-apps/ui-lifecycle/src/main.ts @@ -0,0 +1,154 @@ +import { createServiceContext } from "@vrtmrz/livesync-commonlib/context"; +import { EVENT_PLUGIN_UNLOADED } from "@vrtmrz/livesync-commonlib/compat/events/coreEvents"; +import { MemoryValueStore } from "@vrtmrz/value-store"; + +import { Menu } from "@/apps/browser/BrowserMenu"; +import { createLiveSyncBrowserServiceHub } from "@/apps/browser/createLiveSyncBrowserServiceHub"; +import TestLifecycleDialog from "./TestLifecycleDialog.svelte"; + +type OwnerName = "owner-a" | "owner-b" | "owner-c" | "owner-d"; +type DialogOutcome = + | { readonly kind: "resolved"; readonly value: unknown } + | { readonly kind: "rejected"; readonly message: string }; + +interface Owner { + readonly context: ReturnType; + readonly document: Document; + readonly hub: ReturnType; + readonly menu: Menu; + readonly parentAbort?: AbortController; + readonly dialogOutcomes: DialogOutcome[]; + readonly menuOutcomes: string[]; + savedBackdrop?: HTMLElement; +} + +interface BrowserUiLifecycleFixture { + readonly ready: boolean; + startDialog(name: OwnerName, initialData?: string): void; + startMenu(name: OwnerName, title: string): void; + clickSavedBackdrop(name: OwnerName): void; + hideMenu(name: OwnerName): void; + unload(name: OwnerName): void; + abortParent(name: OwnerName): void; + state(name: OwnerName): { + readonly dialogs: readonly DialogOutcome[]; + readonly menus: readonly string[]; + readonly hasSavedBackdrop: boolean; + }; +} + +declare global { + interface Window { + browserUiLifecycle?: BrowserUiLifecycleFixture; + } +} + +const owners = new Map(); + +function ownerDocument(name: OwnerName): Document { + const frame = document.getElementById(name); + if (!(frame instanceof HTMLIFrameElement) || !frame.contentDocument) { + throw new Error(`The ${name} frame is not ready`); + } + return frame.contentDocument; +} + +function createOwner(name: OwnerName, options: { parentAbort?: AbortController; preAborted?: boolean } = {}): Owner { + const frameDocument = ownerDocument(name); + const parentAbort = options.parentAbort; + if (options.preAborted) { + parentAbort?.abort(new Error("Fixture UI owner ended")); + } + const context = createServiceContext(); + const hub = createLiveSyncBrowserServiceHub({ + context, + deviceValues: new MemoryValueStore(), + ui: { + document: frameDocument, + ...(parentAbort ? { signal: parentAbort.signal } : {}), + }, + }); + return { + context, + document: frameDocument, + hub, + menu: new Menu({ document: frameDocument, signal: hub.browserUi.signal }), + ...(parentAbort ? { parentAbort } : {}), + dialogOutcomes: [], + menuOutcomes: [], + }; +} + +const ownerCAbort = new AbortController(); +const ownerDAbort = new AbortController(); +ownerDAbort.abort(new Error("Fixture UI owner ended")); + +owners.set("owner-a", createOwner("owner-a")); +owners.set("owner-b", createOwner("owner-b")); +owners.set("owner-c", createOwner("owner-c", { parentAbort: ownerCAbort })); +owners.set("owner-d", createOwner("owner-d", { parentAbort: ownerDAbort, preAborted: true })); + +const fixture: BrowserUiLifecycleFixture = { + ready: true, + startDialog(name, initialData) { + const owner = owners.get(name)!; + try { + void owner.hub.UI.dialogManager.open(TestLifecycleDialog, initialData).then( + (value) => + owner.dialogOutcomes.push({ kind: "resolved", value: value === undefined ? "undefined" : value }), + (error: unknown) => + owner.dialogOutcomes.push({ + kind: "rejected", + message: error instanceof Error ? error.message : String(error), + }) + ); + } catch (error) { + owner.dialogOutcomes.push({ + kind: "rejected", + message: error instanceof Error ? error.message : String(error), + }); + } + }, + startMenu(name, title) { + const owner = owners.get(name)!; + owner.savedBackdrop = owner.document.querySelector(".background") ?? undefined; + try { + void owner.menu + .addItem((item) => item.setTitle(title)) + .showAtPosition({ x: 12, y: 12 }) + .then( + () => owner.menuOutcomes.push("resolved"), + (error: unknown) => + owner.menuOutcomes.push(`rejected:${error instanceof Error ? error.message : String(error)}`) + ); + } catch (error) { + owner.menuOutcomes.push(`rejected:${error instanceof Error ? error.message : String(error)}`); + } + }, + clickSavedBackdrop(name) { + const owner = owners.get(name)!; + const window = owner.document.defaultView; + if (owner.savedBackdrop && window) { + owner.savedBackdrop.dispatchEvent(new window.MouseEvent("click", { bubbles: true })); + } + }, + hideMenu(name) { + owners.get(name)!.menu.hide(); + }, + unload(name) { + owners.get(name)!.context.events.emitEvent(EVENT_PLUGIN_UNLOADED); + }, + abortParent(name) { + owners.get(name)!.parentAbort?.abort(new Error("Fixture UI owner ended")); + }, + state(name) { + const owner = owners.get(name)!; + return { + dialogs: [...owner.dialogOutcomes], + menus: [...owner.menuOutcomes], + hasSavedBackdrop: owner.savedBackdrop !== undefined, + }; + }, +}; + +window.browserUiLifecycle = fixture; diff --git a/test/browser-apps/ui-lifecycle/tsconfig.json b/test/browser-apps/ui-lifecycle/tsconfig.json new file mode 100644 index 00000000..e83a2d8e --- /dev/null +++ b/test/browser-apps/ui-lifecycle/tsconfig.json @@ -0,0 +1,9 @@ +{ + "extends": "../../../tsconfig.json", + "compilerOptions": { + "target": "ES2020", + "lib": ["ES2020", "DOM", "DOM.Iterable"] + }, + "include": ["src/**/*.ts", "src/**/*.svelte"], + "exclude": [] +} diff --git a/test/browser-apps/ui-lifecycle/vite.config.ts b/test/browser-apps/ui-lifecycle/vite.config.ts new file mode 100644 index 00000000..5c10c5b4 --- /dev/null +++ b/test/browser-apps/ui-lifecycle/vite.config.ts @@ -0,0 +1,25 @@ +import { fileURLToPath } from "node:url"; +import { dirname, resolve } from "node:path"; +import { svelte } from "@sveltejs/vite-plugin-svelte"; +import { defineConfig } from "vite"; + +const fixtureDirectory = dirname(fileURLToPath(import.meta.url)); +const repositoryRoot = resolve(fixtureDirectory, "../../.."); + +export default defineConfig({ + root: fixtureDirectory, + plugins: [svelte()], + resolve: { + alias: { + "@": resolve(repositoryRoot, "src"), + }, + }, + base: "./", + build: { + outDir: resolve(repositoryRoot, "_testdata/browser-ui-lifecycle"), + emptyOutDir: true, + rollupOptions: { + input: resolve(fixtureDirectory, "index.html"), + }, + }, +}); diff --git a/test/e2e-obsidian/scripts/conflict-dialog-policy.ts b/test/e2e-obsidian/scripts/conflict-dialog-policy.ts index ed53222a..62638685 100644 --- a/test/e2e-obsidian/scripts/conflict-dialog-policy.ts +++ b/test/e2e-obsidian/scripts/conflict-dialog-policy.ts @@ -38,7 +38,10 @@ type ObsidianTestApp = { | { core?: { services?: { - conflict?: { ensureAllProcessed(): Promise }; + conflict?: { + ensureAllProcessed(): Promise; + conflictProcessQueueCount: { value: number }; + }; }; }; } @@ -607,11 +610,40 @@ async function main(): Promise { await requestRepeatedConflictChecks(cliBinary, repeatedSession.cliEnv, repeatedPath, 3); await page.waitForFunction((element) => !element.isConnected, original, { timeout: uiTimeoutMs }); await modal.waitFor({ state: "visible", timeout: uiTimeoutMs }); - await page.waitForTimeout(250); + // All enqueue calls have settled. Only the final resolver may remain + // while its dialogue waits for a response; earlier checks also count. + await page.waitForFunction( + (targetPath) => { + const conflict = (globalThis as ObsidianTestGlobal).app?.plugins?.plugins?.["obsidian-livesync"] + ?.core?.services?.conflict; + const visible = Array.from(document.querySelectorAll(".modal-container")).filter( + (element) => + element.getClientRects().length > 0 && + element.querySelector(".modal-title")?.textContent?.includes("Conflicting changes") && + element.textContent?.includes(targetPath) + ); + return conflict?.conflictProcessQueueCount.value === 1 && visible.length === 1; + }, + repeatedPath, + { timeout: uiTimeoutMs } + ); const visibleCount = await conflictDialogue(page, repeatedPath).evaluateAll( (elements) => elements.filter((element) => element.getClientRects().length > 0).length ); if (visibleCount !== 1) { + const dialogues = await page.locator(".modal-container").evaluateAll((elements) => + elements.map((element) => ({ + visible: element.getClientRects().length > 0, + text: element.textContent?.slice(0, 1200), + })) + ); + const fixture = await readConflictFixture(cliBinary, repeatedSession.cliEnv, repeatedPath); + console.error(JSON.stringify({ sameFileReplacementFailure: { visibleCount, dialogues, fixture } })); + await captureObsidianPage( + repeatedSession.remoteDebuggingPort, + "conflict-dialog-same-file-replacement.failure.png", + async () => {} + ); throw new Error(`Expected one newest same-file dialogue, but found ${visibleCount}`); } }); diff --git a/test/e2e-obsidian/scripts/document-history-nav.ts b/test/e2e-obsidian/scripts/document-history-nav.ts index 9daf0752..7f25ed1f 100644 --- a/test/e2e-obsidian/scripts/document-history-nav.ts +++ b/test/e2e-obsidian/scripts/document-history-nav.ts @@ -265,7 +265,51 @@ async function main(): Promise { ); screenshotPaths.push(await captureStep(page, "06-after-search-next-match")); + const highlightDiff = modal.locator('.history-highlight-diff-label input[type="checkbox"]'); + await highlightDiff.check(); + const diffBlocks = modal.locator(".history-added, .history-deleted"); + await diffBlocks.first().waitFor({ state: "visible", timeout: 10000 }); + const diffIndicator = modal.locator(".diff-nav-indicator"); + const diffCount = await diffBlocks.count(); + const assertDiffSelection = async (index: number, count: number) => { + assertEqual( + (await diffIndicator.innerText()).trim(), + `${index + 1}/${count}`, + "Unexpected diff indicator." + ); + const focused = await diffBlocks.evaluateAll((elements) => + elements.flatMap((element, index) => (element.classList.contains("diff-focused") ? [index] : [])) + ); + assertEqual( + JSON.stringify(focused), + JSON.stringify([index]), + "Diff focus did not move to exactly one block." + ); + }; + await assertDiffSelection(0, diffCount); + const diffPrev = modal.locator(".diff-nav-btn").first(); + const diffNext = modal.locator(".diff-nav-btn").last(); + await diffPrev.click(); + await assertDiffSelection(diffCount - 1, diffCount); + await diffNext.click(); + await assertDiffSelection(0, diffCount); + for (let index = 0; index < diffCount; index++) await diffNext.click(); + await assertDiffSelection(0, diffCount); + + const oldBlock = await diffBlocks.first().elementHandle(); + if (oldBlock === null) throw new Error("Could not retain the previous revision's diff block"); + const changeRevision = (await nextBtn.isDisabled()) ? prevBtn : nextBtn; + await changeRevision.click(); + await page.waitForFunction((element) => !element.isConnected, oldBlock, { timeout: 10000 }); + await diffBlocks.first().waitFor({ state: "visible", timeout: 10000 }); + const replacementDiffCount = await diffBlocks.count(); + await assertDiffSelection(0, replacementDiffCount); + await diffPrev.click(); + await assertDiffSelection(replacementDiffCount - 1, replacementDiffCount); + screenshotPaths.push(await captureStep(page, "07-diff-navigation-after-revision-change")); + return [ + `diffNavigation: ${diffCount} blocks; replacement revision: ${replacementDiffCount} blocks`, `initialIndicator: ${initialIndicator}`, `afterPrevIndicator: ${afterPrevIndicator}`, `afterNextIndicator: ${afterNextIndicator}`, diff --git a/updates.md b/updates.md index c217ef94..32f0f46a 100644 --- a/updates.md +++ b/updates.md @@ -16,6 +16,10 @@ Earlier releases remain available in the 1.0 release history, the 1.0 preview hi - Service log calls stay with the plug-in instance that registered them. Its log connection is released after the full shutdown, so stale instances cannot receive later service messages. +### Browser applications + +- WebApp and WebPeer dialogues and menus remain with their owning application + and close on shutdown; closing or replacing a menu releases its component. ## 1.0.42