Skip to content

feat(design-system): add shared contextual menus - #89

Open
tellaho wants to merge 1 commit into
mainfrom
tho/sidebar-menus
Open

tellaho wants to merge 1 commit into
mainfrom
tho/sidebar-menus

Conversation

@tellaho

@tellaho tellaho commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Overview

Category: infrastructure
User Impact: This prerequisite does not change the Messages sidebar yet; it supplies the shared menus used by the following channel-action and sorting PRs.

Problem: Saved-group actions and section sorting need the same accessible, viewport-aware menu behavior, but keeping that machinery inside either feature makes the feature split harder to review.

Solution: Extract the existing Base UI menu wrappers and shared floating-surface styling into one narrow stack base. Grouping/Star and sorting will consume it in separate feature PRs; no relay, preference, or sidebar behavior is included here.

Changes

File changes

src/shared/design-system/ui/Menu.tsx
Provides menu/context-menu roots and triggers, positioned popups, action/link items, radio/checkbox choices, and nested submenus. Base UI retains interaction and dismissal ownership.

src/shared/design-system/styles/components.css
Adds shared floating menu surfaces, choice indicators, highlighted/disabled states, viewport bounds, and reduced-motion treatment.

src/shared/design-system/ui/registry.ts
Records the menu component and its Base UI backing parts.

tests/fixtures/design-system/ui/componentSpecimens.tsx
Adds a menu specimen for exercising action, checkbox, radio, and nested-menu composition. This is documentation, not evidence that the production sidebar works.

Reproduction Steps

  1. Start the design-system viewer and open the Menu component.
  2. Open More actions, toggle Notifications, and open Sort to choose Recent or A–Z.
  3. Check keyboard navigation, Escape dismissal, disabled items, and constrained-window positioning in light and dark mode.
  4. The production Messages sidebar is unchanged in this prerequisite. Its consumer PRs must exercise the real sidebar before integration.

Validation and limits

At clean 57dd36eab159367c27063c8632930d88fd6463c9, based on freshly fetched main a9194b24a3cc208abf6f9a9a05eed77583e6c0fa:

  • Staged formatting/lint, application typecheck, design typecheck, design guards, and design build passed.
  • Full design:test: 7 files / 39 tests passed. These are existing design-system tests, not new menu-interaction coverage.
  • Normal pre-push checks passed: TypeScript and 4 related test files / 14 tests.
  • Full diff and DCO/actual-author audit passed; only the four files above changed.
  • No browser cases added or removed. Production consumer wiring, both-engine keyboard/focus/collision journeys, native checks, and actual-product screenshots remain deferred to the feature slices. No specimen screenshot is presented as production acceptance.
  • Design build emitted its large-chunk warning. No bundle restructuring is included.

Kept draft: a standalone primitive extraction is not a claim that the downstream features are validated or ready to merge.

Implemented/extracted by Carl (AI agent), at the operator request.

Add Base UI menu and context-menu primitives, shared floating styles, and the component registry/specimen entry.

Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Reviewed head 57dd36eab159367c27063c8632930d88fd6463c9 against base a9194b24a3cc208abf6f9a9a05eed77583e6c0fa.

Changes requested

P2: Preserve the design system’s keyboard-only focus ring on menu items.

components.css:718–732 unconditionally sets outline: none on every item wrapper and only adds the same data-highlighted fill used for pointer navigation. The popup’s data-buzz-ui boundary excludes its descendants from the host’s legacy focus outline, and neither host nor viewer supplies a replacement menu-item ring. This loses the focused-control ring explicitly required by the existing design contract and component rules.

Reproduction path for verification: in the Menu specimen, Tab to More actions, open with ArrowDown, navigate the action/checkbox items, then enter Sort and navigate the radio items. Base UI moves focus, but the new CSS suppresses its outline; only the shared hover/highlight background remains. This is a source-established styling-contract defect, not a claim that the existing highlight is invisible or that navigation itself is broken. I did not execute this browser path.

Smallest fix / exit criteria: add a visible item focus ring gated by html[data-keyboard-navigation] and :focus-visible, retaining quiet pointer focus and Base UI’s interaction ownership. Add a focused regression proving the keyboard ring and pointer suppression on the composed menu, including a submenu item, in light/dark mode. No broader menu framework or sidebar integration is required to address this finding.

Scope and evidence

  • This is a shared-menu prerequisite. Production sidebar behavior, persistence and consumer integration are not delivered by these four files and are not requested as fixes here. API/prop composition, context-menu primitive reuse, controlled choice callbacks, portal styling boundaries and shared state attributes were source-reviewed against pinned Base UI 1.7.0.
  • All 12 existing head-bound hosted checks passed. The JavaScript job passed 147 Vitest files / 1,552 tests. Its synthetic merge d9beaa683d38b04989effa57a60133fbe3bc4cce has the pinned base/head as parents and the same source tree as this reviewed head.
  • Those checks do not prove this new menu’s interactions: the specimen/registry checks do not open it. Review was source-only plus existing hosted evidence, with no checkout, build, test execution or CI rerun. Rendered collision/layering and live consumer acceptance remain unverified; no speculative overlay defect is asserted.

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