From 01f3b133e202d36406af47220e14520514ff2ac3 Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 15 Aug 2026 12:16:19 -0400 Subject: [PATCH 1/3] fix(layout): persist Browser and Comments panel toggles The Settings -> Layout toggles for the Browser and Comments right panels acted only on the live right-panel registry. Nothing was written to `desktopSettings.layout`, and both registration hooks unconditionally opened their panel on mount, so every launch reopened a panel the user had turned off. Startup mode made no difference: reopening the last project restores layers and the camera, never these panels. Both panels now have a persisted `layout.browserPanelVisible` / `layout.commentsPanelVisible` setting (defaulting to on, so settings saved before the keys existed keep today's behavior), the registration hooks seed from it, and the Settings toggles write it. The toggles apply live rather than on Save, so they patch the dialog draft as well; the draft is snapshotted when the dialog opens and Save writes it wholesale, which would otherwise revert the toggle the user just made. Reset moves the panels too, since those two rows render the live registry state. Fixes #1935 --- .../src/components/layout/SettingsDialog.tsx | 26 +++++++++-- .../src/hooks/useDesktopSettings.ts | 21 +++++++++ .../src/hooks/useRegisterBrowserPanel.ts | 19 +++++--- .../src/hooks/useRegisterCommentsPanel.ts | 16 +++++-- tests/layout-panel-settings.test.ts | 44 +++++++++++++++++++ 5 files changed, 115 insertions(+), 11 deletions(-) create mode 100644 tests/layout-panel-settings.test.ts diff --git a/apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx b/apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx index 60d6d4c8b..152492fa5 100644 --- a/apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx +++ b/apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx @@ -417,12 +417,25 @@ 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, so their checkboxes read the + // live registry state (the user can also close them from their own header) + // while the toggle writes the matching persisted layout setting. The setting + // is what their registration hooks seed from on the next launch, so the + // toggle survives a restart (#1935). const rightPanelState = useRightPanelState(); const browserPanelOpen = rightPanelState.visibleIds.includes(BROWSER_PANEL_ID); const commentsPanelOpen = rightPanelState.visibleIds.includes(COMMENTS_PANEL_ID); + // These apply live rather than on Save, so the draft is patched alongside the + // saved settings: the draft was snapshotted when the dialog opened, and Save + // writes it wholesale, which would otherwise revert the toggle the user just + // made in this same dialog. + const applyPanelVisibility = ( + key: "browserPanelVisible" | "commentsPanelVisible", + visible: boolean, + ) => { + updateSavedLayoutSettings({ [key]: visible }); + updateDraftLayoutSettings({ [key]: visible }); + }; // 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. @@ -433,6 +446,7 @@ export function SettingsDialog({ } else { closeRightPanel(BROWSER_PANEL_ID); } + applyPanelVisibility("browserPanelVisible", show); }; // Collapsed for the same reason as Browser above, and to match the state // Comments registers itself in on mount. @@ -443,6 +457,7 @@ export function SettingsDialog({ } else { closeRightPanel(COMMENTS_PANEL_ID); } + applyPanelVisibility("commentsPanelVisible", 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. @@ -932,6 +947,11 @@ export function SettingsDialog({ const resetLayoutSettings = () => { updateDraftLayoutSettings(DEFAULT_DESKTOP_LAYOUT_SETTINGS); + // The Browser/Comments checkboxes render the live registry state, not the + // draft, so reset has to move the panels themselves or those two rows would + // ignore the button. + toggleBrowserPanel(DEFAULT_DESKTOP_LAYOUT_SETTINGS.browserPanelVisible); + toggleCommentsPanel(DEFAULT_DESKTOP_LAYOUT_SETTINGS.commentsPanelVisible); }; // The accent scheme applies live (instant preview) rather than waiting for 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..a63f375d2 100644 --- a/apps/geolibre-desktop/src/hooks/useRegisterBrowserPanel.ts +++ b/apps/geolibre-desktop/src/hooks/useRegisterBrowserPanel.ts @@ -1,6 +1,7 @@ import { collapseRightPanel, openRightPanel, registerRightPanel } from "@geolibre/plugins"; import { useEffect } from "react"; import i18n from "../i18n"; +import { useDesktopSettingsStore } from "./useDesktopSettings"; /** Stable id of the Browser (Data Source Manager) right panel. */ export const BROWSER_PANEL_ID = "browser"; @@ -24,8 +25,10 @@ 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: turning the panel off + * in Settings → Layout keeps it off across restarts rather than having the + * toggle silently reset (#1935). */ export function useRegisterBrowserPanel(): void { useEffect(() => { @@ -38,9 +41,15 @@ export function useRegisterBrowserPanel(): void { 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); + // so it is present without burying the map on first load. Read the setting + // here rather than subscribing: this seeds the panel's startup state only. + // Once mounted the Settings toggle drives the registry directly, so the + // user can also close the panel from its own header without that being + // written back as a preference. + if (useDesktopSettingsStore.getState().desktopSettings.layout.browserPanelVisible) { + openRightPanel(BROWSER_PANEL_ID); + collapseRightPanel(BROWSER_PANEL_ID); + } return dispose; }, []); } diff --git a/apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts b/apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts index 0e6c7e6c5..66a74d946 100644 --- a/apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts +++ b/apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts @@ -1,6 +1,7 @@ import { collapseRightPanel, openRightPanel, registerRightPanel } from "@geolibre/plugins"; import { useEffect } from "react"; import i18n from "../i18n"; +import { useDesktopSettingsStore } from "./useDesktopSettings"; /** Stable id of the Comments right panel. */ export const COMMENTS_PANEL_ID = "comments"; @@ -10,7 +11,10 @@ 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: a user who turned the panel + * off in Settings → Layout gets it back off on the next launch instead of having + * the toggle silently reset (#1935). */ export function useRegisterCommentsPanel(): void { useEffect(() => { @@ -22,8 +26,14 @@ export function useRegisterCommentsPanel(): void { dock: "replace-style", render: () => {}, }); - openRightPanel(COMMENTS_PANEL_ID); - collapseRightPanel(COMMENTS_PANEL_ID); + // Read the setting here rather than subscribing: this seeds the panel's + // startup state only. Once mounted the Settings toggle drives the registry + // directly, so the user can also close the panel from its own header + // without that being written back as a preference. + if (useDesktopSettingsStore.getState().desktopSettings.layout.commentsPanelVisible) { + openRightPanel(COMMENTS_PANEL_ID); + collapseRightPanel(COMMENTS_PANEL_ID); + } return dispose; }, []); } 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); + }); +}); From c0678c228b33cbe710de9befe98d416e41386117 Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 15 Aug 2026 12:30:05 -0400 Subject: [PATCH 2/3] fix(layout): give the panel rows the dialog's draft semantics Review follow-up. The Browser/Comments rows in the Layout dialog applied live while the other four rows waited for Save, which left two seams: Reset followed by Cancel stuck those two flags while reverting the rest, and the rows rendered live registry state that Save did not write, so closing a panel from its own header showed an unchecked box the dialog then saved back as visible. Both rows are now draft-backed like their neighbours. The draft seeds them from the live registry when the dialog opens, so a panel closed from its header still shows unchecked and Save persists exactly what was displayed; Reset and Cancel behave as they do for every other row. Save applies the committed values to the registry, skipping panels already in the requested state so saving an untouched dialog cannot collapse an expanded panel. The Settings dropdown keeps applying on the spot and persisting in one step, since it has no Save to wait for. --- .../src/components/layout/SettingsDialog.tsx | 102 +++++++++++------- 1 file changed, 61 insertions(+), 41 deletions(-) diff --git a/apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx b/apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx index 152492fa5..0c75ebb42 100644 --- a/apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx +++ b/apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx @@ -11,7 +11,12 @@ import { type ProjectPreferences, type RuntimeEnvironmentVariable, } from "@geolibre/core"; -import { closeRightPanel, collapseRightPanel, openRightPanel } from "@geolibre/plugins"; +import { + closeRightPanel, + collapseRightPanel, + isRightPanelVisible, + openRightPanel, +} from "@geolibre/plugins"; import { Button, Dialog, @@ -417,47 +422,45 @@ export function SettingsDialog({ const showSettingsItem = (id: string) => isMenuItemVisible(desktopSettings.uiProfile, id); const [open, setOpen] = useState(false); const [section, setSection] = useState("map"); - // Browser and Comments are dockable right panels, so their checkboxes read the - // live registry state (the user can also close them from their own header) - // while the toggle writes the matching persisted layout setting. The setting - // is what their registration hooks seed from on the next launch, so the - // toggle survives a restart (#1935). + // Browser and Comments are dockable right panels: the registry owns whether + // they are on screen, and `layout.browserPanelVisible` / + // `layout.commentsPanelVisible` persist that across restarts so the toggle no + // longer resets on every launch (#1935). The dropdown's checkboxes read the + // live registry (they apply on the spot, with no Save to wait for); the + // dialog's read its draft like every other row there. const rightPanelState = useRightPanelState(); const browserPanelOpen = rightPanelState.visibleIds.includes(BROWSER_PANEL_ID); const commentsPanelOpen = rightPanelState.visibleIds.includes(COMMENTS_PANEL_ID); - // These apply live rather than on Save, so the draft is patched alongside the - // saved settings: the draft was snapshotted when the dialog opened, and Save - // writes it wholesale, which would otherwise revert the toggle the user just - // made in this same dialog. - const applyPanelVisibility = ( - key: "browserPanelVisible" | "commentsPanelVisible", - visible: boolean, - ) => { - updateSavedLayoutSettings({ [key]: visible }); - updateDraftLayoutSettings({ [key]: visible }); - }; // 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) => { + // Bails out when the panel is already where it is being asked to go, so + // saving the dialog without touching these rows cannot collapse a panel the + // user had expanded. + const applyPanelVisibility = (panelId: string, show: boolean) => { + if (isRightPanelVisible(panelId) === show) return; if (show) { - openRightPanel(BROWSER_PANEL_ID); - collapseRightPanel(BROWSER_PANEL_ID); + openRightPanel(panelId); + collapseRightPanel(panelId); } else { - closeRightPanel(BROWSER_PANEL_ID); + closeRightPanel(panelId); } - applyPanelVisibility("browserPanelVisible", show); }; + const applyBrowserPanelVisibility = (show: boolean) => + applyPanelVisibility(BROWSER_PANEL_ID, show); // Collapsed for the same reason as Browser above, and to match the state // Comments registers itself in on mount. + const applyCommentsPanelVisibility = (show: boolean) => + applyPanelVisibility(COMMENTS_PANEL_ID, show); + // The dropdown toggles have no Save step, so they move the panel and persist + // the preference in one go, matching the other live entries in that menu. + const toggleBrowserPanel = (show: boolean) => { + applyBrowserPanelVisibility(show); + updateSavedLayoutSettings({ browserPanelVisible: show }); + }; const toggleCommentsPanel = (show: boolean) => { - if (show) { - openRightPanel(COMMENTS_PANEL_ID); - collapseRightPanel(COMMENTS_PANEL_ID); - } else { - closeRightPanel(COMMENTS_PANEL_ID); - } - applyPanelVisibility("commentsPanelVisible", show); + applyCommentsPanelVisibility(show); + updateSavedLayoutSettings({ commentsPanelVisible: 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. @@ -617,9 +620,18 @@ export function SettingsDialog({ } const seededPreferences = clonePreferences(useAppStore.getState().preferences); setDraftPreferences(seededPreferences); - setDraftDesktopSettings( - cloneDesktopSettings(useDesktopSettingsStore.getState().desktopSettings), - ); + const seededSettings = cloneDesktopSettings(useDesktopSettingsStore.getState().desktopSettings); + // Browser and Comments are dockable panels the user can also close from the + // panel's own header, which is a session action that writes no setting. Seed + // those two rows from the live registry rather than the stored value so the + // dialog opens showing what is actually on screen; Save then persists + // exactly what the checkboxes showed. + seededSettings.layout = { + ...seededSettings.layout, + browserPanelVisible: isRightPanelVisible(BROWSER_PANEL_ID), + commentsPanelVisible: isRightPanelVisible(COMMENTS_PANEL_ID), + }; + setDraftDesktopSettings(seededSettings); // Land the AI section on the first profile's provider, or the first // available provider if no profiles exist, so the user sees something // relevant without extra clicks. @@ -947,11 +959,6 @@ export function SettingsDialog({ const resetLayoutSettings = () => { updateDraftLayoutSettings(DEFAULT_DESKTOP_LAYOUT_SETTINGS); - // The Browser/Comments checkboxes render the live registry state, not the - // draft, so reset has to move the panels themselves or those two rows would - // ignore the button. - toggleBrowserPanel(DEFAULT_DESKTOP_LAYOUT_SETTINGS.browserPanelVisible); - toggleCommentsPanel(DEFAULT_DESKTOP_LAYOUT_SETTINGS.commentsPanelVisible); }; // The accent scheme applies live (instant preview) rather than waiting for @@ -1225,6 +1232,11 @@ export function SettingsDialog({ updates: draftDesktopSettings.updates, startup: draftDesktopSettings.startup, }); + // The dockable panels are the one layout row the store cannot apply on its + // own: their registration hooks only read the setting at mount, so move the + // registry here to match what was just saved. + applyBrowserPanelVisibility(draftDesktopSettings.layout.browserPanelVisible); + applyCommentsPanelVisibility(draftDesktopSettings.layout.commentsPanelVisible); setOpen(false); }; @@ -1847,8 +1859,12 @@ export function SettingsDialog({ toggleBrowserPanel(event.target.checked)} + checked={draftDesktopSettings.layout.browserPanelVisible} + onChange={(event) => + updateDraftLayoutSettings({ + browserPanelVisible: event.target.checked, + }) + } /> {t("settings.layout.showBrowserPanel")} @@ -1857,8 +1873,12 @@ export function SettingsDialog({ toggleCommentsPanel(event.target.checked)} + checked={draftDesktopSettings.layout.commentsPanelVisible} + onChange={(event) => + updateDraftLayoutSettings({ + commentsPanelVisible: event.target.checked, + }) + } /> {t("settings.layout.showCommentsPanel")} From 66280e0fdca7c9ebf2f30f08daeb629a1b4d3b62 Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 15 Aug 2026 12:52:41 -0400 Subject: [PATCH 3/3] fix(layout): mirror panel visibility instead of persisting it on Save Review follow-up. Seeding the dialog draft from the live registry fixed the checkbox lying about what Save would write, but it moved the seam rather than closing it: a panel closed from its own header stayed a session-only state that an unrelated Save then converted into a permanent preference. Both directions now go through one place. `registerPersistedRightPanel` seeds the panel from its setting at registration and subscribes to the registry afterwards, writing every later visibility change back. For these panels closing is not a transient collapse, it removes the rail entry entirely and only Settings can restore it, so it is a preference however it was reached. With the setting and the registry always in step, the checkbox cannot disagree with either, and there is no session-only state left for a Save to silently promote. Two registry events are deliberately not mirrored: the emit from registration (the panel is legitimately not visible yet, so the subscription is attached after the seed) and the emit from unregistering on unmount, which would otherwise persist false on every teardown. Being displaced by another panel is not a close, so it writes nothing. The dialog rows keep the draft semantics of the previous commit, and Save re-applies them to the registry, skipping a panel already in the requested state. `tests/persisted-right-panel.test.ts` covers the seed, the header close, the collapse and displacement non-events, and the teardown guard; the module imports the registry subpath so it stays a leaf the suite can load without the plugin barrel. --- .../src/components/layout/SettingsDialog.tsx | 76 +++------- .../src/hooks/useRegisterBrowserPanel.ts | 46 +++--- .../src/hooks/useRegisterCommentsPanel.ts | 44 +++--- .../src/lib/persisted-right-panel.ts | 98 +++++++++++++ packages/plugins/package.json | 1 + tests/persisted-right-panel.test.ts | 133 ++++++++++++++++++ 6 files changed, 289 insertions(+), 109 deletions(-) create mode 100644 apps/geolibre-desktop/src/lib/persisted-right-panel.ts create mode 100644 tests/persisted-right-panel.test.ts diff --git a/apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx b/apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx index 0c75ebb42..2bbbd6eb1 100644 --- a/apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx +++ b/apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx @@ -11,12 +11,6 @@ import { type ProjectPreferences, type RuntimeEnvironmentVariable, } from "@geolibre/core"; -import { - closeRightPanel, - collapseRightPanel, - isRightPanelVisible, - openRightPanel, -} from "@geolibre/plugins"; import { Button, Dialog, @@ -96,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"; @@ -423,45 +418,16 @@ export function SettingsDialog({ const [open, setOpen] = useState(false); const [section, setSection] = useState("map"); // Browser and Comments are dockable right panels: the registry owns whether - // they are on screen, and `layout.browserPanelVisible` / - // `layout.commentsPanelVisible` persist that across restarts so the toggle no - // longer resets on every launch (#1935). The dropdown's checkboxes read the - // live registry (they apply on the spot, with no Save to wait for); the - // dialog's read its draft like every other row there. + // 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. - // Bails out when the panel is already where it is being asked to go, so - // saving the dialog without touching these rows cannot collapse a panel the - // user had expanded. - const applyPanelVisibility = (panelId: string, show: boolean) => { - if (isRightPanelVisible(panelId) === show) return; - if (show) { - openRightPanel(panelId); - collapseRightPanel(panelId); - } else { - closeRightPanel(panelId); - } - }; - const applyBrowserPanelVisibility = (show: boolean) => - applyPanelVisibility(BROWSER_PANEL_ID, show); - // Collapsed for the same reason as Browser above, and to match the state - // Comments registers itself in on mount. - const applyCommentsPanelVisibility = (show: boolean) => - applyPanelVisibility(COMMENTS_PANEL_ID, show); - // The dropdown toggles have no Save step, so they move the panel and persist - // the preference in one go, matching the other live entries in that menu. - const toggleBrowserPanel = (show: boolean) => { - applyBrowserPanelVisibility(show); - updateSavedLayoutSettings({ browserPanelVisible: show }); - }; - const toggleCommentsPanel = (show: boolean) => { - applyCommentsPanelVisibility(show); - updateSavedLayoutSettings({ commentsPanelVisible: show }); - }; + 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); @@ -620,18 +586,9 @@ export function SettingsDialog({ } const seededPreferences = clonePreferences(useAppStore.getState().preferences); setDraftPreferences(seededPreferences); - const seededSettings = cloneDesktopSettings(useDesktopSettingsStore.getState().desktopSettings); - // Browser and Comments are dockable panels the user can also close from the - // panel's own header, which is a session action that writes no setting. Seed - // those two rows from the live registry rather than the stored value so the - // dialog opens showing what is actually on screen; Save then persists - // exactly what the checkboxes showed. - seededSettings.layout = { - ...seededSettings.layout, - browserPanelVisible: isRightPanelVisible(BROWSER_PANEL_ID), - commentsPanelVisible: isRightPanelVisible(COMMENTS_PANEL_ID), - }; - setDraftDesktopSettings(seededSettings); + setDraftDesktopSettings( + cloneDesktopSettings(useDesktopSettingsStore.getState().desktopSettings), + ); // Land the AI section on the first profile's provider, or the first // available provider if no profiles exist, so the user sees something // relevant without extra clicks. @@ -1232,11 +1189,12 @@ export function SettingsDialog({ updates: draftDesktopSettings.updates, startup: draftDesktopSettings.startup, }); - // The dockable panels are the one layout row the store cannot apply on its - // own: their registration hooks only read the setting at mount, so move the - // registry here to match what was just saved. - applyBrowserPanelVisibility(draftDesktopSettings.layout.browserPanelVisible); - applyCommentsPanelVisibility(draftDesktopSettings.layout.commentsPanelVisible); + // 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); }; diff --git a/apps/geolibre-desktop/src/hooks/useRegisterBrowserPanel.ts b/apps/geolibre-desktop/src/hooks/useRegisterBrowserPanel.ts index a63f375d2..628bb9d60 100644 --- a/apps/geolibre-desktop/src/hooks/useRegisterBrowserPanel.ts +++ b/apps/geolibre-desktop/src/hooks/useRegisterBrowserPanel.ts @@ -1,7 +1,6 @@ -import { collapseRightPanel, openRightPanel, registerRightPanel } from "@geolibre/plugins"; import { useEffect } from "react"; import i18n from "../i18n"; -import { useDesktopSettingsStore } from "./useDesktopSettings"; +import { registerPersistedRightPanel } from "../lib/persisted-right-panel"; /** Stable id of the Browser (Data Source Manager) right panel. */ export const BROWSER_PANEL_ID = "browser"; @@ -26,30 +25,25 @@ export const BROWSER_PANEL_ID = "browser"; * 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). "By default" means the default - * of the persisted `layout.browserPanelVisible` setting: turning the panel off - * in Settings → Layout keeps it off across restarts rather than having the - * toggle silently reset (#1935). + * 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. Read the setting - // here rather than subscribing: this seeds the panel's startup state only. - // Once mounted the Settings toggle drives the registry directly, so the - // user can also close the panel from its own header without that being - // written back as a preference. - if (useDesktopSettingsStore.getState().desktopSettings.layout.browserPanelVisible) { - 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 66a74d946..17f00bff3 100644 --- a/apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts +++ b/apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts @@ -1,7 +1,6 @@ -import { collapseRightPanel, openRightPanel, registerRightPanel } from "@geolibre/plugins"; import { useEffect } from "react"; import i18n from "../i18n"; -import { useDesktopSettingsStore } from "./useDesktopSettings"; +import { registerPersistedRightPanel } from "../lib/persisted-right-panel"; /** Stable id of the Comments right panel. */ export const COMMENTS_PANEL_ID = "comments"; @@ -12,28 +11,25 @@ export const COMMENTS_PANEL_ID = "comments"; * * Comments is enabled by default but collapsed onto the Style rail, so it is * discoverable without taking map space. "By default" means the default of the - * persisted `layout.commentsPanelVisible` setting: a user who turned the panel - * off in Settings → Layout gets it back off on the next launch instead of having - * the toggle silently reset (#1935). + * 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: () => {}, - }); - // Read the setting here rather than subscribing: this seeds the panel's - // startup state only. Once mounted the Settings toggle drives the registry - // directly, so the user can also close the panel from its own header - // without that being written back as a preference. - if (useDesktopSettingsStore.getState().desktopSettings.layout.commentsPanelVisible) { - 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/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); + }); +});