Skip to content

feat: configurable click behavior to close tabs#218

Open
AryanRogye wants to merge 6 commits intothe-ora:mainfrom
AryanRogye:feature/double_click_exit_fullscreen
Open

feat: configurable click behavior to close tabs#218
AryanRogye wants to merge 6 commits intothe-ora:mainfrom
AryanRogye:feature/double_click_exit_fullscreen

Conversation

@AryanRogye
Copy link
Copy Markdown
Contributor

@AryanRogye AryanRogye commented Mar 9, 2026

adds feature for #192
I wanted this feature but with option click, so I made it configurable in the settings, supports middle mouse and Option+click

Configure in settings

Screenshot 2026-03-08 at 9 34 42 PM
Screen.Recording.2026-03-08.at.9.13.02.PM.mov

only thing is that I cant test the middle mouse button because I dont have one
but claude told me that checking event.buttonNumber == 2 would handle that

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 9, 2026

Greptile Summary

This PR adds a configurable click gesture (middle-click or Option+Click) to close the active browser tab directly from the sidebar, exposed via a new "Close active tab with:" picker in General Settings. The implementation is clean overall — a new ClickDetector NSViewRepresentable wraps an NSEvent local monitor, SettingsStore persists the choice in UserDefaults, and the detector is wired into SidebarView as a background overlay.

Key finding:

  • Event passthrough issue: Both middleMouse and optionClick handlers return the event rather than consuming it (return nil). This means after closeActiveTab() fires, the same gesture continues to AppKit's normal hit-testing and can trigger a secondary action on whatever sidebar element is under the cursor (e.g., selecting a tab on middle-click, triggering an Option-modified action on Option+Click). Since the feature is named "Close active tab with:", these should be exclusive gestures — the event should be consumed by returning nil from the monitor handlers instead.

The core feature works correctly and persistence is handled properly. The persistence layer and settings UI are straightforward and well-implemented.

Confidence Score: 2/5

  • PR has a real behavioral issue with event passthrough that can cause double-actions, but the fix is straightforward.
  • The event-passthrough behavior is a concrete functional issue: clicking with the configured gesture will both close the active tab AND trigger the underlying AppKit action on the sidebar element (e.g., selecting a different tab on middle-click). This contradicts the feature name/intent. The fix is clear and isolated to two return statements in the monitor handlers. The persistence layer and UI are correctly implemented. Once the event consumption is fixed, the PR will be safe to merge.
  • ora/Core/Platform/Representables/ClickDetector.swift — the event monitor handlers need to consume events by returning nil instead of return event.

Sequence Diagram

sequenceDiagram
    participant User
    participant AppKit
    participant Coordinator as ClickDetector.Coordinator
    participant TabManager
    participant SidebarView

    User->>AppKit: Middle-click or Option+Click
    AppKit->>Coordinator: NSEvent local monitor callback
    Coordinator->>Coordinator: isOverView(event)?
    alt cursor inside sidebar bounds
        Coordinator->>Coordinator: fire(onClick) — throttle check
        Coordinator->>TabManager: closeActiveTab()
        Coordinator-->>AppKit: return event (not consumed)
        AppKit->>SidebarView: event propagates to normal hit-testing
    else cursor outside sidebar
        Coordinator-->>AppKit: return event (pass through)
    end
Loading

Last reviewed commit: 78bda0d

@AryanRogye
Copy link
Copy Markdown
Contributor Author

lmk, I’ll fix the lint issues soon. Just pushed this while taking a break from studying for an exam.

@tembo tembo bot added the enhancement improvements label Mar 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant