Implement ScrollView::autoScrollTo() in both renderers

[MhAhmadAli]

Fixes the dead API reported in NativePHP/mobile-air#366.

Reviewed on device by @gwleuverink, who found two real bugs (a one-behind .onChange on iOS, and clamping dragging the reader around on content changes). Both are fixed — see the review reply for the reasoning and for two open design questions.

The problem

ScrollView::autoScrollTo($index) builds a prop, the prop crosses the wire intact, and then nothing reads it. Confirmed end to end:

ScrollView::make(...)->autoScrollTo(1)->toArray(...)
  => type: scroll_view
     props: {"auto_scroll_to":1}

auto_scroll_to appears zero times in this repo — neither NativeUIScrollViewRenderer.swift nor ScrollViewRenderer in ContainerRenderers.kt looks at it. scroll_anchor right next to it is read on both platforms; this one was just never wired up. The docs describe it, and mobile-air's own BenchmarkComponent drives its "Large List 10k FPS" scenario with it — so that benchmark has been measuring a list that never moves.

The fix

Read the prop on both platforms and drive the list from it. Semantics are matched across iOS and Android:

Behaviour	Rule
Meaning of the index	A direct child, as documented
Absent / negative	No target — the author isn't driving scroll position
Index past the end	Ignored — nothing happens until the child exists
When it fires	When the resolved target changes — not on every publish
First vs. later	First application jumps, later ones animate (same rule scroll-anchor follows)
vs. scroll-anchor="bottom"	An explicit auto_scroll_to request wins
Resting position	Start of the viewport (.top / .leading, matching Compose's scrollToItem)

Three choices worth calling out:

An out-of-range index is ignored, not clamped. Clamping to the last child looks like a kindness, but it ties the scroll to a row that moves whenever the content does rather than when the author's intent does. On device that meant removing rows dragged the reader down to the new end, and a list streaming in scrolled repeatedly on its way to a target it hadn't reached yet. Ignoring keeps the useful half: nothing happens until the named child exists, then the list goes there once.

Precedence keys on the request, not the resolved target. These are deliberately two ideas — autoScrollRequest (did the author name a child at all? content-independent) and autoScrollIndex (does it exist yet? drives the scroll). scroll-anchor="bottom" loses to the request, so an author who sets both gets a list that stays put until the row exists and then goes to it, rather than sitting at the bottom and jumping once it appears.

The effect keys on the resolved target. That's what stops a re-publish from yanking the reader: a screen that re-renders for an unrelated reason carries the same index and nothing fires.

Horizontal scroll views are covered too — the iOS one gains a ScrollViewReader and the Android LazyRow an explicit list state, neither of which it had.

axis="both" is skipped on iOS, where children are layered at their own frames rather than sequenced, so an index has no position to scroll to. Android has no 2D path to skip: it never reads axis, and both() sets only axis and never horizontal, so such a view already falls through to the vertical list and auto-scroll applies to it. That Android gap predates this PR and wants its own issue rather than a guard here.

Testing

Everything below ran in Docker.

Repo CI, at parity — pest (217 passed, 735 assertions), pint --test (104 files), php -l over src/ (86 files), swiftc -parse over all 46 resources/ios/*.swift.

The resolution logic, executed. Both resolvers are extracted verbatim from the committed files into standalone programs and run, so the shipped expressions are what get exercised. iOS and Android are given the same truth table — in range, boundary, one-past-end, far-past-end, negative, empty children, single child, and the benchmark's own 1-child/index-9999 shape — and agree on every case. A second table checks that the request stays content-independent, which is what makes scroll-anchor precedence stable.

Publish-sequence tests, replaying realistic frame sequences through the real resolver and counting the scrolls the list would perform:

same index republished 5x            -> [7]        one scroll, reader not yanked
index changes 0,0,4,4,9              -> [0, 4, 9]  one per distinct target
fixed target 2, list grows 10->12    -> [2]        content churn doesn't drag the user
target removed mid-sequence          -> [3, 3]     control handed back and retaken
no prop across publishes             -> []

Including regressions for both bugs found on device:

removal: target 25, list 30 -> 20    -> [25]       was [25, 19]        (the drag)
growth:  target 20, list 5,10,15,21  -> [20]       was [4,9,14,20]     (the stutter)

And one that documents a known gap rather than asserting good behaviour — a search screen holding autoScrollTo(0) across new result sets fires only once, because 0 → 0 isn't a change. That needs an author-supplied re-arm token, which needs a builder argument that doesn't exist yet; discussed in the review thread.

Kotlin type-check against real Compose. The new functions plus the exact call-site wiring compile against real androidx.compose.* artifacts (Compose Multiplatform 1.7.3 + Compose compiler plugin, Kotlin 2.1.0) with zero errors and zero warnings from frontend analysis.

Negative controls, because a green check that can't fail proves nothing: injected bugs are each caught — wrong import, wrong argument type, wrong parameter name, suspend call outside a coroutine — and mutating the clamp or the negative guard produces failing assertions rather than silence.

What I could not test: on-device behaviour. No simulator or emulator here. @gwleuverink covered this round on an iPhone 17 Pro simulator and an API 36 emulator, which is how both bugs surfaced — notably, swiftc -parse performs no semantic analysis and so emits no deprecation diagnostics, and a pure-logic test can't reach SwiftUI's event wiring. Another pass would be welcome on the changed paths.

Not included

Two follow-ups belong in mobile-air, not here:

1. There is no auto-scroll-to Blade attribute, so <native:scroll-view> still can't express this — only the PHP builder can. scroll-anchor is mapped in NativeElementCollector; this isn't.
2. BenchmarkComponent::renderLargeListFpsScreen() calls autoScrollTo($itemCount - 1) on a scroll view whose only direct child is a wrapping Column. Index 9999 against one child now resolves to nothing at all, so that benchmark scenario still does not scroll and this PR does not fix it — it needs restructuring so the rows are direct children.

Related: #27 implements this alongside the Android gap fix and chrome-collapse work, and is blocked on NativePHP/mobile-air#241. This PR is the isolated auto_scroll_to piece; happy to defer to #27 or have it drop that section, whichever the maintainers prefer.

[gwleuverink]

Nice work tracing this one end to end, and thanks for being upfront about not having a device. I put the branch on an iPhone 17 Pro simulator and an API 36 emulator to close that gap. The index maths is right, and on Android everything behaves the way your table says. Three things behave differently on a real screen. The first two have fixes I've built and run, the third is a design call I'd rather make with you.

How I read the results, so the numbers below mean something: every row in the test screen is a solid block of colour that encodes its own number, so a screenshot tells you exactly which row is at the top and how far into it the list has scrolled.

1. On iOS the list goes where you told it last time

Change the index and nothing else, over and over:

set 3  -> list doesn't move
set 6  -> list goes to 3
set 20 -> list goes to 6
set 8  -> list goes to 20
set 8  -> list doesn't move   (right, the index didn't change)
set 14 -> list goes to 8

It is always one behind, on three separate builds, and the same on a horizontal list. If a screen opens with no index and gets one later, it never scrolls at all, because that first change is measured against nothing.

The cause is .onChange(of:perform:). When the value changes, it hands your closure the new index but runs it against the previous version of the view, so applyAutoScroll looks up autoScrollIndex and node.children again and gets the old ones. .onAppear reads the current view, which is why opening a screen lands correctly and only later changes are wrong.

The two-parameter version reads the current view, and the app targets 18.2 so it's available:

.onChange(of: autoScrollIndex) { _, _ in
    applyAutoScroll(proxy: proxy, anchor: .top, animated: true)
}

Both handlers need it, the .leading one too. With that in, the sequence above lands 6 for 6, a target that was out of reach still fires once the list grows to it, a render that swaps the rows and the index at the same time lands, and the same holds inside a bottom sheet and a sheet pane.

Two things worth knowing. Passing the new index into applyAutoScroll instead isn't enough on its own, because the row list is old in that same closure, so a render that changes rows and index together still lands wrong. And swiftc -parse was never going to catch this: it doesn't check deprecations, so it missed the warnings too, and the bug lives in the SwiftUI event wiring rather than in the maths your standalone test exercised.

2. Adding or removing rows moves the reader

Tying the scroll to the row it settles on is the right instinct, but that row moves when the content changes, not just when the author changes their mind.

Removing rows: index 25 of 30, I scrolled back to the top by hand, then the list dropped to 20 rows with the index untouched. The reader gets pulled down to the new last row.

Adding rows: index 20 held fixed while the list filled 5, 10, 15, 21. It scrolls twice on the way before landing, so a list that streams in stutters after the reader instead of arriving once.

Both come from clamping an index that's past the end. If you ignore it instead, you keep the good part, the list still jumps to the row the moment it exists, and both symptoms go:

private var autoScrollIndex: Int? {
    let requested = node.props.getInt("auto_scroll_to", default: -1)
    guard requested >= 0, requested < node.children.count else { return nil }

    return requested
}

private fun resolveAutoScrollTarget(node: NativeUINode): Int {
    val requested = node.props.getInt("auto_scroll_to", -1)
    if (requested < 0 || requested >= node.children.size) return -1

    return requested
}

I built that and ran it on both platforms:

• Removing rows: same setup, reader at the top, list drops 30 to 20. They stay at the top. Before, they were dragged to the end.
• Adding rows: same 5, 10, 15, 21 fill. One scroll, at 11.5 seconds, the moment the 21st row made index 20 real. Before, it scrolled at 8.4 and 11.4 seconds on the way there.
• Nothing else changed: a normal index still parks that row at the top, changing from one valid index to another still lands, a negative or missing index still does nothing, horizontal lists are unaffected, scroll-anchor="bottom" still loses to an explicit index, and the fill-height case is untouched. Same on iOS.

Two knock-on effects, better decided than discovered. An index past the end now does nothing at all, so autoScrollTo(99) on 30 rows no longer lands on the last one. And on a list that also has scroll-anchor="bottom", an index that's still out of reach leaves the anchor in charge until the row exists: I watched it sit at the bottom with 14 rows, then jump to row 25 once the list reached 30. For a chat that's probably what you'd want, but it does mean the "clamped" row of your table needs rewriting alongside the code.

3. Coming back to a list starts it over

This is the one I'd most like your thinking on. It needs no unusual layout, and the fix in point 1 doesn't touch it.

A horizontal row of cards inside a normal vertical feed. The row opened on index 10, I swiped it across to 15, scrolled the feed down past it and back up, and it was sitting on 10 again. Twice. On iOS the same thing happens when the row comes back into view. So wherever the reader had dragged that row to is thrown away every time it leaves the screen and returns.

It shows up again when you leave a screen and come back, and when you close and reopen a panel, where it isn't even consistent between panels:

panel	opens on the current index	index changed while open	close and reopen
BottomSheet	yes	iOS no, Android yes	starts over, reader's position lost
Modal	yes	iOS no, Android yes	starts over
SheetPane	yes	iOS no, Android yes	stays where it was

Sheets and modals build their contents fresh each time they open. The sheet pane builds once at its tallest size and only slides, so it never starts over.

Underneath it's the same thing in each case: the flag that remembers "I've already scrolled this list" belongs to the on-screen view, so it dies when that view goes away and is back to zero when it returns. For it to survive, it has to belong to the list itself rather than to the view showing it. On Android rememberSaveable gets you most of the way; on iOS it needs something keyed off node.id. That's a bigger decision than the other two and it's yours to make, so I stopped at the diagnosis rather than guessing. Tell me which way you'd take it and I'll test it.

One more for while you're in there: on iOS the one-behind problem is worse inside a panel, because the index it remembers is the one the panel opened with. So a sheet that's meant to open a list and scroll to a particular message shows nothing at all rather than being one step off. Point 1 fixes that too, I checked.

Smaller things

axis="both" isn't excluded on both platforms. iOS skips it on purpose, but Android has nothing to skip: it never looks at axis, and both() only sets axis and never horizontal, so the view falls through to the vertical path and scrolls anyway. Index 3 with six 300pt children scrolled to the third one. Not yours to fix, but the description reads as though both sides opt out.

The benchmark still won't move, on either platform. With the rows wrapped in a single Column the index means "the first and only child" today, and means nothing at all if you take point 2, so either way it's worth keeping out of any merge note that sounds like the FPS scenario is fixed. You already say so in "Not included".

Two I couldn't reach: the drawer wouldn't open for me without touching the screen, and accordions, carousels and tab bodies all throw their contents away when hidden, so I'd expect them to behave like the sheet row above without having watched it happen.

Everything else behaves

Jumping to a row once it exists, doing nothing for a negative or missing index, beating scroll-anchor="bottom" and handing control back when the index is removed, horizontal lists parking the row at the left edge, the fill-height case, and leaving a reader alone through 50 re-renders and a hand swipe. The first scroll on a screen arrives instantly and later ones animate over about a quarter second. And a scroll view with no index set renders pixel for pixel the same as before your change on both platforms, so the new ScrollViewReader and list state cost nothing on the untouched path.

Requesting changes for points 1 and 2, both small and both verified on a device. Point 3 is the one worth settling together before this lands.

[gwleuverink]

I can only submit review with a comment. See my last message 👆🏻

[gwleuverink]

One more, from wondering what a search field would do to this.

Asking for the same row twice never fires, so a list can't be sent back to the top once its results are replaced.

The screen is a search where the results are the scroll view's children and the author wants the top of the results for every query:

public function render(): Element
{
    return ScrollView::make(...$this->resultsFor($this->query))
        ->autoScrollTo(0)
        ->fillWidth()
        ->flexGrow(1);
}

resultsFor() returns different rows each time the query changes. autoScrollTo(0) never changes, because the author's intent never changes.

Driven on Android:

1. First query returns 16 rows. The list opens at the top. Correct.
2. The reader scrolls down, about two thirds of the way.
3. Second query returns 16 completely different rows. The list doesn't move. The reader is two thirds of the way down results they've never seen.
4. Third query returns 14 rows, so the count changes too. Still doesn't move.

Index 0 to index 0 isn't a change, so nothing fires, and there's no way for the author to ask again. I drove the queries from a probe that swaps the row set on command rather than a real text field, but the tree the renderer receives is what a search screen would publish.

Keying on the identity of the row at that index doesn't help. Element ids are positional, so two different result sets hand the renderer the same ids:

results A (alpha, beta, gamma)   -> child ids 2,3,4
results B (delta, epsilon, zeta) -> child ids 2,3,4

I built that version and it changes nothing on a device. It also survives the earlier suggestion about ignoring out-of-range indices, since 0 stays 0 either way.

Something that does work

Let the author re-arm it with a token the renderer watches alongside the index. On Android that's reading the prop, threading it into AutoScrollToEffect, and adding it to the effect key:

val autoScrollToken = node.props.getString("auto_scroll_token", "")
// ... passed through to
LaunchedEffect(targetIndex, token) {

On iOS it's one computed property:

private var autoScrollTrigger: String? {
    guard let index = autoScrollIndex else { return nil }

    return "\(index)|" + node.props.getString("auto_scroll_token", default: "")
}

watched by .onChange(of: autoScrollTrigger) in place of .onChange(of: autoScrollIndex). On the PHP side the author would pass whatever already identifies the query, something like:

ScrollView::make(...$this->resultsFor($this->query))
    ->autoScrollTo(0, token: $this->query)

I built this and ran it on both platforms.

Android, index held at 0 throughout: each new query with a new token puts the list at the top of the new results, including from two thirds of the way down. Then with the token unchanged, 15 re-renders and a content growth left the reader exactly where they were.

iOS, index held at 5, starting from a list too short to scroll so that firing and not firing land in different places: with the token changed the list moved to row 5 of the new results, with the token unchanged it stayed at the top. Same behaviour on both.

The builder argument is in mobile-air rather than here, so this is only half a suggestion. But the renderer half is small, and it's the only shape I found that fixes the search case without also re-scrolling on content changes the author didn't ask about.

Other things I checked

Two things I checked that turned out fine: rows inserted above the list while a sheet is closed shift which row the index names, which is the prop behaving as documented, and rotating the device keeps the reader where they were rather than snapping back.

[simonhamp]

Looks to partially duplicate #27, but this may be a better piece in isolation...

[MhAhmadAli]

Thank you — putting this on a simulator and an emulator is exactly the gap I couldn't close, and both bugs are real. I've taken both fixes. Points 3 and 4 are decisions rather than defects, so my thinking is below rather than a patch.

1. The one-behind .onChange — fixed

Your diagnosis holds up independently. Two things I checked before taking it:

• IPHONEOS_DEPLOYMENT_TARGET = 18.2 in mobile-air's project.pbxproj, and platform :ios, '18.2' in the Podfile. So the two-parameter form is available.
• It is already the house style here: 25 of the 27 .onChange call sites in this repo use { _, new in }. Mine were two of the three that didn't. I wrote the outlier and it bit exactly where you'd expect.

Both handlers now use the two-parameter form.

Your point about why my checks missed it is worth recording precisely, because it generalises: swiftc -parse stops before semantic analysis, so it emits no deprecation diagnostics at all — and the standalone test exercised the resolver, while the bug lived in the SwiftUI event wiring. Neither was ever going to reach it. A parse pass and a pure-logic test are the two cheapest things to run and they share a blind spot exactly where SwiftUI keeps its sharp edges.

I've left the two remaining one-parameter sites alone: NativeUIDrawerHost.swift:213, and the messageSignal handler directly above mine. The latter has the same staleness in principle, but it scrolls to a constant anchor id, so nothing it reads goes stale — and it's the path you exercised most heavily. I'd rather not destabilise a verified path inside an unrelated PR. Happy to fix it here or separately, your call.

2. Clamping — fixed, and the precedence gate with it

You're right and my reasoning was wrong. I clamped to cover a list that is momentarily shorter than the index; ignoring out-of-range covers that case just as well — nothing fires until the row exists, then it fires once — without tying the scroll to a row that moves whenever the content does. Taken as you wrote it, on both platforms.

One thing I changed beyond your patch. You flagged that under "ignore", an out-of-reach index hands control back to scroll-anchor="bottom" until the row appears, and that the table needed rewriting around it. Rather than document that hand-off I removed it. Precedence now keys on the author's request (the raw prop) rather than on the resolved target, so these are two separate ideas:

• autoScrollRequest — did the author name a child at all? Content-independent.
• autoScrollIndex — does that child exist yet? Content-dependent, and what actually drives the scroll.

scroll-anchor loses to the request. An author who sets both now gets a list that stays put until the row exists and then goes to it, instead of sitting at the bottom and then jumping. I'd rather the renderer be predictable than have it guess which of two contradictory instructions was meant. That said, you watched the other behaviour on a real chat and thought it read well — if you'd rather keep it, it's a one-identifier change and I'll put it back.

Both of your scenarios are now regression tests, driven through the resolver extracted verbatim from the committed file:

removal: target 25, list 30 -> 20     fires [25]   (was [25, 19] — the drag you saw)
growth:  target 20, list 5,10,15,21   fires [20]   (was [4, 9, 14, 20] — the stutter)

3. Coming back to a list — where I'd take it

Your diagnosis is right, and I'd refine it in one place that changes the options. On Android rememberLazyListState is itself rememberSaveable-backed, and lazy layouts wrap keyed items in a SaveableStateProvider — our items do have keys. So for the card row inside a feed I'd expect the position to be restored already, and then immediately overwritten, because LaunchedEffect(targetIndex) re-runs on re-entering composition and my flag only chooses jump-vs-animate. If that's right, the reader's position isn't being lost so much as clobbered — worth a look on your rig, since it's your setup that can tell.

Either way, I don't think this should be settled implicitly, because the two real cases want opposite things:

• A chat opened with autoScrollTo($last): leave, come back, and showing the latest message is right.
• Your horizontal card row in a feed: scroll past and back, and not throwing away the reader's swipe is right.

The renderer can't tell those apart. Both are "the view was rebuilt and the author's index is still N". So whichever default we hard-code is wrong half the time, and I'd rather not encode a guess as a persistence mechanism — the mechanism is the expensive part to undo later.

My recommendation: keep the current behaviour in this PR — a freshly built view honours the author's declared position, which is at least what the prop literally says — and settle re-entry as its own change, as a prop (auto-scroll-once versus the current always), because that's the shape that lets both cases be right.

One hazard for whoever builds it, from your own second comment: element ids are positional. So a node.id-keyed store on the iOS side would collide across genuinely different screens that happen to mint the same id — the same property that defeated keying on child identity. Worth designing around rather than discovering.

If you'd rather have the other default now, say so and I'll build it: rememberSaveable plus a last-applied-target check on Android, and a node-id-keyed store on iOS, with that collision handled. I stopped here because it's a bigger commitment than the two fixes above and you asked to settle it together.

4. The search case, and the re-arm token

This one is a real hole and your diagnosis of it is airtight, including ruling out child identity empirically — that was the first thing I'd have reached for.

I don't want to add the renderer half on its own, though, and I think you'll agree once it's stated: a auto_scroll_token prop that no PHP builder can set is a prop that reads the wire and finds nothing there. That is precisely the failure this PR exists to fix, in the opposite direction. It would sit here looking implemented until someone tried to use it.

So: agreed on the shape, and I'd like to land it as one change across both repos — autoScrollTo(int $index, ?string $token = null) in mobile-air plus the renderer read here — so the API and the thing that honours it arrive together. Happy to open both. It also wants a moment's thought about what happens when the author passes a token that changes on every render, since that turns into "scroll on every publish", which is the behaviour the resolved-index keying exists to prevent.

Smaller things

axis="both" on Android — you're right and I've fixed the description. I checked: Android never reads axis, and both() in mobile-air sets only axis, never horizontal, so such a view falls through to the vertical path. My description claimed both platforms opt out; only iOS does. I've not added an Android skip, because Android has no 2D path to fall back to — skipping would leave those views less functional than they are today, and the real gap is that Android doesn't implement 2D at all. That deserves its own issue rather than a guard here.

The benchmark is worse than "still won't move", now that clamping is gone: with the rows inside one Column, index 9999 against one child used to clamp to that child and now resolves to nothing at all. Either way it can't measure a scroll. It stays in "Not included", and I've made the wording unambiguous so no merge note can read as though that scenario is fixed.

#27 — noted, and I'd rather not duplicate Shane's work. That PR bundles this fix with the Android gap fix and the chrome-collapse work, and is blocked on mobile-air#241; its own description marks the auto_scroll_to half as not yet exercised on device, which is the half you've now put on two devices here. If #27 lands first I'll close this or rebase to whatever's left; if this lands first, #27 drops its section 1 and keeps the two fixes that are genuinely its own. Whichever you prefer — I've no attachment to it being this PR.

What I ran this round

All in Docker. Both resolvers are re-extracted verbatim from the committed files at test-generation time, so the tests exercise the shipped expressions rather than a copy.

	iOS	Android
Resolution truth table	17/17	17/17
Publish sequences	—	8/8
Parse / type-check	swiftc -parse, 46/46 files clean	Compose type-check, 0 errors 0 warnings

The two platforms agree on every case in the shared table, including the ones your review changed: one-past-end, far-past-end, and the benchmark's 1-child/index-9999 shape all now resolve to "nothing", and the request stays content-independent so precedence can't flicker.

Negative controls, since a check that can't fail proves nothing — I mutated the shipped logic back to both bugs you found and confirmed the tests catch them:

reintroduce the clamp    iOS 5 failing assertions,  Android 7
drop the negative guard  iOS 6 failing assertions,  Android 2

The Android 7 includes both of your device scenarios, which is the check I actually wanted: if anyone reintroduces clamping, the removal and growth cases fail rather than going quiet.

Repo CI at parity: Pint 104 files pass, php -l 86 files clean. Pest is 214 passed / 3 failed, and the 3 are environmental — my usual Debian PHP image was evicted mid-session and Alpine's musl PHP doesn't define GLOB_BRACE, which src/Fonts/GoogleFonts.php:480 uses. Same three pass on glibc, this branch touches no PHP at all, and CI runs on ubuntu, so they'll be green there. Flagging it rather than quietly reporting 214.

Still not device-tested. Everything above is reasoning, parsing and type-checking — and both bugs you found lived in exactly the gap that leaves: one in SwiftUI's event wiring, one in behaviour over time that only shows up with real content changing under a real finger. If you have the rig up, the changed paths worth a pass are the one-behind sequence, removal and growth, and an author setting both autoScrollTo and scroll-anchor="bottom" — that last one is now my behaviour rather than the one you observed, so it's the most likely place for me to have chosen wrong.