Skip to content

Commit 6311ce4

Browse files
authored
fix(layout): persist Browser and Comments panel toggles (#1940)
* 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 * 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. * 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.
1 parent 5f77287 commit 6311ce4

8 files changed

Lines changed: 366 additions & 62 deletions

File tree

apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx

Lines changed: 27 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
type ProjectPreferences,
1212
type RuntimeEnvironmentVariable,
1313
} from "@geolibre/core";
14-
import { closeRightPanel, collapseRightPanel, openRightPanel } from "@geolibre/plugins";
1514
import {
1615
Button,
1716
Dialog,
@@ -91,6 +90,7 @@ import { COMMENTS_PANEL_ID } from "../../hooks/useRegisterCommentsPanel";
9190
import { useRightPanelState } from "../../hooks/useRightPanels";
9291
import type { ThemeMode } from "../../hooks/useThemeMode";
9392
import { isTauri } from "../../lib/is-tauri";
93+
import { applyRightPanelVisibility } from "../../lib/persisted-right-panel";
9494
import { COORDINATE_FORMATS, normalizeCoordinateFormat } from "../../lib/coordinate-format";
9595
import { THEME_SCHEMES, normalizeHexColor, type ThemeScheme } from "../../lib/theme-schemes";
9696
import { IS_MAS_BUILD } from "../../lib/build-flags";
@@ -417,33 +417,17 @@ export function SettingsDialog({
417417
const showSettingsItem = (id: string) => isMenuItemVisible(desktopSettings.uiProfile, id);
418418
const [open, setOpen] = useState(false);
419419
const [section, setSection] = useState<SettingsSection>("map");
420-
// The Browser is a dockable right panel (open/close via the registry), not a
421-
// persisted layout preference, so its Layout toggle acts on the live registry
422-
// state directly rather than through the draft settings.
420+
// Browser and Comments are dockable right panels: the registry owns whether
421+
// they are on screen and `registerPersistedRightPanel` mirrors that into
422+
// `layout.browserPanelVisible` / `layout.commentsPanelVisible`, so the toggle
423+
// no longer resets on every launch (#1935). Because the mirror is the single
424+
// writer, moving the panel is all these controls have to do: the setting
425+
// follows, so the two can never disagree about what the checkbox should say.
423426
const rightPanelState = useRightPanelState();
424427
const browserPanelOpen = rightPanelState.visibleIds.includes(BROWSER_PANEL_ID);
425428
const commentsPanelOpen = rightPanelState.visibleIds.includes(COMMENTS_PANEL_ID);
426-
// Show it collapsed on the shared Layers rail, matching its default state, so
427-
// re-enabling from Settings doesn't jump to an expanded panel that buries the
428-
// Layers panel.
429-
const toggleBrowserPanel = (show: boolean) => {
430-
if (show) {
431-
openRightPanel(BROWSER_PANEL_ID);
432-
collapseRightPanel(BROWSER_PANEL_ID);
433-
} else {
434-
closeRightPanel(BROWSER_PANEL_ID);
435-
}
436-
};
437-
// Collapsed for the same reason as Browser above, and to match the state
438-
// Comments registers itself in on mount.
439-
const toggleCommentsPanel = (show: boolean) => {
440-
if (show) {
441-
openRightPanel(COMMENTS_PANEL_ID);
442-
collapseRightPanel(COMMENTS_PANEL_ID);
443-
} else {
444-
closeRightPanel(COMMENTS_PANEL_ID);
445-
}
446-
};
429+
const toggleBrowserPanel = (show: boolean) => applyRightPanelVisibility(BROWSER_PANEL_ID, show);
430+
const toggleCommentsPanel = (show: boolean) => applyRightPanelVisibility(COMMENTS_PANEL_ID, show);
447431
// A field a deep-link asked us to focus once its section renders; cleared
448432
// after the focus lands so a later open without a focus request stays put.
449433
const [pendingFocus, setPendingFocus] = useState<SettingsFocusTarget | null>(null);
@@ -1205,6 +1189,12 @@ export function SettingsDialog({
12051189
updates: draftDesktopSettings.updates,
12061190
startup: draftDesktopSettings.startup,
12071191
});
1192+
// The dockable panels are the one layout row nothing renders from the store:
1193+
// the registry owns what is on screen, so move it to match what was just
1194+
// saved (a no-op for a panel already there, so an untouched Save cannot
1195+
// collapse one the user had expanded).
1196+
applyRightPanelVisibility(BROWSER_PANEL_ID, draftDesktopSettings.layout.browserPanelVisible);
1197+
applyRightPanelVisibility(COMMENTS_PANEL_ID, draftDesktopSettings.layout.commentsPanelVisible);
12081198
setOpen(false);
12091199
};
12101200

@@ -1827,8 +1817,12 @@ export function SettingsDialog({
18271817
<input
18281818
className="h-4 w-4"
18291819
type="checkbox"
1830-
checked={browserPanelOpen}
1831-
onChange={(event) => toggleBrowserPanel(event.target.checked)}
1820+
checked={draftDesktopSettings.layout.browserPanelVisible}
1821+
onChange={(event) =>
1822+
updateDraftLayoutSettings({
1823+
browserPanelVisible: event.target.checked,
1824+
})
1825+
}
18321826
/>
18331827
<FolderTree className="h-4 w-4 text-muted-foreground" />
18341828
<span>{t("settings.layout.showBrowserPanel")}</span>
@@ -1837,8 +1831,12 @@ export function SettingsDialog({
18371831
<input
18381832
className="h-4 w-4"
18391833
type="checkbox"
1840-
checked={commentsPanelOpen}
1841-
onChange={(event) => toggleCommentsPanel(event.target.checked)}
1834+
checked={draftDesktopSettings.layout.commentsPanelVisible}
1835+
onChange={(event) =>
1836+
updateDraftLayoutSettings({
1837+
commentsPanelVisible: event.target.checked,
1838+
})
1839+
}
18421840
/>
18431841
<MessageSquare className="h-4 w-4 text-muted-foreground" />
18441842
<span>{t("settings.layout.showCommentsPanel")}</span>

apps/geolibre-desktop/src/hooks/useDesktopSettings.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,17 @@ export interface UpdateSettings {
110110
}
111111

112112
export interface DesktopLayoutSettings {
113+
/**
114+
* Whether the Browser (Data Source Manager) right panel is registered as
115+
* visible. Unlike {@link layerPanelVisible} this does not describe a fixed
116+
* dock slot: the Browser is a dockable right panel, so the flag is the
117+
* persisted seed its registration hook applies on mount (open + collapsed onto
118+
* its rail, or closed). Without it the panel reopened on every launch no
119+
* matter what the Settings toggle said (#1935).
120+
*/
121+
browserPanelVisible: boolean;
122+
/** Same as {@link browserPanelVisible}, for the Comments right panel. */
123+
commentsPanelVisible: boolean;
113124
layerPanelVisible: boolean;
114125
showProjectInfo: boolean;
115126
stylePanelVisible: boolean;
@@ -157,6 +168,8 @@ interface DesktopSettingsState {
157168
}
158169

159170
export const DEFAULT_DESKTOP_LAYOUT_SETTINGS: DesktopLayoutSettings = {
171+
browserPanelVisible: true,
172+
commentsPanelVisible: true,
160173
layerPanelVisible: true,
161174
showProjectInfo: true,
162175
stylePanelVisible: true,
@@ -410,6 +423,14 @@ function normalizeDesktopLayoutSettings(layout: unknown): DesktopLayoutSettings
410423
// cannot smuggle non-boolean values into the layout settings.
411424
const candidate = layout as Partial<DesktopLayoutSettings>;
412425
return {
426+
browserPanelVisible:
427+
typeof candidate.browserPanelVisible === "boolean"
428+
? candidate.browserPanelVisible
429+
: DEFAULT_DESKTOP_LAYOUT_SETTINGS.browserPanelVisible,
430+
commentsPanelVisible:
431+
typeof candidate.commentsPanelVisible === "boolean"
432+
? candidate.commentsPanelVisible
433+
: DEFAULT_DESKTOP_LAYOUT_SETTINGS.commentsPanelVisible,
413434
layerPanelVisible:
414435
typeof candidate.layerPanelVisible === "boolean"
415436
? candidate.layerPanelVisible
Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import { collapseRightPanel, openRightPanel, registerRightPanel } from "@geolibre/plugins";
21
import { useEffect } from "react";
32
import i18n from "../i18n";
3+
import { registerPersistedRightPanel } from "../lib/persisted-right-panel";
44

55
/** Stable id of the Browser (Data Source Manager) right panel. */
66
export const BROWSER_PANEL_ID = "browser";
@@ -24,23 +24,26 @@ export const BROWSER_PANEL_ID = "browser";
2424
* The panel is **on by default but collapsed** onto the shared Layers rail: on
2525
* mount it is opened and immediately collapsed, so it shows as a rail entry
2626
* beside Layers rather than covering the map. The user expands it from that
27-
* rail (or toggles it off in Settings → Layout). It reopens collapsed on the
28-
* next load, matching the "on by default" behavior of the Layout toggle.
27+
* rail (or toggles it off in Settings → Layout). "By default" means the default
28+
* of the persisted `layout.browserPanelVisible` setting, which
29+
* {@link registerPersistedRightPanel} seeds from and then keeps in step with the
30+
* panel: turning it off stays off across restarts instead of the toggle silently
31+
* resetting on every launch (#1935).
2932
*/
3033
export function useRegisterBrowserPanel(): void {
31-
useEffect(() => {
32-
// i18n.t (not the useTranslation hook) so registration carries no
33-
// render-time dependency; the body still localizes live via useTranslation.
34-
const dispose = registerRightPanel({
35-
id: BROWSER_PANEL_ID,
36-
title: () => i18n.t("browser.title"),
37-
dock: "replace-layers",
38-
render: () => {},
39-
});
40-
// Default on, but docked collapsed to the Layers rail (open then collapse),
41-
// so it is present without burying the map on first load.
42-
openRightPanel(BROWSER_PANEL_ID);
43-
collapseRightPanel(BROWSER_PANEL_ID);
44-
return dispose;
45-
}, []);
34+
useEffect(
35+
() =>
36+
registerPersistedRightPanel(
37+
{
38+
id: BROWSER_PANEL_ID,
39+
// i18n.t (not the useTranslation hook) so registration carries no
40+
// render-time dependency; the body localizes live via useTranslation.
41+
title: () => i18n.t("browser.title"),
42+
dock: "replace-layers",
43+
render: () => {},
44+
},
45+
"browserPanelVisible",
46+
),
47+
[],
48+
);
4649
}
Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import { collapseRightPanel, openRightPanel, registerRightPanel } from "@geolibre/plugins";
21
import { useEffect } from "react";
32
import i18n from "../i18n";
3+
import { registerPersistedRightPanel } from "../lib/persisted-right-panel";
44

55
/** Stable id of the Comments right panel. */
66
export const COMMENTS_PANEL_ID = "comments";
@@ -10,20 +10,26 @@ export const COMMENTS_PANEL_ID = "comments";
1010
* sidebar's rail (`replace-style`).
1111
*
1212
* Comments is enabled by default but collapsed onto the Style rail, so it is
13-
* discoverable without taking map space.
13+
* discoverable without taking map space. "By default" means the default of the
14+
* persisted `layout.commentsPanelVisible` setting, which
15+
* {@link registerPersistedRightPanel} seeds from and then keeps in step with the
16+
* panel: turning it off stays off across restarts instead of the toggle silently
17+
* resetting on every launch (#1935).
1418
*/
1519
export function useRegisterCommentsPanel(): void {
16-
useEffect(() => {
17-
// i18n.t (not the useTranslation hook) so registration carries no
18-
// render-time dependency; the rail entry re-resolves the getter on render.
19-
const dispose = registerRightPanel({
20-
id: COMMENTS_PANEL_ID,
21-
title: () => i18n.t("comments.title"),
22-
dock: "replace-style",
23-
render: () => {},
24-
});
25-
openRightPanel(COMMENTS_PANEL_ID);
26-
collapseRightPanel(COMMENTS_PANEL_ID);
27-
return dispose;
28-
}, []);
20+
useEffect(
21+
() =>
22+
registerPersistedRightPanel(
23+
{
24+
id: COMMENTS_PANEL_ID,
25+
// i18n.t (not the useTranslation hook) so registration carries no
26+
// render-time dependency; the rail entry re-resolves it on render.
27+
title: () => i18n.t("comments.title"),
28+
dock: "replace-style",
29+
render: () => {},
30+
},
31+
"commentsPanelVisible",
32+
),
33+
[],
34+
);
2935
}
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// The registry subpath rather than the package barrel: this module is a leaf the
2+
// test suite imports directly, and the barrel pulls in the whole built-in plugin
3+
// registry (including CSS imports Node cannot load). See CLAUDE.md on testing
4+
// against a leaf module.
5+
import {
6+
collapseRightPanel,
7+
closeRightPanel,
8+
getRightPanel,
9+
isRightPanelVisible,
10+
openRightPanel,
11+
registerRightPanel,
12+
subscribeRightPanels,
13+
} from "@geolibre/plugins/right-panel-registry";
14+
import type { GeoLibreRightPanelRegistration } from "@geolibre/plugins";
15+
import { useDesktopSettingsStore, type DesktopLayoutSettings } from "../hooks/useDesktopSettings";
16+
17+
/**
18+
* Layout settings that persist a dockable right panel's visibility. The Browser
19+
* and Comments panels are the two built-in panels that work this way; plugin
20+
* panels are owned by their plugin and are not persisted here.
21+
*/
22+
export type PersistedPanelKey = Extract<
23+
keyof DesktopLayoutSettings,
24+
"browserPanelVisible" | "commentsPanelVisible"
25+
>;
26+
27+
/** Read a panel's persisted visibility, bypassing React so callers can seed. */
28+
export function isPanelVisibleInSettings(key: PersistedPanelKey): boolean {
29+
return useDesktopSettingsStore.getState().desktopSettings.layout[key];
30+
}
31+
32+
/** Persist a panel's visibility. A no-op when the value already matches. */
33+
export function setPanelVisibleInSettings(key: PersistedPanelKey, visible: boolean): void {
34+
const { desktopSettings, setDesktopSettings } = useDesktopSettingsStore.getState();
35+
if (desktopSettings.layout[key] === visible) return;
36+
setDesktopSettings({
37+
...desktopSettings,
38+
layout: { ...desktopSettings.layout, [key]: visible },
39+
});
40+
}
41+
42+
/**
43+
* Move a panel to `visible`, showing it collapsed on its rail so re-enabling it
44+
* does not jump to an expanded panel that buries its neighbour. Bails out when
45+
* the panel is already where it is being asked to go, so a caller re-applying an
46+
* unchanged value cannot collapse a panel the user had expanded.
47+
*/
48+
export function applyRightPanelVisibility(panelId: string, visible: boolean): void {
49+
if (isRightPanelVisible(panelId) === visible) return;
50+
if (visible) {
51+
openRightPanel(panelId);
52+
collapseRightPanel(panelId);
53+
} else {
54+
closeRightPanel(panelId);
55+
}
56+
}
57+
58+
/**
59+
* Register a dockable right panel whose visibility is a persisted layout
60+
* setting, and keep the two in step. Returns a disposer that unsubscribes
61+
* before unregistering.
62+
*
63+
* Visibility is seeded from the setting at registration, so a panel the user
64+
* turned off stays off across restarts instead of reopening on every launch
65+
* (GeoLibre#1935). From then on the setting mirrors the registry, which matters
66+
* because the panel can be closed from its own header as well as from Settings
67+
* → Layout: for these panels closing is not a transient collapse, it removes
68+
* the rail entry entirely and only Settings can bring it back, so it is a
69+
* preference either way. Mirroring is what keeps the Settings checkbox, the
70+
* panel on screen, and the stored value from ever disagreeing.
71+
*
72+
* Two registry events are deliberately *not* mirrored:
73+
*
74+
* - The `emit` from registration itself, because the panel is legitimately not
75+
* visible yet. The subscription is therefore attached after the seed.
76+
* - The `emit` from unregistering on unmount, which would otherwise persist
77+
* `false` every time the shell tears down. `unregisterRightPanel` removes the
78+
* panel from the registry before it emits, so the `getRightPanel` guard
79+
* catches it; the disposer unsubscribing first makes that belt-and-braces.
80+
*
81+
* Being displaced by another panel is not a close (the registry keeps a
82+
* displaced panel in `visibleIds`), so it correctly writes nothing.
83+
*/
84+
export function registerPersistedRightPanel(
85+
registration: GeoLibreRightPanelRegistration,
86+
key: PersistedPanelKey,
87+
): () => void {
88+
const dispose = registerRightPanel(registration);
89+
applyRightPanelVisibility(registration.id, isPanelVisibleInSettings(key));
90+
const unsubscribe = subscribeRightPanels(() => {
91+
if (!getRightPanel(registration.id)) return;
92+
setPanelVisibleInSettings(key, isRightPanelVisible(registration.id));
93+
});
94+
return () => {
95+
unsubscribe();
96+
dispose();
97+
};
98+
}

packages/plugins/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
"./local-netcdf": "./src/plugins/local-netcdf.ts",
1313
"./maplibre-graticule": "./src/plugins/maplibre-graticule.ts",
1414
"./raster-symbology": "./src/plugins/raster-symbology.ts",
15+
"./right-panel-registry": "./src/right-panel-registry.ts",
1516
"./zarr-time-axis": "./src/plugins/zarr-time-axis.ts"
1617
},
1718
"dependencies": {
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import assert from "node:assert/strict";
2+
import { describe, it } from "node:test";
3+
import {
4+
DEFAULT_DESKTOP_LAYOUT_SETTINGS,
5+
normalizeDesktopSettings,
6+
} from "../apps/geolibre-desktop/src/hooks/useDesktopSettings";
7+
8+
// The Browser and Comments right panels used to be session-only: their Settings
9+
// → Layout toggles moved the panel registry but nothing was persisted, so every
10+
// launch reopened them (GeoLibre#1935). They are now layout settings like the
11+
// Layers/Style panels, which means they have to round-trip through
12+
// normalizeDesktopSettings and keep defaulting to on for existing users whose
13+
// stored settings predate the keys.
14+
describe("dockable panel layout settings", () => {
15+
it("defaults both dockable panels to visible", () => {
16+
assert.equal(DEFAULT_DESKTOP_LAYOUT_SETTINGS.browserPanelVisible, true);
17+
assert.equal(DEFAULT_DESKTOP_LAYOUT_SETTINGS.commentsPanelVisible, true);
18+
});
19+
20+
it("keeps a disabled panel disabled across a load", () => {
21+
const layout = normalizeDesktopSettings({
22+
layout: { browserPanelVisible: false, commentsPanelVisible: false },
23+
}).layout;
24+
assert.equal(layout.browserPanelVisible, false);
25+
assert.equal(layout.commentsPanelVisible, false);
26+
});
27+
28+
it("falls back to the defaults for settings saved before the keys existed", () => {
29+
const layout = normalizeDesktopSettings({
30+
layout: { layerPanelVisible: false, stylePanelVisible: true, toolbarLabels: true },
31+
}).layout;
32+
assert.equal(layout.layerPanelVisible, false);
33+
assert.equal(layout.browserPanelVisible, true);
34+
assert.equal(layout.commentsPanelVisible, true);
35+
});
36+
37+
it("rejects non-boolean values from tampered storage", () => {
38+
const layout = normalizeDesktopSettings({
39+
layout: { browserPanelVisible: "no", commentsPanelVisible: 0 },
40+
}).layout;
41+
assert.equal(layout.browserPanelVisible, true);
42+
assert.equal(layout.commentsPanelVisible, true);
43+
});
44+
});

0 commit comments

Comments
 (0)