feat(channels): move session creation into the context menu - #209
Conversation
Align shared menu styling, token guidance and geometry regression with the parent-menu foundation in #209. Keep startup presentation and grouping behavior unchanged. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Review clear: no established code/product/security blockers at 3a01b8210d9896243f16de4bf5abf7fda7dd1a07 against 21ea3513b468c204e936d57f981cb1e37d7acd3e.
Reviewed action eligibility and menu retirement across row relocation/removal/action loss, draft-resume and focus handoff, separate disclosure/child/Activity controls, and shared menu-row styling. The small page-owned state hook and existing session/design-system owners are proportionate to the contract.
Validation: source-only review, with an independent styling/interaction lane. Existing exact-head CI and DCO passed; no PR code was executed for this review. Windows native validation was skipped. The dedicated design-viewer browser run is author-reported, not independently established by the app CI result.
Remaining gaps, not demonstrated defects: native keyboard/assistive/touch acceptance, Activity-popup-child context input, and focus after involuntary menu retirement. No session-submission/native acceptance is claimed. These gaps do not justify reversing the explicit select-only context-menu design or restoring the removed ellipsis.
Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
3a01b82 to
8f84071
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Review clear: no established code/product/security blockers at 8f8407152707531bb349ee456c34d28193d560e7 against e02fe33220fa497a2e7ee780c8295517b4f967fa.
Re-review verified the previously clear patch’s forward rebase: main’s New message route/selection and design-viewer focus policy are preserved; the only new patch content is the selection regression assertions. Checked integrated menu eligibility/retirement, draft focus, separate disclosure/Activity controls, and shared radius behavior, with an independent focused verification.
Validation: source-only; existing exact-head app CI #987 passed, including Chromium/WebKit session-menu journeys. No PR code was executed. Windows native was skipped; a dedicated design-viewer browser run remains author-reported, not established by app CI. Native keyboard/assistive/touch, involuntary-retirement focus, Activity-popup-child context input, and actual session submission remain acceptance gaps, not demonstrated regressions.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Clear for human approval at 8f8407152707531bb349ee456c34d28193d560e7 against e02fe33220fa497a2e7ee780c8295517b4f967fa. No material blockers found. This is a bounded follow-up to the clear review at 3a01b821, not a new blanket runtime certification.
The range-diff preserves the reviewed menu eligibility/lifetime and focus-transfer implementation. Conflict resolutions retain main’s New message route, conditional sidebar selection and viewer keyboard/focus expectations; the added New message assertions cover selected → composing/unselected → confirmed/selected. Pill geometry coverage remains additive. Independent review checked the styling/test resolutions.
Exact-head CI and DCO pass; GitHub reports MERGEABLE, with no review threads outstanding. Required reviewer/code-owner approval remains. This follow-up was source/CI evidence review, not a local suite rerun. Windows native validation was skipped; native/assistive/touch and actual session-submission acceptance remain outside this verdict. No approval or merge performed.
…rs-support * origin/main: feat: show owner-view agent memories in profiles (#231) Add opt-in Canvas-backed channel Todos (#222) Skip hidden folders when discovering plugins in a folder (#229) feat: add owned local agent actions to profiles (#190) feat(channels): move session creation into the context menu (#209) Add Goose as an agent harness option (#214) feat: preview channel agent activity in profiles (#187) Use context-aware identity names with human-first priority (#167) feat: add managed agents to channels from profiles (#196) Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz> # Conflicts: # src/features/relay/outbox.ts
Overview
Category: improvement
User Impact: Users can start a session from a channel’s right-click menu without a separate ellipsis button cluttering the sidebar.
Problem: Session creation uses a separate row menu, while the sidebar needs a consistent home for contextual actions.
Solution: Move New session into the channel context menu, preserve keyboard access and draft focus, and remove the ellipsis button. Determine availability from actual actions and retire open-menu state when its row moves or becomes unavailable, so later sidebar actions can integrate without inheriting Sessions-specific eligibility.
Changes
File changes
docs/channels.md
Document action composition, rendered section identity, menu lifetime, and the separate disclosure/child-row surfaces.
docs/sessions/README.md
Document right-click and keyboard session entry, plugin eligibility, and focus behavior.
src/bundled/channels/ChannelSidebarItem.tsx
Keep the context trigger inside the memoized row, separate from the activity button’s popup semantics and child-session controls.
src/bundled/channels/ChannelSidebarItem.test.tsx
Cover keyboard menu entry and preserve disclosure/child navigation boundaries.
src/bundled/channels/ChannelSidebarRow.tsx
Remove the ellipsis menu while retaining session disclosure, draft navigation, and DM removal.
src/bundled/channels/ChannelSidebarRow.module.css
Remove obsolete ellipsis-control styling.
src/bundled/channels/ChannelSidebarRow.test.tsx
Remove obsolete ellipsis assertions; session entry now has connected-row and full-app coverage.
src/bundled/channels/ChannelsPage.tsx
Compose actual menu items with individual eligibility, own the controlled menu, and preserve focus transfer to the row or draft composer.
src/bundled/channels/useChannelRowMenu.ts
Retire menu state when its channel leaves the rendered section or loses its last action; use full section keys to avoid placement collisions.
src/bundled/channels/useChannelRowMenu.test.tsx
Mount the real hook under StrictMode to cover relocation, removal, hide/archive, lost/restored actions, independent DM eligibility, and stable keyboard anchors/callbacks.
src/shared/design-system/styles/components.css
Use the existing
--radius-pilltoken for every menu row, with no first/last exceptions or one-off inset radius. The global navigation-row radius stays unchanged.src/shared/design-system/DESIGN.md
Document the shared menu corner recipe and its ownership.
src/shared/design-system/tokens/registry.ts
Document menu rows as a use of the existing pill-radius token.
tests/fixtures/design-system/viewer.spec.ts
Extend the existing keyboard/focus journey to verify fully rounded action, checkbox, submenu and grouped radio items in both themes at narrow, intermediate and wide widths.
tests/browser/navigation-session-menu.spec.mjs
Exercise actual menu-to-composer focus transfer, keyboard dismissal, draft resumption, rendered corner geometry, and non-resurrection after a preference-driven row move.
tests/browser/navigation-sidebar.spec.mjs
Use the new entry point and verify disabling the only available action does not expose an empty menu.
tests/browser/new-message.spec.mjs
Verify the previous DM is selected before composing, deselected while composing, and selected again after confirmed delivery.
tests/browser/thread-unread.spec.mjs
Preserve activity-popover accessibility and focus alongside the new context trigger; explicitly finish the hover state before testing keyboard opening.
Validation
Current head:
8f8407152707531bb349ee456c34d28193d560e7, rebased onto maine02fe33220fa497a2e7ee780c8295517b4f967fa.Conflict resolution: Retained main’s New message route and conditional sidebar selection alongside action-specific context-menu eligibility. Preserved both channel-menu and direct-message documentation. Combined pill-radius coverage with main’s newer viewer focus expectations; no mainline behavior or keyboard assertions were removed. App focus-ring checks remain unchanged: the app and design viewer currently load separate stylesheet entry points.
At the clean rebased head:
navigation-session-menu,navigation-sidebar,thread-unread,new-message): 34/34 passed, Chromium and WebKit.selected={selected}made the new deselection assertion fail in both engines; restored conditional selection passes.Earlier validation (before this rebase)
CI repair: The original Chromium shard failed because Activity remained hover-open after context-menu dismissal. The test pressed Enter (closing it), then accepted the still-visible exit animation as an open popup. The hosted trace shows
aria-expanded="true"before Enter and adata-closedpopup afterward. The test now leaves hover, waits for the popup to unmount, then verifies keyboard entry setsaria-expanded="true"before focusing its item. No production code, sleeps, timeouts or assertion weakening. The completethread-unread.spec.mjsfile passed 4/4 cases in Chromium and WebKit at88b1bc9plus the exact test delta committed as3a01b82; commit/push hooks passed at the new head. That pre-rebase head later passed all automatic hosted checks (Windows intentionally skipped).Full-rounded follow-up: At
d936662plus the CSS/docs/browser-test delta now committed in88b1bc9, all 84 design-system unit tests, 50 design-system browser cases, and 4 session-menu app cases passed. Browser runs included Chromium and WebKit; geometry assertions cover both themes and narrow/intermediate/wide layouts. Design and app builds passed. The later token-registry change only clarifies usage wording; pre-push hooks at88b1bc9passed TypeScript, 51 related unit tests, design typecheck and design guards. Pre-commit secret scanning and formatting/lint also passed.Restoring the previous positional CSS made the final radius regression fail in both engines (expected the pill token’s computed value, received 19px); the full-rounded CSS was then restored. No new browser scenario was added for this follow-up: it extends existing layout/focus journeys.
Original foundation evidence (historical, not a blanket rerun at this head):
d936662736733eca7d266a3d889580e2bffe54cematched all 15 reviewed file hashes. On base21ea3513b468c204e936d57f981cb1e37d7acd3eplus that reviewed candidate:navigation-session-menu,navigation-sidebar, andthread-unreadbrowser files: 32 cases passed, Chromium and WebKit.channel-openingfile: 4 serial measurement cases passed, both engines.Browser-only coverage: The foundation added two scenarios (four engine cases) for actual portal/composer focus and layout, plus page/store/portal wiring during row relocation. Transition permutations stay in mounted component tests. The relocation case failed before the repair in both engines: the menu reappeared when the row returned to its old section. It passes after the repair. No sleeps establish asynchronous completion.
Scope and gaps: No grouping, mute/read, or lifecycle writers are added. The session disclosure and child rows deliberately remain outside the parent trigger. Later actions still need their own integration checks; actual session submission/native acceptance and hosted CI are not claimed here. Screenshot capture exercised the real connected app’s local draft entry without sending a message or creating a session.
Reproduction Steps
Screenshots / Demos
Illustrative reference — requested placeholder items, not shipped functionality.
All three rows now use the same full-rounded token; hover position no longer changes the shape.
Captured from the running app in dark mode with the product’s purple accent. The actual New session menu is shown over the left nav; two inert placeholder items illustrate multi-item shape and size. Nearby channel names are replaced for privacy. These browser-only changes are not part of the implementation or PR diff; the shipped menu has only New session.
Prepared with Carl (AI); independent local review by Princess Donut (AI).
Originating conversation: buzz://message?channel=b9ab2a04-14c4-440d-8c82-aebfbc1caa68&id=ac0238c98faf6824a4b2cb1cfb7bad2132b4b1438b990698b369b0af07f6b9dc