Skip to content

fix: let the native context menu through where text is selectable - #27

Open
ashproto wants to merge 2 commits into
nextfrom
fix/scope-native-context-menu
Open

fix: let the native context menu through where text is selectable#27
ashproto wants to merge 2 commits into
nextfrom
fix/scope-native-context-menu

Conversation

@ashproto

@ashproto ashproto commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Reported by Codex review on #26 as P2. The code is not part of that PR — src/routes/+page.svelte:298 came from 15e51c3 (2026-07-17), and #26 only ever touched that file to add a --diff-ring token — so it is fixed here on its own branch.

The problem

A window-level handler cancelled every native context menu in the app:

const suppressNativeContextMenu = (event: MouseEvent) => event.preventDefault();
window.addEventListener("contextmenu", suppressNativeContextMenu);

Right-clicking a commit-message box, a PR review textarea, diff code, or rendered Markdown gave nothing at all — no Copy, no Paste, no Look Up, no spelling suggestions. On macOS that reads as broken.

Two findings that made this smaller than it looked

The intent was already satisfied by the build config. The handler was presumably written to hide WebKit's developer menu (Reload / Inspect Element). But src-tauri/Cargo.toml declares tauri = { version = "2", features = ["macos-private-api"] } — no devtools feature. Tauri enables the inspector on debug builds only, so "Inspect Element" never appears in a release build. Nothing needed guarding in the shipped app.

The global handler was not what made the custom menus work. Every one of them already calls preventDefault() itself — GraphHistory.svelte:247, Sidebar.svelte:238, WorktreePanel.svelte:67, and the WorkingCopyView / RefTree callbacks. Removing it would not have produced two stacked menus.

The fix

The stylesheet immediately below the handler already curates where text matters — a no-select body, opted back in for form fields, .diff-cell, .md, .body-msg, .sha, .selectable. That set is, near enough by definition, where a native menu earns its place. So the handler reads the computed value rather than keeping a second list in sync with it:

const el = event.target as Element | null;
const style = el && getComputedStyle(el);
if (style && (style.webkitUserSelect || style.userSelect) !== "none") return;
event.preventDefault();

user-select inherits, so descendants of .md / .selectable are covered without enumerating them, and DiffView's line-number gutter — which sets its own user-select: none so gutter-drag staging never selects text — keeps suppressing its menu for free. Both the prefixed and unprefixed properties are read: minimumSystemVersion is 12.3 (Monterey / Safari 15.x), where unprefixed support is not a given.

Design doc: docs/superpowers/specs/2026-08-08-context-menu-scoping-design.md.

Behavior

Right-click target Before After
Commit row, sidebar ref, worktree, working-copy file app menu app menu (unchanged)
Toolbar, panel header, tabs, labels nothing nothing (unchanged)
Commit-message input, review textarea nothing native Copy / Paste / spelling
Diff code, rendered Markdown, commit body, SHA nothing native Copy / Look Up
Diff line-number gutter nothing nothing (own user-select: none)

Verification

Ran the handler against the live stylesheet in the dev server and dispatched contextmenu at each shape:

target computed outcome
chrome div, nested toolbar span, diff gutter none suppressed ✓
input, textarea text native menu ✓
.md + descendants, .diff-cell, .sha, .body-msg, deep .selectable child text native menu ✓

Still wants a human look: what WKWebView actually renders for that menu is a decision no test environment reproduces. The gate logic above is verified; the rendered menu is not. Worth a right-click in a bundled build before merging — and note a debug build legitimately shows "Inspect Element" there, which the shipped app will not.

Gates: npm run check 524 files / 0 errors · npm test 330 passed · cargo test unaffected (TS-only change; the one failure on this branch is the git_ops.rs:356 init-repo flake that #25 fixes).

Rejected alternatives

  • An explicit selector constant mirroring the CSS list. More obvious and unit-testable, but duplicates the allowlist — they drift the first time someone adds a selectable class to one and not the other, and the failure is silent.
  • Deleting the handler outright. The most surgical option, and all custom handlers already self-suppress. Rejected because WKWebView's default menu would then appear on chrome, and what it contains in a release build cannot be confirmed from a debug build — the inspector item is exactly what differs between them.

🤖 Generated with Claude Code

ashproto and others added 2 commits August 8, 2026 13:02
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A window-level handler cancelled every native context menu in the app. The
handful of row types with their own menu already call preventDefault
themselves, so all it did elsewhere was take functionality away: right-clicking
a commit-message box, a review textarea, diff code, or rendered Markdown gave
nothing at all — no Copy, Paste, Look Up, or spelling. On macOS that reads as
broken.

The intent was presumably to hide WebKit's developer menu, but `devtools` is
not in the Tauri feature list, so the inspector only exists on debug builds and
never ships. Nothing needed guarding in release.

Rather than invent a second notion of "editable or selectable", read the
computed `user-select`: the stylesheet right below already curates that set,
opting a no-select body back in for form fields, diff code, rendered markdown,
commit bodies and `.sha`. It inherits, so descendants of `.md`/`.selectable`
are covered without enumerating them, and DiffView's gutter — which sets its
own `user-select: none` so gutter-drag staging never selects text — keeps
suppressing its menu for free. Both the prefixed and unprefixed properties are
read: minimumSystemVersion is 12.3, where unprefixed support is not a given.

Verified by running the handler against the live stylesheet: chrome, nested
toolbar spans and the gutter suppress; input, textarea, .md and its
descendants, .diff-cell, .sha, .body-msg and deep .selectable children get the
native menu. What that menu RENDERS is a WKWebView decision no test
environment reproduces, so it still wants a look in a bundled build.

npm run check 524 files 0 errors; npm test 330 passed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

if (
!pendingAutomaticUpdate ||
checking ||
presenting ||
dialogs.state.kind !== "none"

P2 Badge Defer updates for every open overlay

When an automatic check completes while Settings, Manage Repository, amend/rebase, branch-color, or a GitHub action modal is open, dialogs.state is still none, so this proceeds to presentUpdate() and mounts the higher-z-index update dialog over the active workflow. The repository already centralizes these states in anyOverlayOpen() (src/lib/overlays.ts); use that full predicate here so startup/background checks wait instead of interrupting an in-progress modal.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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.

1 participant