From f509c39ebfc4e5cc4e8e262ee70d916b09600f61 Mon Sep 17 00:00:00 2001 From: Rob MacEachern Date: Wed, 5 Aug 2026 11:20:50 -0500 Subject: [PATCH 1/3] fix: invalidate forwarded presentations when hosts detach --- .../ModalHostContainerViewController.swift | 80 +++++++++- ...odalHostContainerViewControllerTests.swift | 124 +++++++++++++++ .../Sources/ModalHostContainer.swift | 93 ++++++++++- .../Tests/ModalHostContainerTests.swift | 144 ++++++++++++++++++ 4 files changed, 430 insertions(+), 11 deletions(-) diff --git a/Modals/Sources/ModalHostContainerViewController.swift b/Modals/Sources/ModalHostContainerViewController.swift index 01b821d2..6b103d17 100644 --- a/Modals/Sources/ModalHostContainerViewController.swift +++ b/Modals/Sources/ModalHostContainerViewController.swift @@ -31,6 +31,7 @@ public final class ModalHostContainerViewController: UIViewController, ModalHost private var needsModalUpdate = true private var isInModalUpdate = false + private weak var forwardingAncestorModalHost: ModalHost? var logger = ModalsLogging.logger @@ -43,7 +44,15 @@ public final class ModalHostContainerViewController: UIViewController, ModalHost public var presentationFilter: ModalPresentationFilter? { didSet { if presentationFilter?.identifier != oldValue?.identifier { + let formerAncestorModalHost = oldValue != nil && presentationFilter == nil + ? ancestorModalHost + : nil + setNeedsModalUpdate() + + // `setNeedsModalUpdate()` only forwards through the current filter. If this host + // has stopped forwarding, the former ancestor still needs to remove its snapshot. + formerAncestorModalHost?.setNeedsModalUpdate() } } } @@ -98,6 +107,9 @@ public final class ModalHostContainerViewController: UIViewController, ModalHost }, presentationViews: { [unowned modalPresentation, unowned toastPresentation] in [modalPresentation, toastPresentation].map { $0.view } + }, + windowDidChange: { [weak self] window in + self?.modalHostWindowDidChange(window) } ) @@ -120,6 +132,26 @@ public final class ModalHostContainerViewController: UIViewController, ModalHost updatePreferredContentSize() } + public override func willMove(toParent parent: UIViewController?) { + if parent == nil { + clearForwardingAncestorModalHost( + fallback: hasPresentationFilter ? ancestorModalHost : nil + ) + } + + super.willMove(toParent: parent) + } + + public override func didMove(toParent parent: UIViewController?) { + super.didMove(toParent: parent) + + // A host may already own presentations when it is attached to an active hierarchy. + // Ensure the new ancestor includes any forwarded presentations in its next update. + if parent != nil { + setForwardingAncestorModalHostNeedsUpdate() + } + } + public override func viewWillLayoutSubviews() { super.viewWillLayoutSubviews() @@ -188,11 +220,7 @@ public final class ModalHostContainerViewController: UIViewController, ModalHost viewIfLoaded?.setNeedsLayout() modalPresentation.viewIfLoaded?.setNeedsLayout() - if hasPresentationFilter, let ancestorModalHost { - // Some modals may be forwarded to an ancestor host. - // Inform it so that it may update. - ancestorModalHost.setNeedsModalUpdate() - } + setForwardingAncestorModalHostNeedsUpdate() } private func updateModalsIfNeeded() { @@ -240,6 +268,38 @@ public final class ModalHostContainerViewController: UIViewController, ModalHost presentationFilter != nil } + private func modalHostWindowDidChange(_ window: UIWindow?) { + if window == nil { + // An indirect containment removal does not call `willMove(toParent:)` on this host. + // Its view still leaves the window, so invalidate the ancestor cached while attached. + clearForwardingAncestorModalHost() + } else { + setForwardingAncestorModalHostNeedsUpdate() + } + } + + private func setForwardingAncestorModalHostNeedsUpdate() { + let currentAncestorModalHost = hasPresentationFilter ? ancestorModalHost : nil + + if forwardingAncestorModalHost !== currentAncestorModalHost { + // The former host may still display this host's last forwarded snapshot. + forwardingAncestorModalHost?.setNeedsModalUpdate() + forwardingAncestorModalHost = currentAncestorModalHost + } + + // Some presentations may be forwarded to the current ancestor host. + forwardingAncestorModalHost?.setNeedsModalUpdate() + } + + private func clearForwardingAncestorModalHost(fallback: ModalHost? = nil) { + let formerAncestorModalHost = forwardingAncestorModalHost ?? fallback + forwardingAncestorModalHost = nil + + // A forwarding host is part of its ancestor's aggregated modal list. Invalidate that + // snapshot while the former ancestor is still reachable. + formerAncestorModalHost?.setNeedsModalUpdate() + } + // MARK: ToastPresentationViewControllerDelegate public func toastPresentationViewControllerDidChange(hasVisiblePresentations: Bool) { @@ -270,15 +330,18 @@ private final class ModalHostView: UIView { private let passthroughSizeThatFits: (CGSize) -> CGSize private let ancestorPresentationView: () -> UIView? private let presentationViews: () -> [UIView] + private let windowDidChange: (UIWindow?) -> Void init( frame: CGRect, sizeThatFits: @escaping (CGSize) -> CGSize, ancestorPresentationView: @escaping () -> UIView?, - presentationViews: @escaping () -> [UIView] + presentationViews: @escaping () -> [UIView], + windowDidChange: @escaping (UIWindow?) -> Void ) { self.ancestorPresentationView = ancestorPresentationView self.presentationViews = presentationViews + self.windowDidChange = windowDidChange passthroughSizeThatFits = sizeThatFits super.init(frame: frame) @@ -288,6 +351,11 @@ private final class ModalHostView: UIView { fatalError() } + override func didMoveToWindow() { + super.didMoveToWindow() + windowDidChange(window) + } + override func sizeThatFits(_ size: CGSize) -> CGSize { passthroughSizeThatFits(size) } diff --git a/Modals/Tests/ModalHostContainerViewControllerTests.swift b/Modals/Tests/ModalHostContainerViewControllerTests.swift index 716a586b..4593402e 100644 --- a/Modals/Tests/ModalHostContainerViewControllerTests.swift +++ b/Modals/Tests/ModalHostContainerViewControllerTests.swift @@ -125,6 +125,130 @@ final class ModalHostContainerViewControllerTests: XCTestCase { ) } } + + func test_attaching_forwarding_host_invalidates_ancestor_for_existing_toast() { + let innerContent = UIViewController() + let innerHost = ModalHostContainerViewController(content: innerContent) + let outerHost = ModalHostContainerViewController(content: UIViewController()) + + let lifetime = innerContent.toastPresenter.present( + UIViewController(), + style: .init(ToastPresentationStyleFixture()), + accessibilityAnnouncement: "Toast." + ) + defer { lifetime.dismiss() } + + show(vc: outerHost) { outerHost in + XCTAssertTrue(outerHost.toastPresentation.presentedViewControllers.isEmpty) + + nest(innerHost, in: outerHost) + outerHost.view.layoutIfNeeded() + + XCTAssertTrue(innerHost.toastPresentation.presentedViewControllers.isEmpty) + XCTAssertEqual(outerHost.toastPresentation.presentedViewControllers.count, 1) + } + } + + func test_removing_forwarding_host_invalidates_ancestor_for_toast() { + let innerContent = UIViewController() + let innerHost = ModalHostContainerViewController(content: innerContent) + let outerHost = ModalHostContainerViewController(content: UIViewController()) + nest(innerHost, in: outerHost) + + let lifetime = innerContent.toastPresenter.present( + UIViewController(), + style: .init(ToastPresentationStyleFixture()), + accessibilityAnnouncement: "Toast." + ) + defer { lifetime.dismiss() } + + show(vc: outerHost) { outerHost in + innerHost.view.layoutIfNeeded() + XCTAssertEqual(outerHost.toastPresentation.presentedViewControllers.count, 1) + + innerHost.willMove(toParent: nil) + innerHost.view.removeFromSuperview() + innerHost.removeFromParent() + outerHost.view.layoutIfNeeded() + + XCTAssertEqual(innerContent.aggregateModals().toasts.count, 1) + XCTAssertTrue(outerHost.toastPresentation.presentedViewControllers.isEmpty) + } + } + + func test_removing_ancestor_of_forwarding_host_invalidates_outer_host_for_toast() { + let innerContent = UIViewController() + let innerHost = ModalHostContainerViewController(content: innerContent) + + let container = UIViewController() + container.addChild(innerHost) + container.view.addSubview(innerHost.view) + innerHost.didMove(toParent: container) + + let outerHost = ModalHostContainerViewController(content: UIViewController()) + nest(container, in: outerHost) + + let lifetime = innerContent.toastPresenter.present( + UIViewController(), + style: .init(ToastPresentationStyleFixture()), + accessibilityAnnouncement: "Toast." + ) + defer { lifetime.dismiss() } + + show(vc: outerHost) { outerHost in + innerHost.view.layoutIfNeeded() + XCTAssertEqual(outerHost.toastPresentation.presentedViewControllers.count, 1) + + container.willMove(toParent: nil) + container.view.removeFromSuperview() + container.removeFromParent() + outerHost.view.layoutIfNeeded() + + XCTAssertEqual(innerContent.aggregateModals().toasts.count, 1) + XCTAssertTrue(outerHost.toastPresentation.presentedViewControllers.isEmpty) + } + } + + func test_stopping_forwarding_invalidates_former_ancestor_for_modal() { + let innerContent = UIViewController() + let innerHost = ModalHostContainerViewController( + content: innerContent, + toastContainerStyle: .fixture, + presentationFilter: .containsUniqueKey(TestModalInfoKey.self) + ) + let outerHost = ModalHostContainerViewController(content: UIViewController()) + nest(innerHost, in: outerHost) + + let lifetime = innerContent.modalPresenter.present( + UIViewController(), + style: .testFull(), + info: .empty(), + completion: nil + ) + defer { lifetime.dismiss() } + + show(vc: outerHost) { outerHost in + innerHost.view.layoutIfNeeded() + XCTAssertTrue(innerHost.modalPresentation.presentedViewControllers.isEmpty) + XCTAssertEqual(outerHost.modalPresentation.presentedViewControllers.count, 1) + + innerHost.presentationFilter = nil + innerHost.view.layoutIfNeeded() + outerHost.view.layoutIfNeeded() + + XCTAssertEqual(innerHost.modalPresentation.presentedViewControllers.count, 1) + XCTAssertTrue(outerHost.modalPresentation.presentedViewControllers.isEmpty) + } + } + + private func nest( + _ child: UIViewController, + in outerHost: ModalHostContainerViewController + ) { + outerHost.content.addChild(child) + outerHost.content.view.addSubview(child.view) + child.didMove(toParent: outerHost.content) + } } private enum TestModalInfoKey: UniqueModalInfoKey {} diff --git a/WorkflowModals/Sources/ModalHostContainer.swift b/WorkflowModals/Sources/ModalHostContainer.swift index 412454ac..83001b5e 100644 --- a/WorkflowModals/Sources/ModalHostContainer.swift +++ b/WorkflowModals/Sources/ModalHostContainer.swift @@ -148,6 +148,10 @@ extension ModalHostContainer: Screen where Content: Screen { private var needsModalUpdate = true private var isInModalUpdate = false + private weak var forwardingAncestorModalHost: ModalHost? + private lazy var modalHostWindowObserverView = ModalHostWindowObserverView { [weak self] window in + self?.modalHostWindowDidChange(window) + } required init(screen: ModalHostContainer, environment: ViewEnvironment) { content = screen @@ -175,6 +179,7 @@ extension ModalHostContainer: Screen where Content: Screen { modalPresentationController.view.frame = view.bounds view.addSubview(modalPresentationController.view) + view.addSubview(modalHostWindowObserverView) addOrRemoveToastPresentationSubviewIfNecessary( hasVisiblePresentations: toastPresentationController.hasVisiblePresentations @@ -183,6 +188,26 @@ extension ModalHostContainer: Screen where Content: Screen { updatePreferredContentSize() } + override func willMove(toParent parent: UIViewController?) { + if parent == nil { + clearForwardingAncestorModalHost( + fallback: hasPresentationFilter ? ancestorModalHost : nil + ) + } + + super.willMove(toParent: parent) + } + + override func didMove(toParent parent: UIViewController?) { + super.didMove(toParent: parent) + + // A host may already own presentations when it is attached to an active hierarchy. + // Ensure the new ancestor includes any forwarded presentations in its next update. + if parent != nil { + setForwardingAncestorModalHostNeedsUpdate() + } + } + public override func viewWillLayoutSubviews() { super.viewWillLayoutSubviews() modalPresentationController.view.frame = view.bounds @@ -200,7 +225,16 @@ extension ModalHostContainer: Screen where Content: Screen { ) if previousScreen.presentationFilter?.identifier != screen.presentationFilter?.identifier { + let formerAncestorModalHost = previousScreen.presentationFilter != nil + && screen.presentationFilter == nil + ? ancestorModalHost + : nil + setNeedsModalUpdate() + + // `setNeedsModalUpdate()` only forwards through the current filter. If this host + // has stopped forwarding, the former ancestor still needs to remove its snapshot. + formerAncestorModalHost?.setNeedsModalUpdate() } } @@ -251,11 +285,7 @@ extension ModalHostContainer: Screen where Content: Screen { viewIfLoaded?.setNeedsLayout() modalPresentationController.viewIfLoaded?.setNeedsLayout() - if hasPresentationFilter, let ancestorModalHost { - // Some modals may be forwarded to an ancestor host. - // Inform it so that it may update. - ancestorModalHost.setNeedsModalUpdate() - } + setForwardingAncestorModalHostNeedsUpdate() } private func updateModalsIfNeeded() { @@ -303,6 +333,38 @@ extension ModalHostContainer: Screen where Content: Screen { screen.presentationFilter != nil } + private func modalHostWindowDidChange(_ window: UIWindow?) { + if window == nil { + // An indirect containment removal does not call `willMove(toParent:)` on this host. + // Its view still leaves the window, so invalidate the ancestor cached while attached. + clearForwardingAncestorModalHost() + } else { + setForwardingAncestorModalHostNeedsUpdate() + } + } + + private func setForwardingAncestorModalHostNeedsUpdate() { + let currentAncestorModalHost = hasPresentationFilter ? ancestorModalHost : nil + + if forwardingAncestorModalHost !== currentAncestorModalHost { + // The former host may still display this host's last forwarded snapshot. + forwardingAncestorModalHost?.setNeedsModalUpdate() + forwardingAncestorModalHost = currentAncestorModalHost + } + + // Some presentations may be forwarded to the current ancestor host. + forwardingAncestorModalHost?.setNeedsModalUpdate() + } + + private func clearForwardingAncestorModalHost(fallback: ModalHost? = nil) { + let formerAncestorModalHost = forwardingAncestorModalHost ?? fallback + forwardingAncestorModalHost = nil + + // A forwarding host is part of its ancestor's aggregated modal list. Invalidate that + // snapshot while the former ancestor is still reachable. + formerAncestorModalHost?.setNeedsModalUpdate() + } + // MARK: ToastPresentationViewControllerDelegate func toastPresentationViewControllerDidChange(hasVisiblePresentations: Bool) { @@ -338,6 +400,27 @@ extension ModalHostContainer: Screen where Content: Screen { } } +private final class ModalHostWindowObserverView: UIView { + private let windowDidChange: (UIWindow?) -> Void + + init(windowDidChange: @escaping (UIWindow?) -> Void) { + self.windowDidChange = windowDidChange + super.init(frame: .zero) + isHidden = true + isUserInteractionEnabled = false + } + + @available(*, unavailable) + required init?(coder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } + + override func didMoveToWindow() { + super.didMoveToWindow() + windowDidChange(window) + } +} + extension ModalHostContainer: SingleScreenContaining where Content: Screen { public var primaryScreen: Screen { content diff --git a/WorkflowModals/Tests/ModalHostContainerTests.swift b/WorkflowModals/Tests/ModalHostContainerTests.swift index bc677472..1ec124ae 100644 --- a/WorkflowModals/Tests/ModalHostContainerTests.swift +++ b/WorkflowModals/Tests/ModalHostContainerTests.swift @@ -380,6 +380,118 @@ class ModalHostContainerTests: XCTestCase { } } + func test_attaching_forwarding_host_invalidates_ancestor_for_existing_toast() { + let innerHost = makeToastHost() + let outerHost = makeToastHost() + + let lifetime = innerHost.content.toastPresenter.present( + UIViewController(), + style: .init(ToastPresentationStyleFixture()), + accessibilityAnnouncement: "Toast." + ) + defer { lifetime.dismiss() } + + show(vc: outerHost) { outerHost in + XCTAssertTrue(outerHost.toastPresentationController.presentedViewControllers.isEmpty) + + nest(innerHost, in: outerHost) + outerHost.view.layoutIfNeeded() + + XCTAssertTrue(innerHost.toastPresentationController.presentedViewControllers.isEmpty) + XCTAssertEqual(outerHost.toastPresentationController.presentedViewControllers.count, 1) + } + } + + func test_removing_forwarding_host_invalidates_ancestor_for_toast() { + let innerHost = makeToastHost() + let outerHost = makeToastHost() + nest(innerHost, in: outerHost) + + let lifetime = innerHost.content.toastPresenter.present( + UIViewController(), + style: .init(ToastPresentationStyleFixture()), + accessibilityAnnouncement: "Toast." + ) + defer { lifetime.dismiss() } + + show(vc: outerHost) { outerHost in + innerHost.view.layoutIfNeeded() + XCTAssertEqual(outerHost.toastPresentationController.presentedViewControllers.count, 1) + + innerHost.willMove(toParent: nil) + innerHost.view.removeFromSuperview() + innerHost.removeFromParent() + outerHost.view.layoutIfNeeded() + + XCTAssertEqual(innerHost.content.aggregateModals().toasts.count, 1) + XCTAssertTrue(outerHost.toastPresentationController.presentedViewControllers.isEmpty) + } + } + + func test_removing_ancestor_of_forwarding_host_invalidates_outer_host_for_toast() { + let innerHost = makeToastHost() + let outerHost = makeToastHost() + + let container = UIViewController() + container.addChild(innerHost) + container.view.addSubview(innerHost.view) + innerHost.didMove(toParent: container) + nest(container, in: outerHost) + + let lifetime = innerHost.content.toastPresenter.present( + UIViewController(), + style: .init(ToastPresentationStyleFixture()), + accessibilityAnnouncement: "Toast." + ) + defer { lifetime.dismiss() } + + show(vc: outerHost) { outerHost in + innerHost.view.layoutIfNeeded() + XCTAssertEqual(outerHost.toastPresentationController.presentedViewControllers.count, 1) + + container.willMove(toParent: nil) + container.view.removeFromSuperview() + container.removeFromParent() + outerHost.view.layoutIfNeeded() + + XCTAssertEqual(innerHost.content.aggregateModals().toasts.count, 1) + XCTAssertTrue(outerHost.toastPresentationController.presentedViewControllers.isEmpty) + } + } + + func test_stopping_forwarding_invalidates_former_ancestor_for_modal() { + let innerHost = makeModalFilteringHost() + let outerHost = makeToastHost() + nest(innerHost, in: outerHost) + + let lifetime = innerHost.content.modalPresenter.present( + UIViewController(), + style: .init(FullScreenModalStyle()), + info: .empty(), + completion: nil + ) + defer { lifetime.dismiss() } + + show(vc: outerHost) { outerHost in + innerHost.view.layoutIfNeeded() + XCTAssertTrue(innerHost.modalPresentationController.presentedViewControllers.isEmpty) + XCTAssertEqual(outerHost.modalPresentationController.presentedViewControllers.count, 1) + + innerHost.update( + screen: .init( + content: EmptyScreen(), + toastContainerStyle: .fixture, + presentationFilter: nil + ) + ) + innerHost.view.layoutIfNeeded() + outerHost.view.layoutIfNeeded() + + XCTAssertEqual(innerHost.modalPresentationController.presentedViewControllers.count, 1) + XCTAssertTrue(outerHost.modalPresentationController.presentedViewControllers.isEmpty) + } + } + func test_preferredContentSize() { struct TestScreen: Screen { @@ -417,4 +529,36 @@ class ModalHostContainerTests: XCTestCase { hostContainer.view.layoutIfNeeded() XCTAssertEqual(hostContainer.preferredContentSize, CGSize(width: axisSize, height: axisSize)) } + + private func makeToastHost() -> ModalHostContainer.ViewController { + ModalHostContainer.ViewController( + screen: .init( + content: EmptyScreen(), + toastContainerStyle: .fixture + ), + environment: .empty + ) + } + + private func makeModalFilteringHost() -> ModalHostContainer.ViewController { + ModalHostContainer.ViewController( + screen: .init( + content: EmptyScreen(), + toastContainerStyle: .fixture, + presentationFilter: .containsUniqueKey(ForwardingTestModalInfoKey.self) + ), + environment: .empty + ) + } + + private func nest( + _ child: UIViewController, + in outerHost: ModalHostContainer.ViewController + ) { + outerHost.content.addChild(child) + outerHost.content.view.addSubview(child.view) + child.didMove(toParent: outerHost.content) + } } + +private enum ForwardingTestModalInfoKey: UniqueModalInfoKey {} From d769d00a8286d191cb7c39c0fdc56cf92ec2c8ed Mon Sep 17 00:00:00 2001 From: Rob MacEachern Date: Wed, 5 Aug 2026 12:06:27 -0500 Subject: [PATCH 2/3] test: cover forwarded presentation ancestor changes --- .../ModalHostContainerViewController.swift | 4 +- ...odalHostContainerViewControllerTests.swift | 41 +++++++++++++++++++ .../Sources/ModalHostContainer.swift | 4 +- .../Tests/ModalHostContainerTests.swift | 40 ++++++++++++++++++ 4 files changed, 85 insertions(+), 4 deletions(-) diff --git a/Modals/Sources/ModalHostContainerViewController.swift b/Modals/Sources/ModalHostContainerViewController.swift index 6b103d17..0f2e3613 100644 --- a/Modals/Sources/ModalHostContainerViewController.swift +++ b/Modals/Sources/ModalHostContainerViewController.swift @@ -50,8 +50,8 @@ public final class ModalHostContainerViewController: UIViewController, ModalHost setNeedsModalUpdate() - // `setNeedsModalUpdate()` only forwards through the current filter. If this host - // has stopped forwarding, the former ancestor still needs to remove its snapshot. + // `setNeedsModalUpdate()` invalidates a previously tracked ancestor. If forwarding + // was never tracked, invalidate the ancestor still reachable through containment. formerAncestorModalHost?.setNeedsModalUpdate() } } diff --git a/Modals/Tests/ModalHostContainerViewControllerTests.swift b/Modals/Tests/ModalHostContainerViewControllerTests.swift index 4593402e..0b9bd9ea 100644 --- a/Modals/Tests/ModalHostContainerViewControllerTests.swift +++ b/Modals/Tests/ModalHostContainerViewControllerTests.swift @@ -209,6 +209,47 @@ final class ModalHostContainerViewControllerTests: XCTestCase { } } + func test_changing_forwarding_ancestor_invalidates_former_and_current_hosts() { + let innerContent = UIViewController() + let innerHost = ModalHostContainerViewController(content: innerContent) + let container = UIViewController() + let formerOuterHost = ModalHostContainerViewController(content: UIViewController()) + let currentOuterHost = ModalHostContainerViewController(content: UIViewController()) + + formerOuterHost.content.addChild(container) + container.didMove(toParent: formerOuterHost.content) + container.addChild(innerHost) + innerHost.didMove(toParent: container) + + let lifetime = innerContent.toastPresenter.present( + UIViewController(), + style: .init(ToastPresentationStyleFixture()), + accessibilityAnnouncement: "Toast." + ) + defer { lifetime.dismiss() } + + formerOuterHost.view.layoutIfNeeded() + currentOuterHost.view.layoutIfNeeded() + XCTAssertEqual(formerOuterHost.toastPresentation.presentedViewControllers.count, 1) + XCTAssertTrue(currentOuterHost.toastPresentation.presentedViewControllers.isEmpty) + + // Reparent an intermediate container without loading or moving the inner host's view. + // Its next modal update must invalidate both the cached and newly resolved ancestors. + container.willMove(toParent: nil) + container.removeFromParent() + currentOuterHost.content.addChild(container) + container.didMove(toParent: currentOuterHost.content) + XCTAssertFalse(innerHost.isViewLoaded) + + innerHost.setNeedsModalUpdate() + formerOuterHost.view.layoutIfNeeded() + currentOuterHost.view.layoutIfNeeded() + + XCTAssertTrue(formerOuterHost.toastPresentation.presentedViewControllers.isEmpty) + XCTAssertEqual(currentOuterHost.toastPresentation.presentedViewControllers.count, 1) + XCTAssertEqual(innerContent.aggregateModals().toasts.count, 1) + } + func test_stopping_forwarding_invalidates_former_ancestor_for_modal() { let innerContent = UIViewController() let innerHost = ModalHostContainerViewController( diff --git a/WorkflowModals/Sources/ModalHostContainer.swift b/WorkflowModals/Sources/ModalHostContainer.swift index 83001b5e..55b6e2b6 100644 --- a/WorkflowModals/Sources/ModalHostContainer.swift +++ b/WorkflowModals/Sources/ModalHostContainer.swift @@ -232,8 +232,8 @@ extension ModalHostContainer: Screen where Content: Screen { setNeedsModalUpdate() - // `setNeedsModalUpdate()` only forwards through the current filter. If this host - // has stopped forwarding, the former ancestor still needs to remove its snapshot. + // `setNeedsModalUpdate()` invalidates a previously tracked ancestor. If forwarding + // was never tracked, invalidate the ancestor still reachable through containment. formerAncestorModalHost?.setNeedsModalUpdate() } } diff --git a/WorkflowModals/Tests/ModalHostContainerTests.swift b/WorkflowModals/Tests/ModalHostContainerTests.swift index 1ec124ae..8d9a0d23 100644 --- a/WorkflowModals/Tests/ModalHostContainerTests.swift +++ b/WorkflowModals/Tests/ModalHostContainerTests.swift @@ -459,6 +459,46 @@ class ModalHostContainerTests: XCTestCase { } } + func test_changing_forwarding_ancestor_invalidates_former_and_current_hosts() { + let innerHost = makeToastHost() + let container = UIViewController() + let formerOuterHost = makeToastHost() + let currentOuterHost = makeToastHost() + + formerOuterHost.content.addChild(container) + container.didMove(toParent: formerOuterHost.content) + container.addChild(innerHost) + innerHost.didMove(toParent: container) + + let lifetime = innerHost.content.toastPresenter.present( + UIViewController(), + style: .init(ToastPresentationStyleFixture()), + accessibilityAnnouncement: "Toast." + ) + defer { lifetime.dismiss() } + + formerOuterHost.view.layoutIfNeeded() + currentOuterHost.view.layoutIfNeeded() + XCTAssertEqual(formerOuterHost.toastPresentationController.presentedViewControllers.count, 1) + XCTAssertTrue(currentOuterHost.toastPresentationController.presentedViewControllers.isEmpty) + + // Reparent an intermediate container without loading or moving the inner host's view. + // Its next modal update must invalidate both the cached and newly resolved ancestors. + container.willMove(toParent: nil) + container.removeFromParent() + currentOuterHost.content.addChild(container) + container.didMove(toParent: currentOuterHost.content) + XCTAssertFalse(innerHost.isViewLoaded) + + innerHost.setNeedsModalUpdate() + formerOuterHost.view.layoutIfNeeded() + currentOuterHost.view.layoutIfNeeded() + + XCTAssertTrue(formerOuterHost.toastPresentationController.presentedViewControllers.isEmpty) + XCTAssertEqual(currentOuterHost.toastPresentationController.presentedViewControllers.count, 1) + XCTAssertEqual(innerHost.content.aggregateModals().toasts.count, 1) + } + func test_stopping_forwarding_invalidates_former_ancestor_for_modal() { let innerHost = makeModalFilteringHost() let outerHost = makeToastHost() From 96783d5cde298d5ffa48fcb3d633b21743a23406 Mon Sep 17 00:00:00 2001 From: Rob MacEachern Date: Thu, 6 Aug 2026 09:43:01 -0500 Subject: [PATCH 3/3] fix: invalidate local presentations when forwarding changes --- .../ModalHostContainerViewController.swift | 21 +++++++++++++++++-- ...odalHostContainerViewControllerTests.swift | 5 +++++ .../Sources/ModalHostContainer.swift | 21 +++++++++++++++++-- .../Tests/ModalHostContainerTests.swift | 5 +++++ 4 files changed, 48 insertions(+), 4 deletions(-) diff --git a/Modals/Sources/ModalHostContainerViewController.swift b/Modals/Sources/ModalHostContainerViewController.swift index 0f2e3613..81ad3a30 100644 --- a/Modals/Sources/ModalHostContainerViewController.swift +++ b/Modals/Sources/ModalHostContainerViewController.swift @@ -214,13 +214,20 @@ public final class ModalHostContainerViewController: UIViewController, ModalHost // MARK: ModalHost + /// Marks this host's local presentations for recomputation and notifies any forwarding + /// ancestor that its aggregated presentation snapshot may have changed. public func setNeedsModalUpdate() { + setNeedsLocalModalUpdate() + setForwardingAncestorModalHostNeedsUpdate() + } + + /// Marks only this host's presentation controllers for recomputation on their next layout, + /// without propagating the invalidation to an ancestor. + private func setNeedsLocalModalUpdate() { needsModalUpdate = true viewIfLoaded?.setNeedsLayout() modalPresentation.viewIfLoaded?.setNeedsLayout() - - setForwardingAncestorModalHostNeedsUpdate() } private func updateModalsIfNeeded() { @@ -278,6 +285,8 @@ public final class ModalHostContainerViewController: UIViewController, ModalHost } } + /// Reconciles the tracked forwarding ancestor with the current hierarchy, invalidating any + /// former or current ancestor snapshot and refreshing local filtering when it changes. private func setForwardingAncestorModalHostNeedsUpdate() { let currentAncestorModalHost = hasPresentationFilter ? ancestorModalHost : nil @@ -285,6 +294,9 @@ public final class ModalHostContainerViewController: UIViewController, ModalHost // The former host may still display this host's last forwarded snapshot. forwardingAncestorModalHost?.setNeedsModalUpdate() forwardingAncestorModalHost = currentAncestorModalHost + + // Local filtering changes depending on whether presentations can be forwarded. + setNeedsLocalModalUpdate() } // Some presentations may be forwarded to the current ancestor host. @@ -295,6 +307,11 @@ public final class ModalHostContainerViewController: UIViewController, ModalHost let formerAncestorModalHost = forwardingAncestorModalHost ?? fallback forwardingAncestorModalHost = nil + if formerAncestorModalHost != nil { + // Without an ancestor, presentations that were forwarded must become local again. + setNeedsLocalModalUpdate() + } + // A forwarding host is part of its ancestor's aggregated modal list. Invalidate that // snapshot while the former ancestor is still reachable. formerAncestorModalHost?.setNeedsModalUpdate() diff --git a/Modals/Tests/ModalHostContainerViewControllerTests.swift b/Modals/Tests/ModalHostContainerViewControllerTests.swift index 0b9bd9ea..ed1a508b 100644 --- a/Modals/Tests/ModalHostContainerViewControllerTests.swift +++ b/Modals/Tests/ModalHostContainerViewControllerTests.swift @@ -138,6 +138,9 @@ final class ModalHostContainerViewControllerTests: XCTestCase { ) defer { lifetime.dismiss() } + innerHost.view.layoutIfNeeded() + XCTAssertEqual(innerHost.toastPresentation.presentedViewControllers.count, 1) + show(vc: outerHost) { outerHost in XCTAssertTrue(outerHost.toastPresentation.presentedViewControllers.isEmpty) @@ -169,9 +172,11 @@ final class ModalHostContainerViewControllerTests: XCTestCase { innerHost.willMove(toParent: nil) innerHost.view.removeFromSuperview() innerHost.removeFromParent() + innerHost.view.layoutIfNeeded() outerHost.view.layoutIfNeeded() XCTAssertEqual(innerContent.aggregateModals().toasts.count, 1) + XCTAssertEqual(innerHost.toastPresentation.presentedViewControllers.count, 1) XCTAssertTrue(outerHost.toastPresentation.presentedViewControllers.isEmpty) } } diff --git a/WorkflowModals/Sources/ModalHostContainer.swift b/WorkflowModals/Sources/ModalHostContainer.swift index 55b6e2b6..04a4b055 100644 --- a/WorkflowModals/Sources/ModalHostContainer.swift +++ b/WorkflowModals/Sources/ModalHostContainer.swift @@ -279,13 +279,20 @@ extension ModalHostContainer: Screen where Content: Screen { // MARK: ModalHost + /// Marks this host's local presentations for recomputation and notifies any forwarding + /// ancestor that its aggregated presentation snapshot may have changed. func setNeedsModalUpdate() { + setNeedsLocalModalUpdate() + setForwardingAncestorModalHostNeedsUpdate() + } + + /// Marks only this host's presentation controllers for recomputation on their next layout, + /// without propagating the invalidation to an ancestor. + private func setNeedsLocalModalUpdate() { needsModalUpdate = true viewIfLoaded?.setNeedsLayout() modalPresentationController.viewIfLoaded?.setNeedsLayout() - - setForwardingAncestorModalHostNeedsUpdate() } private func updateModalsIfNeeded() { @@ -343,6 +350,8 @@ extension ModalHostContainer: Screen where Content: Screen { } } + /// Reconciles the tracked forwarding ancestor with the current hierarchy, invalidating any + /// former or current ancestor snapshot and refreshing local filtering when it changes. private func setForwardingAncestorModalHostNeedsUpdate() { let currentAncestorModalHost = hasPresentationFilter ? ancestorModalHost : nil @@ -350,6 +359,9 @@ extension ModalHostContainer: Screen where Content: Screen { // The former host may still display this host's last forwarded snapshot. forwardingAncestorModalHost?.setNeedsModalUpdate() forwardingAncestorModalHost = currentAncestorModalHost + + // Local filtering changes depending on whether presentations can be forwarded. + setNeedsLocalModalUpdate() } // Some presentations may be forwarded to the current ancestor host. @@ -360,6 +372,11 @@ extension ModalHostContainer: Screen where Content: Screen { let formerAncestorModalHost = forwardingAncestorModalHost ?? fallback forwardingAncestorModalHost = nil + if formerAncestorModalHost != nil { + // Without an ancestor, presentations that were forwarded must become local again. + setNeedsLocalModalUpdate() + } + // A forwarding host is part of its ancestor's aggregated modal list. Invalidate that // snapshot while the former ancestor is still reachable. formerAncestorModalHost?.setNeedsModalUpdate() diff --git a/WorkflowModals/Tests/ModalHostContainerTests.swift b/WorkflowModals/Tests/ModalHostContainerTests.swift index 8d9a0d23..6544940c 100644 --- a/WorkflowModals/Tests/ModalHostContainerTests.swift +++ b/WorkflowModals/Tests/ModalHostContainerTests.swift @@ -391,6 +391,9 @@ class ModalHostContainerTests: XCTestCase { ) defer { lifetime.dismiss() } + innerHost.view.layoutIfNeeded() + XCTAssertEqual(innerHost.toastPresentationController.presentedViewControllers.count, 1) + show(vc: outerHost) { outerHost in XCTAssertTrue(outerHost.toastPresentationController.presentedViewControllers.isEmpty) @@ -421,9 +424,11 @@ class ModalHostContainerTests: XCTestCase { innerHost.willMove(toParent: nil) innerHost.view.removeFromSuperview() innerHost.removeFromParent() + innerHost.view.layoutIfNeeded() outerHost.view.layoutIfNeeded() XCTAssertEqual(innerHost.content.aggregateModals().toasts.count, 1) + XCTAssertEqual(innerHost.toastPresentationController.presentedViewControllers.count, 1) XCTAssertTrue(outerHost.toastPresentationController.presentedViewControllers.isEmpty) } }