Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves popup positioning and fixes hit-testing issues on iOS 16.4-16.26 by introducing a more reliable CALayer-based hit detection mechanism and adding automatic positioning capabilities.
Changes:
- Refactored hit-testing to use CALayer.hitTest with a PopupBackgroundFrameMarker for reliable tap detection on popup backgrounds vs content
- Added
fitsScreenparameter to.anchorRelativeand new.autopositioning mode that automatically selects the best corner to minimize overflow - Downgraded minimum iOS version from 17.0 to 16.0
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| WindowManager.swift | Replaced UIPassthroughVC class and refactored hitTest logic to use CALayer-based detection with PopupBackgroundFrameMarker |
| Utils.swift | Added PopupBackgroundFrameMarker UIViewRepresentable and wrapped AnimatedBackgroundView in ZStack to support new hit-testing |
| PublicAPI.swift | Added fitsScreen parameter to anchorRelative and new .auto positioning case |
| AnchoredAnimationManager.swift | Implemented automatic positioning algorithm with overflow scoring, clamping logic, and safe area bounds calculation |
| Package.swift | Downgraded iOS deployment target from 17.0 to 16.0 |
| README.md | Updated documentation to describe new positioning options |
| ProfileView.swift | Updated example to demonstrate .auto positioning |
| project.pbxproj | Updated example project deployment target to 16.0 |
| .gitignore | Added .derivedData/ entry |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let tieBreaker = abs(point.x - preferredX) * 0.001 + abs(point.y - preferredY) * 0.001 | ||
| return overflow + tieBreaker |
There was a problem hiding this comment.
The tie-breaker weight of 0.001 in the overflow scoring could theoretically cause issues if the total overflow is very small (e.g., less than 0.001 points). While unlikely in practice, this could lead to the tie-breaker dominating the decision. Consider using a more robust tie-breaking mechanism, such as comparing overflow scores first and only using the preference as a secondary criterion when overflow values are equal.
| private func safeAreaBounds() -> CGRect { | ||
| if let window = WindowManager.shared.windows[id] { | ||
| return window.bounds.inset(by: window.safeAreaInsets) | ||
| } | ||
|
|
||
| return UIScreen.main.bounds | ||
| } |
There was a problem hiding this comment.
The fallback to UIScreen.main.bounds when the window is not found doesn't account for safe area insets, which is inconsistent with the primary path that uses window.bounds.inset(by: window.safeAreaInsets). This could lead to incorrect positioning when the window hasn't been created yet. Consider using a consistent approach that accounts for safe area insets in both cases, or document why the fallback intentionally ignores safe area.
| public enum AnchoredPopupPosition { | ||
| case anchorRelative(_ point: UnitPoint) // popup view will be aligned to anchor view at corresponding proportion | ||
| case anchorRelative(_ point: UnitPoint, fitsScreen: Bool = true) // popup view will be aligned to anchor view at corresponding proportion | ||
| case auto // similar to `anchorRelative(..., fitsScreen: true)` but auto-picks the best anchor `UnitPoint` to keep the popup within safe area |
There was a problem hiding this comment.
The new .auto positioning case lacks inline documentation explaining what it does, while the other cases have comments. Consider adding a more detailed comment explaining that .auto automatically selects the best corner (topLeading, topTrailing, bottomLeading, or bottomTrailing) to minimize popup overflow outside the safe area, preferring to stay on the same side of the screen as the anchor.
| case auto // similar to `anchorRelative(..., fitsScreen: true)` but auto-picks the best anchor `UnitPoint` to keep the popup within safe area | |
| case auto // automatically chooses one of the four corner alignments (topLeading, topTrailing, bottomLeading, bottomTrailing) to minimize popup overflow outside the safe area, while preferring to stay on the same side of the screen as the anchor; conceptually similar to `anchorRelative(..., fitsScreen: true)` but with automatic `UnitPoint` selection |
| let superlayerDelegateName = layerHitTestResult?.superlayer?.delegate.map { String(describing: type(of: $0)) } | ||
| let isTappedOnBackground = superlayerDelegateName?.contains(String(describing: PopupBackgroundFrameMarker.self)) ?? false |
There was a problem hiding this comment.
The hit-testing logic relies on string matching against the delegate type name which is fragile. If the PopupBackgroundFrameMarker is ever renamed or if the view hierarchy changes, this detection will silently fail. Consider adding a protocol conformance or using a more robust marker mechanism (e.g., a unique tag or associated object) to identify the background marker view.
| let superlayerDelegateName = layerHitTestResult?.superlayer?.delegate.map { String(describing: type(of: $0)) } | |
| let isTappedOnBackground = superlayerDelegateName?.contains(String(describing: PopupBackgroundFrameMarker.self)) ?? false | |
| let isTappedOnBackground = layerHitTestResult?.superlayer?.delegate is PopupBackgroundFrameMarker |
| // If popup is larger than bounds, keep as much visible as possible by clamping using bounds half-size | ||
| let clampedHalfW = min(halfW, bounds.width / 2) | ||
| let clampedHalfH = min(halfH, bounds.height / 2) | ||
|
|
||
| let minCenterX = bounds.minX + clampedHalfW | ||
| let maxCenterX = bounds.maxX - clampedHalfW | ||
| let minCenterY = bounds.minY + clampedHalfH | ||
| let maxCenterY = bounds.maxY - clampedHalfH |
There was a problem hiding this comment.
The clamping logic for popups larger than bounds may not work as expected. When clampedHalfW and clampedHalfH are clamped to bounds.width / 2 and bounds.height / 2 respectively, and the popup is larger than the bounds, the min and max center constraints become equal (minCenterX == maxCenterX and minCenterY == maxCenterY), which effectively centers the popup but may not be the intended behavior. Consider documenting this edge case behavior or adding a comment explaining the intended outcome when the popup exceeds bounds.
This PR fixes incorrect hit-testing/passthrough behavior on iOS 16.0-26 for AnchoredPopup windows by reliably distinguishing taps on popup background (including EmptyView) vs taps on popup content.
It adds
fitsScreen: Booloption foranchorRelativeposition. WhenfitsScreenistrueit will try to keep the popup within the screen safe areaAlso, it adds '.auto' positioning mode which works like
.anchorRelative(..., fitsScreen: true), but automatically picks the bestUnitPoint(corner) to keep the popup within the safe areaDowngrades min iOS version to 16 as it works fine on it.
Regarding the hit testing thing:
Problem
UIKit view hit-testing in an overlay UIWindow can return inconsistent results for SwiftUI-hosted hierarchies (especially when the background is .none/EmptyView). This caused taps on content to be misclassified as "outside/background" (or vice versa), breaking closeOnTapOutside and passthrough behavior.
Solution
Add a lightweight SwiftUI -> UIKit marker: PopupBackgroundFrameMarker (a UIViewRepresentable backed by a clear UIView).
Insert it into AnimatedBackgroundView so it always exists for the popup background layer
Update UIPassthroughWindow.hitTest to use CALayer.hitTest and inspect the superlayer.delegate chain to detect when the hit came from the background marker
If tapped on background -> optionally dismiss (closeOnTapOutside), and either passthrough (nil) or swallow
If tapped on content -> return super.hitTest(...) to preserve normal interaction (deepest descendant view)