mirror of
https://github.com/vrtmrz/obsidian-livesync.git
synced 2026-10-26 02:53:21 +00:00
refactor: detect platform via APIService.isMobile() instead of Platform.isDesktopApp
Address the maintainer review on #949: determine the platform through the plugin's own service layer (services.API.isMobile()) rather than Obsidian's Platform API directly, matching the existing call in ObsidianLiveSyncSettingTab. Applies to both PR-introduced sites: the runtime guard (ModuleObsidianEvents) and the settings-pane toggle (PaneSyncSettings). The TFile import becomes type-only so deps.ts is no longer pulled at runtime; the unit test drives the platform through the services.API.isMobile() mock. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
1 parent
c78e583399
commit
292a6b9e1e
3 files changed
+11
-20
No files matched your search
@@ -2,7 +2,7 @@ import { AbstractObsidianModule } from "../AbstractObsidianModule.ts";
|
|||||||
import { EVENT_FILE_RENAMED, EVENT_LEAF_ACTIVE_CHANGED, eventHub } from "../../common/events.js";
|
import { EVENT_FILE_RENAMED, EVENT_LEAF_ACTIVE_CHANGED, eventHub } from "../../common/events.js";
|
||||||
import { LOG_LEVEL_NOTICE, LOG_LEVEL_VERBOSE } from "octagonal-wheels/common/logger";
|
import { LOG_LEVEL_NOTICE, LOG_LEVEL_VERBOSE } from "octagonal-wheels/common/logger";
|
||||||
import { scheduleTask } from "octagonal-wheels/concurrency/task";
|
import { scheduleTask } from "octagonal-wheels/concurrency/task";
|
||||||
import { Platform, type TFile } from "../../deps.ts";
|
import type { TFile } from "../../deps.ts";
|
||||||
import { fireAndForget } from "octagonal-wheels/promises";
|
import { fireAndForget } from "octagonal-wheels/promises";
|
||||||
import { type FilePathWithPrefix } from "../../lib/src/common/types.ts";
|
import { type FilePathWithPrefix } from "../../lib/src/common/types.ts";
|
||||||
import { reactive, reactiveSource, type ReactiveSource } from "octagonal-wheels/dataobject/reactive";
|
import { reactive, reactiveSource, type ReactiveSource } from "octagonal-wheels/dataobject/reactive";
|
||||||
@@ -146,7 +146,7 @@ export class ModuleObsidianEvents extends AbstractObsidianModule {
|
|||||||
const keepActiveInBackground =
|
const keepActiveInBackground =
|
||||||
this.settings.keepReplicationActiveInBackground &&
|
this.settings.keepReplicationActiveInBackground &&
|
||||||
(this.settings.liveSync || this.settings.periodicReplication) &&
|
(this.settings.liveSync || this.settings.periodicReplication) &&
|
||||||
Platform.isDesktopApp;
|
!this.services.API.isMobile();
|
||||||
|
|
||||||
if (isHidden) {
|
if (isHidden) {
|
||||||
if (!keepActiveInBackground) await this.services.appLifecycle.onSuspending();
|
if (!keepActiveInBackground) await this.services.appLifecycle.onSuspending();
|
||||||
|
|||||||
@@ -1,11 +1,5 @@
|
|||||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
import { describe, it, expect, vi, afterEach } from "vitest";
|
||||||
|
|
||||||
// Unit tests stub out `obsidian`, so deps.ts (which re-exports from it) can't be loaded for real.
|
|
||||||
// ModuleObsidianEvents only needs `Platform` from deps at runtime; provide a mutable stub so each
|
|
||||||
// test can choose desktop vs mobile.
|
|
||||||
vi.mock("../../deps.ts", () => ({ Platform: { isDesktopApp: true } }));
|
|
||||||
|
|
||||||
import { Platform } from "../../deps.ts";
|
|
||||||
import { ModuleObsidianEvents } from "./ModuleObsidianEvents";
|
import { ModuleObsidianEvents } from "./ModuleObsidianEvents";
|
||||||
import { DEFAULT_SETTINGS, REMOTE_COUCHDB } from "@lib/common/types";
|
import { DEFAULT_SETTINGS, REMOTE_COUCHDB } from "@lib/common/types";
|
||||||
|
|
||||||
@@ -15,6 +9,8 @@ type SetupOptions = {
|
|||||||
isLastHidden?: boolean;
|
isLastHidden?: boolean;
|
||||||
hasFocus?: boolean;
|
hasFocus?: boolean;
|
||||||
isSuspended?: boolean;
|
isSuspended?: boolean;
|
||||||
|
// Platform is read via services.API.isMobile(); default desktop (false) so the feature applies.
|
||||||
|
isMobile?: boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
function setup(opts: SetupOptions) {
|
function setup(opts: SetupOptions) {
|
||||||
@@ -35,6 +31,7 @@ function setup(opts: SetupOptions) {
|
|||||||
registerWindow: vi.fn(),
|
registerWindow: vi.fn(),
|
||||||
addRibbonIcon: vi.fn(),
|
addRibbonIcon: vi.fn(),
|
||||||
registerProtocolHandler: vi.fn(),
|
registerProtocolHandler: vi.fn(),
|
||||||
|
isMobile: vi.fn(() => opts.isMobile ?? false),
|
||||||
},
|
},
|
||||||
setting: { saveSettingData: vi.fn(async () => undefined) },
|
setting: { saveSettingData: vi.fn(async () => undefined) },
|
||||||
appLifecycle,
|
appLifecycle,
|
||||||
@@ -60,15 +57,10 @@ function setup(opts: SetupOptions) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
describe("watchWindowVisibilityAsync — keepReplicationActiveInBackground", () => {
|
describe("watchWindowVisibilityAsync — keepReplicationActiveInBackground", () => {
|
||||||
beforeEach(() => {
|
|
||||||
(Platform as any).isDesktopApp = true;
|
|
||||||
});
|
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
// The handler reads a global `activeWindow`; the Platform mock is module-scoped. Both would
|
// The handler reads a global `activeWindow`; clear it so it doesn't leak into sibling spec
|
||||||
// otherwise leak into sibling spec files running in the same worker, so reset them here.
|
// files running in the same worker.
|
||||||
delete (globalThis as any).activeWindow;
|
delete (globalThis as any).activeWindow;
|
||||||
(Platform as any).isDesktopApp = true;
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it("does NOT suspend on hide when enabled in LiveSync mode on the desktop app", async () => {
|
it("does NOT suspend on hide when enabled in LiveSync mode on the desktop app", async () => {
|
||||||
@@ -162,11 +154,11 @@ describe("watchWindowVisibilityAsync — keepReplicationActiveInBackground", ()
|
|||||||
expect(appLifecycle.onResumed).toHaveBeenCalledTimes(1);
|
expect(appLifecycle.onResumed).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("does not apply on a non-desktop app even if the flag is set", async () => {
|
it("does not apply on mobile even if the flag is set", async () => {
|
||||||
(Platform as any).isDesktopApp = false;
|
|
||||||
const { module, appLifecycle } = setup({
|
const { module, appLifecycle } = setup({
|
||||||
settings: { keepReplicationActiveInBackground: true, liveSync: true },
|
settings: { keepReplicationActiveInBackground: true, liveSync: true },
|
||||||
hidden: true,
|
hidden: true,
|
||||||
|
isMobile: true,
|
||||||
});
|
});
|
||||||
await module.watchWindowVisibilityAsync();
|
await module.watchWindowVisibilityAsync();
|
||||||
expect(appLifecycle.onSuspending).toHaveBeenCalledTimes(1);
|
expect(appLifecycle.onSuspending).toHaveBeenCalledTimes(1);
|
||||||
|
|||||||
@@ -11,7 +11,6 @@ import { EVENT_REQUEST_COPY_SETUP_URI, eventHub } from "../../../common/events.t
|
|||||||
import type { ObsidianLiveSyncSettingTab } from "./ObsidianLiveSyncSettingTab.ts";
|
import type { ObsidianLiveSyncSettingTab } from "./ObsidianLiveSyncSettingTab.ts";
|
||||||
import type { PageFunctions } from "./SettingPane.ts";
|
import type { PageFunctions } from "./SettingPane.ts";
|
||||||
import { visibleOnly } from "./SettingPane.ts";
|
import { visibleOnly } from "./SettingPane.ts";
|
||||||
import { Platform } from "../../../deps.ts";
|
|
||||||
export function paneSyncSettings(
|
export function paneSyncSettings(
|
||||||
this: ObsidianLiveSyncSettingTab,
|
this: ObsidianLiveSyncSettingTab,
|
||||||
paneEl: HTMLElement,
|
paneEl: HTMLElement,
|
||||||
@@ -193,7 +192,7 @@ export function paneSyncSettings(
|
|||||||
// Desktop app only, and only for the sync modes that keep a background replication channel
|
// Desktop app only, and only for the sync modes that keep a background replication channel
|
||||||
// (LiveSync and Periodic). Ignored on mobile, where suspending preserves battery. The
|
// (LiveSync and Periodic). Ignored on mobile, where suspending preserves battery. The
|
||||||
// visibility predicate mirrors the runtime guard in ModuleObsidianEvents.
|
// visibility predicate mirrors the runtime guard in ModuleObsidianEvents.
|
||||||
if (Platform.isDesktopApp) {
|
if (!this.services.API.isMobile()) {
|
||||||
new Setting(paneEl).setClass("wizardHidden").autoWireToggle("keepReplicationActiveInBackground", {
|
new Setting(paneEl).setClass("wizardHidden").autoWireToggle("keepReplicationActiveInBackground", {
|
||||||
onUpdate: visibleOnly(
|
onUpdate: visibleOnly(
|
||||||
() => this.isConfiguredAs("syncMode", "LIVESYNC") || this.isConfiguredAs("syncMode", "PERIODIC")
|
() => this.isConfiguredAs("syncMode", "LIVESYNC") || this.isConfiguredAs("syncMode", "PERIODIC")
|
||||||
|
|||||||
Reference in new issue
Block a user