Skip to content

Commit b40483b

Browse files
authored
fix: portal before host (#36)
## 📜 Description Fixed a problem when portal is not moved to the host if it's rendered before. ## 💡 Motivation and Context First of all it's important to mention that only Android doesn't have this bug. It happens because on Android we have following lifecycle: - set `hostName` - register `Host` - mount children (call corresponding `addView`) On iOS (and web) from the other side the sequence looks next: - mount children - update props (set `hostName`) - register `Host` > The problem is not present with `@gorhom/portal`, because there we simply store children in `context` and render them when `PortalHost` is mounted. I decided to follow similar pattern, so after registering host we need to check if we can attach portals to it and if we can then attach. To implement this I added `registerPendingPortal`/`unregisterPendingPortals` methods to `PortalRegistry`. So when we want to move `PortalView`, but host is not found -> we register pending portals. When we call `registerHost` then we check if it has pending portals and if yes, then we go through each pending portal and call `onHostAvailableMethod`. Additionally inside `PortalView` we track `isWaitingForHost` to properly cleanup pending portals. Even though Android doesn't have this problem I decided to implement this fix there too, because I think it can be a timing issue and may be changed in the future (so let's have a fix in advance). Web implementation follows the same concept, except the view registration. Instead of view we register a callback that can teleport our view. Closes #35 ## 📢 Changelog <!-- High level overview of important changes --> <!-- For example: fixed status bar manipulation; added new types declarations; --> <!-- If your changes don't affect one of platform/language below - then remove this platform/language --> ### Example - added "portal before host" example; - removed "lottie" example (because we have messenger example now that better showcase a use-case of the lib). ### E2E - added "portal before host" test case; ### Web - added `registerPendingPortal`/`unregisterPendingPortals` to `PortalRegistry`; - rename `setHost` -> `registerHost` to match iOS/Android naming; ### iOS - added `registerPendingPortal`/`unregisterPendingPortals` to `PortalRegistry`; - added `onHostAvailable` to `PortalView`; - call `onHostAvailable` from `registerHost` if we have `pendingPortals` for this host; - added `isWaitingForHost` to `PortalView` to remove `pendingPortals` if we change host/detach view; ### Android - added `registerPendingPortal`/`unregisterPendingPortals` to `PortalRegistry`; - added `onHostAvailable` to `PortalView`; - call `onHostAvailable` from `registerHost` if we have `pendingPortals` for this host; - added `isWaitingForHost` to `PortalView` to remove `pendingPortals` if we change host/detach view; ## 🤔 How Has This Been Tested? Tested on Pixel 9 Pro (API 33), iPhone 16 Pro (iOS 18.5), Chrome 141.0.7390.108 ## 📸 Screenshots (if appropriate): |When|Android|iOS|Web| |------|--------|---|----| |Before|<img width="382" height="807" alt="image" src="https://github.com/user-attachments/assets/472f8685-23a5-45a2-8145-f28b704213a6" />|<img width="400" height="874" alt="image" src="https://github.com/user-attachments/assets/6541b050-001d-4ab6-91c0-49d70c99266d" />|<img width="387" height="828" alt="image" src="https://github.com/user-attachments/assets/780b4d59-7737-4f46-b0d7-cd143e902f40" />| |After|<img width="375" height="797" alt="image" src="https://github.com/user-attachments/assets/90709cc7-1056-40f2-9727-462c188811f6" />|<img width="400" height="874" alt="image" src="https://github.com/user-attachments/assets/a4fbf2d4-ce60-4de3-b953-2c5b63b06945" />|<img width="382" height="822" alt="image" src="https://github.com/user-attachments/assets/96be9964-2c37-4f81-a282-fa0dbbbc20f4" />| ## 📝 Checklist - [x] CI successfully passed - [x] I added new mocks and corresponding unit-tests if library API was changed
1 parent b393658 commit b40483b

File tree

21 files changed

+326
-3110
lines changed

21 files changed

+326
-3110
lines changed
Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,52 @@
11
package com.teleport.global
22

33
import com.teleport.host.PortalHostView
4+
import com.teleport.portal.PortalView
45
import java.lang.ref.WeakReference
56

67
object PortalRegistry {
78
private val hosts: MutableMap<String, WeakReference<PortalHostView>> = HashMap()
9+
private val pendingPortals: MutableMap<String, MutableList<WeakReference<PortalView>>> = HashMap()
810

9-
fun register(
11+
fun registerHost(
1012
name: String,
1113
view: PortalHostView,
1214
) {
1315
hosts[name] = WeakReference(view)
16+
17+
pendingPortals[name]?.let { portals ->
18+
val iterator = portals.iterator()
19+
while (iterator.hasNext()) {
20+
val portalRef = iterator.next()
21+
portalRef.get()?.onHostAvailable() ?: iterator.remove()
22+
}
23+
pendingPortals.remove(name)
24+
}
1425
}
1526

16-
fun unregister(name: String) {
27+
fun unregisterHost(name: String) {
1728
hosts.remove(name)
1829
}
1930

2031
fun getHost(name: String?): PortalHostView? = hosts[name]?.get()
32+
33+
fun registerPendingPortal(
34+
hostName: String,
35+
portal: PortalView,
36+
) {
37+
val portals = pendingPortals.getOrPut(hostName) { mutableListOf() }
38+
portals.add(WeakReference(portal))
39+
}
40+
41+
fun unregisterPendingPortal(
42+
hostName: String,
43+
portal: PortalView,
44+
) {
45+
pendingPortals[hostName]?.let { portals ->
46+
portals.removeAll { it.get() == null || it.get() == portal }
47+
if (portals.isEmpty()) {
48+
pendingPortals.remove(hostName)
49+
}
50+
}
51+
}
2152
}

android/src/main/java/com/teleport/host/PortalHostView.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,15 @@ class PortalHostView(
1212
fun setName(newName: String?) {
1313
if (name == newName) return
1414

15-
name?.let { PortalRegistry.unregister(it) }
15+
name?.let { PortalRegistry.unregisterHost(it) }
1616
name = newName
17-
newName?.let { PortalRegistry.register(it, this) }
17+
newName?.let { PortalRegistry.registerHost(it, this) }
1818
}
1919

2020
// region Lifecycle
2121
override fun onDetachedFromWindow() {
2222
super.onDetachedFromWindow()
23-
name?.let { PortalRegistry.unregister(it) }
23+
name?.let { PortalRegistry.unregisterHost(it) }
2424
}
2525
// endregion
2626
}

android/src/main/java/com/teleport/portal/PortalView.kt

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,15 @@ class PortalView(
1111
context: Context,
1212
) : ReactViewGroup(context) {
1313
private var hostName: String? = null
14+
private var isWaitingForHost = false
1415

1516
fun setHostName(name: String?) {
17+
if (isWaitingForHost) {
18+
hostName?.let { PortalRegistry.unregisterPendingPortal(it, this) }
19+
isWaitingForHost = false
20+
}
21+
22+
// Gather children BEFORE updating hostName, since getChildCount() and getChildAt() depend on it
1623
val children = mutableListOf<View>()
1724
val count = childCount
1825
for (i in count - 1 downTo 0) {
@@ -24,17 +31,40 @@ class PortalView(
2431
hostName = name
2532

2633
val target: ViewGroup =
27-
if (hostName != null) {
28-
PortalRegistry.getHost(hostName) ?: this
29-
} else {
30-
this
31-
}
34+
hostName?.let { hostNameValue ->
35+
val host = PortalRegistry.getHost(hostNameValue)
36+
if (host != null) {
37+
host
38+
} else {
39+
PortalRegistry.registerPendingPortal(hostNameValue, this)
40+
isWaitingForHost = true
41+
this
42+
}
43+
} ?: this
3244

3345
for (child in children) {
3446
target.addView(child)
3547
}
48+
}
49+
50+
internal fun onHostAvailable() {
51+
isWaitingForHost = false
52+
53+
val host = PortalRegistry.getHost(hostName)
54+
if (host != null) {
55+
// Use super methods to get actual children from this view, not from the host
56+
val children = mutableListOf<View>()
57+
val count = super.getChildCount()
58+
for (i in count - 1 downTo 0) {
59+
val child = super.getChildAt(i) ?: continue
60+
children.add(0, child)
61+
super.removeViewAt(i)
62+
}
3663

37-
requestLayout()
64+
for (child in children) {
65+
host.addView(child)
66+
}
67+
}
3868
}
3969

4070
private fun isTeleported(): Boolean = hostName != null && PortalRegistry.getHost(hostName) != null
@@ -103,4 +133,14 @@ class PortalView(
103133
// When teleported, do nothing—children are handled by the host's accessibility tree
104134
}
105135
// endregion
136+
137+
// region Lifecycle
138+
override fun onDetachedFromWindow() {
139+
super.onDetachedFromWindow()
140+
if (isWaitingForHost) {
141+
hostName?.let { PortalRegistry.unregisterPendingPortal(it, this) }
142+
isWaitingForHost = false
143+
}
144+
}
145+
// endregion
106146
}

e2e/flows/portal-before-host.yml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
appId: teleport.example
2+
---
3+
- launchApp
4+
- tapOn:
5+
id: "portal_before_host"
6+
- takeScreenshot:
7+
path: e2e/PortalBeforeHost
8+
- runScript:
9+
file: ../scripts/compare-screenshots.js
10+
env:
11+
base: screenshots/${DEVICE}/PortalBeforeHost
12+
target: PortalBeforeHost
13+
diff: 0.3
6.48 KB
Loading
21.9 KB
Loading
63.6 KB
Loading

0 commit comments

Comments
 (0)