Skip to content

fix(web): anchor menus to the clicked trigger - #254

Merged
kinqsradio merged 3 commits into
orbyn-dev:feat/web-rebuildfrom
luannn010:fix/web-menu-anchor
Nov 3, 2026
Merged

kinqsradio merged 3 commits into
orbyn-dev:feat/web-rebuildfrom
luannn010:fix/web-menu-anchor

Conversation

@luannn010

@luannn010 luannn010 commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

See CONTRIBUTING before starting engineering work.

Change

The Workspace actions menu (and every other shared Menu in web/) sometimes opened in the top-left corner instead of under its trigger.

Cause. Menu took its anchor from document.activeElement when opened. WebKit (Safari, and WebKit-based desktop webviews) does not focus a <button> on mouse click, so the anchor was <body> and the menu was placed at (8, 8). The menu was also positioned once, so resizing the window while it was open left it stranded.

Fix (web/src/ui/kit.tsx and one call site):

  • open now accepts the click event and anchors to event.currentTarget. Call sites that already use onClick={open} get this without changes; callers that use bare open() keep the previous focus-based fallback.
  • The shared-task row (TaskRow.tsx) now passes its click event, so its options menu gets the same trigger anchoring.
  • The open menu is re-placed on window resize and on scroll (capture), using the same clamping rules as before.

No styles or tokens changed. Keyboard behaviour is unchanged: focus moves to the first item, arrow keys cycle, and Escape closes and returns focus to the trigger (this now also works after an unfocused click).

Workspace shell height (9c3020f1)

After scrolling Today, the app could slide up and show the grey page background below it.

Cause. The workspace shell is fixed at 100dvh and only .workspace-frame-content scrolls. That scroller was not positioned, so absolutely positioned descendants (the .sr-only labels in Today's plan rows) used the initial containing block. They escaped the scroller and stretched the document to 1299px in a 900px viewport, so the whole shell could scroll up by about 400px.

Fix (web/src/app/workspace/workspace.css only): .workspace-frame-content gets position: relative, so those descendants stay inside the scroller.

Screenshots

Captured in the built-in Chromium browser against the local Vite dev server at http://127.0.0.1:5175/app (Today screen, synthetic demo data), 100% zoom, device pixel ratio 2. Each menu was opened with the trigger not focused (blur() then click()) to reproduce WebKit's click behaviour in Chromium.

Revision Viewport Theme Menu box (left, top) Trigger box (left, bottom)
Before cadf0db6 1280×800 light (8, 8) (1145, 113)
After d6942ee9 1280×800 light (1072, 117) (1145, 113)
After d6942ee9 1280×800 dark (1072, 117) (1145, 113)
After d6942ee9 375×812 dark (167, 117) (323, 113)

Before — menu opens over the sidebar at the top-left:

Before: menu at top-left, 1280 light

After — menu opens under the Workspace button:

After: 1280 light

After: 1280 dark

After: 375 phone, dark

Screenshots are stored on the fork's pr-evidence/web-menu-anchor branch so this PR's diff stays at the two source files above.

Checks and limits

Shell height, at 9c3020f1 in the same browser and dev server: the document's scroll height equals the viewport height after scrolling each frame to the bottom, with no horizontal overflow, at 375×812 (8 routes including Today and Assistant), 768×1024 and 1440×900 (Today), and 1440×600 (all 18 sidebar routes). Before the fix, Today measured 1299px in a 900px viewport. npx prettier --check web/src/app/workspace/workspace.css passes. Not checked: real Safari, the desktop webview and mobile clients.

Menu anchoring, run on fix/web-menu-anchor at d6942ee9:

  • npx tsc --noEmit in web/: exit 0.
  • npx prettier --check web/src/ui/kit.tsx: passes.
  • Rendered checks in Chromium as above, plus: the open menu followed the trigger when the viewport was resized from 375 to 1280 wide (before the fix it stayed at its first position); keyboard open, arrow and Escape behaviour as described.

Follow-up 99818e49 (3 Nov 2026): the TaskRow call site was updated; web/ typechecking passes with the ES2023 array lib, Prettier and vite build pass. The stock npm run build still stops at the pre-existing Array.findLast/ES2022 error in library/page/Source.tsx. No new browser pass was run.

Not verified:

  • Real Safari or the desktop app's webview. The bug was reproduced by emulating WebKit's no-focus-on-click in Chromium, not in WebKit itself.
  • Other screens' menus were not checked visually; they share the same component and their onClick={open} triggers take the same path.
  • No automated tests were added; the web package has no test script.
  • Native/mobile (Expo) clients are unaffected by this file and were not run.

Workspace

  • Used the outer Orbyn Engineering workspace with this target nested under repo/, and read the relevant project guides.

This contributor confirmation is not an automated claim that a host loaded policies.

Agent disclosure

  • Tools: Claude Code (desktop app), built-in Chromium browser pane, git, gh, tsc, prettier.
  • Model: Claude Opus 5.5 (claude-opus-5-5); effort as set by the session.
  • Human role: @luannn010 reported the bug from the running app, approved creating the fork and requested this PR; later reported the workspace gap from a screenshot and asked for the fix to be added here.
  • External sources: none.

🤖 Generated with Claude Code

Follow-up fixes (3 Nov 2026)

  • Tools: Freebuff Desktop coding agent, terminal, git, gh, npm, tsc, Prettier, Vite.
  • Model: selected by the host and not exposed to the session; exact identifier unavailable.
  • Effort: host default; not separately set.
  • Human role: maintainer requested the review fixes and the feat(web): tidy workspace menus, dock and sidebar controls #256 rebase; no human code edits.
  • External sources: none.

luannn010 and others added 2 commits November 3, 2026 14:20
WebKit does not focus a button on mouse click, so Menu read <body> from
document.activeElement and opened the popup at the top-left corner. Anchor
to the click's currentTarget, keeping focus as the fallback for bare open(),
and re-place the open menu on resize and scroll.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Absolutely positioned descendants such as .sr-only labels escaped the
scrolling frame and extended the document, so the shell could scroll up
and expose the page background below it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@kinqsradio kinqsradio left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed #254 against feat/web-rebuild (cadf0db6). The event-based anchor is the right WebKit fix, the scroll/resize re-placement is a real improvement, and the .workspace-frame-content { position: relative } change is the right containment fix for the .sr-only labels; the existing absolute tab-actions popover already has a positioned parent (.workspace-control-tab), so it keeps its anchor.

Verdict: approve with one follow-up. tsc --noEmit -p web/tsconfig.json, Prettier on the changed files and npm run build -w web pass locally. No CI checks are reported for this PR. I did not run real Safari; the PR's blur-then-click Chromium reproduction is a reasonable emulation, and the before/after measurements are good evidence.

The follow-up is on kit.tsx: the fallback is still reachable from one in-tree menu. web/src/screens/shared-task/TaskRow.tsx:84 calls bare open() from its kebab onClick (after stopPropagation()), so on WebKit that task-row menu still anchors to document.activeElement, often <body>. Pass the event there (open(event)), or explicitly state that the task-row menu is out of scope. I found no other bare open() inside a Menu trigger.

Comment thread web/src/ui/kit.tsx
}: {
label: string;
trigger: (open: () => void, expanded: boolean) => ReactNode;
trigger: (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Keeping event optional preserves the focus fallback, but one in-tree menu still uses it: web/src/screens/shared-task/TaskRow.tsx:84 calls open() from its kebab onClick after stopPropagation(). On WebKit that still anchors to document.activeElement (often <body>), so the task-row menu remains broken. Passing the event there (open(event)) would cover it. I found no other bare open() inside a Menu trigger; the other wrapped call sites are local dialog/opening functions, not shared Menu triggers.

@kinqsradio

Copy link
Copy Markdown
Member

Follow-up pushed in 99818e49: TaskRow now calls open(event), so its menu anchors to the kebab trigger on WebKit too.

Checks: Prettier and vite build pass; tsc --noEmit passes with the pre-existing Array.findLast lib workaround. No browser/Safari pass was run.

@kinqsradio kinqsradio left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@kinqsradio
kinqsradio merged commit 14e5ab9 into orbyn-dev:feat/web-rebuild Nov 3, 2026
7 checks passed
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