Repository navigation
Fix browser panel resize flicker during split drag - #2513
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR modifies the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes visible flicker in browser panes during split-divider drags by skipping the redundant portal-wide geometry sync when an interactive drag is already in progress. The change is small and well-targeted:
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["NSSplitView.didResizeSubviewsNotification fires"] --> B{"splitView.window === window?"}
B -- No --> Z["return false\n(ignore)"]
B -- Yes --> C{"splitView.isDescendant\nof hostView?"}
C -- Yes --> Z2["return false\n(DevTools internal resize — ignore)"]
C -- No --> D{"isInteractiveSplitDividerDrag\nin window?"}
D -- "pressedMouseButtons bit 0 == 0\nor no currentEvent\nor event stale > 0.1s\nor event.window ≠ window\nor event type not drag/down" --> E["return true\n→ scheduleExternalGeometrySynchronize()"]
D -- "Left button held + recent drag event\nin same window" --> F["return false\n(suppress — browser host anchor\nalready handling geometry)"]
Reviews (1): Last reviewed commit: "Avoid duplicate browser portal sync duri..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| private static func isInteractiveSplitDividerDrag(in window: NSWindow) -> Bool { | ||
| guard (NSEvent.pressedMouseButtons & 1) != 0 else { return false } | ||
| guard let event = NSApp.currentEvent else { return false } | ||
| let now = ProcessInfo.processInfo.systemUptime | ||
| guard (now - event.timestamp) < 0.1 else { return false } | ||
| guard event.window === window else { return false } | ||
| switch event.type { | ||
| case .leftMouseDown, .leftMouseDragged: | ||
| return true | ||
| default: | ||
| return false | ||
| } |
There was a problem hiding this comment.
isInteractiveSplitDividerDrag is broader than its name implies
The function name suggests it specifically detects a split-divider drag, but the implementation detects any left-mouse drag (.leftMouseDown / .leftMouseDragged) occurring in window. There is no check that the drag event originated on an NSSplitView divider hit-test region.
In practice this means: if the user is performing a non-divider left-button drag in the same window — for example, selecting text in a terminal pane, dragging a tab, or drawing a selection rectangle — and a NSSplitView.didResizeSubviewsNotification fires concurrently (e.g., from a programmatic split-position change), shouldTreatSplitResizeAsExternalGeometry will return false and the external-geometry sync will be silently skipped. The portal would then be stale until the next unrelated trigger fires the sync.
The risk is low in the current codebase since programmatic split resizes during unrelated drags are uncommon, but the name and intent are misleading. Consider either (a) narrowing the check to events whose locationInWindow falls within any NSSplitView divider rect in the window, or (b) renaming the function and adding a comment that it is intentionally a broader "left-mouse-drag" heuristic.
| guard (NSEvent.pressedMouseButtons & 1) != 0 else { return false } | ||
| guard let event = NSApp.currentEvent else { return false } | ||
| let now = ProcessInfo.processInfo.systemUptime | ||
| guard (now - event.timestamp) < 0.1 else { return false } |
There was a problem hiding this comment.
0.1 should be a named constant
The staleness threshold 0.1 (seconds) is an unexplained magic number. Extracting it into a named constant makes the intent clear and makes future tuning easier.
| guard (now - event.timestamp) < 0.1 else { return false } | |
| let eventStalenessThreshold = 0.1 // seconds; events older than this are not treated as current drag activity | |
| guard (now - event.timestamp) < eventStalenessThreshold else { return false } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/BrowserWindowPortal.swift">
<violation number="1" location="Sources/BrowserWindowPortal.swift:2129">
P2: This branch classifies any left-button drag in the window as a split-divider drag. Narrow the detection to actual `NSSplitView` divider interaction (e.g., divider hit-testing), otherwise external geometry sync can be skipped during unrelated drags and leave portal geometry stale until a later trigger.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| guard (now - event.timestamp) < 0.1 else { return false } | ||
| guard event.window === window else { return false } | ||
| switch event.type { | ||
| case .leftMouseDown, .leftMouseDragged: |
There was a problem hiding this comment.
P2: This branch classifies any left-button drag in the window as a split-divider drag. Narrow the detection to actual NSSplitView divider interaction (e.g., divider hit-testing), otherwise external geometry sync can be skipped during unrelated drags and leave portal geometry stale until a later trigger.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/BrowserWindowPortal.swift, line 2129:
<comment>This branch classifies any left-button drag in the window as a split-divider drag. Narrow the detection to actual `NSSplitView` divider interaction (e.g., divider hit-testing), otherwise external geometry sync can be skipped during unrelated drags and leave portal geometry stale until a later trigger.</comment>
<file context>
@@ -2111,7 +2111,26 @@ final class WindowBrowserPortal: NSObject {
+ guard (now - event.timestamp) < 0.1 else { return false }
+ guard event.window === window else { return false }
+ switch event.type {
+ case .leftMouseDown, .leftMouseDragged:
+ return true
+ default:
</file context>
This branch was successfully deployed
1 participant
Summary
isInteractiveSplitDividerDragcheck to prevent double WebKit refresh that caused visible flicker in browser panesCloses #2503
Test plan
🤖 Generated with Claude Code
Summary by cubic
Fixes browser pane flicker during split resize by skipping redundant portal-wide geometry sync while a divider drag is in progress. This prevents double WebKit refresh and makes resizing smooth.
isInteractiveSplitDividerDraghelper that detects recent left-mouse drag events on the same window.Written for commit 2dbb8c2. Summary will update on new commits.
Summary by CodeRabbit