Conversation
Fix missing touches on the popup view
There was a problem hiding this comment.
Pull request overview
This PR updates the window-based popup presentation to improve background hit-testing so that .allowTapThroughBG(true) can still support outside-tap dismissal (and broader gesture dismissal behavior), including iOS 26+ behavior changes, by introducing a dedicated hit-test marker view.
Changes:
- Refactors
WindowManager.showInNewWindowto always use a customUIPassthroughWindowwith configurable pass-through + outside-tap dismissal behavior. - Reworks
UIPassthroughWindow.hitTestto detect “background hits” using a dedicatedPopupHitTestingBackgroundmarker. - Updates popup background rendering to include the new hit-test marker view.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| Sources/PopupView/WindowManager.swift | Refactors the window creation + implements new layer-based background hit-testing and dismissal in UIPassthroughWindow. |
| Sources/PopupView/PopupBackgroundView.swift | Adds PopupHitTestingBackground marker view to support the new hit-testing approach. |
| Sources/PopupView/FullscreenPopup.swift | Updates .window display mode to use the new showInNewWindow API and dismissal closure wiring. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| PopupHitTestingBackground() // Hit testing workaround | ||
| .ignoresSafeArea() |
There was a problem hiding this comment.
The previous tap-outside dismissal logic (using closeOnTapOutside + dismissEnabled with a tap gesture/content shape) was removed from PopupBackgroundView. Because this view is also used in .overlay and .sheet modes, those modes will no longer dismiss on outside taps. Please restore the dismissal gesture for non-window presentations (and keep the hit-test marker approach scoped to the .window/UIWindow path).
| PopupHitTestingBackground() // Hit testing workaround | |
| .ignoresSafeArea() | |
| .contentShape(Rectangle()) | |
| .onTapGesture { | |
| guard closeOnTapOutside, dismissEnabled.wrappedValue else { return } | |
| dismissSource = .tapOutside | |
| isPresented = false | |
| item = nil | |
| } | |
| if allowTapThroughBG { | |
| PopupHitTestingBackground() // Hit testing workaround for .window presentations | |
| .ignoresSafeArea() | |
| } |
| /// A special view to handle hit-testing on background parts of popup content | ||
| struct PopupHitTestingBackground: UIViewRepresentable { | ||
| func makeUIView(context: Context) -> UIView { | ||
| let view = UIView() | ||
| view.backgroundColor = .clear |
There was a problem hiding this comment.
PopupHitTestingBackground is declared as a UIViewRepresentable without #if os(iOS) || os(tvOS) guarding. Since this package supports macOS/watchOS too, this will fail to compile on non-UIKit platforms. Wrap this type (and its usage) in appropriate platform conditionals and/or provide an NSViewRepresentable implementation for macOS.
| closeOnTapOutside: closeOnTapOutside, | ||
| allowTapThroughBG: allowTapThroughBG, | ||
| dismissClosure: { | ||
| dismissSource = .tapOutside |
There was a problem hiding this comment.
The dismissClosure passed to showInNewWindow sets dismissSource = .tapOutside, but the same closure is also injected as @Environment(\.popupDismiss) inside WindowManager.showInNewWindow. That will cause programmatic dismissals from inside the popup to be reported as .tapOutside. Consider splitting tap-outside vs programmatic dismiss into separate closures (or pass a DismissSource into the closure).
| dismissSource = .tapOutside |
|
|
||
| if didTapBackground { | ||
| if closeOnTapOutside { | ||
| dismissClosure?() |
There was a problem hiding this comment.
hitTest can run multiple times per user interaction (e.g., multi-touch gestures, repeated hit-testing), but when didTapBackground is true it calls dismissClosure?() unconditionally. This can trigger dismissal logic multiple times. Add a guard so dismissal fires only once (e.g., an isDismissing flag or nil out the closure after the first call).
| dismissClosure?() | |
| if let dismiss = dismissClosure { | |
| dismissClosure = nil | |
| dismiss() | |
| } |
| private func isTouchInsideSubview(point: CGPoint, vc: UIView) -> UIView? { | ||
| for subview in vc.subviews { | ||
| if subview.frame.contains(point) { | ||
| return subview | ||
| } |
There was a problem hiding this comment.
isTouchInsideSubview(point:vc:) is no longer referenced anywhere in UIPassthroughWindow after the new layer-based hit-testing logic. Please remove this dead code (or reintroduce a call if it’s still needed) to keep the hit-testing implementation focused.
| id: UUID, | ||
| closeOnTapOutside: Bool, | ||
| allowTapThroughBG: Bool, | ||
| dismissClosure: SendableClosure?, |
There was a problem hiding this comment.
dismissClosure is captured by an escaping environment closure (.environment(\.popupDismiss) { ... }) and stored on the window, but the showInNewWindow parameter isn’t marked @escaping. This will not compile (capturing/storing a non-escaping closure). Mark the parameter as escaping (e.g. dismissClosure: (@escaping SendableClosure)?, or make it non-optional and @escaping).
| dismissClosure: SendableClosure?, | |
| dismissClosure: (@escaping SendableClosure)?, |
| init(windowScene: UIWindowScene, closeOnTapOutside: Bool, isPassthrough: Bool, dismissClosure: SendableClosure?) { | ||
| self.closeOnTapOutside = closeOnTapOutside | ||
| self.isPassthrough = isPassthrough | ||
| self.dismissClosure = dismissClosure | ||
| super.init(windowScene: windowScene) |
There was a problem hiding this comment.
In UIPassthroughWindow.init, dismissClosure is assigned to a stored property, but the initializer parameter is not @escaping. This won’t compile with Swift’s non-escaping default for closure parameters. Update the initializer signature to accept an escaping closure (including the optional case).
| isPresented = false | ||
| item = nil | ||
| }) { | ||
| AnyView(constructPopup()) |
There was a problem hiding this comment.
is it at all possible to avoid AnyView here?
| closeOnTapOutside: Bool, | ||
| allowTapThroughBG: Bool, | ||
| dismissClosure: @escaping ()->(), | ||
| content: @escaping () -> AnyView |
There was a problem hiding this comment.
also here - could you please use generics trick instead of eplicit casting to AnyView?
Fix missing touch handling in the popups view when using
.allowTapThroughBG(true)Supports iOS 26+ with backward compatibility. Uses a dedicated
PopupHitTestingBackgroundview as a hit-test marker.Before:
Screen.Recording.2026-02-07.at.12.37.21.mov
After:
Screen.Recording.2026-02-07.at.12.55.38.mov
It also enables this combination, which now passes the touch through while still dismissing the popup. Previously, the popup did not dismiss:
Also, for this combination, dismissal no longer requires a strict tap gesture (press and release). It now works for swipes, pinch, pan, two-finger gestures, etc: