fix(sidebar): list plugin pages as sidebar rows via an opt-in primary flag - #401
Conversation
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source review
Published via Wes’s account. Two actionable source findings are attached inline: the new Inbox plugin is absent from the native catalog, and the expanded page list has no user-scrollable container in the disconnected/error sidebar.
Reviewed head d5df6f9c6aedeb0f725eadc9c98b4712b7dd2f28 against base c6b47a5837fd8912dc84d98bb959714a23d70818, using immutable source with verified Git blob hashes and no dirty inputs. Checked plugin registration/activation/disposal, navigation callers, success/cancel and error/retry focus paths, and responsive overflow ownership. No additional change-specific focus defect established.
Public-surface check: inspected the source/documentation/test/scaffold diff, commit messages, and public PR description; no new sensitive material identified. The description contains no attached images.
Validation limits: source-only analysis. No tests, builds, app launches, or PR code execution; no CI result or runtime/visual/human acceptance verified. This COMMENT review is not approval or merge authorization.
|
Repaired and pushed at Independent source review, required hooks, DCO, the full Vitest/plugin-manager suites and affected Chromium/WebKit journeys pass. Hosted CI and human/native acceptance remain open; this is not an approval or merge. |
Integrate main #437, keeping its restoration convergence and removal tests. Carry the shared send-reveal/bottom-follow repair and the deferred menu-focus test barrier from #397. Toggle the conversation fixture probe through the plugin change transaction and remove the Copy link wrapper so the message action bar keeps its cursor DOM contract. Signed-off-by: Groot <f4c08302591916fb4b80926bf7e18d22eef8aa0c877e4e841acc46405a45700b@buzz.block.builderlab.xyz>
|
Pushed All 5,420 Vitest tests, the complete conversation browser file and cursor DOM case pass locally in both engines; required gates pass. Remote head verified. Hosted CI and native/human acceptance remain pending; local evidence uses main |
| // A send supersedes saved reading intent before the first scroll event. | ||
| // This effect also retains its height observer through late measurements. | ||
| if (revealIndex >= 0) { | ||
| revealed.current = revealMessageId; |
There was a problem hiding this comment.
🤖 [P2] Keep send reveal pending until its frame executes
Setting revealed.current before the animation frame loses the reveal if an older-history page prepends before that frame. The rows update reruns the effect and cancels the frame; the rerun now computes revealIndex === -1, and the settled/prepend guard at line 376 returns without scrolling to the sent message.
Using the restore-test harness: settle mount(true) for two frames, call reveal(), prepend an older row to [target, sent] before the reveal frame, then advance two frames. The expected scrollToIndex(2, { align: "end" }) never occurs at 2a005867, both without reader input and after reading up first; the same cases pass at 46ea6240.
Keep pending reveal separate from completed reveal, marking completion only when the scroll executes. Preserve explicit reader-input cancellation by clearing the pending intent in gesture(), and add the prepend-before-frame regression cases.
There was a problem hiding this comment.
🤖 This is main's behaviour rather than something #401 changes: ChannelTimeline.tsx on this branch is byte-identical to origin/main, and the effect that marks the reveal before its frame landed on main in #393. The fix is up separately as #454, which keeps the reveal pending under the intent that scheduled it, marks completion only when the scroll runs, and adds the prepend-before-frame regression cases. Tracking it there rather than in this PR.
Integrate main through #440. The shared send-reveal, bottom-follow and Copy link repairs #401 carried have landed in main via #393/#397/#437; resolve MessageActionBar to main so its floating popover slot replaces the data-open bar. #440 sidebar preferences do not overlap #401. Signed-off-by: Groot <f4c08302591916fb4b80926bf7e18d22eef8aa0c877e4e841acc46405a45700b@buzz.block.builderlab.xyz>
|
Reconciled main through |
The 640-row paging guard counts every document node. Main measured 1792 under its 1800 ceiling; #401 measures 1804 with the same 54 mounted timeline rows, from its restored sidebar page rows (5 nodes each). Raise the whole-document ceiling to 1850 so it still catches an unvirtualized history. The non-ready sidebar focus check compared the Retry outline to the scrollport with exact equality. Scroll offsets snap to whole pixels while row heights can be fractional, which fails on hosted WebKit. Allow less than one pixel at the scroll end. Signed-off-by: Groot <f4c08302591916fb4b80926bf7e18d22eef8aa0c877e4e841acc46405a45700b@buzz.block.builderlab.xyz>
Restore the strict outline containment check and exercise it with fractional content heights. The scroll area's 4px end padding matched the 4px ring exactly, so a whole-pixel scroll offset could clip it on hosted WebKit. Use the 6px spacing token instead. Scope the paging node guard to the timeline. Main measured 1792 document and 1573 timeline nodes under its 1800 document ceiling; #401 measures 1804 and 1573. Keep that same timeline budget (< 1581) and report whole-document counts as diagnostics. A 2400px Virtua buffer still fails it (2182) while passing the 100-row guard. Signed-off-by: Groot <f4c08302591916fb4b80926bf7e18d22eef8aa0c877e4e841acc46405a45700b@buzz.block.builderlab.xyz>
|
Delivered at Full Vitest passes (5,542 tests), as do 74 Chromium/WebKit cases and required local gates. Returned to draft pending hosted CI/DCO and native/human acceptance; please test the Inbox toggle/legacy destinations and short-sidebar keyboard focus steps in the updated description. No approval or merge performed. |
Staged linearised merge commit 2a00586 ("Merge main and carry shared timeline repairs into #401") while rebasing onto origin/main. A rebase does not replay merge commits, so this commit carries the hand edits that merge introduced beyond the automatic merge of its parents; without it that content would be lost. Files: src/app/App.profile.test.tsx src/app/shell/ProfileButton.test.tsx src/features/messages/ChannelTimeline.restore.test.tsx src/features/messages/ChannelTimeline.tsx src/features/messages/MessageActionBar.tsx tests/fixtures/conversation.tsx Signed-off-by: Matt Toohey <contact@matttoohey.com>
The 640-row paging guard counts every document node. Main measured 1792 under its 1800 ceiling; #401 measures 1804 with the same 54 mounted timeline rows, from its restored sidebar page rows (5 nodes each). Raise the whole-document ceiling to 1850 so it still catches an unvirtualized history. The non-ready sidebar focus check compared the Retry outline to the scrollport with exact equality. Scroll offsets snap to whole pixels while row heights can be fractional, which fails on hosted WebKit. Allow less than one pixel at the scroll end. Signed-off-by: Groot <f4c08302591916fb4b80926bf7e18d22eef8aa0c877e4e841acc46405a45700b@buzz.block.builderlab.xyz> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Restore the strict outline containment check and exercise it with fractional content heights. The scroll area's 4px end padding matched the 4px ring exactly, so a whole-pixel scroll offset could clip it on hosted WebKit. Use the 6px spacing token instead. Scope the paging node guard to the timeline. Main measured 1792 document and 1573 timeline nodes under its 1800 document ceiling; #401 measures 1804 and 1573. Keep that same timeline budget (< 1581) and report whole-document counts as diagnostics. A 2400px Virtua buffer still fails it (2182) while passing the 100-row guard. Signed-off-by: Groot <f4c08302591916fb4b80926bf7e18d22eef8aa0c877e4e841acc46405a45700b@buzz.block.builderlab.xyz> Signed-off-by: Matt Toohey <contact@matttoohey.com>
0484b32 to
2ae4958
Compare
PR #235 replaced the shell's plugin-aware page list with three hardcoded rows (Inbox, Bestie, Agents). Inbox and Bestie opened "Content coming soon" placeholders, and Bestie showed even with the buzz.bestie plugin disabled. Projects, Workflows, Sessions and external plugin pages were reachable only through search. App now passes the shell's page navigation back through the sidebar render prop, and ChannelSidebar hosts it above the channel list in the ready, connecting and error states. The hardcoded rows and the agentsEnabled prop are gone. The shell renders page icons in an 18px box at the compact size so rows line up with sidebar sections, and the page list no longer scrolls on its own; it scrolls with the roster. The Inbox and Bestie placeholder routes stay valid for deep links. Browser specs open them by address through a new openChannelPlaceholder helper. A new App-level test checks that the sidebar lists only active plugin pages and drops a row when its plugin is disabled. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Every registered page used to get a sidebar row, so Channels and Sessions sat beside Projects, Agents and Workflows even though nothing marks them as top-level destinations. The page contract now has an opt-in `primary` flag. The shell lists only primary pages in the sidebar; search still lists every active page and deep links stay valid. Projects, Agents, Workflows and Link Lab declare `primary: true`. Channels and Sessions stay registered without rows: Messages opens by default, from any channel row and from search, and Sessions opens from Messages and search. The scaffold template, example plugins and browser fixture pages set the flag so they keep their rows. Docs describe the split and call the startup page the default destination so "landing" no longer covers two ideas. Browser specs expect Projects, Agents and Workflows as the sidebar rows. A service test validates the flag, and an App-level test checks that an unflagged page is active and searchable but has no row. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Inbox had no plugin and Bestie registered only a companion panel, so neither could declare a sidebar row once rows became opt-in. The rows PR #235 drew by hand opened "Content coming soon" placeholders inside the Channels page through the Inbox and Bestie route params. A new buzz.inbox plugin registers an Inbox page, and buzz.bestie registers a Bestie page beside its panel. Both are workspace pages with primary: true, so each gets a sidebar row that disappears with its plugin. Inbox keeps the placeholder copy; the Bestie page shares the companion card's body until agent chat connects. The shell ranks them after Messages and before Projects, gives Inbox the bell icon, and draws Bestie's artwork through a new BestieIcon in the icon gateway so the search palette's icon contract still holds. The Channels placeholders are gone. Channel routes no longer accept the Inbox or Bestie params, ChannelsPage drops the placeholder branch and its style, and leaving Settings with no prior destination now opens plain Messages rather than the Inbox placeholder. Browser specs open the new pages from their sidebar rows and the openChannelPlaceholder helper is removed. Tests cover the ordering, the icons, the rejected route params, bundled page counts and the App-level sidebar rows. Docs list the new rows and the inbox plugin. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Every primary page comes from an optional plugin, so disabling Inbox, Bestie, Projects, Agents and Workflows left the shell rendering an empty <nav aria-label="Pages"> that screen readers still announced, with ChannelSidebar's destinations wrapper keeping its bottom margin around nothing. AppShell now passes null through the sidebar render prop when the primary list is empty, and ChannelSidebar skips the destinations wrapper when it receives no page navigation. The App-level sidebar test disables the remaining primary plugins and checks the landmark is gone while the channel sidebar stays; ChannelSidebar tests cover the missing wrapper in the ready, connecting and error states and prove the class selector against the populated case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Staged linearised merge commit 46ea624 ("Merge main and repair primary-page navigation integration") while rebasing onto origin/main. A rebase does not replay merge commits, so this commit carries the hand edits that merge introduced beyond the automatic merge of its parents; without it that content would be lost. Files: crates/plugin-manager/src/lib.rs crates/plugin-manager/tests/management.rs docs/shell-design.md src/app/App.profile.test.tsx src/app/entity-navigation.test.tsx src/app/navigation.ts src/bundled/channels/Channels.module.css src/bundled/channels/ChannelsPage.tsx src/features/channel-navigation/ChannelSidebar.tsx src/features/messages/ChannelTimeline.restore.test.tsx tests/browser/app-style-order.spec.mjs tests/browser/sidenav-polish.spec.mjs Signed-off-by: Matt Toohey <contact@matttoohey.com>
Staged linearised merge commit 2a00586 ("Merge main and carry shared timeline repairs into #401") while rebasing onto origin/main. A rebase does not replay merge commits, so this commit carries the hand edits that merge introduced beyond the automatic merge of its parents; without it that content would be lost. Files: src/app/App.profile.test.tsx src/app/shell/ProfileButton.test.tsx src/features/messages/ChannelTimeline.restore.test.tsx src/features/messages/ChannelTimeline.tsx src/features/messages/MessageActionBar.tsx tests/fixtures/conversation.tsx Signed-off-by: Matt Toohey <contact@matttoohey.com>
The 640-row paging guard counts every document node. Main measured 1792 under its 1800 ceiling; #401 measures 1804 with the same 54 mounted timeline rows, from its restored sidebar page rows (5 nodes each). Raise the whole-document ceiling to 1850 so it still catches an unvirtualized history. The non-ready sidebar focus check compared the Retry outline to the scrollport with exact equality. Scroll offsets snap to whole pixels while row heights can be fractional, which fails on hosted WebKit. Allow less than one pixel at the scroll end. Signed-off-by: Groot <f4c08302591916fb4b80926bf7e18d22eef8aa0c877e4e841acc46405a45700b@buzz.block.builderlab.xyz> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Restore the strict outline containment check and exercise it with fractional content heights. The scroll area's 4px end padding matched the 4px ring exactly, so a whole-pixel scroll offset could clip it on hosted WebKit. Use the 6px spacing token instead. Scope the paging node guard to the timeline. Main measured 1792 document and 1573 timeline nodes under its 1800 document ceiling; #401 measures 1804 and 1573. Keep that same timeline budget (< 1581) and report whole-document counts as diagnostics. A 2400px Virtua buffer still fails it (2182) while passing the 100-row guard. Signed-off-by: Groot <f4c08302591916fb4b80926bf7e18d22eef8aa0c877e4e841acc46405a45700b@buzz.block.builderlab.xyz> Signed-off-by: Matt Toohey <contact@matttoohey.com>
2ae4958 to
5dbec07
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
No change-specific blockers found in this follow-up. The earlier native Inbox catalog, legacy Inbox/Bestie destination, and non-ready sidebar overflow findings are addressed; I found no material regression in their repairs.
Reviewed head 5dbec07ca0b3581d00ba2ed34cca484185169847 against base 5d2b08e2ff3bb40dc62f04015298ff319f4a4f8a, including independent source checks of the native/plugin and sidebar/focus boundaries.
- Validation: hosted CI passed on merge
f6a73d8ea36943e1d5a393e54add2f025ba8ce61of those exact revisions: 456 Vitest files / 5,588 tests, Rust/tool integration, browser journeys in Chromium and WebKit, and measurements. Logs specifically confirm legacy-route regressions, native catalog toggling, and short-sidebar Retry/focus coverage. DCO is green. I inspected source and existing CI evidence; no duplicate local suites or native launch. - Remaining acceptance: rebuilt desktop Inbox activation/toggling and human acceptance are still unverified here. Exercise the native toggle, legacy destinations, and short-sidebar keyboard outline before claiming that acceptance. This is a comment, not approval or merge authorization.
- Optional housekeeping: refresh the description’s stale head/draft/pending-CI text; GitHub currently reports
5dbec07c, ready for review, with successful CI.
* origin/main: (27 commits) Let plugin pages publish NIP-AR artifacts and embed the host thread view (#434) test(app): migrate entity-navigation test off removed buzz://open locator API (#463) Show agent activity in navigation (#423) test(browser): hold motion when it commits, not on its start event (#459) fix(navigation): ignore unknown query parameters on Buzz links and remove the buzz://open locator (#457) feat(design-system): distinguish controls on floating surfaces (#429) feat(native): add community extras and media preparation (#450) Clone inventory identities through reviewed text and fresh identity creation (#289) feat(communities): add right-click actions to the community rail (#400) fix(messages): keep a send reveal pending until its scroll runs (#454) fix(messages): reserve a stable scrollbar gutter on the channel feed (#451) fix(sidebar): list plugin pages as sidebar rows via an opt-in primary flag (#401) feat(channels): surface canvas content in channel settings (#426) fix(profiles): remove redundant presence status row (#394) test(browser): count live retries once the page handles startup controls (#443) feat(composer): host-owned resource links for the Projects picker (#445) feat: support native read state and recent channel activity (#444) feat(native): serve relay media and uploads in packaged builds (#433) feat(channels): suggest joined channels in the composer (#446) feat: support native agent activity, library, memories, and community resolution (#441) ... Signed-off-by: Codex <noreply@openai.com>
PR #235 swapped the shell's plugin-aware page list for three hardcoded rows: Inbox, Bestie and Agents. Inbox and Bestie opened "Content coming soon" placeholders inside Channels, and Bestie still showed with the buzz.bestie plugin disabled. Projects, Workflows, Sessions and external plugin pages could only be reached through search.
Changes
Apppasses the shell's page navigation toChannelSidebarthrough its render prop, and the sidebar shows it above the channel list in the ready, connecting and error states. The hardcoded rows and theagentsEnabledprop are gone. At the compact size, page icons sit in an 18px box so they line up with the sidebar sections, and the page list scrolls with the roster.primaryflag. Only pages that setprimary: trueget a sidebar row. Search still lists every active page, and deep links still work. Projects, Agents, Workflows and Link Lab set the flag. Channels and Sessions stay registered without rows. The scaffold template, example plugins and browser fixture pages also set it, so they keep their rows.buzz.inboxplugin registers an Inbox page (bell icon, placeholder copy for now).buzz.bestieregisters a Bestie page beside its panel, using the companion card's body and a newBestieIconin the icon gateway. Both are primary and appear after Messages and before Projects. Each row disappears when its plugin is disabled.ChannelsPagedrops the placeholder branch. Leaving Settings with no earlier destination now opens Messages.plugin-architecture.md,shell-design.md) cover the primary/search split and the new rows. They now say "default destination" for the startup page.Tests
🤖 Generated with Claude Code
Integration and review repairs
Current head:
0484b32a, integrating main4e1cf4b8without conflicts. Earlier integration evidence remains in the prior delivery comment.<1581) and≤100mounted rows; whole-document counts remain diagnostic. A deliberate buffer-size regression fails the node guard in both engines even while passing the row guard.Validation at 0484b32
Browser-only justification: real clipping, fractional geometry, scrolling and virtualization cannot be established in jsdom. This follow-up adds no standalone browser cases and removes none; it strengthens the existing short-height case with fractional content and replaces the document-wide structural assertion with the timeline-owned equivalent. No timeout, retry or clipping tolerance was increased. Local scoped browser execution took 7.0 minutes; this is not a before/after performance claim. An earlier mis-scoped command selected the full suite and was terminated at the command's five-minute deadline; it is not a full-suite pass.
Deferred acceptance
Kept draft: hosted CI on this head, native desktop Inbox activation/toggle smoke testing, and human acceptance remain outstanding. Agent browser checks are not native or human acceptance; no approval or merge is implied.
Please verify Inbox appears in desktop, disabling/re-enabling it removes/restores its row, legacy Inbox/Bestie destinations open correctly, and a short sidebar can scroll to Retry with a fully visible keyboard outline. Existing Inbox/Bestie desktop toggle policy is unchanged: those standalone pages keep the sidebar visible.