Skip to content

feat: open ⋯ menu on right-click/contextual click on session items#286

Merged
PureWeen merged 2 commits intomainfrom
fix/right-click-or-contextual-click-on-sessi-20260305-1358
Mar 5, 2026
Merged

feat: open ⋯ menu on right-click/contextual click on session items#286
PureWeen merged 2 commits intomainfrom
fix/right-click-or-contextual-click-on-sessi-20260305-1358

Conversation

@StephaneDelcroix
Copy link
Copy Markdown
Collaborator

Summary

Right-clicking (contextual click) on a session in the left sidebar now opens the same ⋯ actions menu that the ellipsis button opens. The browser's default context menu is suppressed.

Changes

  • SessionListItem.razor: Added @oncontextmenu handler with preventDefault on the session-item div. The OpenContextMenu method only opens the menu if it isn't already open (prevents accidental close on repeated right-clicks).
  • RightClickContextMenuTests.cs: 5 unit tests verifying the handler exists, prevents default, uses the correct method, only opens when closed, and is on the correct element.
  • mode-switch-scenarios.json: Added UI scenario right-click-opens-session-menu with CDP test steps.

Testing

  • All 5 new tests pass
  • All 1970 existing passing tests still pass (13 pre-existing failures in PopupThemeTests unrelated)
  • Mac Catalyst build succeeds

Add @oncontextmenu handler to the session-item div in SessionListItem.razor
that opens the same actions menu as the ⋯ button. The browser's default
context menu is suppressed via @oncontextmenu:preventDefault. The handler
only opens the menu if it isn't already open, preventing accidental close
on repeated right-clicks.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PureWeen
Copy link
Copy Markdown
Owner

PureWeen commented Mar 5, 2026

PR #286 Review — "feat: open ⋯ menu on right-click/contextual click on session items"

CI Status: ⚠️ No checks reported
Prior reviews/comments: None (first review)


Consensus Findings (flagged by 3+ of 5 models)


🟡 MODERATE — PolyPilot.Tests/Scenarios/mode-switch-scenarios.json — Step 3 asserts DOM state synchronously after synthetic event with no wait

The right-click-opens-session-menu scenario fires a contextmenu event via JS, then immediately checks that .session-menu is visible:

{ "action": "evaluate", "selector": "...", "expression": "item.dispatchEvent(evt)" },
{ "action": "assertExists", "selector": ".session-menu" }   // ← no wait step

In MAUI Blazor Hybrid, dispatchEvent triggers JS synchronously but the Blazor C# handler (OpenContextMenuOnToggleMenu.InvokeAsyncStateHasChanged → DOM update) requires multiple async round-trips through the BlazorWebView/SignalR circuit. The assertion will fire before Blazor re-renders, making the test unreliable (always-false or flaky depending on timing).

Fix: Insert a wait or poll step between the dispatchEvent and the assertExists, or use a waitFor-style assertion with a timeout.

Flagged by 3/5 models.


Below Consensus Threshold (2/5 models — included for informational purposes only, not required to block)

  • 🟢 MINOR — RightClickContextMenuTests.cs — The SessionListItem_ContextMenuOnSessionItemDiv test uses a 200-character proximity heuristic to find the @oncontextmenu attribute near session-item-container. This will produce a false negative if the code is reformatted, or a false positive if an unrelated element with oncontextmenu is added nearby. A more structural check (e.g., parsing the Razor AST or matching on element attributes directly) would be more reliable. Flagged by 2/5 models; informational only.

Positive observations

  • @oncontextmenu:preventDefault="true" correctly suppresses the native browser context menu.
  • The C# code change is minimal and self-contained.

Summary

# Severity Location Finding
1 🟡 MODERATE mode-switch-scenarios.json step 3 Missing wait step after synthetic contextmenu event before asserting DOM state

Recommended action: ⚠️ Request changes — The scenario test will be flaky/unreliable without a wait step after the synthetic event dispatch. Otherwise the implementation is correct.


Review by 5-model parallel dispatch (claude-opus-4.6 ×2, claude-sonnet-4.6, gemini-3-pro-preview, gpt-5.3-codex) with 2+ model consensus filter.

@PureWeen
Copy link
Copy Markdown
Owner

PureWeen commented Mar 5, 2026

PR Review Squad — PR #286 Re-Review Round 2

CI Status: ⚠️ No checks reported

Previous Findings

  • F1 🟡 Scenario timing race — STILL PRESENT
  • F2 🟢 Proximity heuristic — STILL PRESENT

🟡 MODERATE

  1. F1 mode-switch-scenarios.json:~1458 (5/5) — Step 3 checks .session-menu immediately after step 2 dispatches contextmenu. No wait/poll between them. Blazor C# handler runs async via the JS→.NET bridge; DOM wont have updated yet. Step 3 will always fail against a live app.

  2. New M1 SessionListItem.razor:26 (2/5) — @oncontextmenu:preventDefault="true" on parent .session-item div captures right-clicks from child elements. The rename <input> has @onclick:stopPropagation but no @oncontextmenu:stopPropagation. Right-clicking inside the rename input suppresses the native text-selection context menu (cut/copy/paste) and opens the session ⋯ menu instead — usability regression.

🟢 MINOR

  • F2: RightClickContextMenuTests.cs:70Math.Abs(...) < 200 proximity heuristic still brittle

Recommended Action: ⚠️ Request changes

  1. Add { "action": "wait", "ms": 200 } between scenario steps 2 and 3
  2. Add @oncontextmenu:stopPropagation="true" to the rename <input>

- Add 200ms wait between dispatchEvent and .session-menu check in
  right-click-opens-session-menu scenario (Blazor renders async)
- Add @oncontextmenu:stopPropagation to rename input to preserve native
  text-selection context menu when right-clicking inside rename field

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PureWeen PureWeen merged commit c3ff36c into main Mar 5, 2026
@PureWeen PureWeen deleted the fix/right-click-or-contextual-click-on-sessi-20260305-1358 branch March 5, 2026 16:32
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