feat: open the Style panel only when explicitly requested - #1775
Conversation
Selecting a layer no longer pops the Style panel open over the map, which made the workspace jump around during ordinary layer picking. The panel now expands only from the new "Open Style panel" item in a layer's menu.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughLayer actions now select a layer and explicitly request Style panel opening. DesktopShell passes request counters to StylePanel instances. StylePanel no longer expands automatically on layer selection. Localization and tutorial text describe the updated interaction. ChangesStyle panel opening flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant LayerPanel
participant DesktopShell
participant StylePanel
LayerPanel->>LayerPanel: select layer
LayerPanel->>DesktopShell: invoke onOpenStylePanel
DesktopShell->>DesktopShell: increment stylePanelOpenRequest
DesktopShell->>StylePanel: pass openRequest
StylePanel->>StylePanel: expand on openRequest change
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. 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. Comment |
🔍 Cloudflare PR preview
|
🔍 GitHub Pages PR preview
Note GitHub Pages built this preview successfully, but its serving edge returned HTTP 403 when checked. The links may still be propagating. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Code reviewBugs
Security
Performance
Quality
CLAUDE.md
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/components/panels/LayerPanel.tsx`:
- Around line 3370-3378: Gate the style-panel DropdownMenuItem in LayerPanel
behind the current StylePanel visibility state used by DesktopShell, so it is
hidden or disabled when stylePanelVisible is false. Update the item around
selectLayer and onOpenStylePanel, preserving normal selection and opening
behavior when the panel is mounted.
🪄 Autofix
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: d9bed045-aa34-46a6-8bf3-e92f47dbf568
📒 Files selected for processing (5)
apps/geolibre-desktop/src/components/layout/DesktopShell.tsxapps/geolibre-desktop/src/components/panels/LayerPanel.tsxapps/geolibre-desktop/src/components/panels/StylePanel.tsxapps/geolibre-desktop/src/i18n/locales/en.jsondocs/tutorials/first-map.md
- Gate the Style panel's open-request effect on `autoCollapse`, so a request made while the notebook or a story-map presentation owns the workspace can no longer force the panel open over them. The `autoCollapse` effect acts only on transitions, so such an expand would have stuck until the notebook was closed and reopened. - Hide the "Open Style panel" menu item when the Style panel is hidden (Settings → "Show Style panel"). The panel is not mounted then, so the request was dropped rather than queued and the item did nothing.
| onOpenStylePanel(); | ||
| }} | ||
| > | ||
| <Palette className="me-2 h-3.5 w-3.5" /> |
There was a problem hiding this comment.
Minor icon-consistency nit: Palette is already used a few lines below (and in this same layer's menu) for "Open Style Manager" (line ~3757). Elsewhere in the app the Style panel itself is always represented by SlidersHorizontal (the shared-rail tab in DesktopShell.tsx, and the collapsed-rail icon in StylePanel.tsx). Reusing Palette here makes two different actions in the same context menu look identical, and misses the icon users already associate with the Style panel.
| <Palette className="me-2 h-3.5 w-3.5" /> | |
| <SlidersHorizontal className="me-2 h-3.5 w-3.5" /> |
(SlidersHorizontal would need to be added to the lucide-react import list in this file.)
Confidence: medium — this is a UX/consistency nit, not a functional bug.
| // An explicit request (Layers → "Open Style panel") expands the panel from its | ||
| // rail. Skipped while `autoCollapse` holds it closed (the notebook or a | ||
| // story-map presentation owns the workspace), so a request made there cannot | ||
| // pop Style back open over them: the `autoCollapse` effect below acts only on | ||
| // transitions, so an expand that slipped through would stick until the | ||
| // notebook was closed and reopened. The request is still consumed so it does | ||
| // not fire later. | ||
| const previousOpenRequest = useRef(openRequest); | ||
| 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]); | ||
| if (openRequest === previousOpenRequest.current) return; | ||
| previousOpenRequest.current = openRequest; | ||
| if (!autoCollapse) setIsCollapsed(false); | ||
| }, [autoCollapse, openRequest, setIsCollapsed]); |
There was a problem hiding this comment.
The "Open Style panel" menu item in LayerPanel is only hidden/disabled based on layoutOptions.stylePanelVisible — it stays enabled while autoCollapse is true for another reason (notebook open, story-map presenting, panelsCollapsed, etc., depending on the render mode). In that state this effect still consumes the request (previousOpenRequest.current = openRequest) but skips the setIsCollapsed(false), so clicking the menu item does nothing visible and gives no feedback that the request was dropped.
This mirrors the previous auto-expand-on-select behavior, so it may be an accepted tradeoff, but now that opening the panel is an explicit user action (vs. an incidental side effect of selecting a layer), a silent no-op is more likely to read as a bug to users. Worth considering whether the menu item should be disabled/hidden while autoCollapse would swallow the request, or whether the effect should defer the expand until autoCollapse clears instead of dropping it.
Confidence: low-medium — plausible UX gap rather than a clear defect, since the surrounding comments suggest this drop is intentional.
Code reviewBugs: None found. The refactor from selection-based auto-expand to an explicit Security: None found. No new user input handling, injection surface, or secrets involved. Performance: None found. The change replaces a Quality:
CLAUDE.md: No violations found — the new string uses |
Summary
openRequestprop wired fromDesktopShell.Test plan
pre-commit run --files <changed paths>(includes the full npm build, eslint, oxfmt)npm run test:frontendSummary by CodeRabbit
New Features
Bug Fixes
Documentation