FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(screen): preserve hook state across Android pop-back · pythonnative/pythonnative@6b27f0c · GitHub

Repository navigation

Commit 6b27f0c

Browse files
committed
fix(screen): preserve hook state across Android pop-back
1 parent a94859f commit 6b27f0c

5 files changed

Lines changed: 94 additions & 14 deletions

File tree

‎src/pythonnative/screen.py‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,23 @@ def _flush_scheduled_renders(hosts: Sequence[Any]) -> None:
251251
def _on_create(host: Any) -> None:
252252
from .hooks import NavigationHandle, Provider, _NavigationContext
253253

254+
# ``on_create`` is idempotent across native-view recreations. On
255+
# Android the FragmentManager destroys and recreates a screen's
256+
# view every time the user pops back to it, and the platform
257+
# template calls ``screen.on_create()`` again from
258+
# ``onViewCreated`` — but the Python screen object (and therefore
259+
# the reconciler, hook state, focus subscribers, etc.) persists
260+
# across that. Re-running the full mount path here would reset
261+
# use_state, clobber use_focus_effect subscriptions, and break
262+
# navigation handles held by existing components, which is why
263+
# the focus counter never advanced past ``1`` before this guard.
264+
# If we're already mounted, just re-attach the existing root view
265+
# to the (newly created) native container — ``on_resume`` will
266+
# fire the focus subscribers separately.
267+
if host._reconciler is not None and host._root_native_view is not None:
268+
host._attach_root(host._root_native_view)
269+
return
270+
254271
host._nav_handle = NavigationHandle(host)
255272
host._reconciler = _new_reconciler(host)
256273

@@ -815,6 +832,18 @@ def _attach_root(self, native_view: Any) -> None:
815832
container.removeAllViews()
816833
except Exception:
817834
pass
835+
# When the user pops back to a previously mounted screen,
836+
# ``native_view`` is the root from the prior mount and may
837+
# still be parented under the old (destroyed) FrameLayout.
838+
# ViewGroup.addView() throws if a view already has a
839+
# parent, so detach it from the old one before re-attaching
840+
# to the freshly created container.
841+
try:
842+
old_parent = native_view.getParent()
843+
if old_parent is not None:
844+
old_parent.removeView(native_view)
845+
except Exception:
846+
pass
818847
LayoutParams = jclass("android.view.ViewGroup$LayoutParams")
819848
lp = LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT)
820849
container.addView(native_view, lp)

‎tests/e2e/AGENTS.md‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,22 @@ The build step (`pn run <platform> --no-logs`) only needs to run once per change
8989

9090
This is intentional and the source of the suite's speed. When debugging a flow, **don't** "simplify" `open_demo` to always `launchApp` + go home + go to category — that's the slow path the smart logic was written to avoid (about 15 min vs. 3 min of pure navigation overhead across the full iOS suite). If a flow needs a guaranteed clean app launch, set up that state in the flow itself.
9191

92+
Gate conditions in `open_demo` deliberately use signals that work on **both** platforms. The two cross-platform asymmetries that matter here:
93+
94+
- **Scroll preservation.** iOS preserves a ScrollView's offset across navigation; Android resets it to the top. A condition like `visible: "Back to home"` (button at the bottom of a category list) works on iOS after a return-from-demo but fails on Android because the list is back at the top. The helper gates on `"Demos in .*"` (top of the list, visible on both) and `notVisible: "Back to list"` (i.e. we left the demo screen) instead.
95+
- **Native-view recreation on Android.** The Android FragmentManager destroys and rebuilds a screen's view tree on pop-back; `pythonnative.screen._on_create` short-circuits the second call so hook state, `use_focus_effect` subscriptions, and `use_navigation` handles persist. If a future change to `screen.py` or `ScreenFragment.kt` breaks that idempotency, `flows/navigation/focus_effect.yaml` is the canary — it'll regress to `Focus count: 1` on pop-back.
96+
97+
### Scrolling fixed-height containers (ScrollView / FlatList)
98+
99+
Maestro's `scrollUntilVisible` always swipes from the screen center. That works for the outer page ScrollView (which fills the screen) but **not** for small in-page containers like the 200 dp `ScrollView` / `FlatList` demos — the screen center sits below those containers, so the swipe lands outside them and never moves the contents.
100+
101+
`flows/components/scroll_view.yaml` and `flows/components/flat_list.yaml` work around this with an explicit-coordinate swipe loop wrapped in `repeat: while: notVisible: ...`. Two reasons for the loop rather than a fixed `times: N`:
102+
103+
- Per-swipe scroll travel is platform-dependent — Android's `NestedScrollView` flings more aggressively than iOS's `UIScrollView`, so a count tuned for one platform overshoots on the other.
104+
- iOS preserves the inner ScrollView's offset across navigation, so a re-entry into the demo may already have the target row in view; `while: notVisible` exits the loop immediately in that case.
105+
106+
When adding a new flow that needs to scroll a non-fullscreen container, copy this pattern (small swipes ~10% of screen height, ~500 ms each, `times` cap as a safety net) rather than calling `scrollUntilVisible`.
107+
92108
### Suite-level retry
93109

94110
`scripts/run-e2e.sh` re-invokes the whole `maestro test` once if the first attempt exits non-zero. The retry exists to absorb Maestro's iOS XCUITest driver flake (transient `Application is not running` / `Request for viewHierarchy failed`) — not to paper over real failures.

‎tests/e2e/flows/components/flat_list.yaml‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,13 @@
22
#
33
# Demo screen: examples/e2e-suite/app/screens/components/flat_list.py
44
# Source under test: src/pythonnative/components.py :: FlatList
5+
#
6+
# Same pattern as components/scroll_view.yaml: the FlatList is a
7+
# small fixed-height (200 dp) container near the top of the screen,
8+
# so Maestro's ``scrollUntilVisible`` (which swipes from the screen
9+
# center) misses the container entirely. We loop with explicit
10+
# coordinates and ``while: notVisible`` so the test exits as soon as
11+
# the target row is in view, regardless of per-platform fling physics.
512
appId: ${APP_ID}
613
---
714
- runFlow:
@@ -10,10 +17,15 @@ appId: ${APP_ID}
1017
CATEGORY: Components
1118
DEMO_TITLE: FlatList
1219
- assertVisible: "FlatRow 1"
13-
- scrollUntilVisible:
14-
element: "FlatRow 20"
15-
direction: DOWN
16-
timeout: 15000
20+
- repeat:
21+
times: 15
22+
while:
23+
notVisible: "FlatRow 20"
24+
commands:
25+
- swipe:
26+
start: 50%, 20%
27+
end: 50%, 10%
28+
duration: 500
1729
- assertVisible: "FlatRow 20"
1830
- runFlow:
1931
file: ../../helpers/close_demo.yaml

‎tests/e2e/flows/components/scroll_view.yaml‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,15 @@
1111
# scrollables is a custom swipe loop with explicit coordinates that
1212
# stay inside the scroll container's bounds. See:
1313
# https://docs.maestro.dev/examples/recipes/custom-scrolling-for-screen-fragments
14+
#
15+
# Per-swipe scroll travel is platform-dependent — Android's
16+
# ``NestedScrollView`` flings more aggressively per gesture than iOS's
17+
# ``UIScrollView``, so a fixed ``times: N`` overshoots on one platform
18+
# and undershoots on the other. We loop with ``while: notVisible``
19+
# instead, which exits as soon as ``ScrollRow 20`` enters the viewport
20+
# regardless of how many swipes that took on this platform. Swipes are
21+
# kept small (10% of screen height) so a single fling can't shoot
22+
# past row 20.
1423
appId: ${APP_ID}
1524
---
1625
- runFlow:
@@ -19,16 +28,15 @@ appId: ${APP_ID}
1928
CATEGORY: Components
2029
DEMO_TITLE: ScrollView
2130
- assertVisible: "ScrollRow 1"
22-
# Each swipe moves the inner ScrollView roughly 4-5 rows. ``ScrollRow 20``
23-
# sits ~12 rows in, so 3 swipes lands it inside the visible 200 dp area
24-
# without overshooting past the end of the list.
2531
- repeat:
26-
times: 3
32+
times: 15
33+
while:
34+
notVisible: "ScrollRow 20"
2735
commands:
2836
- swipe:
29-
start: 50%, 22%
30-
end: 50%, 6%
31-
duration: 300
37+
start: 50%, 20%
38+
end: 50%, 10%
39+
duration: 500
3240
- assertVisible: "ScrollRow 20"
3341
- runFlow:
3442
file: ../../helpers/close_demo.yaml

‎tests/e2e/helpers/open_demo.yaml‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,34 @@
3636
appId: ${APP_ID}
3737
---
3838
# 1. If we landed on a demo screen, pop one level to its category list.
39+
# Confirm the pop by waiting for ``Back to list`` to disappear rather
40+
# than for the category's ``Back to home`` button to appear: Android
41+
# resets the category ScrollView's offset to the top after navigation,
42+
# so ``Back to home`` (at the bottom of the category list) starts
43+
# below the visible fold and isn't a reliable post-tap signal. iOS
44+
# preserves the offset so the latter would work there — but the
45+
# former works on both.
3946
- runFlow:
4047
when:
4148
visible: "Back to list"
4249
commands:
4350
- tapOn: "Back to list"
4451
- extendedWaitUntil:
45-
visible: "Back to home"
52+
notVisible: "Back to list"
4653
timeout: 15000
4754

48-
# 2. If we're on the wrong category, go back to home.
55+
# 2. If we're on the wrong category, go back to home. We gate on the
56+
# category title (``Demos in ...``) being visible rather than the
57+
# ``Back to home`` button: Android resets the category ScrollView's
58+
# offset to the top after navigation, so ``Back to home`` (at the
59+
# bottom of the list) starts off-screen and isn't a reliable signal.
60+
# The title is at the top of the scroll content and ``close_demo``
61+
# always scrolls back to it before exiting, so it's visible on both
62+
# platforms when the next flow's ``open_demo`` runs.
4963
- runFlow:
5064
when:
51-
visible: "Back to home"
65+
visible:
66+
text: "Demos in .*"
5267
notVisible: "Demos in ${CATEGORY}"
5368
commands:
5469
- scrollUntilVisible:

0 commit comments

Comments
 (0)

Back | FazBrowse Home | New Git URL