Restore plugin pane chrome and the Docs plugin sidebar - #1036
Conversation
Four regressions landed together and left plugin panes with broken chrome. Three came from #991, one predates it. Docs sidebar. #991 restyled the sidebar to bg-sidebar and painted a fake background up into the app header. #1019 then gave every header a seam, which that background cut short, so the plugin's own panel read as the host's shared sidebar. The sidebar sits below the header again and the seam runs edge to edge. Docs header toggle. #991 also deleted the shared sidebar store when its mounts hit zero. The header and the sidebar live in separate React subtrees and find each other only through that map, so a deleted entry left each on its own store: two toggles, and the header one did nothing. Entries now live for the session; a reopened pane still starts clean because the view resets state on its first mount. Plugin pane controls. #991 stripped full screen and move-to-side from plugin panes, and dropped their reserve for the host's pinned panel toggle. That toggle then landed on Close pane whenever a plugin pane held the workspace corner. Position alone decides the reserve now, and plugin panes carry the same pane controls as any other pane. Empty right panel. #962 disabled the panel toggle for panes that publish no panel and deleted the placeholder; #991 removed the control outright. Both are back from #736: the toggle always works, and a pane without a panel shows "This pane has no right panel." The corner button stays visible while that empty state is open, since it has no chrome of its own to close from. Also: the reserve no longer trails an empty slot when the host hides its toggle (a full-screen pane), and the corner button moved to right-4 so it shares the pane header's action axis. Tests: 2364 pass in @bb/app; 59 in bb-plugin-simple-notes. Typecheck clean, lint reports no errors. Verified in the dev app: header seam, one working Docs toggle, plugin full screen and restore, no button collision, the empty-state panel, and even header spacing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🚨 SLOP COP 🚨 · I am the SlopCop. I am reviewing this pull request for security, code quality, performance, architecture, and end-to-end behavior. The stack contains PR #1036 only. I will post a comment after each review phase. |
|
🚨 SLOP COP 🚨 · Security phase complete. I found no security issues. The new controls use host-owned pane IDs. The empty panel adds no untrusted HTML, URL, or filesystem input. I also checked plugin trust boundaries, composer scope, store keys, and denial-of-service risks. |
|
🚨 SLOP COP 🚨 · Performance phase complete. I found one low-severity issue. The map can grow across many Docs vaults. A bounded cache or safe delayed cleanup would prevent this long-session heap growth. I found no other performance issue. All GitHub checks passed. |
|
🚨 SLOP COP 🚨 · Code quality and architecture phase complete. I found one blocking lifecycle defect. An empty panel starts at zero size for a focused plugin pane. The panel can call A later thread pane then closes its persisted-open panel. The current panel mock does not reproduce this library callback. The code should ignore collapse until the empty panel first reports a positive size. A regression test should cover the plugin-first path. |
|
🚨 SLOP COP 🚨 · End-to-end phase complete on the top PR. I installed the Docs plugin in an isolated development instance. I opened Docs beside a new-thread pane. Both panes showed Full Screen and Close pane controls. The Docs sidebar expanded to 288 pixels and showed its action toolbar. The plugin pane entered and left full-screen mode. The host toggle opened and closed the empty right-panel state. The visual check showed no control overlap. This path passed. |
| defaultSize={isOpen ? panelWidthPercent : 0} | ||
| minSize={THREAD_SECONDARY_PANEL_MIN_SIZE_PERCENT} | ||
| maxSize={THREAD_SECONDARY_PANEL_MAX_SIZE_PERCENT} | ||
| onCollapse={handleEmptyPanelCollapse} |
There was a problem hiding this comment.
🚨 slopcop/review — The initial collapse overwrites the visibility sentinel.
This panel starts at zero size when a plugin pane has focus. React Resizable Panels calls onCollapse for that initial size.
The callback changes isPanelVisible from null to false. A later persisted-open thread then closes instead of adopting its saved state.
Ignore collapse until this panel first reports a positive size. Add a test that calls onCollapse for the initial zero size.
| // deleted entry lets one subtree keep the old store while the other creates a | ||
| // new one, which silently breaks the header toggle. A reopened pane starts | ||
| // clean anyway, because the view resets the state on its first mount. | ||
| const notesSidebarStores = new Map<string, NotesSidebarStore>(); |
There was a problem hiding this comment.
🚨 slopcop/review — This map keeps inactive stores for the full application session.
Each distinct pane and vault key adds a state object and listener set. Unmounting both React subtrees no longer removes the entry.
Use safe delayed cleanup or a bounded cache. Cancel cleanup when either subtree mounts again.
There was a problem hiding this comment.
Thanks — I looked at this one and am leaving it as is. Deleting entries is what broke the toggle in the first place, so I would rather pay a small bound than reintroduce that failure mode.
The key is ${paneScope}:${vaultKey}. Pane scopes come from live split-pane IDs, capped at 8 panes, and vault keys come from the user's own vault list. So the map tops out at a few dozen entries per session, each a small object plus an empty listener set. It grows only when the user opens new panes or vaults, not on a loop.
Delayed cleanup with cancellation would restore the race in a subtler form: the header and the sidebar mount in separate React subtrees with no ordering guarantee, so any window where the entry is gone can hand the two subtrees different store objects again. That is exactly the desync this PR fixes — a dead header toggle and a duplicate control, with no error to point at it.
If the map ever needs bounding, the safe shape is a cap on total entries with eviction of keys whose panes no longer exist in the split layout, checked against the live layout rather than against mount counts. That is worth doing when there is evidence of real growth; today it would add machinery and risk for a few dozen small objects.
| const toggleLabel = isOpen ? "Hide right panel" : "Show right panel"; | ||
| // An open pane panel carries the toggle in its own chrome, and a full-screen | ||
| // pane hides it. The empty state has no chrome, so it keeps the corner. | ||
| const pinsCornerToggle = !isPaneMaximized && !(isOpen && model !== null); |
There was a problem hiding this comment.
🚨 slopcop/review — An open empty panel leaves a blank header action slot.
The empty panel contains the visible toggle. The separate right-edge plugin pane still reserves 28 pixels for that toggle.
Reserve this slot only when the closed panel places the toggle over the main pane. Add an open-empty-panel test.
SawyerHood
left a comment
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Review complete for the one-PR stack: #1036.
I found one blocking defect and two low-severity defects.
- The initial empty-panel collapse clears the null visibility sentinel.
- The Docs sidebar store map grows for the full application session.
- An open empty panel leaves a blank 28-pixel plugin header slot.
The first defect can close a persisted-open thread panel after a plugin pane first gains focus.
The fix should ignore empty-panel collapse until that panel first reports a positive size. A regression test must simulate the initial collapse callback.
I found no security issue.
The empty panel repeats collapse and resize behavior from ThreadSecondaryPanel. A shared lifecycle helper could prevent more behavior differences.
Local validation passed:
- The app test passed 38 tests.
- The Docs test passed 24 tests.
- Both package typechecks passed.
- All GitHub checks passed.
- The browser test passed the Docs sidebar, split chrome, full-screen, and empty-panel flows.
The GPT-5.6 final gate returned REQUEST CHANGES. I posted a comment-only review and did not use GitHub approval controls.
Initial collapse. The empty panel mounts at zero width for a focused plugin pane, and react-resizable-panels reports that first layout as a collapse. Honoring it turned the "adopt the first publisher's state" sentinel into a hard closed, so the next thread lost its persisted-open panel. A collapse now counts only after the panel has reported a real width. Corner reserve. An open empty panel holds the toggle over itself, but the right-edge pane header still reserved 28px for it, which left the blank slot this branch set out to remove. The reserve now follows whether the button lands on a pane header, which an open panel of either kind ends. The panel mock records each panel's lifecycle callbacks, so a test can fire the initial collapse the real library fires on its own. Both new tests fail against the previous code. Tests: 2366 pass in @bb/app. Typecheck clean, lint reports no errors. Verified in the dev app: closed keeps the reserve, open ends the header at Close pane with the toggle over the panel. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Summary
Four regressions left plugin panes with broken chrome. Three came from #991; one predates it. Each was reproduced in the dev app before the fix.
Docs sidebar looked like the host's shared sidebar. #991 restyled it to
bg-sidebarand painted a fake background up into the app header. #1019 then gave every header its seam, which that background cut short. The sidebar sits below the header again, with the seam edge to edge and the plugin's ownbg-muted/20tint.The Docs header toggle did nothing. #991 deleted the shared sidebar store once its mounts hit zero. The header and the sidebar live in separate React subtrees and find each other only through that module-level map, so a deleted entry left each subtree on a different store object: two toggles rendered, and the header one was dead (measured 288px to 288px on click). Entries now live for the session. A reopened pane still starts clean, because the view resets the state on its first mount.
Plugin panes lost full screen, move-to-side, and their corner reserve. #991 excluded
plugin-panelcontent from all three. Without the reserve, the host's pinned panel toggle landed on Close pane whenever a plugin pane held the workspace corner — measured overlap of 22 of 28px, with the toggle winning the click. Position alone decides the reserve now, and plugin panes carry the same pane controls as every other pane.A pane with no right panel lost its toggle and placeholder. #736 shipped both. #962 disabled the button and deleted the placeholder; #991 removed the control outright. Both are restored: the toggle always works (including
Ctrl+J), and a pane that publishes no panel showsThis pane has no right panel.in a resizable panel whose width persists. Since that empty state has no chrome of its own, the corner button stays visible while it is open — otherwise no button could close it. The host exposes this aspinsCornerToggle, and pane headers reserve the corner from the same flag.Two smaller fixes in the same area:
right-2.5toright-4, so it shares the pane header'spx-4action axis. Measured after: Full Screen 1652–1680, Close pane 1684–1712, toggle 1716–1744 — an even 4px rhythm, with the toggle landing exactly in the reserved slot.Tests
turbo run test --filter=@bb/app: 2364 pass (315 files).turbo run test --filter=bb-plugin-simple-notes: 59 pass.turbo run typecheckpasses for@bb/appandbb-plugin-simple-notes. Lint reports 0 errors.QA
Checked in the branch dev app with the Docs, Tasks, and GitHub plugins installed: the header seam, a single working Docs toggle, plugin full screen and restore, no button collision, the empty-state panel opening and collapsing to 0px, and even header spacing.
Risks
Restoring move-to-side for plugin panes was not requested directly. It came out in the same #991 block as full screen, so I put it back with the rest. Say the word if it should stay off.
🤖 Generated with Claude Code