feat(app): close search & session drawers on outside click#2682
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: f1806be The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Greptile SummaryAdds a small
Confidence Score: 5/5Safe to merge; the change is additive and isolated to drawer dismiss behaviour with clean opt-out paths. The hook is narrow in scope, the depth guards correctly replicate the existing Esc behaviour in both DBRowSidePanelErrorBoundary and SessionSidePanel, and existing consumers that render inside their own role=dialog are naturally neutralised by the floating-layer exemption. The only unaddressed gap is that right-click mousedown events will trigger close, which is a minor UX rough edge rather than a logic defect. packages/app/src/hooks/useCloseOnClickOutside.ts — the right-click filtering gap is the only item worth a follow-up look. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[mousedown captured on document] --> B{event.target instanceof Element?}
B -- No --> Z[return]
B -- Yes --> C{target.closest FLOATING_LAYER_SELECTOR?}
C -- Yes --> Z
C -- No --> D{keepOpenSelector provided?}
D -- Yes --> E{target.closest keepOpenSelector?}
E -- Yes --> Z
E -- No --> F[stableOnClose]
D -- No --> F
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[mousedown captured on document] --> B{event.target instanceof Element?}
B -- No --> Z[return]
B -- Yes --> C{target.closest FLOATING_LAYER_SELECTOR?}
C -- Yes --> Z
C -- No --> D{keepOpenSelector provided?}
D -- Yes --> E{target.closest keepOpenSelector?}
E -- Yes --> Z
E -- No --> F[stableOnClose]
D -- No --> F
Reviews (6): Last reviewed commit: "Merge branch 'main' into elizabet/close-..." | Re-trigger Greptile |
E2E Test Results✅ All tests passed • 240 passed • 1 skipped • 1051s
Tests ran across 4 shards in parallel. |
Co-authored-by: Cursor <cursoragent@cursor.com>
…lickOutside Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
🔵 Tier 2 — Low RiskSmall, isolated change with no API route or data model modifications. Why this tier:
Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns. Stats
|
Deep ReviewScope: New No data-loss, auth, injection, or guaranteed-crash issues were found. The feature is purely client-side UI dismissal built on ✅ No critical issues found. 🟡 P2 -- recommended
🔵 P3 nitpicks (4)
Reviewers (7): testing, maintainability, project-standards, kieran-typescript, previous-comments, agent-native, learnings. Correctness, adversarial, and frontend-races were still running at synthesis time; their primary angles (open/close race, portaled-content dismissal, mouse-button handling) were verified directly against the code and folded into the findings above. Testing gaps:
|
Why
When a detail drawer is open on the Search page, clicking outside of the logs table should dismiss it. Same for the session drawer on the Sessions page. Previously these drawers only closed via the X button, Esc, or breadcrumb navigation.
These drawers are Mantine
Drawers rendered withwithOverlay={false}(so the page stays interactive behind them), which means there is no overlay to catch outside clicks — hence Mantine's built-incloseOnClickOutsidedoes nothing.What
Added a small reusable hook,
useCloseOnClickOutside, and wired it into the row-table / session detail drawers. It's on by default (opt out withcloseOnClickOutside={false}).DBSqlRowTableWithSidebar(row detail drawer) andDirectTraceSidePanel. Keep-open zone = the results panel ([data-testid="search-results-panel"]).SessionSidePanel. Keep-open zone = the session list ([data-testid="session-card-list"]). Gated to the root view (!selectedEvent), mirroring the existing Esc behavior so deep in-panel navigation isn't skipped.DBSqlRowTableWithSidebardefaults its keep-open zone to its own results table ([data-testid="search-results-table"]), so every consumer gets clean row-switching without a page needing to opt in.Behavior
role="dialog"/tooltip/listbox/menu, dropdown classes, overlays) as "inside", which is what makes it robust against portaled content.Reuse safety
DBSqlRowTableWithSidebar/DBRowSidePanelare reused in several places. The other consumers (PodDetailsSidePanel,PatternSidePanel, K8s node/namespace/pod detail panels) render them inside their own MantineDrawer(role="dialog"), so the floating-layer exemption neutralizes the behavior there — clicks inside the parent drawer count as "inside". Dashboards render the table directly, where click-outside-to-close is the desired behavior and the default table keep-open zone prevents any close/reopen jank.Note / possible follow-up
On Search, "outside of the logs table" is taken fairly literally: the keep-open zone is the results panel (table + find-in-page), so clicking the histogram/stats above it also closes the drawer. If we'd rather keep it open across the whole results area, that's a one-line change to which element carries the
data-testid.Testing
useCloseOnClickOutside.test.tsx(4 cases) — passing.SessionSidePanel,DirectTraceSidePanel,DBSearchPage.directTrace, andDBRowSidePanelsuites — passing (30 tests across 7 suites).Made with Cursor