Repository navigation
Route portal drop overlays through content - #117
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR conditionally suppresses inline drop overlays for portal-rendered tab kinds by adding ChangesPortal Drop Overlay Routing
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 introduces a mechanism to suppress Bonsplit's inline SwiftUI drop placeholder for tab kinds whose content renders its own overlay above an AppKit portal (e.g. terminal, browser), while still forwarding the active drop zone to those views via the
Confidence Score: 4/5Safe to merge; the change is well-scoped and the drop interaction path is not regressed. The routing logic is correct and the environment forwarding of activeDropZone to portal content is preserved. One test case uses nil as both input and expected value, so it cannot detect a regression where the suppression guard is removed. Tests/BonsplitTests/BonsplitTests.swift — the nil-input assertion in the suppression test. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Drag enters pane] --> B[UnifiedPaneDropDelegate sets activeDropZone]
B --> C[PaneContainerView recomputes inlineDropZone]
C --> D{selectedTabKind in portalDropOverlayTabKinds?}
D -- Yes --> E[inlineDropZone = nil - Suppress SwiftUI placeholder]
D -- No / nil --> F[inlineDropZone = activeDropZone - Show SwiftUI placeholder]
E --> G[PaneDropInteractionContainer renders invisible placeholder]
F --> G2[PaneDropInteractionContainer renders visible placeholder]
B --> H[contentArea receives .environment paneDropZone activeDropZone]
H --> I[Portal-hosted AppKit content renders its own overlay]
Reviews (1): Last reviewed commit: "Route portal drop overlays through conte..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| XCTAssertEqual(PaneDropOverlayRouting.inlineDropZone( | ||
| activeDropZone: .left, | ||
| selectedTabKind: "browser", | ||
| portalDropOverlayTabKinds: config.portalDropOverlayTabKinds | ||
| ), .left) | ||
| XCTAssertNil(PaneDropOverlayRouting.inlineDropZone( |
There was a problem hiding this comment.
This assertion passes regardless of whether the suppression logic exists: when activeDropZone is nil, both the suppression path (return nil) and the passthrough path (return activeDropZone) produce nil. A failing test would not be generated if the guard were removed and the function simply returned activeDropZone in all cases. Replacing the input with a non-nil drop zone (e.g., .left) would make this case actually verify that the overlay is suppressed for a portal kind.
1 participant
Summary
portalDropOverlayTabKindsto identify tab content that renders pane drop overlays above AppKit portals.paneDropZoneto content.terminalandbrowser.Testing
swift test, 114 tests passed.Summary by cubic
Routes pane drop overlays through tab content for portal-backed tabs to avoid conflicts with AppKit portals. Defaults to
terminalandbrowser, and can be configured.BonsplitConfiguration.portalDropOverlayTabKinds(default:["terminal", "browser"]).paneDropZoneso content can render the overlay above the portal.PaneDropOverlayRouting.inlineDropZoneto compute the effective inline drop zone.Written for commit 5b38ac9. Summary will update on new commits.
Summary by CodeRabbit
New Features
portalDropOverlayTabKindsconfiguration option to control which tab types display drop overlays from portal sources. Defaults to "terminal" and "browser" tabs.Tests