diff --git a/apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx b/apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx index 60d6d4c8b..2bbbd6eb1 100644 --- a/apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx +++ b/apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx @@ -11,7 +11,6 @@ import { type ProjectPreferences, type RuntimeEnvironmentVariable, } from "@geolibre/core"; -import { closeRightPanel, collapseRightPanel, openRightPanel } from "@geolibre/plugins"; import { Button, Dialog, @@ -91,6 +90,7 @@ import { COMMENTS_PANEL_ID } from "../../hooks/useRegisterCommentsPanel"; import { useRightPanelState } from "../../hooks/useRightPanels"; import type { ThemeMode } from "../../hooks/useThemeMode"; import { isTauri } from "../../lib/is-tauri"; +import { applyRightPanelVisibility } from "../../lib/persisted-right-panel"; import { COORDINATE_FORMATS, normalizeCoordinateFormat } from "../../lib/coordinate-format"; import { THEME_SCHEMES, normalizeHexColor, type ThemeScheme } from "../../lib/theme-schemes"; import { IS_MAS_BUILD } from "../../lib/build-flags"; @@ -417,33 +417,17 @@ export function SettingsDialog({ const showSettingsItem = (id: string) => isMenuItemVisible(desktopSettings.uiProfile, id); const [open, setOpen] = useState(false); const [section, setSection] = useState("map"); - // The Browser is a dockable right panel (open/close via the registry), not a - // persisted layout preference, so its Layout toggle acts on the live registry - // state directly rather than through the draft settings. + // Browser and Comments are dockable right panels: the registry owns whether + // they are on screen and `registerPersistedRightPanel` mirrors that into + // `layout.browserPanelVisible` / `layout.commentsPanelVisible`, so the toggle + // no longer resets on every launch (#1935). Because the mirror is the single + // writer, moving the panel is all these controls have to do: the setting + // follows, so the two can never disagree about what the checkbox should say. const rightPanelState = useRightPanelState(); const browserPanelOpen = rightPanelState.visibleIds.includes(BROWSER_PANEL_ID); const commentsPanelOpen = rightPanelState.visibleIds.includes(COMMENTS_PANEL_ID); - // Show it collapsed on the shared Layers rail, matching its default state, so - // re-enabling from Settings doesn't jump to an expanded panel that buries the - // Layers panel. - const toggleBrowserPanel = (show: boolean) => { - if (show) { - openRightPanel(BROWSER_PANEL_ID); - collapseRightPanel(BROWSER_PANEL_ID); - } else { - closeRightPanel(BROWSER_PANEL_ID); - } - }; - // Collapsed for the same reason as Browser above, and to match the state - // Comments registers itself in on mount. - const toggleCommentsPanel = (show: boolean) => { - if (show) { - openRightPanel(COMMENTS_PANEL_ID); - collapseRightPanel(COMMENTS_PANEL_ID); - } else { - closeRightPanel(COMMENTS_PANEL_ID); - } - }; + const toggleBrowserPanel = (show: boolean) => applyRightPanelVisibility(BROWSER_PANEL_ID, show); + const toggleCommentsPanel = (show: boolean) => applyRightPanelVisibility(COMMENTS_PANEL_ID, show); // A field a deep-link asked us to focus once its section renders; cleared // after the focus lands so a later open without a focus request stays put. const [pendingFocus, setPendingFocus] = useState(null); @@ -1205,6 +1189,12 @@ export function SettingsDialog({ updates: draftDesktopSettings.updates, startup: draftDesktopSettings.startup, }); + // The dockable panels are the one layout row nothing renders from the store: + // the registry owns what is on screen, so move it to match what was just + // saved (a no-op for a panel already there, so an untouched Save cannot + // collapse one the user had expanded). + applyRightPanelVisibility(BROWSER_PANEL_ID, draftDesktopSettings.layout.browserPanelVisible); + applyRightPanelVisibility(COMMENTS_PANEL_ID, draftDesktopSettings.layout.commentsPanelVisible); setOpen(false); }; @@ -1827,8 +1817,12 @@ export function SettingsDialog({ toggleBrowserPanel(event.target.checked)} + checked={draftDesktopSettings.layout.browserPanelVisible} + onChange={(event) => + updateDraftLayoutSettings({ + browserPanelVisible: event.target.checked, + }) + } /> {t("settings.layout.showBrowserPanel")} @@ -1837,8 +1831,12 @@ export function SettingsDialog({ toggleCommentsPanel(event.target.checked)} + checked={draftDesktopSettings.layout.commentsPanelVisible} + onChange={(event) => + updateDraftLayoutSettings({ + commentsPanelVisible: event.target.checked, + }) + } /> {t("settings.layout.showCommentsPanel")} diff --git a/apps/geolibre-desktop/src/hooks/useDesktopSettings.ts b/apps/geolibre-desktop/src/hooks/useDesktopSettings.ts index f3adc2d18..73f268c5e 100644 --- a/apps/geolibre-desktop/src/hooks/useDesktopSettings.ts +++ b/apps/geolibre-desktop/src/hooks/useDesktopSettings.ts @@ -110,6 +110,17 @@ export interface UpdateSettings { } export interface DesktopLayoutSettings { + /** + * Whether the Browser (Data Source Manager) right panel is registered as + * visible. Unlike {@link layerPanelVisible} this does not describe a fixed + * dock slot: the Browser is a dockable right panel, so the flag is the + * persisted seed its registration hook applies on mount (open + collapsed onto + * its rail, or closed). Without it the panel reopened on every launch no + * matter what the Settings toggle said (#1935). + */ + browserPanelVisible: boolean; + /** Same as {@link browserPanelVisible}, for the Comments right panel. */ + commentsPanelVisible: boolean; layerPanelVisible: boolean; showProjectInfo: boolean; stylePanelVisible: boolean; @@ -157,6 +168,8 @@ interface DesktopSettingsState { } export const DEFAULT_DESKTOP_LAYOUT_SETTINGS: DesktopLayoutSettings = { + browserPanelVisible: true, + commentsPanelVisible: true, layerPanelVisible: true, showProjectInfo: true, stylePanelVisible: true, @@ -410,6 +423,14 @@ function normalizeDesktopLayoutSettings(layout: unknown): DesktopLayoutSettings // cannot smuggle non-boolean values into the layout settings. const candidate = layout as Partial; return { + browserPanelVisible: + typeof candidate.browserPanelVisible === "boolean" + ? candidate.browserPanelVisible + : DEFAULT_DESKTOP_LAYOUT_SETTINGS.browserPanelVisible, + commentsPanelVisible: + typeof candidate.commentsPanelVisible === "boolean" + ? candidate.commentsPanelVisible + : DEFAULT_DESKTOP_LAYOUT_SETTINGS.commentsPanelVisible, layerPanelVisible: typeof candidate.layerPanelVisible === "boolean" ? candidate.layerPanelVisible diff --git a/apps/geolibre-desktop/src/hooks/useRegisterBrowserPanel.ts b/apps/geolibre-desktop/src/hooks/useRegisterBrowserPanel.ts index a3a4cb764..628bb9d60 100644 --- a/apps/geolibre-desktop/src/hooks/useRegisterBrowserPanel.ts +++ b/apps/geolibre-desktop/src/hooks/useRegisterBrowserPanel.ts @@ -1,6 +1,6 @@ -import { collapseRightPanel, openRightPanel, registerRightPanel } from "@geolibre/plugins"; import { useEffect } from "react"; import i18n from "../i18n"; +import { registerPersistedRightPanel } from "../lib/persisted-right-panel"; /** Stable id of the Browser (Data Source Manager) right panel. */ export const BROWSER_PANEL_ID = "browser"; @@ -24,23 +24,26 @@ export const BROWSER_PANEL_ID = "browser"; * The panel is **on by default but collapsed** onto the shared Layers rail: on * mount it is opened and immediately collapsed, so it shows as a rail entry * beside Layers rather than covering the map. The user expands it from that - * rail (or toggles it off in Settings → Layout). It reopens collapsed on the - * next load, matching the "on by default" behavior of the Layout toggle. + * rail (or toggles it off in Settings → Layout). "By default" means the default + * of the persisted `layout.browserPanelVisible` setting, which + * {@link registerPersistedRightPanel} seeds from and then keeps in step with the + * panel: turning it off stays off across restarts instead of the toggle silently + * resetting on every launch (#1935). */ export function useRegisterBrowserPanel(): void { - useEffect(() => { - // i18n.t (not the useTranslation hook) so registration carries no - // render-time dependency; the body still localizes live via useTranslation. - const dispose = registerRightPanel({ - id: BROWSER_PANEL_ID, - title: () => i18n.t("browser.title"), - dock: "replace-layers", - render: () => {}, - }); - // Default on, but docked collapsed to the Layers rail (open then collapse), - // so it is present without burying the map on first load. - openRightPanel(BROWSER_PANEL_ID); - collapseRightPanel(BROWSER_PANEL_ID); - return dispose; - }, []); + useEffect( + () => + registerPersistedRightPanel( + { + id: BROWSER_PANEL_ID, + // i18n.t (not the useTranslation hook) so registration carries no + // render-time dependency; the body localizes live via useTranslation. + title: () => i18n.t("browser.title"), + dock: "replace-layers", + render: () => {}, + }, + "browserPanelVisible", + ), + [], + ); } diff --git a/apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts b/apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts index 0e6c7e6c5..17f00bff3 100644 --- a/apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts +++ b/apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts @@ -1,6 +1,6 @@ -import { collapseRightPanel, openRightPanel, registerRightPanel } from "@geolibre/plugins"; import { useEffect } from "react"; import i18n from "../i18n"; +import { registerPersistedRightPanel } from "../lib/persisted-right-panel"; /** Stable id of the Comments right panel. */ export const COMMENTS_PANEL_ID = "comments"; @@ -10,20 +10,26 @@ export const COMMENTS_PANEL_ID = "comments"; * sidebar's rail (`replace-style`). * * Comments is enabled by default but collapsed onto the Style rail, so it is - * discoverable without taking map space. + * discoverable without taking map space. "By default" means the default of the + * persisted `layout.commentsPanelVisible` setting, which + * {@link registerPersistedRightPanel} seeds from and then keeps in step with the + * panel: turning it off stays off across restarts instead of the toggle silently + * resetting on every launch (#1935). */ export function useRegisterCommentsPanel(): void { - useEffect(() => { - // i18n.t (not the useTranslation hook) so registration carries no - // render-time dependency; the rail entry re-resolves the getter on render. - const dispose = registerRightPanel({ - id: COMMENTS_PANEL_ID, - title: () => i18n.t("comments.title"), - dock: "replace-style", - render: () => {}, - }); - openRightPanel(COMMENTS_PANEL_ID); - collapseRightPanel(COMMENTS_PANEL_ID); - return dispose; - }, []); + useEffect( + () => + registerPersistedRightPanel( + { + id: COMMENTS_PANEL_ID, + // i18n.t (not the useTranslation hook) so registration carries no + // render-time dependency; the rail entry re-resolves it on render. + title: () => i18n.t("comments.title"), + dock: "replace-style", + render: () => {}, + }, + "commentsPanelVisible", + ), + [], + ); } diff --git a/apps/geolibre-desktop/src/lib/persisted-right-panel.ts b/apps/geolibre-desktop/src/lib/persisted-right-panel.ts new file mode 100644 index 000000000..8814a3e37 --- /dev/null +++ b/apps/geolibre-desktop/src/lib/persisted-right-panel.ts @@ -0,0 +1,98 @@ +// The registry subpath rather than the package barrel: this module is a leaf the +// test suite imports directly, and the barrel pulls in the whole built-in plugin +// registry (including CSS imports Node cannot load). See CLAUDE.md on testing +// against a leaf module. +import { + collapseRightPanel, + closeRightPanel, + getRightPanel, + isRightPanelVisible, + openRightPanel, + registerRightPanel, + subscribeRightPanels, +} from "@geolibre/plugins/right-panel-registry"; +import type { GeoLibreRightPanelRegistration } from "@geolibre/plugins"; +import { useDesktopSettingsStore, type DesktopLayoutSettings } from "../hooks/useDesktopSettings"; + +/** + * Layout settings that persist a dockable right panel's visibility. The Browser + * and Comments panels are the two built-in panels that work this way; plugin + * panels are owned by their plugin and are not persisted here. + */ +export type PersistedPanelKey = Extract< + keyof DesktopLayoutSettings, + "browserPanelVisible" | "commentsPanelVisible" +>; + +/** Read a panel's persisted visibility, bypassing React so callers can seed. */ +export function isPanelVisibleInSettings(key: PersistedPanelKey): boolean { + return useDesktopSettingsStore.getState().desktopSettings.layout[key]; +} + +/** Persist a panel's visibility. A no-op when the value already matches. */ +export function setPanelVisibleInSettings(key: PersistedPanelKey, visible: boolean): void { + const { desktopSettings, setDesktopSettings } = useDesktopSettingsStore.getState(); + if (desktopSettings.layout[key] === visible) return; + setDesktopSettings({ + ...desktopSettings, + layout: { ...desktopSettings.layout, [key]: visible }, + }); +} + +/** + * Move a panel to `visible`, showing it collapsed on its rail so re-enabling it + * does not jump to an expanded panel that buries its neighbour. Bails out when + * the panel is already where it is being asked to go, so a caller re-applying an + * unchanged value cannot collapse a panel the user had expanded. + */ +export function applyRightPanelVisibility(panelId: string, visible: boolean): void { + if (isRightPanelVisible(panelId) === visible) return; + if (visible) { + openRightPanel(panelId); + collapseRightPanel(panelId); + } else { + closeRightPanel(panelId); + } +} + +/** + * Register a dockable right panel whose visibility is a persisted layout + * setting, and keep the two in step. Returns a disposer that unsubscribes + * before unregistering. + * + * Visibility is seeded from the setting at registration, so a panel the user + * turned off stays off across restarts instead of reopening on every launch + * (GeoLibre#1935). From then on the setting mirrors the registry, which matters + * because the panel can be closed from its own header as well as from Settings + * → Layout: for these panels closing is not a transient collapse, it removes + * the rail entry entirely and only Settings can bring it back, so it is a + * preference either way. Mirroring is what keeps the Settings checkbox, the + * panel on screen, and the stored value from ever disagreeing. + * + * Two registry events are deliberately *not* mirrored: + * + * - The `emit` from registration itself, because the panel is legitimately not + * visible yet. The subscription is therefore attached after the seed. + * - The `emit` from unregistering on unmount, which would otherwise persist + * `false` every time the shell tears down. `unregisterRightPanel` removes the + * panel from the registry before it emits, so the `getRightPanel` guard + * catches it; the disposer unsubscribing first makes that belt-and-braces. + * + * Being displaced by another panel is not a close (the registry keeps a + * displaced panel in `visibleIds`), so it correctly writes nothing. + */ +export function registerPersistedRightPanel( + registration: GeoLibreRightPanelRegistration, + key: PersistedPanelKey, +): () => void { + const dispose = registerRightPanel(registration); + applyRightPanelVisibility(registration.id, isPanelVisibleInSettings(key)); + const unsubscribe = subscribeRightPanels(() => { + if (!getRightPanel(registration.id)) return; + setPanelVisibleInSettings(key, isRightPanelVisible(registration.id)); + }); + return () => { + unsubscribe(); + dispose(); + }; +} diff --git a/packages/plugins/package.json b/packages/plugins/package.json index 1680978c4..e2dae116e 100644 --- a/packages/plugins/package.json +++ b/packages/plugins/package.json @@ -12,6 +12,7 @@ "./local-netcdf": "./src/plugins/local-netcdf.ts", "./maplibre-graticule": "./src/plugins/maplibre-graticule.ts", "./raster-symbology": "./src/plugins/raster-symbology.ts", + "./right-panel-registry": "./src/right-panel-registry.ts", "./zarr-time-axis": "./src/plugins/zarr-time-axis.ts" }, "dependencies": { diff --git a/tests/layout-panel-settings.test.ts b/tests/layout-panel-settings.test.ts new file mode 100644 index 000000000..5be09c2f9 --- /dev/null +++ b/tests/layout-panel-settings.test.ts @@ -0,0 +1,44 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { + DEFAULT_DESKTOP_LAYOUT_SETTINGS, + normalizeDesktopSettings, +} from "../apps/geolibre-desktop/src/hooks/useDesktopSettings"; + +// The Browser and Comments right panels used to be session-only: their Settings +// → Layout toggles moved the panel registry but nothing was persisted, so every +// launch reopened them (GeoLibre#1935). They are now layout settings like the +// Layers/Style panels, which means they have to round-trip through +// normalizeDesktopSettings and keep defaulting to on for existing users whose +// stored settings predate the keys. +describe("dockable panel layout settings", () => { + it("defaults both dockable panels to visible", () => { + assert.equal(DEFAULT_DESKTOP_LAYOUT_SETTINGS.browserPanelVisible, true); + assert.equal(DEFAULT_DESKTOP_LAYOUT_SETTINGS.commentsPanelVisible, true); + }); + + it("keeps a disabled panel disabled across a load", () => { + const layout = normalizeDesktopSettings({ + layout: { browserPanelVisible: false, commentsPanelVisible: false }, + }).layout; + assert.equal(layout.browserPanelVisible, false); + assert.equal(layout.commentsPanelVisible, false); + }); + + it("falls back to the defaults for settings saved before the keys existed", () => { + const layout = normalizeDesktopSettings({ + layout: { layerPanelVisible: false, stylePanelVisible: true, toolbarLabels: true }, + }).layout; + assert.equal(layout.layerPanelVisible, false); + assert.equal(layout.browserPanelVisible, true); + assert.equal(layout.commentsPanelVisible, true); + }); + + it("rejects non-boolean values from tampered storage", () => { + const layout = normalizeDesktopSettings({ + layout: { browserPanelVisible: "no", commentsPanelVisible: 0 }, + }).layout; + assert.equal(layout.browserPanelVisible, true); + assert.equal(layout.commentsPanelVisible, true); + }); +}); diff --git a/tests/persisted-right-panel.test.ts b/tests/persisted-right-panel.test.ts new file mode 100644 index 000000000..2b078f3e8 --- /dev/null +++ b/tests/persisted-right-panel.test.ts @@ -0,0 +1,133 @@ +import assert from "node:assert/strict"; +import { afterEach, beforeEach, describe, it } from "node:test"; +import { + __resetRightPanelRegistryForTests, + closeRightPanel, + collapseRightPanel, + isRightPanelCollapsed, + isRightPanelVisible, + openRightPanel, + registerRightPanel, +} from "../packages/plugins/src/right-panel-registry"; +import { + applyRightPanelVisibility, + registerPersistedRightPanel, +} from "../apps/geolibre-desktop/src/lib/persisted-right-panel"; +import { useDesktopSettingsStore } from "../apps/geolibre-desktop/src/hooks/useDesktopSettings"; + +// The Browser and Comments right panels persist their visibility so a panel the +// user turned off stays off across restarts (GeoLibre#1935). The setting and the +// registry have to agree in both directions: the panel is seeded from the +// setting at registration, and every later visibility change (Settings toggle or +// the panel's own close button) is written back. +const PANEL_ID = "comments"; +const KEY = "commentsPanelVisible"; + +function registration(id = PANEL_ID) { + return { id, title: "Comments", dock: "replace-style" as const, render: () => {} }; +} + +function setStoredVisibility(visible: boolean): void { + const { desktopSettings, setDesktopSettings } = useDesktopSettingsStore.getState(); + setDesktopSettings({ + ...desktopSettings, + layout: { ...desktopSettings.layout, [KEY]: visible }, + }); +} + +function storedVisibility(): boolean { + return useDesktopSettingsStore.getState().desktopSettings.layout[KEY]; +} + +beforeEach(() => { + __resetRightPanelRegistryForTests(); + setStoredVisibility(true); +}); + +afterEach(() => { + __resetRightPanelRegistryForTests(); + setStoredVisibility(true); +}); + +describe("applyRightPanelVisibility", () => { + it("opens the panel collapsed onto its rail", () => { + registerRightPanel(registration()); + applyRightPanelVisibility(PANEL_ID, true); + assert.equal(isRightPanelVisible(PANEL_ID), true); + assert.equal(isRightPanelCollapsed(), true); + }); + + it("leaves an expanded panel expanded when re-applying `true`", () => { + registerRightPanel(registration()); + openRightPanel(PANEL_ID); + assert.equal(isRightPanelCollapsed(), false); + // Saving the Settings dialog re-applies every layout row, including rows the + // user never touched; that must not collapse a panel they had expanded. + applyRightPanelVisibility(PANEL_ID, true); + assert.equal(isRightPanelCollapsed(), false); + }); + + it("closes the panel", () => { + registerRightPanel(registration()); + applyRightPanelVisibility(PANEL_ID, true); + applyRightPanelVisibility(PANEL_ID, false); + assert.equal(isRightPanelVisible(PANEL_ID), false); + }); +}); + +describe("registerPersistedRightPanel", () => { + it("seeds a visible panel from the setting, collapsed", () => { + const dispose = registerPersistedRightPanel(registration(), KEY); + assert.equal(isRightPanelVisible(PANEL_ID), true); + assert.equal(isRightPanelCollapsed(), true); + dispose(); + }); + + it("leaves a disabled panel closed instead of reopening it on every launch", () => { + setStoredVisibility(false); + const dispose = registerPersistedRightPanel(registration(), KEY); + assert.equal(isRightPanelVisible(PANEL_ID), false); + assert.equal(storedVisibility(), false); + dispose(); + }); + + it("persists a close that came from the panel's own header", () => { + const dispose = registerPersistedRightPanel(registration(), KEY); + closeRightPanel(PANEL_ID); + assert.equal(storedVisibility(), false); + dispose(); + }); + + it("persists a reopen", () => { + setStoredVisibility(false); + const dispose = registerPersistedRightPanel(registration(), KEY); + applyRightPanelVisibility(PANEL_ID, true); + assert.equal(storedVisibility(), true); + dispose(); + }); + + it("does not treat collapsing to the rail as turning the panel off", () => { + const dispose = registerPersistedRightPanel(registration(), KEY); + openRightPanel(PANEL_ID); + collapseRightPanel(PANEL_ID); + assert.equal(storedVisibility(), true); + dispose(); + }); + + it("does not treat being displaced by another panel as a close", () => { + const dispose = registerPersistedRightPanel(registration(), KEY); + registerRightPanel(registration("other")); + openRightPanel("other"); + assert.equal(storedVisibility(), true); + dispose(); + }); + + it("does not persist `false` when the shell unmounts", () => { + // The disposer unregisters, which emits a snapshot with the panel gone. That + // teardown must not be mistaken for the user turning the panel off, or the + // setting would flip to false on every reload. + const dispose = registerPersistedRightPanel(registration(), KEY); + dispose(); + assert.equal(storedVisibility(), true); + }); +});