Skip to content

fix: make the Comments panel opt-in from Settings #1638

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
giswqs merged 5 commits into from
Aug 2, 2026
Merged

fix: make the Comments panel opt-in from Settings #1638

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e67af60
fix: make the Comments panel opt-in from Settings
giswqs Aug 2, 2026
453e726
fix: let dock panels coexist in shared rails
giswqs Aug 2, 2026
2ab7b1c
Address review feedback
giswqs Aug 2, 2026
ef78f30
Address review feedback
giswqs Aug 2, 2026
870653f
Address review feedback
giswqs Aug 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
202 changes: 111 additions & 91 deletions apps/geolibre-desktop/src/components/layout/DesktopShell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,14 @@
el.className = "contents";
return el;
});
const activePanelId = useRightPanelState().activeId;
const rightPanelState = useRightPanelState();
const activePanelId = rightPanelState.activeId;
const replaceStylePanelIds = rightPanelState.visibleIds.filter(
(id) => rightPanelState.panelDocks[id] === "replace-style",
);
const replaceLayersPanelIds = rightPanelState.visibleIds.filter(
(id) => rightPanelState.panelDocks[id] === "replace-layers",
);
const activePanel = activePanelId ? getRightPanel(activePanelId) : undefined;
// The plugins in VIEWER_BLOCKED_PLUGIN_IDS paint drawing and editing controls
// onto the map, which the read-only viewer preset cannot hide the way it
Expand Down Expand Up @@ -1374,7 +1381,7 @@
}
});
return () => setKmlFileImportHandler(null);
}, [addImportedVectorLayers, confirmLargeVectorDataset, t]);

Check warning on line 1384 in apps/geolibre-desktop/src/components/layout/DesktopShell.tsx

View workflow job for this annotation

GitHub Actions / Build and test 

React Hook useEffect has an unnecessary dependency: 'confirmLargeVectorDataset'. Either exclude it or remove the dependency array. Outer scope values like 'confirmLargeVectorDataset' aren't valid dependencies because mutating them doesn't re-render the component

const addDroppedPhotos = useCallback(
(result: GeotaggedPhotoResult | null): number => {
Expand Down Expand Up @@ -1650,7 +1657,7 @@
disposed = true;
unlisten?.();
};
}, [

Check warning on line 1660 in apps/geolibre-desktop/src/components/layout/DesktopShell.tsx

View workflow job for this annotation

GitHub Actions / Build and test 

React Hook useEffect has a missing dependency: 't'. Either include it or remove the dependency array
clearDropMessageLater,
finishDrop,
addDroppedRasters,
Expand Down Expand Up @@ -1794,7 +1801,7 @@
clearDropMessageLater();
}
},
[

Check warning on line 1804 in apps/geolibre-desktop/src/components/layout/DesktopShell.tsx

View workflow job for this annotation

GitHub Actions / Build and test 

React Hook useCallback has a missing dependency: 't'. Either include it or remove the dependency array
clearDropMessageLater,
finishDrop,
addDroppedRasters,
Expand Down Expand Up @@ -2075,55 +2082,13 @@
{/* Map-only / hidden-panels embeds show nothing but the map: skip the
whole left side-dock (Layers, plugin panels, and the shared rail that
hosts the Browser entry), not just the built-in Layers panel. */}
{layoutOptions.panelsHidden ? null : replaceLayersPanelId && !layoutOptions.viewer ? (
// Shared-rail mode on the Layers (left) side: the plugin panel shares
// the Layers sidebar surface, so a single rail lists both the workbench
// and Layers instead of the two positional plugin slots flanking it.
<SectionErrorBoundary
label="Shared left sidebar"
displayName={t("shell.section.sharedLeftSidebar")}
>
<SharedSidebar
key={replaceLayersPanelId}
side="layers"
pluginId={replaceLayersPanelId}
pluginContentEl={dockContentEl}
pluginWidth={pluginPanelWidth}
onPluginWidthChange={setPluginPanelWidth}
builtinVisible={layoutOptions.layerPanelVisible}
builtinTitle={t("sharedRail.layers")}
builtinIcon={<Layers className="h-4 w-4" />}
// The Browser docks here on by default but must not bury Layers:
// start with Layers expanded and Browser a collapsed rail entry.
// On a phone-width viewport both start collapsed (panels overlay
// there), matching the mobile "panels default collapsed" behavior.
initialBuiltinExpanded={
replaceLayersPanelId === BROWSER_PANEL_ID && !getIsMobileViewport()
}
// The story-map presentation is the only standalone Layers
// autoCollapse trigger (the notebook collapses Style, not Layers).
forceBuiltinCollapsed={storymapPresenting}
renderBuiltin={({ collapsed, onCollapsedChange }) => (
<LayerPanel
mapControllerRef={mapControllerRef}
onResizeStart={startLayerPanelResize}
geometryEditLayerId={geometryEditLayerId}
onToggleGeometryEdit={handleToggleGeometryEdit}
onCancelGeometryEdit={handleCancelGeometryEdit}
onMaterializeDuckDBLayer={handleMaterializeDuckDBLayer}
onOpenRasterStylePanel={() =>
openRasterLayerPanel(createAppAPI(mapControllerRef))
}
onOpenRasterSubset={setRasterSubsetLayer}
collapsed={collapsed}
onCollapsedChange={onCollapsedChange}
hideOwnRail
/>
)}
/>
</SectionErrorBoundary>
) : (
{layoutOptions.panelsHidden ? null : (
<>
{/* The positional plugin docks flank whichever middle surface the
Layers side shows (the shared rail or the standalone Layers
panel): a panel moved to left/right-of-layers must stay
reachable while a shared-rail panel such as the Browser is
open. */}
{!layoutOptions.viewer ? (
<SectionErrorBoundary
label="Plugin panel (left of Layers)"
Expand All @@ -2137,7 +2102,55 @@
/>
</SectionErrorBoundary>
) : null}
{layoutOptions.layerPanelVisible ? (
{replaceLayersPanelId && !layoutOptions.viewer ? (
// Shared-rail mode on the Layers (left) side: the plugin panel shares
// the Layers sidebar surface, so a single rail lists both the workbench
// and Layers instead of the built-in panel standing on its own.
<SectionErrorBoundary
label="Shared left sidebar"
displayName={t("shell.section.sharedLeftSidebar")}
>
<SharedSidebar
key={replaceLayersPanelId}
side="layers"
pluginId={replaceLayersPanelId}
additionalPanelIds={replaceLayersPanelIds}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
pluginContentEl={dockContentEl}
pluginWidth={pluginPanelWidth}
onPluginWidthChange={setPluginPanelWidth}
builtinVisible={layoutOptions.layerPanelVisible}
builtinTitle={t("sharedRail.layers")}
builtinIcon={<Layers className="h-4 w-4" />}
// The Browser docks here on by default but must not bury Layers:
// start with Layers expanded and Browser a collapsed rail entry.
// On a phone-width viewport both start collapsed (panels overlay
// there), matching the mobile "panels default collapsed" behavior.
initialBuiltinExpanded={
replaceLayersPanelId === BROWSER_PANEL_ID && !getIsMobileViewport()
}
// The story-map presentation is the only standalone Layers
// autoCollapse trigger (the notebook collapses Style, not Layers).
forceBuiltinCollapsed={storymapPresenting}
renderBuiltin={({ collapsed, onCollapsedChange }) => (
<LayerPanel
mapControllerRef={mapControllerRef}
onResizeStart={startLayerPanelResize}
geometryEditLayerId={geometryEditLayerId}
onToggleGeometryEdit={handleToggleGeometryEdit}
onCancelGeometryEdit={handleCancelGeometryEdit}
onMaterializeDuckDBLayer={handleMaterializeDuckDBLayer}
onOpenRasterStylePanel={() =>
openRasterLayerPanel(createAppAPI(mapControllerRef))
}
onOpenRasterSubset={setRasterSubsetLayer}
collapsed={collapsed}
onCollapsedChange={onCollapsedChange}
hideOwnRail
/>
)}
/>
</SectionErrorBoundary>
) : layoutOptions.layerPanelVisible ? (
<SectionErrorBoundary label="Layer panel" displayName={t("shell.section.layerPanel")}>
{layoutOptions.viewer ? (
<ViewerLayerPanel />
Expand Down Expand Up @@ -2311,45 +2324,13 @@
</main>
{/* Same as the left dock: a map-only / hidden-panels embed skips the
entire right side-dock (Style, plugin panels, and their shared rail). */}
{layoutOptions.panelsHidden || layoutOptions.viewer ? null : replaceStylePanelId ? (
// Shared-rail mode (issue #765): the plugin panel shares the Style
// sidebar surface, so a single rail lists both the workbench and Style
// instead of the two positional plugin slots flanking the Style panel.
<SectionErrorBoundary
label="Shared right sidebar"
displayName={t("shell.section.sharedRightSidebar")}
>
<SharedSidebar
// Key by the active panel id so switching between two replace-style
// plugins remounts the sidebar, resetting its per-panel local state
// (the Style opt-in) rather than carrying the previous plugin over.
key={replaceStylePanelId}
side="style"
pluginId={replaceStylePanelId}
pluginContentEl={dockContentEl}
pluginWidth={pluginPanelWidth}
onPluginWidthChange={setPluginPanelWidth}
builtinVisible={layoutOptions.stylePanelVisible}
builtinTitle={t("sharedRail.style")}
builtinIcon={<SlidersHorizontal className="h-4 w-4" />}
// Mirror the standalone Style panel's autoCollapse triggers so the
// notebook / story-map presentation collapses Style here too.
// `autoCollapsedPanel` is omitted because it is always null in a
// shared-rail mode (the panel is the sole active one).
forceBuiltinCollapsed={notebookOpen || storymapPresenting}
renderBuiltin={({ collapsed, onCollapsedChange }) => (
<StylePanel
mapControllerRef={mapControllerRef}
onResizeStart={startStylePanelResize}
collapsed={collapsed}
onCollapsedChange={onCollapsedChange}
hideOwnRail
/>
)}
/>
</SectionErrorBoundary>
) : (
{layoutOptions.panelsHidden || layoutOptions.viewer ? null : (
<>
{/* Shared-rail panels such as Comments must not remove the ordinary
positional docks: enabled Web Services panels still live in
left/right-of-style and need their vertical rail entries. Both
flank whichever middle surface applies, so they are rendered
once here rather than duplicated per branch. */}
<SectionErrorBoundary
label="Plugin panel (left of Style)"
displayName={t("shell.section.pluginPanelLeftOfStyle")}
Expand All @@ -2361,11 +2342,50 @@
onWidthChange={setPluginPanelWidth}
/>
</SectionErrorBoundary>
{/* The notebook claims the workspace's right half, so the Style panel
{replaceStylePanelId ? (
<SectionErrorBoundary
label="Shared right sidebar"
displayName={t("shell.section.sharedRightSidebar")}
>
<SharedSidebar
// Key by the active panel id so switching between two replace-style
// plugins remounts the sidebar, resetting its per-panel local state
// (the Style opt-in) rather than carrying the previous plugin over.
key={replaceStylePanelId}
side="style"
pluginId={replaceStylePanelId}
additionalPanelIds={replaceStylePanelIds}
pluginContentEl={dockContentEl}
pluginWidth={pluginPanelWidth}
onPluginWidthChange={setPluginPanelWidth}
builtinVisible={layoutOptions.stylePanelVisible}
builtinTitle={t("sharedRail.style")}
builtinIcon={<SlidersHorizontal className="h-4 w-4" />}
// Mirror the standalone Style panel's autoCollapse triggers so the
// notebook / story-map presentation collapses Style here too.
// `autoCollapsedPanel` is omitted because it is always null in a
// shared-rail mode (the panel is the sole active one).
forceBuiltinCollapsed={notebookOpen || storymapPresenting}
renderBuiltin={({ collapsed, onCollapsedChange }) => (
<StylePanel
mapControllerRef={mapControllerRef}
onResizeStart={startStylePanelResize}
collapsed={collapsed}
onCollapsedChange={onCollapsedChange}
// Controlled mode ignores autoCollapse for collapsing (the
// rail owns that via forceBuiltinCollapsed); it is passed so
// a layer selection cannot expand Style over the notebook.
autoCollapse={notebookOpen || storymapPresenting}
hideOwnRail
/>
)}
/>
</SectionErrorBoundary>
) : /* The notebook claims the workspace's right half, so the Style panel
collapses to its rail while the notebook is open (Processing →
Jupyter Notebook) rather than unmounting; the user can re-expand it.
A story map presentation collapses it for the same reason. */}
{layoutOptions.stylePanelVisible ? (
A story map presentation collapses it for the same reason. */
layoutOptions.stylePanelVisible ? (
<SectionErrorBoundary label="Style panel" displayName={t("shell.section.stylePanel")}>
<StylePanel
mapControllerRef={mapControllerRef}
Expand Down
31 changes: 30 additions & 1 deletion apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import {
Locate,
MapPinned,
LayoutPanelTop,
MessageSquare,
Moon,
Palette,
PanelLeft,
Expand Down Expand Up @@ -84,6 +85,7 @@ import {
} from "../../hooks/useDesktopSettings";
import { useLanguage } from "../../hooks/useLanguage";
import { BROWSER_PANEL_ID } from "../../hooks/useRegisterBrowserPanel";
import { COMMENTS_PANEL_ID } from "../../hooks/useRegisterCommentsPanel";
import { useRightPanelState } from "../../hooks/useRightPanels";
import type { ThemeMode } from "../../hooks/useThemeMode";
import { isTauri } from "../../lib/is-tauri";
Expand Down Expand Up @@ -390,7 +392,9 @@ export function SettingsDialog({
// 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.
const browserPanelOpen = useRightPanelState().activeId === BROWSER_PANEL_ID;
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.
Expand All @@ -402,6 +406,14 @@ export function SettingsDialog({
closeRightPanel(BROWSER_PANEL_ID);
}
};
const toggleCommentsPanel = (show: boolean) => {
if (show) {
openRightPanel(COMMENTS_PANEL_ID);
collapseRightPanel(COMMENTS_PANEL_ID);
} else {
closeRightPanel(COMMENTS_PANEL_ID);
}
};
Comment thread
giswqs marked this conversation as resolved.
// 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<SettingsFocusTarget | null>(null);
Expand Down Expand Up @@ -1264,6 +1276,13 @@ export function SettingsDialog({
>
{t("settings.layout.showBrowserPanel")}
</DropdownMenuCheckboxItem>
<DropdownMenuCheckboxItem
checked={commentsPanelOpen}
onCheckedChange={(checked: boolean) => toggleCommentsPanel(checked === true)}
onSelect={(event: Event) => event.preventDefault()}
>
{t("settings.layout.showCommentsPanel")}
</DropdownMenuCheckboxItem>
<DropdownMenuSeparator />
<DropdownMenuItem
onSelect={() => {
Expand Down Expand Up @@ -1738,6 +1757,16 @@ export function SettingsDialog({
<FolderTree className="h-4 w-4 text-muted-foreground" />
<span>{t("settings.layout.showBrowserPanel")}</span>
</label>
<label className="flex items-center gap-3 rounded-md border p-3 text-sm">
<input
className="h-4 w-4"
type="checkbox"
checked={commentsPanelOpen}
onChange={(event) => toggleCommentsPanel(event.target.checked)}
/>
<MessageSquare className="h-4 w-4 text-muted-foreground" />
<span>{t("settings.layout.showCommentsPanel")}</span>
</label>
</div>
{showsAdvancedNotices(desktopSettings.uiProfile) ? (
<div className="rounded-md border bg-muted/40 p-3 text-xs text-muted-foreground">
Expand Down
Loading
Loading