Skip to content

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

Merged
giswqs merged 5 commits into
mainfrom
feat/comments-panel-opt-in
Aug 2, 2026
Merged

fix: make the Comments panel opt-in from Settings#1638
giswqs merged 5 commits into
mainfrom
feat/comments-panel-opt-in

Conversation

@giswqs

@giswqs giswqs commented Aug 2, 2026

Copy link
Copy Markdown
Member

Summary

  • Comments no longer opens itself on mount; dockable right panels share one active registry slot, so it was displacing the Browser panel on startup.
  • Adds a Settings > Layout toggle (menu checkbox and Layout section) for the Comments panel, translated across every shipped locale.
  • Enabling it lands as a collapsed rail entry instead of an expanded panel, and a plugin panel docked outside the shared rail now renders its own collapsed rail so it stays reachable.
  • Selecting a layer re-expands the Style panel when it was sitting collapsed on the rail.

Test plan

  • npm run test:frontend passes
  • pre-commit run --files <changed paths> passes (includes the full build)
  • Fresh load shows Browser on the right rail; Comments is absent until enabled
  • Settings > Layout > Show Comments panel adds a collapsed Comments rail entry without hiding Style or Layers, and unchecking removes it
  • Clicking a layer expands the Style panel when it was collapsed
  • Verify the rail in an RTL locale (Arabic) and in both light and dark themes

Summary by CodeRabbit

  • New Features

    • Added a Layout setting to show or hide the Comments panel.
    • Comments can be opened, collapsed, or closed from Settings and remain hidden until enabled.
    • Added collapsed panel rails for easier access to multiple docked panels.
    • Improved navigation across shared Style and Layers replacement panels.
    • The Style panel expands automatically when switching layers.
  • Localization

    • Added translated Comments panel settings and titles across supported languages.

Comments opened itself on mount, which displaced the Browser panel because
dockable right panels share one active registry slot. It now registers only,
gains a Settings > Layout toggle, and shows a collapsed rail entry so enabling
it never buries Style or Layers. Selecting a layer also re-expands Style.
Copilot AI review requested due to automatic review settings August 2, 2026 02:22
@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8e7496c7-ecd4-486b-af62-24ad5e720758

📥 Commits

Reviewing files that changed from the base of the PR and between ef78f30 and 870653f.

📒 Files selected for processing (1)
  • apps/geolibre-desktop/src/components/panels/PluginRightPanel.tsx

📝 Walkthrough

Walkthrough

The desktop layout now supports multiple visible replacement panels, positional dock rails, opt-in Comments controls, localized labels, and automatic Style panel expansion after non-empty layer selection changes.

Changes

Replacement-panel rails

Layer / File(s) Summary
Panel registry and dock contracts
packages/plugins/src/right-panel-registry.ts, packages/plugins/src/index.ts, packages/plugins/src/plugins/*, packages/plugins/src/types.ts, tests/right-panel-registry.test.ts, docs/plugin-api.md
The registry tracks visible panels and selected docks. Plugin registrations default to replace-style. Tests and documentation reflect the updated contract.
Shared sidebar panel collections
apps/geolibre-desktop/src/components/layout/DesktopShell.tsx, apps/geolibre-desktop/src/components/panels/SharedSidebar.tsx, apps/geolibre-desktop/src/hooks/useRightPanels.ts
Shared sidebars receive all visible replacement panel IDs and render per-panel rail entries.
Positional dock rails
apps/geolibre-desktop/src/components/panels/PluginRightPanel.tsx
Positional docks enumerate registered panels and render multi-panel rails beside the active panel.
Comments controls and Style behavior
apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx, apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts, apps/geolibre-desktop/src/components/panels/StylePanel.tsx, apps/geolibre-desktop/src/i18n/locales/*.json
Comments becomes opt-in and gains layout controls. Style starts collapsed and expands after non-empty layer selection changes. Localized labels were added.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant SettingsDialog
  participant RightPanelRegistry
  participant DesktopShell
  participant SharedSidebar
  User->>SettingsDialog: toggle Comments panel
  SettingsDialog->>RightPanelRegistry: open, collapse, or close panel
  RightPanelRegistry-->>DesktopShell: emit visible panel IDs and dock state
  DesktopShell->>SharedSidebar: pass replacement panel IDs
  SharedSidebar-->>User: render active panel and shared rail
Loading

Possibly related PRs

Suggested reviewers: pythonbass

Poem

A rabbit checks each panel rail,
Shared docks open without fail.
Comments wait for a settings cue,
Style wakes when layers renew.
New docks keep their places right—
Hop, the sidebar shines tonight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 69.23% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: making the Comments panel opt-in through Settings.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/comments-panel-opt-in

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

🔍 Cloudflare PR preview

Item Value
Site https://4d205390.geolibre-preview.pages.dev
Demo app https://4d205390.geolibre-preview.pages.dev/demo/
Commit 870653f

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Makes the Comments right-side panel opt-in (so it no longer displaces other docked panels on startup), adds a Settings > Layout toggle for enabling it, and adjusts right-panel rail behavior so docked plugin panels remain reachable when they’re not the active panel.

Changes:

  • Stop auto-opening the Comments panel on mount; add Settings toggles (dropdown + Layout section) to open/close it as a collapsed rail entry.
  • Ensure plugin panels docked outside the shared rail render their own collapsed rail so they remain reachable when another panel becomes active elsewhere.
  • Re-expand the Style panel when the selected layer changes.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts Removes “open+collapse on mount” behavior for Comments; keeps it as a registered dockable panel.
apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx Adds Layout toggles to show/hide the Comments panel via the right-panel registry.
apps/geolibre-desktop/src/components/layout/DesktopShell.tsx Tweaks shared-rail initial expansion behavior so enabling Comments doesn’t bury Style.
apps/geolibre-desktop/src/components/panels/PluginRightPanel.tsx Adds a per-dock collapsed rail for non-shared docks, keeping docked plugin panels reachable.
apps/geolibre-desktop/src/components/panels/StylePanel.tsx Expands Style panel when the selected layer changes.
apps/geolibre-desktop/src/i18n/locales/en.json Adds translation for the new Layout toggle label.
apps/geolibre-desktop/src/i18n/locales/ar.json Adds translation for the new Layout toggle label.
apps/geolibre-desktop/src/i18n/locales/de.json Adds translation for the new Layout toggle label.
apps/geolibre-desktop/src/i18n/locales/es.json Adds translation for the new Layout toggle label.
apps/geolibre-desktop/src/i18n/locales/fr.json Adds translation for the new Layout toggle label.
apps/geolibre-desktop/src/i18n/locales/hi.json Adds translation for the new Layout toggle label.
apps/geolibre-desktop/src/i18n/locales/id.json Adds translation for the new Layout toggle label.
apps/geolibre-desktop/src/i18n/locales/it.json Adds translation for the new Layout toggle label.
apps/geolibre-desktop/src/i18n/locales/ja.json Adds translation for the new Layout toggle label.
apps/geolibre-desktop/src/i18n/locales/ka.json Adds translation for the new Layout toggle label.
apps/geolibre-desktop/src/i18n/locales/ko.json Adds translation for the new Layout toggle label.
apps/geolibre-desktop/src/i18n/locales/nl.json Adds translation for the new Layout toggle label.
apps/geolibre-desktop/src/i18n/locales/pt.json Adds translation for the new Layout toggle label.
apps/geolibre-desktop/src/i18n/locales/ru.json Adds translation for the new Layout toggle label.
apps/geolibre-desktop/src/i18n/locales/tr.json Adds translation for the new Layout toggle label.
apps/geolibre-desktop/src/i18n/locales/zh.json Adds translation for the new Layout toggle label.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

🔍 GitHub Pages PR preview

Item Value
Site https://opengeos.org/pages-preview/GeoLibre/pr-1638/
Demo app https://opengeos.org/pages-preview/GeoLibre/pr-1638/demo/
Commit 870653f

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts (1)

14-23: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Gate the Comments shared-rail entry behind the opt-in.

registerCommentsPanel registers COMMENTS_PANEL_ID with replace-style, so it becomes useReplaceStylePanelId() and mounts SharedSidebar. SharedSidebar then always renders that panel as a rail entry. Apply the showCommentsPanel opt-in before rendering the shared-rail block and before openRightPanel("comments"). Add fresh-load, enable, and disable tests.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts` around lines 14
- 23, Gate the Comments shared-rail entry on the showCommentsPanel opt-in so
SharedSidebar does not render it when disabled. Update the logic around
useRegisterCommentsPanel and the shared-rail rendering path to apply the same
condition before calling openRightPanel("comments"). Add tests covering fresh
load, enabling, and disabling the opt-in.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts`:
- Around line 14-23: Gate the Comments shared-rail entry on the
showCommentsPanel opt-in so SharedSidebar does not render it when disabled.
Update the logic around useRegisterCommentsPanel and the shared-rail rendering
path to apply the same condition before calling openRightPanel("comments"). Add
tests covering fresh load, enabling, and disabling the opt-in.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b23371c3-442e-4573-aa57-b3142966ff2c

📥 Commits

Reviewing files that changed from the base of the PR and between 4c8f424 and e67af60.

📒 Files selected for processing (21)
  • apps/geolibre-desktop/src/components/layout/DesktopShell.tsx
  • apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx
  • apps/geolibre-desktop/src/components/panels/PluginRightPanel.tsx
  • apps/geolibre-desktop/src/components/panels/StylePanel.tsx
  • apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts
  • apps/geolibre-desktop/src/i18n/locales/ar.json
  • apps/geolibre-desktop/src/i18n/locales/de.json
  • apps/geolibre-desktop/src/i18n/locales/en.json
  • apps/geolibre-desktop/src/i18n/locales/es.json
  • apps/geolibre-desktop/src/i18n/locales/fr.json
  • apps/geolibre-desktop/src/i18n/locales/hi.json
  • apps/geolibre-desktop/src/i18n/locales/id.json
  • apps/geolibre-desktop/src/i18n/locales/it.json
  • apps/geolibre-desktop/src/i18n/locales/ja.json
  • apps/geolibre-desktop/src/i18n/locales/ka.json
  • apps/geolibre-desktop/src/i18n/locales/ko.json
  • apps/geolibre-desktop/src/i18n/locales/nl.json
  • apps/geolibre-desktop/src/i18n/locales/pt.json
  • apps/geolibre-desktop/src/i18n/locales/ru.json
  • apps/geolibre-desktop/src/i18n/locales/tr.json
  • apps/geolibre-desktop/src/i18n/locales/zh.json

Comment thread apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx
Comment thread apps/geolibre-desktop/src/components/panels/StylePanel.tsx Outdated
Comment thread apps/geolibre-desktop/src/components/panels/PluginRightPanel.tsx Outdated
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

All four inline comments are buffered. Here is the final summary comment.

Code review

Bugs

  • Enabling "Show Comments panel" from Settings still collides with the Browser panel: openRightPanel uses a single global activeId (not per-dock), so opting into Comments (SettingsDialog.tsx) silently closes the on-by-default Browser panel, reproducing the exact displacement bug this PR's summary says it fixes — just moved from mount time to opt-in time. High confidence on the mechanism.
  • StylePanel.tsx's new "re-expand on layer selection" effect calls setIsCollapsed(false) unconditionally, without checking autoCollapse, so selecting a different layer while a notebook/story-map presentation is open (which sets autoCollapse specifically to keep Style collapsed) will force the panel open again, defeating that mechanism. Medium-high confidence.

Security

  • None found.

Performance

  • Minor: dockPanels/listRightPanels() is recomputed independently by each of the 4 PluginRightPanel instances on every render; cheap given current panel counts, not worth blocking on.

Quality

  • PluginRightPanel.tsx: the new if (collapsed) return panelRail; early return means collapsed is always false by the time the final JSX renders, making every collapsed ? … : … branch in that block (aria-label, className, the old "expand button" markup) dead code. High confidence.
  • PluginRightPanel.tsx's dockPanels filter compares a panel's raw (optional) registered dock field, which is never normalized to its "right-of-style" default until the panel is first activated — so a plugin registered without an explicit dock stays unreachable from any rail until opened once, undercutting the "stays reachable" goal. Medium confidence (not triggered by any currently bundled plugin, all of which set dock explicitly).
  • No new/updated tests accompany the opt-in Comments toggle, the StylePanel re-expand behavior, or the new plugin-rail rendering path.

CLAUDE.md

  • i18n coverage is complete and consistent: showCommentsPanel was added to all 16 locale files, en.json reads as the source of truth, and the new JSX uses logical Tailwind utilities (border-e/border-s, gap-*) rather than physical ones, matching the RTL guidance. No violations found.

Comment thread apps/geolibre-desktop/src/components/panels/PluginRightPanel.tsx
Comment thread apps/geolibre-desktop/src/components/layout/DesktopShell.tsx Outdated
Comment thread apps/geolibre-desktop/src/components/panels/PluginRightPanel.tsx
// 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}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that multiple panels can be simultaneously visible in the shared Style rail (Comments plus any of the many plugins now defaulting to replace-style), replaceStylePanelId — and therefore this key — changes on every click between two already-visible rail entries, not just on the rare merge/detach action the original comment describes. Each switch fully remounts SharedSidebar, including the wrapped <StylePanel> and SharedSidebar's own builtinOptedIn state.

That interacts badly with the new auto-expand effect in StylePanel.tsx (previousSelectedLayerId = useRef(selectedLayerId) + effect gating on a change from that ref): the ref reinitializes to the current selectedLayerId on every remount, so if a layer is already selected when the remount happens (the common case — a user had a layer selected, then merely clicked to a different panel in the shared rail), the effect sees no "change" and never expands Style. The panel lands collapsed even though a real layer is selected, defeating the "selecting a layer re-expands the Style panel" behavior this PR adds.

Confidence: medium — the remount mechanics and the ref's mount-time initialization are clear from the code; the perceived severity depends on how often users actually have two replace-style panels open at once (Comments + a Web Services plugin).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changing this one — the remount is deliberate and the outcome it produces is the intended layout. Clicking a second shared-rail entry makes that panel the expanded surface, so pluginExpanded is true and Style is collapsed regardless of builtinOptedIn; the reset ref only matters once the user collapses the plugin again, and at that point Style being on the rail is the documented "both collapsed" resting state rather than a defeated auto-expand. Selecting any layer after that still expands Style, because the selection is a real change against the ref. Re-selecting the same layer is a no-op by design (the effect fires on change, not on click). Happy to revisit if the collapse-on-switch reads wrong in use.

Comment thread packages/plugins/src/right-panel-registry.ts
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

All four inline comments posted. Final summary below.

Code review

Bugs

  • The Style-side fix that keeps positionally-docked panels (left-of-style/right-of-style) reachable alongside an active shared rail wasn't mirrored to the Layers side — left-of-layers/right-of-layers panels render nowhere while the Layers shared rail is active, which is the default state since the Browser panel opens on mount at replace-layers. (DesktopShell.tsx, medium-high confidence)
  • PluginRightPanel's new dockPanels/panelRail logic includes the currently active, expanded panel itself, so an actively-expanded panel at a positional dock renders twice: once as the full panel, once as a redundant highlighted icon button in a duplicate mini-rail appended right after it. (PluginRightPanel.tsx, medium-high confidence)
  • With multiple panels now able to share the Style rail simultaneously, switching between them via key={replaceStylePanelId} remounts SharedSidebar/StylePanel far more often than the rare merge/detach case the code originally assumed. Combined with StylePanel's new auto-expand effect (which only fires on a change from the ref's mount-time value), a remount while a layer is already selected leaves Style collapsed instead of auto-expanding. (DesktopShell.tsx / StylePanel.tsx, medium confidence)

Security

  • None found.

Performance

  • None found beyond minor, likely-negligible overhead from listRightPanels() being recomputed on every render of each dock-position component; not flagged given typical panel counts.

Quality

  • packages/plugins/src/types.ts and docs/plugin-api.md (two spots) still document right-of-style as the default dock, but this PR changes the actual default to replace-style; plugin authors relying on the documented default will get a different panel placement with no doc update. (high confidence)

CLAUDE.md

  • No violations of the documented conventions found in the changed files (i18n catalogs updated consistently across all shipped locales; no touched files fall under the special mirrored-constant rules).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
apps/geolibre-desktop/src/components/panels/StylePanel.tsx (1)

1013-1024: 🎯 Functional Correctness | 🟠 Major

Duplicate: prevent selection from overriding autoCollapse.

When autoCollapse is true, this effect still calls setIsCollapsed(false) after a new layer selection. The transition-only effect below does not re-collapse Style while the flag remains true. A notebook, story map, or docked replacement panel can therefore lose its collapsed Style rail.

Gate this expansion with !autoCollapse and add autoCollapse to the dependency list. Keep the manual expand control unchanged.

This repeats the supplied previous StylePanel review finding.

Proposed fix
   useEffect(() => {
     const previous = previousSelectedLayerId.current;
     previousSelectedLayerId.current = selectedLayerId;
     if (
+      !autoCollapse &&
       selectedLayerId &&
       selectedLayerId !== previous &&
       layers.some((candidate) => candidate.id === selectedLayerId)
     ) {
       setIsCollapsed(false);
     }
-  }, [layers, selectedLayerId, setIsCollapsed]);
+  }, [autoCollapse, layers, selectedLayerId, setIsCollapsed]);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/geolibre-desktop/src/components/panels/StylePanel.tsx` around lines 1013
- 1024, Update the selected-layer effect using previousSelectedLayerId to gate
setIsCollapsed(false) with !autoCollapse, and include autoCollapse in the effect
dependencies. Leave the manual expand control unchanged.
apps/geolibre-desktop/src/components/panels/SharedSidebar.tsx (1)

116-140: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fix collapsePlugin/expandBuiltin: call collapseRightPanel with the currently active id.

collapseRightPanel(id) only collapses when activeId === id and the panel is expanded. Since panelIds can include additionalPanelIds, both rail toggles call collapseRightPanel(pluginId) while the active shared-rail panel may have a different id. That makes the active panel fail to collapse and keeps the built-in panel from being able to expand.

Use activeId/the entry id for both collapsePlugin and expandBuiltin.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/geolibre-desktop/src/components/panels/SharedSidebar.tsx` around lines
116 - 140, Update collapsePlugin and expandBuiltin to pass the currently active
panel id to collapseRightPanel instead of the captured pluginId. Ensure both
toggles collapse the active shared-rail entry, including ids from
additionalPanelIds, while preserving the existing expansion behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/geolibre-desktop/src/components/layout/DesktopShell.tsx`:
- Around line 2323-2383: Hoist the shared `PluginRightPanel` sections for
`dock="left-of-style"` and `dock="right-of-style"` out of the
`replaceStylePanelId` conditional, preserving their existing
`SectionErrorBoundary` wrappers and props. Keep only the middle content
conditional: `SharedSidebar` when `replaceStylePanelId` is active and the
standalone `StylePanel` otherwise, without changing either branch’s behavior.

In `@packages/plugins/src/right-panel-registry.ts`:
- Line 56: Update the documentation for GeoLibreRightPanelRegistration,
RightPanelSnapshot.dock, and the new-panel activation comment to match the
implementation: describe "replace-style" as the default dock, document that
later activations restore values from panelDocks, and state that activation
prefers panelDocks.get(id) before the panel’s declared dock.

In `@tests/right-panel-registry.test.ts`:
- Around line 109-136: Extend the existing tests to cover cleanup and
active-dock restoration: after closeRightPanel("browser") in the first test,
assert that panelDocks no longer contains "browser"; after
openRightPanel("comments") in the second test, assert getActiveRightPanelDock()
returns "replace-style".

---

Outside diff comments:
In `@apps/geolibre-desktop/src/components/panels/SharedSidebar.tsx`:
- Around line 116-140: Update collapsePlugin and expandBuiltin to pass the
currently active panel id to collapseRightPanel instead of the captured
pluginId. Ensure both toggles collapse the active shared-rail entry, including
ids from additionalPanelIds, while preserving the existing expansion behavior.

In `@apps/geolibre-desktop/src/components/panels/StylePanel.tsx`:
- Around line 1013-1024: Update the selected-layer effect using
previousSelectedLayerId to gate setIsCollapsed(false) with !autoCollapse, and
include autoCollapse in the effect dependencies. Leave the manual expand control
unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6fc818e5-9d7e-4b04-81b5-177de03cb57b

📥 Commits

Reviewing files that changed from the base of the PR and between e67af60 and 453e726.

📒 Files selected for processing (20)
  • apps/geolibre-desktop/src/components/layout/DesktopShell.tsx
  • apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx
  • apps/geolibre-desktop/src/components/panels/PluginRightPanel.tsx
  • apps/geolibre-desktop/src/components/panels/SharedSidebar.tsx
  • apps/geolibre-desktop/src/components/panels/StylePanel.tsx
  • apps/geolibre-desktop/src/hooks/useRightPanels.ts
  • packages/plugins/src/index.ts
  • packages/plugins/src/plugins/maplibre-annotations.ts
  • packages/plugins/src/plugins/maplibre-arcgis-hub.ts
  • packages/plugins/src/plugins/maplibre-earthdata-gis.ts
  • packages/plugins/src/plugins/maplibre-geolens.ts
  • packages/plugins/src/plugins/maplibre-graticule.ts
  • packages/plugins/src/plugins/maplibre-h3.ts
  • packages/plugins/src/plugins/maplibre-huggingface.ts
  • packages/plugins/src/plugins/maplibre-open-data-catalogs.ts
  • packages/plugins/src/plugins/maplibre-openaerialmap.ts
  • packages/plugins/src/plugins/maplibre-source-coop.ts
  • packages/plugins/src/plugins/maplibre-stac.ts
  • packages/plugins/src/right-panel-registry.ts
  • tests/right-panel-registry.test.ts

Comment thread apps/geolibre-desktop/src/components/layout/DesktopShell.tsx Outdated
Comment thread packages/plugins/src/right-panel-registry.ts
Comment thread tests/right-panel-registry.test.ts
- Register the Comments panel with an i18n title getter (`comments.title`,
  added to every locale) so its rail entry and ARIA labels localize, matching
  the Browser panel.
- Skip the Style panel's select-a-layer expand while `autoCollapse` is on, so a
  selection cannot pop Style open over the notebook or a story-map
  presentation; the shared-rail Style panel now receives the same trigger.
- Drop the now-unreachable collapsed branches in PluginRightPanel: the early
  return for a collapsed panel means everything below it renders expanded.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts`:
- Line 3: Update the Comments rail title flow in useRegisterCommentsPanel to use
the t() function from react-i18next for comments.title instead of the direct
i18n import. Preserve the dynamic registry getter by passing the current t
reference into the title callback, or re-register the entry whenever t changes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 77cdc17d-7e89-425e-a7b9-3d68bc7783f9

📥 Commits

Reviewing files that changed from the base of the PR and between 453e726 and 2ab7b1c.

📒 Files selected for processing (20)
  • apps/geolibre-desktop/src/components/layout/DesktopShell.tsx
  • apps/geolibre-desktop/src/components/panels/PluginRightPanel.tsx
  • apps/geolibre-desktop/src/components/panels/StylePanel.tsx
  • apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts
  • apps/geolibre-desktop/src/i18n/locales/ar.json
  • apps/geolibre-desktop/src/i18n/locales/de.json
  • apps/geolibre-desktop/src/i18n/locales/en.json
  • apps/geolibre-desktop/src/i18n/locales/es.json
  • apps/geolibre-desktop/src/i18n/locales/fr.json
  • apps/geolibre-desktop/src/i18n/locales/hi.json
  • apps/geolibre-desktop/src/i18n/locales/id.json
  • apps/geolibre-desktop/src/i18n/locales/it.json
  • apps/geolibre-desktop/src/i18n/locales/ja.json
  • apps/geolibre-desktop/src/i18n/locales/ka.json
  • apps/geolibre-desktop/src/i18n/locales/ko.json
  • apps/geolibre-desktop/src/i18n/locales/nl.json
  • apps/geolibre-desktop/src/i18n/locales/pt.json
  • apps/geolibre-desktop/src/i18n/locales/ru.json
  • apps/geolibre-desktop/src/i18n/locales/tr.json
  • apps/geolibre-desktop/src/i18n/locales/zh.json

Comment thread apps/geolibre-desktop/src/hooks/useRegisterCommentsPanel.ts
Comment thread apps/geolibre-desktop/src/components/panels/PluginRightPanel.tsx
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Code review

Bugs

  • apps/geolibre-desktop/src/components/panels/PluginRightPanel.tsx:98-102dockPanels includes the currently active and expanded panel at a positional dock (not just displaced/collapsed ones), so expanding any panel there (e.g. via the "Detach" button) renders a redundant duplicate rail entry for itself right alongside the full panel, mislabeled by aria as "(collapsed)" even though it's expanded. High confidence — reproduces via the standard Detach control.
  • apps/geolibre-desktop/src/components/layout/DesktopShell.tsx:2085-2132 — the Layers-side shared-rail branch (replaceLayersPanelId) was not given the same fix as the Style-side branch: it never renders PluginRightPanel dock="left-of-layers"/"right-of-layers". Since the Browser panel is visible by default (replace-layers), any plugin docked left-of-layers/right-of-layers would be completely unreachable whenever Browser is enabled. High confidence on the asymmetry; currently would only affect a third-party plugin since no first-party plugin uses those docks anymore.

Security

  • None found.

Performance

  • None significant found. The new dockPanels/panelIds computations run listRightPanels() on every render of each dock instance, but panel counts are small and this is UI-render-frequency work, not a real concern.

Quality

  • apps/geolibre-desktop/src/components/panels/StylePanel.tsx:1037-1039 — stale comment claims controlled mode "never passes autoCollapse", but DesktopShell.tsx now does pass it there for a different effect. Low confidence, doc-only.
  • The PR migrates the DEFAULT_DOCK and 10 first-party plugins (annotations, ArcGIS Hub, Earthdata GIS, GeoLens, Graticule, H3, Hugging Face, Open Data Catalogs, OpenAerialMap, Source Cooperative, STAC) from "right-of-style" (shown beside Style) to "replace-style" (mutually exclusive with Style). This is a substantial, repo-wide behavior change not mentioned in the PR title/description ("make the Comments panel opt-in"), and it's the underlying reason the new visibleIds/panelDocks tracking machinery had to be built. Worth calling out explicitly in the PR description so reviewers/users aren't surprised that plugins which used to coexist with Style now hide it.
  • No component-level tests were added for PluginRightPanel's new dockPanels rail logic or for the Layers-side branch in DesktopShell, which is where both bugs above live — a test exercising "expand a positional-dock panel" or "a plugin docked left/right-of-layers while Browser is visible" would have caught them.

CLAUDE.md

  • New user-facing strings (settings.layout.showCommentsPanel, comments.title) are added via t() and mirrored across all locale files including en.json as source of truth — consistent with the i18n convention.
  • No adherence issues found otherwise (no tile/host allowlist, WASM-catalog, or lockfile changes are implicated by this diff).

- Render the left/right-of-layers plugin docks alongside the shared Layers
  rail. Browser opens there on nearly every load, so a panel moved to a
  layers-side position previously rendered nowhere at all.
- Hoist both style-side positional docks out of the shared-rail ternary
  (and the layers-side ones likewise) so the two branches cannot diverge.
- Drop the rail beside an expanded positional panel when it would list only
  that panel, which just repeated the header it already renders.
- Correct the documented default dock (`replace-style`, not `right-of-style`)
  in the plugin types and docs/plugin-api.md, and the registry comments on
  restoring a panel's remembered dock.
- Assert the dock cleanup on close and the dock restore on re-activation.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/plugins/src/right-panel-registry.ts (1)

80-83: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Model panelDocks as a sparse map.

panelDocks is emitted from an in-memory map and deleted when panels close or un-register, but Readonly<Record<string, RightPanelDock>> promises a dock for every string key. Consumers reading arbitrary snapshot entries can get undefined.

Use Readonly<Partial<Record<string, RightPanelDock>>>. Update consumers that read arbitrary IDs to handle undefined.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/plugins/src/right-panel-registry.ts` around lines 80 - 83, Change
panelDocks in the right-panel state model to Readonly<Partial<Record<string,
RightPanelDock>>> so absent or unregistered panel IDs are represented safely.
Update consumers of panelDocks, especially arbitrary-ID lookups, to handle
undefined while preserving existing behavior for entries that have a dock.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/geolibre-desktop/src/components/layout/DesktopShell.tsx`:
- Around line 2105-2117: Update the shared left and right Sidebar render
conditions in DesktopShell to use replaceLayersPanelIds and
replaceStylePanelIds, keeping them mounted whenever visible shared-rail panels
exist rather than only when the active ID hook is non-null. Pass the
corresponding active ID to pluginId when available, while preserving the
existing shared-rail behavior and panel collections.

---

Outside diff comments:
In `@packages/plugins/src/right-panel-registry.ts`:
- Around line 80-83: Change panelDocks in the right-panel state model to
Readonly<Partial<Record<string, RightPanelDock>>> so absent or unregistered
panel IDs are represented safely. Update consumers of panelDocks, especially
arbitrary-ID lookups, to handle undefined while preserving existing behavior for
entries that have a dock.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c9636ddc-d2c4-4d08-82e0-a367750ad081

📥 Commits

Reviewing files that changed from the base of the PR and between 2ab7b1c and ef78f30.

📒 Files selected for processing (6)
  • apps/geolibre-desktop/src/components/layout/DesktopShell.tsx
  • apps/geolibre-desktop/src/components/panels/PluginRightPanel.tsx
  • docs/plugin-api.md
  • packages/plugins/src/right-panel-registry.ts
  • packages/plugins/src/types.ts
  • tests/right-panel-registry.test.ts

Comment thread apps/geolibre-desktop/src/components/layout/DesktopShell.tsx
Exclude the expanded panel from a positional dock's rail rather than
suppressing the whole rail when it holds only that panel. The rail is
announced as a collapsed-panel strip, so listing an expanded panel both
duplicated it and mislabeled it; entries now use the shared rail's
"expand <panel>" wording.
Comment on lines +1013 to +1029
// Selecting a real layer expands the panel from its rail. Skipped while
// `autoCollapse` holds it closed (the notebook or a story-map presentation
// owns the workspace), so a selection made there cannot pop Style back open
// over them and defeat the auto-collapse below.
const previousSelectedLayerId = useRef(selectedLayerId);
useEffect(() => {
const previous = previousSelectedLayerId.current;
previousSelectedLayerId.current = selectedLayerId;
if (
!autoCollapse &&
selectedLayerId &&
selectedLayerId !== previous &&
layers.some((candidate) => candidate.id === selectedLayerId)
) {
setIsCollapsed(false);
}
}, [autoCollapse, layers, selectedLayerId, setIsCollapsed]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: this new effect is correctly gated by !autoCollapse (and, in controlled mode, effectively no-ops when DesktopShell forces autoCollapse while the notebook/story-map is open). But the pre-existing comment a few lines below (around "Skipped entirely in controlled mode, where the parent (shared rail) owns collapse and never passes autoCollapse") is now stale — DesktopShell does pass autoCollapse to the controlled StylePanel now (that's what this new effect relies on). Worth a follow-up tweak to that comment so it doesn't mislead future readers; not a functional issue.

Confidence: high, severity: low (doc-only).

Comment on lines +314 to +317
/** Whether a registered panel is enabled in a dock rail. */
export function isRightPanelVisible(id: string): boolean {
return visibleIds.has(id);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isRightPanelVisible is exported (and re-exported from packages/plugins/src/index.ts) but isn't consumed anywhere in this PR — SettingsDialog.tsx computes the same thing itself via rightPanelState.visibleIds.includes(BROWSER_PANEL_ID) / .includes(COMMENTS_PANEL_ID) instead of calling this helper. Not a bug, but consider either using this accessor from SettingsDialog (single source of truth) or dropping it if it's meant purely as future external-plugin API surface — as-is it's untested and unused.

Confidence: medium, severity: low (code quality / duplication).

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Code review

I did a deep pass on the core logic change — the right-panel registry's new visibleIds/panelDocks tracking (packages/plugins/src/right-panel-registry.ts), its consumers (SharedSidebar.tsx, PluginRightPanel.tsx, DesktopShell.tsx, useRightPanels.ts), the new Comments opt-in flow (SettingsDialog.tsx, useRegisterCommentsPanel.ts), and the new Style-panel auto-expand-on-select effect. I traced the invariants the registry relies on (e.g. that panelDocks[activeId] === activeDock always holds, and that SharedSidebar's pluginId prop always equals whichever entry is actually active) and didn't find a way to break them — the registry changes appear correct and are matched by good unit test coverage in tests/right-panel-registry.test.ts. The migration of all 11 built-in right-panel plugins from right-of-style to replace-style is complete and consistent (no stragglers). i18n coverage is complete across all 16 shipped locales for both new keys, and RTL/logical-class conventions are respected.

Bugs: None found with reasonable confidence. Several plausible-looking races (e.g. SharedSidebar.collapsePlugin hard-coding pluginId instead of the clicked entry's id, or the new layer-selection effect forcibly collapsing an active Comments/plugin panel on the shared Style rail) turn out to be either provably safe (given the panelDocks[activeId] === activeDock invariant maintained by the registry) or explicitly intended per the PR's own test-plan checklist ("Clicking a layer expands the Style panel when it was collapsed").

Security: None found — no new user input handling, injection surface, or secret handling in this diff.

Performance: None found — the added .filter()/.some() calls in DesktopShell and StylePanel are small and properly guarded.

Quality:

  • apps/geolibre-desktop/src/components/panels/StylePanel.tsx:1013-1029 (medium confidence, low severity): the new auto-expand effect is correct, but the pre-existing comment just below it ("Skipped entirely in controlled mode, where the parent (shared rail) owns collapse and never passes autoCollapse") is now stale — DesktopShell does pass autoCollapse to the controlled StylePanel today. Doc-only, worth a follow-up tweak.
  • packages/plugins/src/right-panel-registry.ts:314-317 (medium confidence, low severity): the new isRightPanelVisible export is unused — SettingsDialog.tsx duplicates the same check inline via visibleIds.includes(...) instead of calling it. Consider consolidating or dropping it if it's purely forward-looking plugin API surface.

CLAUDE.md: No violations found — i18n keys added via t()/react-i18next across all locales with en.json as source of truth, RTL logical Tailwind utilities used consistently, no new external tile/map hosts requiring CSP updates, and no touched files that need the whitebox-menu-catalog/MAX_VECTOR_PMTILES_ZOOM/MAX_VECTOR_BYTES/MAP_PANEL_SELECTOR mirror updates.

@giswqs
giswqs merged commit 9c86aff into main Aug 2, 2026
30 checks passed
@giswqs
giswqs deleted the feat/comments-panel-opt-in branch August 2, 2026 03:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants