From 4ff5b4dfe801bdbb2c5f054121eeed3f658dda8f Mon Sep 17 00:00:00 2001 From: vorotamoroz Date: Tue, 25 Aug 2026 17:27:31 +0000 Subject: [PATCH 1/4] Fix mobile settings layout overflow --- .../ObsidianLiveSyncSettingTab.ts | 12 +- .../features/SettingDialogue/PanePatches.ts | 2 +- .../SettingDialogue/PanePatches.unit.spec.ts | 8 + .../SettingDialogue/PaneRemoteConfig.ts | 4 +- .../PaneRemoteConfig.unit.spec.ts | 4 + styles.css | 25 +- test/e2e-obsidian/scripts/settings-ui.ts | 335 +++++++++++++++--- updates.md | 6 + 8 files changed, 339 insertions(+), 57 deletions(-) diff --git a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts index 38c1be92..b4e7e779 100644 --- a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts +++ b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts @@ -82,6 +82,14 @@ import { isP2PMainRemote } from "@/common/remoteConfiguration.ts"; // For creating a document // const toc = new Set(); +function registerLiveSyncSettingsModal(component: Component, containerEl: HTMLElement | undefined): void { + if (!containerEl) return; + const modalEl = containerEl.closest(".modal.mod-settings"); + if (!modalEl) return; + modalEl.addClass("sls-setting-modal"); + component.register(() => modalEl.removeClass("sls-setting-modal")); +} + export class ObsidianLiveSyncSettingTab extends PluginSettingTab { plugin: ObsidianLiveSyncPlugin; private _lifetimeComponent?: Component; @@ -701,6 +709,7 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab { private renderCustomPage(page: SettingPage, entry: SettingsPageEntry): Component { if (requireApiVersion("1.13.0")) { const component = this.beginRenderScope(() => page.display()); + registerLiveSyncSettingsModal(component, this.containerEl); this.isShown = true; page.title = entry.name(); page.containerEl.empty(); @@ -1083,8 +1092,9 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab { private displayImperative(): void { const changeDisplay = this.changeDisplay.bind(this); // Make sure the page-owned component is loaded for markdown rendering in panes. - this.beginRenderScope(() => this.displayImperative()); + const component = this.beginRenderScope(() => this.displayImperative()); const { containerEl } = this; + registerLiveSyncSettingsModal(component, containerEl); this.screenElements = {}; if (this._editingSettings == undefined || this.initialSettings == undefined) { this.reloadAllSettings(); diff --git a/src/modules/features/SettingDialogue/PanePatches.ts b/src/modules/features/SettingDialogue/PanePatches.ts index 1816d6d5..5b0262ce 100644 --- a/src/modules/features/SettingDialogue/PanePatches.ts +++ b/src/modules/features/SettingDialogue/PanePatches.ts @@ -177,7 +177,7 @@ export function panePatches(this: ObsidianLiveSyncSettingTab, paneEl: HTMLElemen new Setting(paneEl).autoWireToggle("disableCheckingConfigMismatch"); }); void addPanel(paneEl, "Remediation").then((paneEl) => { - const setting = new Setting(paneEl); + const setting = new Setting(paneEl).setClass("sls-setting-mobile-wrap-controls"); const dateEl = setting.controlEl.createSpan(); setting .addText((text) => { diff --git a/src/modules/features/SettingDialogue/PanePatches.unit.spec.ts b/src/modules/features/SettingDialogue/PanePatches.unit.spec.ts index 60f29b84..9ed2a25b 100644 --- a/src/modules/features/SettingDialogue/PanePatches.unit.spec.ts +++ b/src/modules/features/SettingDialogue/PanePatches.unit.spec.ts @@ -12,11 +12,13 @@ const remediationHarness = vi.hoisted(() => { onChange: vi.fn(), setValue: vi.fn(), }; + const setClass = vi.fn(); return { createSpan, dateElement, inputEl, + setClass, textComponent, }; }); @@ -36,6 +38,11 @@ vi.mock("./LiveSyncSetting.ts", () => ({ return this; } + setClass(value: string): this { + remediationHarness.setClass(value); + return this; + } + addApplyButton(): this { return this; } @@ -93,5 +100,6 @@ describe("panePatches remediation setting", () => { expect(createSpan).not.toHaveBeenCalled(); expect(remediationHarness.createSpan).toHaveBeenCalledOnce(); expect(remediationHarness.dateElement.textContent).toBe("No limit configured"); + expect(remediationHarness.setClass).toHaveBeenCalledWith("sls-setting-mobile-wrap-controls"); }); }); diff --git a/src/modules/features/SettingDialogue/PaneRemoteConfig.ts b/src/modules/features/SettingDialogue/PaneRemoteConfig.ts index bd2143ef..5ecbabeb 100644 --- a/src/modules/features/SettingDialogue/PaneRemoteConfig.ts +++ b/src/modules/features/SettingDialogue/PaneRemoteConfig.ts @@ -105,7 +105,9 @@ export function paneRemoteConfig( void addPanel(paneEl, "E2EE Configuration", () => {}).then((paneEl) => { const infoPanel = new SveltePanel(InfoPanel, paneEl, E2EESummaryWritable); this.lifetimeComponent.register(() => infoPanel.destroy()); - const setupButton = new Setting(paneEl).setName("Configure E2EE"); + const setupButton = new Setting(paneEl) + .setName("Configure E2EE") + .setClass("sls-setting-mobile-wrap-controls"); setupButton .addButton((button) => setButtonDestructiveState(button) diff --git a/src/modules/features/SettingDialogue/PaneRemoteConfig.unit.spec.ts b/src/modules/features/SettingDialogue/PaneRemoteConfig.unit.spec.ts index a11f20f7..8853fad7 100644 --- a/src/modules/features/SettingDialogue/PaneRemoteConfig.unit.spec.ts +++ b/src/modules/features/SettingDialogue/PaneRemoteConfig.unit.spec.ts @@ -30,6 +30,10 @@ vi.mock("./LiveSyncSetting.ts", () => ({ return this; } + setClass() { + return this; + } + addButton() { return this; } diff --git a/styles.css b/styles.css index 241176e0..5dddf444 100644 --- a/styles.css +++ b/styles.css @@ -536,14 +536,35 @@ div.workspace-leaf-content[data-type="bases"] .livesync-status { } .sls-setting-panel-title { - position: sticky; font-size: medium; - top: 2.5em; background-color: var(--background-secondary-alt); border-radius: 10px; padding: 0.5em 1em; } +body.is-mobile .sls-setting button { + max-width: 100%; + white-space: normal; +} + +body.is-mobile .sls-setting-mobile-wrap-controls { + flex-wrap: wrap; +} + +body.is-mobile .sls-setting-mobile-wrap-controls .setting-item-control { + min-width: 0; + flex: 1 1 100%; + flex-wrap: wrap; +} + +body.is-mobile .sls-setting-mobile-wrap-controls .setting-item-control > button { + flex: 1 1 12rem; +} + +body.is-mobile .modal.mod-settings.sls-setting-modal .modal-header { + background-color: var(--background-primary); +} + .active-pane .sls-setting-panel-title { border: 1px solid var(--interactive-accent); } diff --git a/test/e2e-obsidian/scripts/settings-ui.ts b/test/e2e-obsidian/scripts/settings-ui.ts index 31de3061..5d9a3396 100644 --- a/test/e2e-obsidian/scripts/settings-ui.ts +++ b/test/e2e-obsidian/scripts/settings-ui.ts @@ -1,8 +1,12 @@ import { mkdir } from "node:fs/promises"; import { VER } from "@vrtmrz/livesync-commonlib/compat/common/types"; import { discoverObsidianCli, requireObsidianBinary } from "../runner/environment.ts"; -import { waitForLiveSyncCoreReady } from "../runner/liveSyncWorkflow.ts"; -import { assertMobileDialogueLayout, setObsidianMobileTestMode } from "../runner/mobileUi.ts"; +import { createE2eObsidianDeviceLocalState, waitForLiveSyncCoreReady } from "../runner/liveSyncWorkflow.ts"; +import { + assertMobileDialogueLayout, + setObsidianMobileTestMode, + setObsidianMobileTestModeBeforePluginStart, +} from "../runner/mobileUi.ts"; import { startObsidianLiveSyncSession, type ObsidianLiveSyncSession } from "../runner/session.ts"; import { allowPendingObsidianTestVaultOpenAction, @@ -160,39 +164,206 @@ async function setConfiguredStateForLandingInspection(page: Page, configured: bo }, configured); } -async function captureDeclarativeMobileLanding(): Promise { +async function captureDeclarativeMobileSettings(): Promise< + | { + landingPage: string; + maintenance: string; + patches: string; + remoteConfiguration: string; + } + | undefined +> { const port = obsidianRemoteDebuggingPort(); - await setObsidianMobileTestMode(port, true, uiTimeoutMs); - try { - return await withObsidianPage(port, async (page) => { - const settingsNavigator = await openLiveSyncSettings(page, uiTimeoutMs); - if (settingsNavigator.renderer !== "declarative") { - await settingsNavigator.close(); - return undefined; - } - await settingsNavigator.returnToCatalogue(); - await scrollDeclarativeLandingToTop(settingsNavigator.dialogue, true); - await assertDeclarativeLandingOrder(settingsNavigator.dialogue, true); - const remoteConfiguration = settingsNavigator.dialogue - .locator(".setting-item-name") - .filter({ hasText: "Remote Configuration" }) - .first(); - await remoteConfiguration.waitFor({ state: "visible", timeout: uiTimeoutMs }); - const path = `${diagnosticsDirectory}/settings-declarative-landing-mobile.png`; - await settingsNavigator.dialogue.screenshot({ ...settingsScreenshotOptions, path }); - const remotePosition = await remoteConfiguration.evaluate((element) => { - const bounds = element.getBoundingClientRect(); - return { top: bounds.top, bottom: bounds.bottom, viewportHeight: window.innerHeight }; - }); - if (remotePosition.top < 0 || remotePosition.bottom > remotePosition.viewportHeight) { - throw new Error("Remote Configuration was not visible at the top of the mobile settings landing page."); - } + return await withObsidianPage(port, async (page) => { + const settingsNavigator = await openLiveSyncSettings(page, uiTimeoutMs); + if (settingsNavigator.renderer !== "declarative") { await settingsNavigator.close(); - return path; + return undefined; + } + await settingsNavigator.returnToCatalogue(); + await scrollDeclarativeLandingToTop(settingsNavigator.dialogue, true); + await assertDeclarativeLandingOrder(settingsNavigator.dialogue, true); + const remoteConfiguration = settingsNavigator.dialogue + .locator(".setting-item-name") + .filter({ hasText: "Remote Configuration" }) + .first(); + await remoteConfiguration.waitFor({ state: "visible", timeout: uiTimeoutMs }); + const path = `${diagnosticsDirectory}/settings-declarative-landing-mobile.png`; + await settingsNavigator.dialogue.screenshot({ ...settingsScreenshotOptions, path }); + const remotePosition = await remoteConfiguration.evaluate((element) => { + const bounds = element.getBoundingClientRect(); + return { top: bounds.top, bottom: bounds.bottom, viewportHeight: window.innerHeight }; }); - } finally { - await setObsidianMobileTestMode(port, false, uiTimeoutMs); - } + if (remotePosition.top < 0 || remotePosition.bottom > remotePosition.viewportHeight) { + throw new Error("Remote Configuration was not visible at the top of the mobile settings landing page."); + } + + const remotePage = await settingsNavigator.openPage("Remote Configuration"); + const e2eeHeading = remotePage + .locator("h4.sls-setting-panel-title") + .filter({ hasText: "E2EE Configuration" }) + .first(); + const e2eeActions = remotePage.locator(".setting-item").filter({ + has: settingsNavigator.page.getByText("Configure E2EE", { exact: true }), + }); + await e2eeHeading.waitFor({ state: "visible", timeout: uiTimeoutMs }); + await e2eeActions.waitFor({ state: "visible", timeout: uiTimeoutMs }); + + const layoutFailures: string[] = []; + const actionLayout = await e2eeActions.evaluate((setting) => { + const control = setting.querySelector(".setting-item-control"); + if (control === null) throw new Error("The E2EE action row did not contain a control group."); + const settingBounds = setting.getBoundingClientRect(); + const buttonBounds = Array.from(control.querySelectorAll("button")).map((button) => + button.getBoundingClientRect() + ); + return { + controlClientWidth: control.clientWidth, + controlScrollWidth: control.scrollWidth, + rightmostButton: Math.max(...buttonBounds.map((bounds) => bounds.right)), + settingRight: settingBounds.right, + }; + }); + if ( + actionLayout.controlScrollWidth > actionLayout.controlClientWidth + 1 || + actionLayout.rightmostButton > actionLayout.settingRight + 1 + ) { + layoutFailures.push(`the E2EE actions overflowed their setting row (${JSON.stringify(actionLayout)})`); + } + + await remotePage.evaluate((content) => { + content.scrollTop = content.scrollHeight - content.clientHeight; + content.dispatchEvent(new Event("scroll", { bubbles: true })); + }); + await settingsNavigator.page.waitForTimeout(50); + const panelLayout = await e2eeHeading.evaluate((heading) => { + const infoPanel = heading.parentElement?.querySelector(".info-panel"); + if (infoPanel === null || infoPanel === undefined) { + throw new Error("The E2EE section did not contain its information panel."); + } + const modalHeader = heading.closest(".modal.mod-settings")?.querySelector(".modal-header"); + if (modalHeader === null || modalHeader === undefined) { + throw new Error("The mobile settings page did not contain its native header."); + } + const headingBounds = heading.getBoundingClientRect(); + const infoBounds = infoPanel.getBoundingClientRect(); + return { + headingBottom: headingBounds.bottom, + headingPosition: getComputedStyle(heading).position, + headingTop: headingBounds.top, + infoBottom: infoBounds.bottom, + infoTop: infoBounds.top, + modalHeaderBackground: getComputedStyle(modalHeader).backgroundColor, + }; + }); + if ( + panelLayout.headingBottom > panelLayout.infoTop + 1 && + panelLayout.headingTop < panelLayout.infoBottom - 1 + ) { + layoutFailures.push(`the E2EE section heading overlapped its contents (${JSON.stringify(panelLayout)})`); + } + if ( + panelLayout.modalHeaderBackground === "transparent" || + panelLayout.modalHeaderBackground === "rgba(0, 0, 0, 0)" + ) { + layoutFailures.push("the native mobile settings header remained transparent over scrolling content"); + } + + const remotePath = `${diagnosticsDirectory}/settings-declarative-remote-mobile.png`; + await settingsNavigator.dialogue.screenshot({ ...settingsScreenshotOptions, path: remotePath }); + if (layoutFailures.length > 0) { + throw new Error(`The mobile Remote Configuration layout was invalid: ${layoutFailures.join("; ")}.`); + } + + const maintenancePage = await settingsNavigator.openPage("Maintenance"); + const markResolvedButton = maintenancePage + .locator(".op-warn button") + .filter({ hasText: "I've made a backup, mark this device 'resolved'" }) + .first(); + await markResolvedButton.evaluate((button) => { + const warning = button.closest(".op-warn"); + if (warning === null) throw new Error("The Maintenance recovery action had no warning container."); + warning.removeClass("sls-setting-hidden"); + }); + await markResolvedButton.waitFor({ state: "visible", timeout: uiTimeoutMs }); + await markResolvedButton.scrollIntoViewIfNeeded(); + const maintenanceLayout = await markResolvedButton.evaluate((button) => { + const content = button.closest(".vertical-tab-content"); + if (content === null) throw new Error("The Maintenance button was outside the settings content."); + const buttonBounds = button.getBoundingClientRect(); + const contentBounds = content.getBoundingClientRect(); + return { + buttonLeft: buttonBounds.left, + buttonRight: buttonBounds.right, + contentLeft: contentBounds.left, + contentRight: contentBounds.right, + rootClientWidth: document.documentElement.clientWidth, + rootScrollWidth: document.documentElement.scrollWidth, + }; + }); + const maintenancePath = `${diagnosticsDirectory}/settings-declarative-maintenance-mobile.png`; + await settingsNavigator.dialogue.screenshot({ ...settingsScreenshotOptions, path: maintenancePath }); + if ( + maintenanceLayout.buttonLeft < maintenanceLayout.contentLeft - 1 || + maintenanceLayout.buttonRight > maintenanceLayout.contentRight + 1 || + maintenanceLayout.rootScrollWidth > maintenanceLayout.rootClientWidth + 1 + ) { + layoutFailures.push( + `the Maintenance recovery action overflowed the settings pane (${JSON.stringify(maintenanceLayout)})` + ); + } + + const patchesPage = await settingsNavigator.openPage("Patches"); + const remediationSetting = patchesPage.locator(".setting-item").filter({ + has: settingsNavigator.page.locator('input[type="datetime-local"]'), + }); + await remediationSetting.waitFor({ state: "visible", timeout: uiTimeoutMs }); + await remediationSetting.scrollIntoViewIfNeeded(); + const patchesLayout = await remediationSetting.evaluate((setting) => { + const content = setting.closest(".vertical-tab-content"); + const control = setting.querySelector(".setting-item-control"); + if (content === null || control === null) { + throw new Error("The Patches remediation row was incomplete."); + } + const applyButton = control.querySelector("button"); + if (applyButton === null) throw new Error("The Patches remediation row did not contain Apply."); + const settingBounds = setting.getBoundingClientRect(); + const contentBounds = content.getBoundingClientRect(); + const buttonBounds = applyButton.getBoundingClientRect(); + return { + buttonRight: buttonBounds.right, + contentRight: contentBounds.right, + controlClientWidth: control.clientWidth, + controlScrollWidth: control.scrollWidth, + rootClientWidth: document.documentElement.clientWidth, + rootScrollWidth: document.documentElement.scrollWidth, + settingRight: settingBounds.right, + }; + }); + const patchesPath = `${diagnosticsDirectory}/settings-declarative-patches-mobile.png`; + await settingsNavigator.dialogue.screenshot({ ...settingsScreenshotOptions, path: patchesPath }); + if ( + patchesLayout.buttonRight > patchesLayout.settingRight + 1 || + patchesLayout.buttonRight > patchesLayout.contentRight + 1 || + patchesLayout.controlScrollWidth > patchesLayout.controlClientWidth + 1 || + patchesLayout.rootScrollWidth > patchesLayout.rootClientWidth + 1 + ) { + layoutFailures.push( + `the Patches remediation actions overflowed their setting row (${JSON.stringify(patchesLayout)})` + ); + } + + if (layoutFailures.length > 0) { + throw new Error(`The mobile settings layout was invalid: ${layoutFailures.join("; ")}.`); + } + await settingsNavigator.close(); + return { + landingPage: path, + maintenance: maintenancePath, + patches: patchesPath, + remoteConfiguration: remotePath, + }; + }); } async function openSettingsInitialisationDialogueForInspection(isP2P: boolean): Promise { @@ -795,6 +966,72 @@ async function verifyPendingSettingsInitialisationFlow(): Promise<{ choice: stri }); } +function createSettingsPluginData(settingsOnlyRun: boolean): Record { + return { + doctorProcessedVersion: settingsOnlyRun ? "1.0.0" : "0.25.27", + isConfigured: true, + liveSync: false, + versionUpFlash: settingsOnlyRun ? "" : compatibilityReviewMessage, + notifyThresholdOfRemoteStorageSize: 0, + syncOnStart: false, + syncOnSave: false, + syncOnEditorSave: false, + syncOnFileOpen: false, + syncAfterMerge: false, + periodicReplication: false, + handleFilenameCaseSensitive: false, + useAdvancedMode: false, + usePowerUserMode: false, + useEdgeCaseMode: false, + }; +} + +async function captureDeclarativeMobileSettingsInFreshSession( + binary: string, + cliBinary: string +): Promise< + | { + landingPage: string; + maintenance: string; + patches: string; + remoteConfiguration: string; + } + | undefined +> { + // Enter mobile mode before LiveSync first loads so Obsidian fires the + // mobile settings-registration lifecycle used by a real mobile start-up. + const vault = await createTemporaryVault(); + let session: ObsidianLiveSyncSession | undefined; + try { + session = await startObsidianLiveSyncSession({ + binary, + cliBinary, + vault, + startupGraceMs: Number(process.env.E2E_OBSIDIAN_STARTUP_GRACE_MS ?? 1000), + pluginData: { + ...createSettingsPluginData(true), + useAdvancedMode: true, + useEdgeCaseMode: true, + usePowerUserMode: true, + }, + localStorageEntries: createE2eObsidianDeviceLocalState(vault.name), + lifecycle: { + beforePluginStart: async ({ remoteDebuggingPort }) => { + await setObsidianMobileTestModeBeforePluginStart(remoteDebuggingPort, true, uiTimeoutMs); + }, + }, + }); + await waitForLiveSyncCoreReady(cliBinary, session.cliEnv); + await resumePendingCompatibilityReviewForSettings(); + return await captureDeclarativeMobileSettings(); + } finally { + if (session) { + await session.app.stop(); + } + await vault.dispose(); + } +} + async function main(): Promise { const binary = requireObsidianBinary(); const cli = discoverObsidianCli(); @@ -804,29 +1041,14 @@ async function main(): Promise { const vault = await createTemporaryVault(); await mkdir(diagnosticsDirectory, { recursive: true }); let session: ObsidianLiveSyncSession | undefined; + let settingsRenderer: "declarative" | "imperative" | undefined; try { session = await startObsidianLiveSyncSession({ binary, cliBinary: cli.binary, vault, startupGraceMs: Number(process.env.E2E_OBSIDIAN_STARTUP_GRACE_MS ?? 1000), - pluginData: { - doctorProcessedVersion: settingsOnly ? "1.0.0" : "0.25.27", - isConfigured: true, - liveSync: false, - versionUpFlash: settingsOnly ? "" : compatibilityReviewMessage, - notifyThresholdOfRemoteStorageSize: 0, - syncOnStart: false, - syncOnSave: false, - syncOnEditorSave: false, - syncOnFileOpen: false, - syncAfterMerge: false, - periodicReplication: false, - handleFilenameCaseSensitive: false, - useAdvancedMode: false, - usePowerUserMode: false, - useEdgeCaseMode: false, - }, + pluginData: createSettingsPluginData(settingsOnly), lifecycle: settingsOnly ? { afterLaunch: async ({ remoteDebuggingPort }) => { @@ -843,11 +1065,9 @@ async function main(): Promise { await verifyCompatibilityReview(); await verifyConfigDoctorFollowsCompatibilityReview(); } - const settingsRenderer = await verifyEffectiveSettings(); + settingsRenderer = await verifyEffectiveSettings(); const initialisation = await verifyPendingSettingsInitialisationFlow(); const p2pInitialisation = await captureP2PSettingsInitialisationDialogue(); - const mobileLanding = settingsRenderer === "declarative" ? await captureDeclarativeMobileLanding() : undefined; - if (mobileLanding) console.log(`Declarative mobile settings landing page: ${mobileLanding}`); console.log( `Pending-settings initialisation screenshots: ${initialisation.choice}, ${initialisation.fallback}, ${p2pInitialisation}` ); @@ -858,6 +1078,17 @@ async function main(): Promise { } await vault.dispose(); } + + const mobileSettings = + settingsRenderer === "declarative" + ? await captureDeclarativeMobileSettingsInFreshSession(binary, cli.binary) + : undefined; + if (mobileSettings) { + console.log(`Declarative mobile settings landing page: ${mobileSettings.landingPage}`); + console.log(`Declarative mobile Remote Configuration page: ${mobileSettings.remoteConfiguration}`); + console.log(`Declarative mobile Maintenance page: ${mobileSettings.maintenance}`); + console.log(`Declarative mobile Patches page: ${mobileSettings.patches}`); + } } main().catch((error: unknown) => { diff --git a/updates.md b/updates.md index d2c40750..0785b24a 100644 --- a/updates.md +++ b/updates.md @@ -12,6 +12,12 @@ Earlier releases remain available in the 1.0 release history, the 1.0 preview hi ## Unreleased +### Interface and translation + +#### Fixed + +- Remote Configuration section headings no longer overlap their contents when scrolling on mobile. Action buttons in Remote Configuration, Maintenance, and Patches now remain inside the settings pane on narrow screens. + ## 1.0.20 ~~1.0.19~~ was cancelled because prerelease validation exposed an incorrect warning at start-up. From 8644af612840af14c7248b40175bf0cf182e162e Mon Sep 17 00:00:00 2001 From: vorotamoroz Date: Wed, 26 Aug 2026 04:30:44 +0000 Subject: [PATCH 2/4] Narrow mobile settings layout fix --- .../SettingDialogue/ObsidianLiveSyncSettingTab.ts | 12 +----------- src/modules/features/SettingDialogue/PanePatches.ts | 2 +- .../SettingDialogue/PanePatches.unit.spec.ts | 2 +- .../features/SettingDialogue/PaneRemoteConfig.ts | 2 +- .../SettingDialogue/PaneRemoteConfig.unit.spec.ts | 6 +++++- styles.css | 10 +++------- test/e2e-obsidian/scripts/settings-ui.ts | 12 ------------ 7 files changed, 12 insertions(+), 34 deletions(-) diff --git a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts index b4e7e779..38c1be92 100644 --- a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts +++ b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts @@ -82,14 +82,6 @@ import { isP2PMainRemote } from "@/common/remoteConfiguration.ts"; // For creating a document // const toc = new Set(); -function registerLiveSyncSettingsModal(component: Component, containerEl: HTMLElement | undefined): void { - if (!containerEl) return; - const modalEl = containerEl.closest(".modal.mod-settings"); - if (!modalEl) return; - modalEl.addClass("sls-setting-modal"); - component.register(() => modalEl.removeClass("sls-setting-modal")); -} - export class ObsidianLiveSyncSettingTab extends PluginSettingTab { plugin: ObsidianLiveSyncPlugin; private _lifetimeComponent?: Component; @@ -709,7 +701,6 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab { private renderCustomPage(page: SettingPage, entry: SettingsPageEntry): Component { if (requireApiVersion("1.13.0")) { const component = this.beginRenderScope(() => page.display()); - registerLiveSyncSettingsModal(component, this.containerEl); this.isShown = true; page.title = entry.name(); page.containerEl.empty(); @@ -1092,9 +1083,8 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab { private displayImperative(): void { const changeDisplay = this.changeDisplay.bind(this); // Make sure the page-owned component is loaded for markdown rendering in panes. - const component = this.beginRenderScope(() => this.displayImperative()); + this.beginRenderScope(() => this.displayImperative()); const { containerEl } = this; - registerLiveSyncSettingsModal(component, containerEl); this.screenElements = {}; if (this._editingSettings == undefined || this.initialSettings == undefined) { this.reloadAllSettings(); diff --git a/src/modules/features/SettingDialogue/PanePatches.ts b/src/modules/features/SettingDialogue/PanePatches.ts index 5b0262ce..9c813404 100644 --- a/src/modules/features/SettingDialogue/PanePatches.ts +++ b/src/modules/features/SettingDialogue/PanePatches.ts @@ -177,7 +177,7 @@ export function panePatches(this: ObsidianLiveSyncSettingTab, paneEl: HTMLElemen new Setting(paneEl).autoWireToggle("disableCheckingConfigMismatch"); }); void addPanel(paneEl, "Remediation").then((paneEl) => { - const setting = new Setting(paneEl).setClass("sls-setting-mobile-wrap-controls"); + const setting = new Setting(paneEl).setClass("sls-setting-subsequent-buttons"); const dateEl = setting.controlEl.createSpan(); setting .addText((text) => { diff --git a/src/modules/features/SettingDialogue/PanePatches.unit.spec.ts b/src/modules/features/SettingDialogue/PanePatches.unit.spec.ts index 9ed2a25b..efc10bcf 100644 --- a/src/modules/features/SettingDialogue/PanePatches.unit.spec.ts +++ b/src/modules/features/SettingDialogue/PanePatches.unit.spec.ts @@ -100,6 +100,6 @@ describe("panePatches remediation setting", () => { expect(createSpan).not.toHaveBeenCalled(); expect(remediationHarness.createSpan).toHaveBeenCalledOnce(); expect(remediationHarness.dateElement.textContent).toBe("No limit configured"); - expect(remediationHarness.setClass).toHaveBeenCalledWith("sls-setting-mobile-wrap-controls"); + expect(remediationHarness.setClass).toHaveBeenCalledWith("sls-setting-subsequent-buttons"); }); }); diff --git a/src/modules/features/SettingDialogue/PaneRemoteConfig.ts b/src/modules/features/SettingDialogue/PaneRemoteConfig.ts index 5ecbabeb..a8700ea2 100644 --- a/src/modules/features/SettingDialogue/PaneRemoteConfig.ts +++ b/src/modules/features/SettingDialogue/PaneRemoteConfig.ts @@ -107,7 +107,7 @@ export function paneRemoteConfig( this.lifetimeComponent.register(() => infoPanel.destroy()); const setupButton = new Setting(paneEl) .setName("Configure E2EE") - .setClass("sls-setting-mobile-wrap-controls"); + .setClass("sls-setting-subsequent-buttons"); setupButton .addButton((button) => setButtonDestructiveState(button) diff --git a/src/modules/features/SettingDialogue/PaneRemoteConfig.unit.spec.ts b/src/modules/features/SettingDialogue/PaneRemoteConfig.unit.spec.ts index 8853fad7..5417800c 100644 --- a/src/modules/features/SettingDialogue/PaneRemoteConfig.unit.spec.ts +++ b/src/modules/features/SettingDialogue/PaneRemoteConfig.unit.spec.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; const runtime = vi.hoisted(() => ({ panels: [] as Array<{ destroy: ReturnType }>, + settingClasses: [] as string[], })); vi.mock("@vrtmrz/livesync-commonlib/compat/common/types", () => ({ @@ -30,7 +31,8 @@ vi.mock("./LiveSyncSetting.ts", () => ({ return this; } - setClass() { + setClass(value: string) { + runtime.settingClasses.push(value); return this; } @@ -90,6 +92,7 @@ function createPanelElement(): HTMLElement { afterEach(() => { runtime.panels.length = 0; + runtime.settingClasses.length = 0; vi.clearAllMocks(); }); @@ -109,6 +112,7 @@ describe("paneRemoteConfig", () => { paneRemoteConfig.call(host as never, {} as HTMLElement, { addPanel } as never); await vi.waitFor(() => expect(runtime.panels).toHaveLength(1)); + expect(runtime.settingClasses).toContain("sls-setting-subsequent-buttons"); lifetimeComponent.unload(); diff --git a/styles.css b/styles.css index 5dddf444..717253b1 100644 --- a/styles.css +++ b/styles.css @@ -547,24 +547,20 @@ body.is-mobile .sls-setting button { white-space: normal; } -body.is-mobile .sls-setting-mobile-wrap-controls { +body.is-mobile .sls-setting-subsequent-buttons { flex-wrap: wrap; } -body.is-mobile .sls-setting-mobile-wrap-controls .setting-item-control { +body.is-mobile .sls-setting-subsequent-buttons .setting-item-control { min-width: 0; flex: 1 1 100%; flex-wrap: wrap; } -body.is-mobile .sls-setting-mobile-wrap-controls .setting-item-control > button { +body.is-mobile .sls-setting-subsequent-buttons .setting-item-control > button { flex: 1 1 12rem; } -body.is-mobile .modal.mod-settings.sls-setting-modal .modal-header { - background-color: var(--background-primary); -} - .active-pane .sls-setting-panel-title { border: 1px solid var(--interactive-accent); } diff --git a/test/e2e-obsidian/scripts/settings-ui.ts b/test/e2e-obsidian/scripts/settings-ui.ts index 5d9a3396..dcfae599 100644 --- a/test/e2e-obsidian/scripts/settings-ui.ts +++ b/test/e2e-obsidian/scripts/settings-ui.ts @@ -241,10 +241,6 @@ async function captureDeclarativeMobileSettings(): Promise< if (infoPanel === null || infoPanel === undefined) { throw new Error("The E2EE section did not contain its information panel."); } - const modalHeader = heading.closest(".modal.mod-settings")?.querySelector(".modal-header"); - if (modalHeader === null || modalHeader === undefined) { - throw new Error("The mobile settings page did not contain its native header."); - } const headingBounds = heading.getBoundingClientRect(); const infoBounds = infoPanel.getBoundingClientRect(); return { @@ -253,7 +249,6 @@ async function captureDeclarativeMobileSettings(): Promise< headingTop: headingBounds.top, infoBottom: infoBounds.bottom, infoTop: infoBounds.top, - modalHeaderBackground: getComputedStyle(modalHeader).backgroundColor, }; }); if ( @@ -262,13 +257,6 @@ async function captureDeclarativeMobileSettings(): Promise< ) { layoutFailures.push(`the E2EE section heading overlapped its contents (${JSON.stringify(panelLayout)})`); } - if ( - panelLayout.modalHeaderBackground === "transparent" || - panelLayout.modalHeaderBackground === "rgba(0, 0, 0, 0)" - ) { - layoutFailures.push("the native mobile settings header remained transparent over scrolling content"); - } - const remotePath = `${diagnosticsDirectory}/settings-declarative-remote-mobile.png`; await settingsNavigator.dialogue.screenshot({ ...settingsScreenshotOptions, path: remotePath }); if (layoutFailures.length > 0) { From 783fbb8f23a89dfcefbe79a4f15fdc07c105c201 Mon Sep 17 00:00:00 2001 From: vorotamoroz Date: Wed, 26 Aug 2026 05:13:38 +0000 Subject: [PATCH 3/4] Refine mobile settings action layout markers --- .../features/SettingDialogue/PanePatches.ts | 7 +++-- .../SettingDialogue/PanePatches.unit.spec.ts | 10 ++++++- .../SettingDialogue/PaneRemoteConfig.ts | 15 ++++++---- .../PaneRemoteConfig.unit.spec.ts | 28 ++++++++++++++++-- .../features/SettingDialogue/SettingPane.ts | 14 ++++++++- .../SettingDialogue/SettingPane.unit.spec.ts | 29 +++++++++++++++++-- styles.css | 6 ++-- 7 files changed, 91 insertions(+), 18 deletions(-) diff --git a/src/modules/features/SettingDialogue/PanePatches.ts b/src/modules/features/SettingDialogue/PanePatches.ts index 9c813404..0a654692 100644 --- a/src/modules/features/SettingDialogue/PanePatches.ts +++ b/src/modules/features/SettingDialogue/PanePatches.ts @@ -9,7 +9,7 @@ import { Logger } from "@vrtmrz/livesync-commonlib/compat/common/logger"; import { LiveSyncSetting as Setting } from "./LiveSyncSetting.ts"; import type { ObsidianLiveSyncSettingTab } from "./ObsidianLiveSyncSettingTab.ts"; import type { PageFunctions } from "./SettingPane.ts"; -import { visibleOnly } from "./SettingPane.ts"; +import { markSettingRowWithSubsequentButtons, markSubsequentButton, visibleOnly } from "./SettingPane.ts"; import { PouchDB } from "@vrtmrz/livesync-commonlib/compat/pouchdb/pouchdb-browser"; import { ExtraSuffixIndexedDB } from "@vrtmrz/livesync-commonlib/compat/common/types"; import { migrateDatabases } from "./settingUtils.ts"; @@ -177,7 +177,7 @@ export function panePatches(this: ObsidianLiveSyncSettingTab, paneEl: HTMLElemen new Setting(paneEl).autoWireToggle("disableCheckingConfigMismatch"); }); void addPanel(paneEl, "Remediation").then((paneEl) => { - const setting = new Setting(paneEl).setClass("sls-setting-subsequent-buttons"); + const setting = markSettingRowWithSubsequentButtons(new Setting(paneEl)); const dateEl = setting.controlEl.createSpan(); setting .addText((text) => { @@ -215,6 +215,9 @@ export function panePatches(this: ObsidianLiveSyncSettingTab, paneEl: HTMLElemen }) .setAuto("maxMTimeForReflectEvents") .addApplyButton(["maxMTimeForReflectEvents"]); + if (setting.applyButtonComponent) { + markSubsequentButton(setting.applyButtonComponent); + } this.addOnSaved("maxMTimeForReflectEvents", async (key) => { const buttons = ["Restart Now", "Later"] as const; diff --git a/src/modules/features/SettingDialogue/PanePatches.unit.spec.ts b/src/modules/features/SettingDialogue/PanePatches.unit.spec.ts index efc10bcf..5f357b6b 100644 --- a/src/modules/features/SettingDialogue/PanePatches.unit.spec.ts +++ b/src/modules/features/SettingDialogue/PanePatches.unit.spec.ts @@ -12,9 +12,11 @@ const remediationHarness = vi.hoisted(() => { onChange: vi.fn(), setValue: vi.fn(), }; + const addButtonClass = vi.fn(); const setClass = vi.fn(); return { + addButtonClass, createSpan, dateElement, inputEl, @@ -25,6 +27,11 @@ const remediationHarness = vi.hoisted(() => { vi.mock("./LiveSyncSetting.ts", () => ({ LiveSyncSetting: class LiveSyncSetting { + applyButtonComponent = { + buttonEl: { + addClass: remediationHarness.addButtonClass, + }, + }; controlEl = { createSpan: remediationHarness.createSpan, }; @@ -100,6 +107,7 @@ describe("panePatches remediation setting", () => { expect(createSpan).not.toHaveBeenCalled(); expect(remediationHarness.createSpan).toHaveBeenCalledOnce(); expect(remediationHarness.dateElement.textContent).toBe("No limit configured"); - expect(remediationHarness.setClass).toHaveBeenCalledWith("sls-setting-subsequent-buttons"); + expect(remediationHarness.setClass).toHaveBeenCalledWith("sls-setting-row-with-subsequent-buttons"); + expect(remediationHarness.addButtonClass).toHaveBeenCalledWith("sls-setting-subsequent-button"); }); }); diff --git a/src/modules/features/SettingDialogue/PaneRemoteConfig.ts b/src/modules/features/SettingDialogue/PaneRemoteConfig.ts index a8700ea2..98218f73 100644 --- a/src/modules/features/SettingDialogue/PaneRemoteConfig.ts +++ b/src/modules/features/SettingDialogue/PaneRemoteConfig.ts @@ -11,7 +11,12 @@ import { Menu, type ButtonComponent } from "@/deps.ts"; import { $msg } from "@/common/translation"; import { LiveSyncSetting as Setting } from "./LiveSyncSetting.ts"; import type { ObsidianLiveSyncSettingTab } from "./ObsidianLiveSyncSettingTab.ts"; -import { setButtonDestructiveState, type PageFunctions } from "./SettingPane.ts"; +import { + markSettingRowWithSubsequentButtons, + markSubsequentButton, + setButtonDestructiveState, + type PageFunctions, +} from "./SettingPane.ts"; // import { visibleOnly } from "./SettingPane.ts"; import InfoPanel from "./InfoPanel.svelte"; import { writable } from "svelte/store"; @@ -105,12 +110,10 @@ export function paneRemoteConfig( void addPanel(paneEl, "E2EE Configuration", () => {}).then((paneEl) => { const infoPanel = new SveltePanel(InfoPanel, paneEl, E2EESummaryWritable); this.lifetimeComponent.register(() => infoPanel.destroy()); - const setupButton = new Setting(paneEl) - .setName("Configure E2EE") - .setClass("sls-setting-subsequent-buttons"); + const setupButton = markSettingRowWithSubsequentButtons(new Setting(paneEl).setName("Configure E2EE")); setupButton .addButton((button) => - setButtonDestructiveState(button) + setButtonDestructiveState(markSubsequentButton(button)) .onClick(async () => { const setupManager = this.core.getModule(SetupManager); const originalSettings = getSettingsFromEditingSettings(this.editingSettings); @@ -120,7 +123,7 @@ export function paneRemoteConfig( .setButtonText("Configure") ) .addButton((button) => - setButtonDestructiveState(button) + setButtonDestructiveState(markSubsequentButton(button)) .onClick(async () => { const setupManager = this.core.getModule(SetupManager); const originalSettings = getSettingsFromEditingSettings(this.editingSettings); diff --git a/src/modules/features/SettingDialogue/PaneRemoteConfig.unit.spec.ts b/src/modules/features/SettingDialogue/PaneRemoteConfig.unit.spec.ts index 5417800c..edfd32d4 100644 --- a/src/modules/features/SettingDialogue/PaneRemoteConfig.unit.spec.ts +++ b/src/modules/features/SettingDialogue/PaneRemoteConfig.unit.spec.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; const runtime = vi.hoisted(() => ({ + buttonClasses: [] as string[], panels: [] as Array<{ destroy: ReturnType }>, settingClasses: [] as string[], })); @@ -36,7 +37,20 @@ vi.mock("./LiveSyncSetting.ts", () => ({ return this; } - addButton() { + addButton(callback: (button: unknown) => void) { + const button = { + buttonEl: { + addClass: (value: string) => runtime.buttonClasses.push(value), + classList: { toggle: vi.fn() }, + }, + onClick() { + return this; + }, + setButtonText() { + return this; + }, + }; + callback(button); return this; } @@ -91,6 +105,7 @@ function createPanelElement(): HTMLElement { } afterEach(() => { + runtime.buttonClasses.length = 0; runtime.panels.length = 0; runtime.settingClasses.length = 0; vi.clearAllMocks(); @@ -103,7 +118,13 @@ describe("paneRemoteConfig", () => { register: vi.fn((callback: () => unknown) => callbacks.push(callback)), unload: vi.fn(() => callbacks.splice(0).forEach((callback) => callback())), }; - const addPanel = vi.fn((_parent: HTMLElement, _heading: string) => Promise.resolve(createPanelElement())); + const addPanel = vi.fn((_parent: HTMLElement, heading: string) => ({ + then(callback: (paneEl: HTMLElement) => void) { + if (heading === "E2EE Configuration") { + callback(createPanelElement()); + } + }, + })); const host = { editingSettings: { remoteConfigurations: {} }, core: { settings: { remoteConfigurations: {} } }, @@ -112,7 +133,8 @@ describe("paneRemoteConfig", () => { paneRemoteConfig.call(host as never, {} as HTMLElement, { addPanel } as never); await vi.waitFor(() => expect(runtime.panels).toHaveLength(1)); - expect(runtime.settingClasses).toContain("sls-setting-subsequent-buttons"); + expect(runtime.settingClasses).toContain("sls-setting-row-with-subsequent-buttons"); + expect(runtime.buttonClasses).toEqual(["sls-setting-subsequent-button", "sls-setting-subsequent-button"]); lifetimeComponent.unload(); diff --git a/src/modules/features/SettingDialogue/SettingPane.ts b/src/modules/features/SettingDialogue/SettingPane.ts index a21a7e4c..4b789807 100644 --- a/src/modules/features/SettingDialogue/SettingPane.ts +++ b/src/modules/features/SettingDialogue/SettingPane.ts @@ -6,7 +6,7 @@ import { type ConfigLevel, } from "@vrtmrz/livesync-commonlib/compat/common/types"; import type { AllSettingItemKey, AllSettings } from "./settingConstants"; -import type { ButtonComponent } from "@/deps.ts"; +import type { ButtonComponent, Setting } from "@/deps.ts"; export const combineOnUpdate = (func1: OnUpdateFunc, func2: OnUpdateFunc): OnUpdateFunc => { return () => ({ @@ -58,6 +58,18 @@ export function setButtonDestructiveState(button: ButtonComponent, isDestructive return button; } +/** Marks a setting row whose selected action buttons may wrap onto separate lines. */ +export function markSettingRowWithSubsequentButtons(setting: T): T { + setting.setClass("sls-setting-row-with-subsequent-buttons"); + return setting; +} + +/** Marks an action button which may wrap onto a later line in its setting row. */ +export function markSubsequentButton(button: ButtonComponent): ButtonComponent { + button.buttonEl.addClass("sls-setting-subsequent-button"); + return button; +} + export function visibleOnly(cond: () => boolean): OnUpdateFunc { return () => ({ visibility: cond(), diff --git a/src/modules/features/SettingDialogue/SettingPane.unit.spec.ts b/src/modules/features/SettingDialogue/SettingPane.unit.spec.ts index be208eb8..d53be7f3 100644 --- a/src/modules/features/SettingDialogue/SettingPane.unit.spec.ts +++ b/src/modules/features/SettingDialogue/SettingPane.unit.spec.ts @@ -1,6 +1,6 @@ -import type { ButtonComponent } from "@/deps.ts"; +import type { ButtonComponent, Setting } from "@/deps.ts"; import { describe, expect, it, vi } from "vitest"; -import { setButtonDestructiveState } from "./SettingPane.ts"; +import { markSettingRowWithSubsequentButtons, markSubsequentButton, setButtonDestructiveState } from "./SettingPane.ts"; type CompatibleButton = ButtonComponent & { setDestructive?: () => ButtonComponent; @@ -10,6 +10,7 @@ type CompatibleButton = ButtonComponent & { function createButton(overrides: Partial = {}): CompatibleButton { return { buttonEl: { + addClass: vi.fn(), classList: { toggle: vi.fn(), }, @@ -18,6 +19,30 @@ function createButton(overrides: Partial = {}): CompatibleButt } as unknown as CompatibleButton; } +function createSetting(): Setting { + return { + setClass: vi.fn().mockReturnThis(), + } as unknown as Setting; +} + +describe("markSettingRowWithSubsequentButtons", () => { + it("marks only the supplied setting row as containing subsequent actions", () => { + const setting = createSetting(); + + expect(markSettingRowWithSubsequentButtons(setting)).toBe(setting); + expect(setting.setClass).toHaveBeenCalledWith("sls-setting-row-with-subsequent-buttons"); + }); +}); + +describe("markSubsequentButton", () => { + it("marks only the supplied button as a subsequent action", () => { + const button = createButton(); + + expect(markSubsequentButton(button)).toBe(button); + expect(button.buttonEl.addClass).toHaveBeenCalledWith("sls-setting-subsequent-button"); + }); +}); + describe("setButtonDestructiveState", () => { it("uses the native destructive-button API when it is available", () => { const setDestructive = vi.fn(); diff --git a/styles.css b/styles.css index 717253b1..1c9e2705 100644 --- a/styles.css +++ b/styles.css @@ -547,17 +547,17 @@ body.is-mobile .sls-setting button { white-space: normal; } -body.is-mobile .sls-setting-subsequent-buttons { +body.is-mobile .sls-setting-row-with-subsequent-buttons { flex-wrap: wrap; } -body.is-mobile .sls-setting-subsequent-buttons .setting-item-control { +body.is-mobile .sls-setting-row-with-subsequent-buttons .setting-item-control { min-width: 0; flex: 1 1 100%; flex-wrap: wrap; } -body.is-mobile .sls-setting-subsequent-buttons .setting-item-control > button { +body.is-mobile .sls-setting .sls-setting-subsequent-button { flex: 1 1 12rem; } From edeac6f7e2413e3e245d667f0036cdd1940defed Mon Sep 17 00:00:00 2001 From: vorotamoroz Date: Wed, 26 Aug 2026 05:49:37 +0000 Subject: [PATCH 4/4] Separate settings component style helpers --- .../SettingDialogue/LiveSyncSetting.ts | 3 +- .../features/SettingDialogue/PaneHatch.ts | 3 +- .../SettingDialogue/PaneMaintenance.ts | 3 +- .../features/SettingDialogue/PanePatches.ts | 7 ++-- .../SettingDialogue/PanePatches.unit.spec.ts | 29 +++++++------ .../SettingDialogue/PaneRemoteConfig.ts | 14 +++---- .../PaneRemoteConfig.unit.spec.ts | 26 ++++++++---- .../features/SettingDialogue/SettingPane.ts | 32 --------------- .../SettingDialogue/settingComponentStyles.ts | 35 ++++++++++++++++ ...ts => settingComponentStyles.unit.spec.ts} | 41 ++++++++++++++----- styles.css | 6 +-- 11 files changed, 119 insertions(+), 80 deletions(-) create mode 100644 src/modules/features/SettingDialogue/settingComponentStyles.ts rename src/modules/features/SettingDialogue/{SettingPane.unit.spec.ts => settingComponentStyles.unit.spec.ts} (55%) diff --git a/src/modules/features/SettingDialogue/LiveSyncSetting.ts b/src/modules/features/SettingDialogue/LiveSyncSetting.ts index adaf66cb..bed51bd0 100644 --- a/src/modules/features/SettingDialogue/LiveSyncSetting.ts +++ b/src/modules/features/SettingDialogue/LiveSyncSetting.ts @@ -24,7 +24,8 @@ import { type AllBooleanItemKey, } from "./settingConstants.ts"; import { $msg } from "@/common/translation"; -import { setButtonDestructiveState, wrapMemo, type AutoWireOption, type OnUpdateResult } from "./SettingPane.ts"; +import { wrapMemo, type AutoWireOption, type OnUpdateResult } from "./SettingPane.ts"; +import { setButtonDestructiveState } from "./settingComponentStyles.ts"; export class LiveSyncSetting extends Setting { autoWiredComponent?: TextComponent | ToggleComponent | DropdownComponent | ButtonComponent | TextAreaComponent; diff --git a/src/modules/features/SettingDialogue/PaneHatch.ts b/src/modules/features/SettingDialogue/PaneHatch.ts index 376157ec..d14dfd08 100644 --- a/src/modules/features/SettingDialogue/PaneHatch.ts +++ b/src/modules/features/SettingDialogue/PaneHatch.ts @@ -24,7 +24,8 @@ import { import { HiddenFileSync } from "@/features/HiddenFileSync/CmdHiddenFileSync.ts"; import { EVENT_REQUEST_SHOW_HISTORY } from "@/common/obsidianEvents.ts"; import type { ObsidianLiveSyncSettingTab } from "./ObsidianLiveSyncSettingTab.ts"; -import { setButtonDestructiveState, type PageFunctions } from "./SettingPane.ts"; +import type { PageFunctions } from "./SettingPane.ts"; +import { setButtonDestructiveState } from "./settingComponentStyles.ts"; import { isNotFoundError } from "@vrtmrz/livesync-commonlib/compat/common/utils.doc"; import { chooseAndCopyFileDatabaseInfo, diff --git a/src/modules/features/SettingDialogue/PaneMaintenance.ts b/src/modules/features/SettingDialogue/PaneMaintenance.ts index d79a2b8c..a58825d6 100644 --- a/src/modules/features/SettingDialogue/PaneMaintenance.ts +++ b/src/modules/features/SettingDialogue/PaneMaintenance.ts @@ -10,7 +10,8 @@ import { fireAndForget } from "@vrtmrz/livesync-commonlib/compat/common/utils"; import { LiveSyncCouchDBReplicator } from "@vrtmrz/livesync-commonlib/compat/replication/couchdb/LiveSyncReplicator"; import { LiveSyncSetting as Setting } from "./LiveSyncSetting.ts"; import type { ObsidianLiveSyncSettingTab } from "./ObsidianLiveSyncSettingTab"; -import { setButtonDestructiveState, visibleOnly, type PageFunctions } from "./SettingPane"; +import { visibleOnly, type PageFunctions } from "./SettingPane"; +import { setButtonDestructiveState } from "./settingComponentStyles.ts"; export function paneMaintenance( this: ObsidianLiveSyncSettingTab, paneEl: HTMLElement, diff --git a/src/modules/features/SettingDialogue/PanePatches.ts b/src/modules/features/SettingDialogue/PanePatches.ts index 0a654692..87a08c21 100644 --- a/src/modules/features/SettingDialogue/PanePatches.ts +++ b/src/modules/features/SettingDialogue/PanePatches.ts @@ -9,7 +9,8 @@ import { Logger } from "@vrtmrz/livesync-commonlib/compat/common/logger"; import { LiveSyncSetting as Setting } from "./LiveSyncSetting.ts"; import type { ObsidianLiveSyncSettingTab } from "./ObsidianLiveSyncSettingTab.ts"; import type { PageFunctions } from "./SettingPane.ts"; -import { markSettingRowWithSubsequentButtons, markSubsequentButton, visibleOnly } from "./SettingPane.ts"; +import { visibleOnly } from "./SettingPane.ts"; +import { setButtonAdditionalActionState, setSettingAdditionalActionsState } from "./settingComponentStyles.ts"; import { PouchDB } from "@vrtmrz/livesync-commonlib/compat/pouchdb/pouchdb-browser"; import { ExtraSuffixIndexedDB } from "@vrtmrz/livesync-commonlib/compat/common/types"; import { migrateDatabases } from "./settingUtils.ts"; @@ -177,7 +178,7 @@ export function panePatches(this: ObsidianLiveSyncSettingTab, paneEl: HTMLElemen new Setting(paneEl).autoWireToggle("disableCheckingConfigMismatch"); }); void addPanel(paneEl, "Remediation").then((paneEl) => { - const setting = markSettingRowWithSubsequentButtons(new Setting(paneEl)); + const setting = setSettingAdditionalActionsState(new Setting(paneEl)); const dateEl = setting.controlEl.createSpan(); setting .addText((text) => { @@ -216,7 +217,7 @@ export function panePatches(this: ObsidianLiveSyncSettingTab, paneEl: HTMLElemen .setAuto("maxMTimeForReflectEvents") .addApplyButton(["maxMTimeForReflectEvents"]); if (setting.applyButtonComponent) { - markSubsequentButton(setting.applyButtonComponent); + setButtonAdditionalActionState(setting.applyButtonComponent); } this.addOnSaved("maxMTimeForReflectEvents", async (key) => { diff --git a/src/modules/features/SettingDialogue/PanePatches.unit.spec.ts b/src/modules/features/SettingDialogue/PanePatches.unit.spec.ts index 5f357b6b..bcdf5904 100644 --- a/src/modules/features/SettingDialogue/PanePatches.unit.spec.ts +++ b/src/modules/features/SettingDialogue/PanePatches.unit.spec.ts @@ -12,15 +12,15 @@ const remediationHarness = vi.hoisted(() => { onChange: vi.fn(), setValue: vi.fn(), }; - const addButtonClass = vi.fn(); - const setClass = vi.fn(); + const setButtonClassState = vi.fn(); + const setSettingClassState = vi.fn(); return { - addButtonClass, createSpan, dateElement, inputEl, - setClass, + setButtonClassState, + setSettingClassState, textComponent, }; }); @@ -29,7 +29,14 @@ vi.mock("./LiveSyncSetting.ts", () => ({ LiveSyncSetting: class LiveSyncSetting { applyButtonComponent = { buttonEl: { - addClass: remediationHarness.addButtonClass, + classList: { + toggle: remediationHarness.setButtonClassState, + }, + }, + }; + settingEl = { + classList: { + toggle: remediationHarness.setSettingClassState, }, }; controlEl = { @@ -45,11 +52,6 @@ vi.mock("./LiveSyncSetting.ts", () => ({ return this; } - setClass(value: string): this { - remediationHarness.setClass(value); - return this; - } - addApplyButton(): this { return this; } @@ -107,7 +109,10 @@ describe("panePatches remediation setting", () => { expect(createSpan).not.toHaveBeenCalled(); expect(remediationHarness.createSpan).toHaveBeenCalledOnce(); expect(remediationHarness.dateElement.textContent).toBe("No limit configured"); - expect(remediationHarness.setClass).toHaveBeenCalledWith("sls-setting-row-with-subsequent-buttons"); - expect(remediationHarness.addButtonClass).toHaveBeenCalledWith("sls-setting-subsequent-button"); + expect(remediationHarness.setSettingClassState).toHaveBeenCalledWith( + "sls-setting-with-additional-actions", + true + ); + expect(remediationHarness.setButtonClassState).toHaveBeenCalledWith("sls-setting-additional-action", true); }); }); diff --git a/src/modules/features/SettingDialogue/PaneRemoteConfig.ts b/src/modules/features/SettingDialogue/PaneRemoteConfig.ts index 98218f73..68732138 100644 --- a/src/modules/features/SettingDialogue/PaneRemoteConfig.ts +++ b/src/modules/features/SettingDialogue/PaneRemoteConfig.ts @@ -12,11 +12,11 @@ import { $msg } from "@/common/translation"; import { LiveSyncSetting as Setting } from "./LiveSyncSetting.ts"; import type { ObsidianLiveSyncSettingTab } from "./ObsidianLiveSyncSettingTab.ts"; import { - markSettingRowWithSubsequentButtons, - markSubsequentButton, + setButtonAdditionalActionState, setButtonDestructiveState, - type PageFunctions, -} from "./SettingPane.ts"; + setSettingAdditionalActionsState, +} from "./settingComponentStyles.ts"; +import type { PageFunctions } from "./SettingPane.ts"; // import { visibleOnly } from "./SettingPane.ts"; import InfoPanel from "./InfoPanel.svelte"; import { writable } from "svelte/store"; @@ -110,10 +110,10 @@ export function paneRemoteConfig( void addPanel(paneEl, "E2EE Configuration", () => {}).then((paneEl) => { const infoPanel = new SveltePanel(InfoPanel, paneEl, E2EESummaryWritable); this.lifetimeComponent.register(() => infoPanel.destroy()); - const setupButton = markSettingRowWithSubsequentButtons(new Setting(paneEl).setName("Configure E2EE")); + const setupButton = setSettingAdditionalActionsState(new Setting(paneEl).setName("Configure E2EE")); setupButton .addButton((button) => - setButtonDestructiveState(markSubsequentButton(button)) + setButtonDestructiveState(button) .onClick(async () => { const setupManager = this.core.getModule(SetupManager); const originalSettings = getSettingsFromEditingSettings(this.editingSettings); @@ -123,7 +123,7 @@ export function paneRemoteConfig( .setButtonText("Configure") ) .addButton((button) => - setButtonDestructiveState(markSubsequentButton(button)) + setButtonDestructiveState(setButtonAdditionalActionState(button)) .onClick(async () => { const setupManager = this.core.getModule(SetupManager); const originalSettings = getSettingsFromEditingSettings(this.editingSettings); diff --git a/src/modules/features/SettingDialogue/PaneRemoteConfig.unit.spec.ts b/src/modules/features/SettingDialogue/PaneRemoteConfig.unit.spec.ts index edfd32d4..d61a998a 100644 --- a/src/modules/features/SettingDialogue/PaneRemoteConfig.unit.spec.ts +++ b/src/modules/features/SettingDialogue/PaneRemoteConfig.unit.spec.ts @@ -23,6 +23,13 @@ vi.mock("@/common/translation", () => ({ vi.mock("./LiveSyncSetting.ts", () => ({ LiveSyncSetting: class { nameEl = { addClass: vi.fn(), appendText: vi.fn() }; + settingEl = { + classList: { + toggle: (value: string, enabled: boolean) => { + if (enabled) runtime.settingClasses.push(value); + }, + }, + }; setName() { return this; @@ -32,16 +39,17 @@ vi.mock("./LiveSyncSetting.ts", () => ({ return this; } - setClass(value: string) { - runtime.settingClasses.push(value); - return this; - } - addButton(callback: (button: unknown) => void) { const button = { buttonEl: { - addClass: (value: string) => runtime.buttonClasses.push(value), - classList: { toggle: vi.fn() }, + classList: { + toggle: (value: string, enabled: boolean) => { + if (enabled) runtime.buttonClasses.push(value); + }, + }, + }, + setDestructive() { + return this; }, onClick() { return this; @@ -133,8 +141,8 @@ describe("paneRemoteConfig", () => { paneRemoteConfig.call(host as never, {} as HTMLElement, { addPanel } as never); await vi.waitFor(() => expect(runtime.panels).toHaveLength(1)); - expect(runtime.settingClasses).toContain("sls-setting-row-with-subsequent-buttons"); - expect(runtime.buttonClasses).toEqual(["sls-setting-subsequent-button", "sls-setting-subsequent-button"]); + expect(runtime.settingClasses).toContain("sls-setting-with-additional-actions"); + expect(runtime.buttonClasses).toEqual(["sls-setting-additional-action"]); lifetimeComponent.unload(); diff --git a/src/modules/features/SettingDialogue/SettingPane.ts b/src/modules/features/SettingDialogue/SettingPane.ts index 4b789807..442c2f93 100644 --- a/src/modules/features/SettingDialogue/SettingPane.ts +++ b/src/modules/features/SettingDialogue/SettingPane.ts @@ -6,7 +6,6 @@ import { type ConfigLevel, } from "@vrtmrz/livesync-commonlib/compat/common/types"; import type { AllSettingItemKey, AllSettings } from "./settingConstants"; -import type { ButtonComponent, Setting } from "@/deps.ts"; export const combineOnUpdate = (func1: OnUpdateFunc, func2: OnUpdateFunc): OnUpdateFunc => { return () => ({ @@ -39,37 +38,6 @@ export function setStyle(el: HTMLElement, styleHead: string, condition: () => bo } } -/** - * Applies destructive-action styling without requiring Obsidian 1.13 at - * runtime. Older supported versions used the `mod-warning` class for the same - * presentation. - */ -export function setButtonDestructiveState(button: ButtonComponent, isDestructive = true): ButtonComponent { - const compatibleButton = button as unknown as { - setDestructive?: () => ButtonComponent; - removeDestructive?: () => ButtonComponent; - }; - const updateNativeStyle = isDestructive ? compatibleButton.setDestructive : compatibleButton.removeDestructive; - if (typeof updateNativeStyle === "function") { - updateNativeStyle.call(button); - } else { - button.buttonEl.classList.toggle("mod-warning", isDestructive); - } - return button; -} - -/** Marks a setting row whose selected action buttons may wrap onto separate lines. */ -export function markSettingRowWithSubsequentButtons(setting: T): T { - setting.setClass("sls-setting-row-with-subsequent-buttons"); - return setting; -} - -/** Marks an action button which may wrap onto a later line in its setting row. */ -export function markSubsequentButton(button: ButtonComponent): ButtonComponent { - button.buttonEl.addClass("sls-setting-subsequent-button"); - return button; -} - export function visibleOnly(cond: () => boolean): OnUpdateFunc { return () => ({ visibility: cond(), diff --git a/src/modules/features/SettingDialogue/settingComponentStyles.ts b/src/modules/features/SettingDialogue/settingComponentStyles.ts new file mode 100644 index 00000000..f5bf7d1b --- /dev/null +++ b/src/modules/features/SettingDialogue/settingComponentStyles.ts @@ -0,0 +1,35 @@ +import type { ButtonComponent, Setting } from "@/deps.ts"; + +const SETTING_WITH_ADDITIONAL_ACTIONS_CLASS = "sls-setting-with-additional-actions"; +const ADDITIONAL_ACTION_CLASS = "sls-setting-additional-action"; + +/** + * Applies destructive-action styling without requiring Obsidian 1.13 at + * runtime. Older supported versions used the `mod-warning` class for the same + * presentation. + */ +export function setButtonDestructiveState(button: T, isDestructive = true): T { + const compatibleButton = button as unknown as { + setDestructive?: () => ButtonComponent; + removeDestructive?: () => ButtonComponent; + }; + const updateNativeStyle = isDestructive ? compatibleButton.setDestructive : compatibleButton.removeDestructive; + if (typeof updateNativeStyle === "function") { + updateNativeStyle.call(button); + } else { + button.buttonEl.classList.toggle("mod-warning", isDestructive); + } + return button; +} + +/** Sets whether a setting row contains actions which may move onto a later line. */ +export function setSettingAdditionalActionsState(setting: T, hasAdditionalActions = true): T { + setting.settingEl.classList.toggle(SETTING_WITH_ADDITIONAL_ACTIONS_CLASS, hasAdditionalActions); + return setting; +} + +/** Sets whether a button is an additional action which may move onto a later line. */ +export function setButtonAdditionalActionState(button: T, isAdditionalAction = true): T { + button.buttonEl.classList.toggle(ADDITIONAL_ACTION_CLASS, isAdditionalAction); + return button; +} diff --git a/src/modules/features/SettingDialogue/SettingPane.unit.spec.ts b/src/modules/features/SettingDialogue/settingComponentStyles.unit.spec.ts similarity index 55% rename from src/modules/features/SettingDialogue/SettingPane.unit.spec.ts rename to src/modules/features/SettingDialogue/settingComponentStyles.unit.spec.ts index d53be7f3..e8344678 100644 --- a/src/modules/features/SettingDialogue/SettingPane.unit.spec.ts +++ b/src/modules/features/SettingDialogue/settingComponentStyles.unit.spec.ts @@ -1,6 +1,10 @@ import type { ButtonComponent, Setting } from "@/deps.ts"; import { describe, expect, it, vi } from "vitest"; -import { markSettingRowWithSubsequentButtons, markSubsequentButton, setButtonDestructiveState } from "./SettingPane.ts"; +import { + setButtonAdditionalActionState, + setButtonDestructiveState, + setSettingAdditionalActionsState, +} from "./settingComponentStyles.ts"; type CompatibleButton = ButtonComponent & { setDestructive?: () => ButtonComponent; @@ -10,7 +14,6 @@ type CompatibleButton = ButtonComponent & { function createButton(overrides: Partial = {}): CompatibleButton { return { buttonEl: { - addClass: vi.fn(), classList: { toggle: vi.fn(), }, @@ -21,25 +24,41 @@ function createButton(overrides: Partial = {}): CompatibleButt function createSetting(): Setting { return { - setClass: vi.fn().mockReturnThis(), + settingEl: { + classList: { + toggle: vi.fn(), + }, + }, } as unknown as Setting; } -describe("markSettingRowWithSubsequentButtons", () => { - it("marks only the supplied setting row as containing subsequent actions", () => { +describe("setSettingAdditionalActionsState", () => { + it("sets whether the supplied setting row contains additional actions", () => { const setting = createSetting(); - expect(markSettingRowWithSubsequentButtons(setting)).toBe(setting); - expect(setting.setClass).toHaveBeenCalledWith("sls-setting-row-with-subsequent-buttons"); + expect(setSettingAdditionalActionsState(setting, true)).toBe(setting); + expect(setSettingAdditionalActionsState(setting, false)).toBe(setting); + expect(setting.settingEl.classList.toggle).toHaveBeenNthCalledWith( + 1, + "sls-setting-with-additional-actions", + true + ); + expect(setting.settingEl.classList.toggle).toHaveBeenNthCalledWith( + 2, + "sls-setting-with-additional-actions", + false + ); }); }); -describe("markSubsequentButton", () => { - it("marks only the supplied button as a subsequent action", () => { +describe("setButtonAdditionalActionState", () => { + it("sets whether the supplied button is an additional action", () => { const button = createButton(); - expect(markSubsequentButton(button)).toBe(button); - expect(button.buttonEl.addClass).toHaveBeenCalledWith("sls-setting-subsequent-button"); + expect(setButtonAdditionalActionState(button, true)).toBe(button); + expect(setButtonAdditionalActionState(button, false)).toBe(button); + expect(button.buttonEl.classList.toggle).toHaveBeenNthCalledWith(1, "sls-setting-additional-action", true); + expect(button.buttonEl.classList.toggle).toHaveBeenNthCalledWith(2, "sls-setting-additional-action", false); }); }); diff --git a/styles.css b/styles.css index 1c9e2705..6788414f 100644 --- a/styles.css +++ b/styles.css @@ -547,17 +547,17 @@ body.is-mobile .sls-setting button { white-space: normal; } -body.is-mobile .sls-setting-row-with-subsequent-buttons { +body.is-mobile .sls-setting-with-additional-actions { flex-wrap: wrap; } -body.is-mobile .sls-setting-row-with-subsequent-buttons .setting-item-control { +body.is-mobile .sls-setting-with-additional-actions .setting-item-control { min-width: 0; flex: 1 1 100%; flex-wrap: wrap; } -body.is-mobile .sls-setting .sls-setting-subsequent-button { +body.is-mobile .sls-setting .sls-setting-additional-action { flex: 1 1 12rem; }