Skip to content

fix: iOS PiP cleanup #1870

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 4, 2025
Merged

fix: iOS PiP cleanup #1870

merged 1 commit into from
Aug 4, 2025

Conversation

santhoshvai
Copy link
Member

💡 Overview

📝 Implementation notes

uses didMoveToSuperview instead of didMoveToWindow to ensure cleanup is always done on unmount

Here is Gemini 2.5 explanation of why to prefer didMoveToSuperView

Potential pitfalls for "unmounting" detection via didMoveToWindow in RN:

1. View removed from superview, but parent not in a window: If a view is removed from a superview, and that superview (or its ancestor) was never actually added to a window, didMoveToWindow might not have been called with a nil window because the view never had a non-nil window to begin with.

2. Container view removed: If you have a complex view hierarchy (e.g., ViewA contains ViewB, and ViewA is removed from its superview), ViewA and all its subviews (including ViewB) will have their superview become nil. However, didMoveToWindow might only be called on ViewA with nil, and ViewB would receive didMoveToSuperview with nil, but its didMoveToWindow might not fire again if it never had a window. (Though, typically, if a view is removed from a hierarchy that was in a window, its didMoveToWindow would also be called with nil).

3. App backgrounding/foregrounding: When your app goes to the background, views might be removed from the window. When it comes back to the foreground, they might be re-added. didMoveToWindow will be called in these scenarios, which might not be what you consider "unmounting" if you're specifically looking for removal from the view hierarchy rather than screen visibility changes.

@santhoshvai
Copy link
Member Author

santhoshvai commented Jul 31, 2025

Released as below for testing

@stream-io/[email protected]

@santhoshvai santhoshvai marked this pull request as ready for review August 4, 2025 09:55
@santhoshvai
Copy link
Member Author

mrged as it was tested externally and was confirmed to be working

@santhoshvai santhoshvai merged commit 88c87f4 into main Aug 4, 2025
8 checks passed
@santhoshvai santhoshvai deleted the ios-pip-cleanup branch August 4, 2025 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant