From 08f08a45514930ddd6479537df77b181fcd680b2 Mon Sep 17 00:00:00 2001 From: MhAhmadAli Date: Mon, 24 Aug 2026 00:11:20 +0500 Subject: [PATCH 1/2] Implement ScrollView::autoScrollTo() in both renderers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ScrollView::autoScrollTo($index)` has been a dead API: the PHP builder sets an `auto_scroll_to` prop, it serializes and crosses the wire intact, and then neither renderer ever reads it. Nothing scrolls. Read the prop on both platforms and drive the list from it. Semantics, matched across iOS and Android: - The index names a DIRECT child, as the docs describe. - Absent or negative means "no target" — the author isn't driving the scroll position at all. - An index past the end is CLAMPED, not dropped. A list that is still filling in can legitimately be shorter than the index for a frame or two; clamping lands on the last child now and re-fires as the real target arrives, rather than leaving the list parked. - The scroll fires when the RESOLVED index changes, not on every publish. A re-render carrying the same index leaves a reader who has scrolled away exactly where they were. A clamped target still re-fires on its own once the list grows past it. - First application jumps, later ones animate — the same rule `scroll-anchor="bottom"` already follows. - An explicit `auto_scroll_to` takes precedence over `scroll-anchor="bottom"`; both drive the same list state, and the author naming a specific child is the more specific instruction. - The target parks at the start of the viewport on both platforms (`.top` / `.leading` to match Compose's `scrollToItem`). 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. 2D (`axis="both"`) is deliberately excluded — children there are layered at their own frames rather than sequenced, so an index has no position to scroll to. --- resources/android/ContainerRenderers.kt | 69 +++++++++++- .../ios/NativeUIScrollViewRenderer.swift | 100 ++++++++++++++++-- 2 files changed, 160 insertions(+), 9 deletions(-) diff --git a/resources/android/ContainerRenderers.kt b/resources/android/ContainerRenderers.kt index 8d47156..7754103 100644 --- a/resources/android/ContainerRenderers.kt +++ b/resources/android/ContainerRenderers.kt @@ -13,6 +13,7 @@ import androidx.compose.foundation.layout.heightIn import androidx.compose.foundation.layout.offset import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.lazy.LazyRow import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.rememberLazyListState @@ -336,6 +337,57 @@ private fun totalDescendants(node: NativeUINode): Int { return count } +/** + * Resolves `ScrollView::autoScrollTo($index)` into a concrete child index, or + * `-1` for "no target". + * + * The prop names a DIRECT child of the scroll view. An absent or negative + * value means the author isn't driving the scroll position at all. + * + * An index past the end is CLAMPED rather than dropped: PHP publishes the + * index and the children in the same frame, but a list that is still filling + * in (paginated history, a streamed response) can legitimately be shorter than + * the index for a frame or two. Clamping lands on the last child now and + * re-fires as the real target appears; dropping it would leave the list parked + * wherever it was. + */ +private fun resolveAutoScrollTarget(node: NativeUINode): Int { + val requested = node.props.getInt("auto_scroll_to", -1) + if (requested < 0 || node.children.isEmpty()) return -1 + + return requested.coerceAtMost(node.children.size - 1) +} + +/** + * Drives a lazy list from the resolved `auto_scroll_to` target. + * + * Keyed on the RESOLVED index, so the scroll fires when the author's intent + * actually changes — not on every re-publish. A screen that re-renders for an + * unrelated reason (a tick, a toggle elsewhere) carries the same index and + * leaves a reader who has scrolled away exactly where they were. It also means + * a clamped target re-fires on its own once the list grows past it: the + * resolved value moves even though the prop didn't. + * + * First application jumps, later ones animate — matching `scroll-anchor`: + * a screen that opens already scrolled shouldn't visibly fly down from the + * top, but a later move is a state change the user should see happen. + */ +@Composable +private fun AutoScrollToEffect(targetIndex: Int, listState: LazyListState) { + val didInitialScroll = remember { mutableStateOf(false) } + + LaunchedEffect(targetIndex) { + if (targetIndex < 0) return@LaunchedEffect + + if (!didInitialScroll.value) { + didInitialScroll.value = true + listState.scrollToItem(targetIndex) + } else { + listState.animateScrollToItem(targetIndex) + } + } +} + object ScrollViewRenderer { @Composable fun Render(node: NativeUINode, modifier: Modifier) { @@ -345,8 +397,14 @@ object ScrollViewRenderer { detectVerticalDragGestures(onDragStart = { keyboardController?.hide() }) { _, _ -> } } + val autoScrollTarget = resolveAutoScrollTarget(node) + if (horizontal) { - LazyRow(modifier = modifier) { + val rowState = rememberLazyListState() + + AutoScrollToEffect(autoScrollTarget, rowState) + + LazyRow(modifier = modifier, state = rowState) { items(node.children, key = { it.id }) { child -> NodeView(node = child) } @@ -360,7 +418,12 @@ object ScrollViewRenderer { // item with a max offset lands at the very bottom regardless of how // the content is nested. Hooks are called unconditionally to satisfy // Compose's rules; the work is gated on the prop. - val stickBottom = node.props.getString("scroll_anchor", "") == "bottom" + // An explicit `auto_scroll_to` wins over `scroll-anchor="bottom"`. + // Both drive the same LazyListState, so letting them run together + // would have two effects fighting over the same list — the author + // named a specific child, which is the more specific instruction. + val stickBottom = autoScrollTarget < 0 && + node.props.getString("scroll_anchor", "") == "bottom" val listState = rememberLazyListState() val didInitialScroll = remember { mutableStateOf(false) } val contentSignal = if (stickBottom) totalDescendants(node) else 0 @@ -377,6 +440,8 @@ object ScrollViewRenderer { } } + AutoScrollToEffect(autoScrollTarget, listState) + // A `fill` / `h-full` DIRECT child asked to be at least as tall as // the VIEWPORT — the "short screen centred, still scrolls when the // keyboard appears" pattern. A LazyColumn measures its items with diff --git a/resources/ios/NativeUIScrollViewRenderer.swift b/resources/ios/NativeUIScrollViewRenderer.swift index b1d01de..620efa4 100644 --- a/resources/ios/NativeUIScrollViewRenderer.swift +++ b/resources/ios/NativeUIScrollViewRenderer.swift @@ -11,12 +11,23 @@ struct NativeUIScrollViewRenderer: View { /// bottom-anchored list opens at the bottom. @State private var atBottom: Bool = true + /// Whether `auto_scroll_to` has been applied once already. The first + /// application jumps (a screen that opens already scrolled shouldn't fly + /// down from the top); later ones animate, so a move the user didn't + /// initiate is visible rather than teleporting the content. + @State private var didInitialAutoScroll: Bool = false + var body: some View { let horizontal = node.props.getBool("horizontal") let showsIndicators = node.props.getBool("shows_indicators", default: true) let spacing = CGFloat(node.layout?.gap ?? 0) let axis = node.props.getString("axis", default: "") - let stickBottom = node.props.getString("scroll_anchor", default: "") == "bottom" + // An explicit `auto_scroll_to` wins over `scroll-anchor="bottom"`. + // Both drive the same ScrollViewReader, so letting them run together + // would have two handlers fighting over the same list — the author + // named a specific child, which is the more specific instruction. + let stickBottom = autoScrollIndex == nil + && node.props.getString("scroll_anchor", default: "") == "bottom" let messageSignal = stickBottom ? Self.descendantCount(node) : 0 // 2D mode. Bypass the Lazy stacks (which force 1D layout) and use a @@ -34,6 +45,10 @@ struct NativeUIScrollViewRenderer: View { // own `.frame(...)` (set by NodeLayoutModifier from `w-[N]` / // `h-[N]` classes) drives the scrollable size. // + // `auto_scroll_to` is deliberately not honoured here: children in + // 2D mode are layered at their own frames rather than sequenced, + // so "the child at index N" has no position to scroll to. + // // Multi-child 2D scrolls are rare (typical use is one large // image / canvas). For multiple children we layer them in a // ZStack pinned via `.fixedSize` and accept that NavigationStack @@ -53,15 +68,24 @@ struct NativeUIScrollViewRenderer: View { } .scrollDismissesKeyboard(.interactively) } else if horizontal { - ScrollView(.horizontal, showsIndicators: showsIndicators) { - LazyHStack(alignment: .top, spacing: spacing) { - ForEach(node.children) { child in - NodeView(node: child) - .equatable() + ScrollViewReader { proxy in + ScrollView(.horizontal, showsIndicators: showsIndicators) { + LazyHStack(alignment: .top, spacing: spacing) { + ForEach(node.children) { child in + NodeView(node: child) + .equatable() + } } } + .scrollDismissesKeyboard(.interactively) + // `.leading`, so a horizontal auto-scroll parks the target at + // the left edge — the same place Android's `scrollToItem` + // puts it. + .onAppear { applyAutoScroll(proxy: proxy, anchor: .leading, animated: false) } + .onChange(of: autoScrollIndex) { _ in + applyAutoScroll(proxy: proxy, anchor: .leading, animated: true) + } } - .scrollDismissesKeyboard(.interactively) } else if hasFillHeightChild { // A `fill` / `h-full` child asked to be at least as tall as the // VIEWPORT — the "short screen centred, still scrolls when the @@ -184,6 +208,13 @@ struct NativeUIScrollViewRenderer: View { proxy.scrollTo(Self.bottomAnchorID, anchor: .bottom) } } + // `.top`, so the named child parks at the top of the viewport — + // matching Android's `scrollToItem`, which puts the item at the + // start of the list. + .onAppear { applyAutoScroll(proxy: proxy, anchor: .top, animated: false) } + .onChange(of: autoScrollIndex) { _ in + applyAutoScroll(proxy: proxy, anchor: .top, animated: true) + } // The keyboard resizes the scroll viewport in BOTH directions — // it shrinks on the way in (the screen shifts up for keyboard // avoidance) and grows back on the way out. Re-pin on each, so @@ -215,6 +246,61 @@ struct NativeUIScrollViewRenderer: View { } } + /// Resolves `ScrollView::autoScrollTo($index)` into a concrete child + /// index, or `nil` for "no target". + /// + /// The prop names a DIRECT child of the scroll view. An absent or negative + /// value means the author isn't driving the scroll position at all. + /// + /// An index past the end is CLAMPED rather than dropped: PHP publishes the + /// index and the children in the same frame, but a list that is still + /// filling in (paginated history, a streamed response) can legitimately be + /// shorter than the index for a frame or two. Clamping lands on the last + /// child now and re-fires as the real target appears; dropping it would + /// leave the list parked wherever it was. + /// + /// Driving `.onChange` off the RESOLVED index (rather than the raw prop) + /// is what keeps a re-publish from yanking the reader: a screen that + /// re-renders for an unrelated reason carries the same index and nothing + /// fires. It also makes a clamped target re-fire on its own once the list + /// grows past it — the resolved value moves even though the prop didn't. + private var autoScrollIndex: Int? { + let requested = node.props.getInt("auto_scroll_to", default: -1) + guard requested >= 0, !node.children.isEmpty else { return nil } + + return min(requested, node.children.count - 1) + } + + /// Brings the `auto_scroll_to` child into view. + /// + /// Targets the child's node id, which is the identity `ForEach` already + /// assigns to each row (`NativeUINode: Identifiable`), so no extra `.id()` + /// is needed on the row's modifier chain. + /// + /// `animated` is decided by the caller's context, but the FIRST successful + /// application always jumps regardless — an `.onChange` can be the first + /// thing to fire when the prop arrives after the initial layout. + private func applyAutoScroll(proxy: ScrollViewProxy, anchor: UnitPoint, animated: Bool) { + guard let index = autoScrollIndex else { return } + + let targetID = node.children[index].id + let shouldAnimate = animated && didInitialAutoScroll + didInitialAutoScroll = true + + // Defer past first layout — lazy content isn't measured yet inside + // `onAppear`, so an immediate `scrollTo` no-ops. Same reason the + // bottom-anchor pin defers. + DispatchQueue.main.async { + if shouldAnimate { + withAnimation(.easeOut(duration: 0.25)) { + proxy.scrollTo(targetID, anchor: anchor) + } + } else { + proxy.scrollTo(targetID, anchor: anchor) + } + } + } + /// Scroll the bottom anchor back into view, in step with the keyboard. /// /// Shared by the show and hide observers so the two transitions animate From c46e19ecbcc2528b4eff361af215eec34735777c Mon Sep 17 00:00:00 2001 From: MhAhmadAli Date: Sat, 5 Sep 2026 16:49:28 +0500 Subject: [PATCH 2/2] Address device review: stale onChange, and clamping that moved the reader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs found on an iPhone 17 Pro simulator and an API 36 emulator. iOS scrolled to the PREVIOUS target. `.onChange(of:perform:)` hands the closure the new value but runs it against the previous version of the view, so `applyAutoScroll` re-read `autoScrollIndex` and `node.children` from a stale `self` and always landed one step behind. A screen that opened with no index and gained one later never scrolled at all, because that first change was measured against nothing. Both handlers now use the two-parameter form, which reads the current view; the deployment target is 18.2, and 25 of the 27 `.onChange` sites in this repo already use it. Clamping an out-of-range index tied the scroll to a row that moves with the CONTENT rather than with the author's intent. Removing rows dragged a reader who had scrolled away down to the new last row; a list streaming in scrolled repeatedly on its way to a target it hadn't reached. An index past the end is now ignored, which keeps the useful half — nothing happens until the named child exists, then the list goes there once. Precedence over `scroll-anchor="bottom"` now keys on the author's REQUEST rather than the resolved target. Gating on the resolved target would hand control back to the anchor whenever the index was out of reach, so a list filling in would sit at the bottom and then jump. Splitting the two ideas keeps the renderer predictable when an author sets both: - `autoScrollRequest` — did the author name a child? Content-independent. - `autoScrollIndex` — does it exist yet? Drives the scroll. --- resources/android/ContainerRenderers.kt | 46 +++++++++++----- .../ios/NativeUIScrollViewRenderer.swift | 55 ++++++++++++------- 2 files changed, 68 insertions(+), 33 deletions(-) diff --git a/resources/android/ContainerRenderers.kt b/resources/android/ContainerRenderers.kt index 7754103..0edd765 100644 --- a/resources/android/ContainerRenderers.kt +++ b/resources/android/ContainerRenderers.kt @@ -338,24 +338,35 @@ private fun totalDescendants(node: NativeUINode): Int { } /** - * Resolves `ScrollView::autoScrollTo($index)` into a concrete child index, or - * `-1` for "no target". + * The author's declared intent: the index passed to + * `ScrollView::autoScrollTo($index)`, or `-1` when they aren't driving the + * scroll position at all (prop absent, or negative). * - * The prop names a DIRECT child of the scroll view. An absent or negative - * value means the author isn't driving the scroll position at all. + * Deliberately independent of the children, so that precedence over + * `scroll-anchor` doesn't flicker while a list fills in. + */ +private fun autoScrollRequest(node: NativeUINode): Int { + val requested = node.props.getInt("auto_scroll_to", -1) + + return if (requested < 0) -1 else requested +} + +/** + * The child to actually bring into view, or `-1` when there is nothing to + * scroll to yet. * - * An index past the end is CLAMPED rather than dropped: PHP publishes the - * index and the children in the same frame, but a list that is still filling - * in (paginated history, a streamed response) can legitimately be shorter than - * the index for a frame or two. Clamping lands on the last child now and - * re-fires as the real target appears; dropping it would leave the list parked - * wherever it was. + * An index past the end is IGNORED, not clamped. Clamping looked like a + * kindness — land on the last child now, correct it later — but it ties the + * scroll to a row that moves whenever the CONTENT does rather than when the + * author's intent does. Removing rows then drags a reader down to the new end, + * and a list streaming in scrolls repeatedly on its way to a target it hasn't + * reached. Ignoring the index keeps the useful half: nothing happens until the + * named child exists, and then the list goes there exactly once. */ private fun resolveAutoScrollTarget(node: NativeUINode): Int { - val requested = node.props.getInt("auto_scroll_to", -1) - if (requested < 0 || node.children.isEmpty()) return -1 + val requested = autoScrollRequest(node) - return requested.coerceAtMost(node.children.size - 1) + return if (requested >= node.children.size) -1 else requested } /** @@ -422,7 +433,14 @@ object ScrollViewRenderer { // Both drive the same LazyListState, so letting them run together // would have two effects fighting over the same list — the author // named a specific child, which is the more specific instruction. - val stickBottom = autoScrollTarget < 0 && + // + // Gated on the REQUEST rather than the resolved target: an author + // who named a child owns the scroll position from that moment, + // including the frames before the child exists. Gating on the + // resolved target would hand control back to the anchor whenever + // the index is out of reach, so a list filling in would sit at the + // bottom and then jump. + val stickBottom = autoScrollRequest(node) < 0 && node.props.getString("scroll_anchor", "") == "bottom" val listState = rememberLazyListState() val didInitialScroll = remember { mutableStateOf(false) } diff --git a/resources/ios/NativeUIScrollViewRenderer.swift b/resources/ios/NativeUIScrollViewRenderer.swift index 620efa4..5fb2206 100644 --- a/resources/ios/NativeUIScrollViewRenderer.swift +++ b/resources/ios/NativeUIScrollViewRenderer.swift @@ -26,7 +26,13 @@ struct NativeUIScrollViewRenderer: View { // Both drive the same ScrollViewReader, so letting them run together // would have two handlers fighting over the same list — the author // named a specific child, which is the more specific instruction. - let stickBottom = autoScrollIndex == nil + // + // Gated on the REQUEST rather than the resolved target: an author who + // named a child owns the scroll position from that moment, including + // the frames before the child exists. Gating on the resolved target + // would hand control back to the anchor whenever the index is out of + // reach, so a list filling in would sit at the bottom and then jump. + let stickBottom = autoScrollRequest == nil && node.props.getString("scroll_anchor", default: "") == "bottom" let messageSignal = stickBottom ? Self.descendantCount(node) : 0 @@ -82,7 +88,7 @@ struct NativeUIScrollViewRenderer: View { // the left edge — the same place Android's `scrollToItem` // puts it. .onAppear { applyAutoScroll(proxy: proxy, anchor: .leading, animated: false) } - .onChange(of: autoScrollIndex) { _ in + .onChange(of: autoScrollIndex) { _, _ in applyAutoScroll(proxy: proxy, anchor: .leading, animated: true) } } @@ -212,7 +218,7 @@ struct NativeUIScrollViewRenderer: View { // matching Android's `scrollToItem`, which puts the item at the // start of the list. .onAppear { applyAutoScroll(proxy: proxy, anchor: .top, animated: false) } - .onChange(of: autoScrollIndex) { _ in + .onChange(of: autoScrollIndex) { _, _ in applyAutoScroll(proxy: proxy, anchor: .top, animated: true) } // The keyboard resizes the scroll viewport in BOTH directions — @@ -246,29 +252,40 @@ struct NativeUIScrollViewRenderer: View { } } - /// Resolves `ScrollView::autoScrollTo($index)` into a concrete child - /// index, or `nil` for "no target". + /// The author's declared intent: the index passed to + /// `ScrollView::autoScrollTo($index)`, or `nil` when they aren't driving + /// the scroll position at all (prop absent, or negative). /// - /// The prop names a DIRECT child of the scroll view. An absent or negative - /// value means the author isn't driving the scroll position at all. + /// Deliberately independent of the children, so that precedence over + /// `scroll-anchor` doesn't flicker while a list fills in. + private var autoScrollRequest: Int? { + let requested = node.props.getInt("auto_scroll_to", default: -1) + + return requested >= 0 ? requested : nil + } + + /// The child to actually bring into view, or `nil` when there is nothing + /// to scroll to yet. /// - /// An index past the end is CLAMPED rather than dropped: PHP publishes the - /// index and the children in the same frame, but a list that is still - /// filling in (paginated history, a streamed response) can legitimately be - /// shorter than the index for a frame or two. Clamping lands on the last - /// child now and re-fires as the real target appears; dropping it would - /// leave the list parked wherever it was. + /// An index past the end is IGNORED, not clamped. Clamping looked like a + /// kindness — land on the last child now, correct it later — but it ties + /// the scroll to a row that moves whenever the CONTENT does rather than + /// when the author's intent does. Removing rows then drags a reader down + /// to the new end, and a list streaming in scrolls repeatedly on its way + /// to a target it hasn't reached. Ignoring the index keeps the useful + /// half: nothing happens until the named child exists, and then the list + /// goes there exactly once. /// - /// Driving `.onChange` off the RESOLVED index (rather than the raw prop) + /// Driving `.onChange` off this RESOLVED value (rather than the raw prop) /// is what keeps a re-publish from yanking the reader: a screen that /// re-renders for an unrelated reason carries the same index and nothing - /// fires. It also makes a clamped target re-fire on its own once the list - /// grows past it — the resolved value moves even though the prop didn't. + /// fires. private var autoScrollIndex: Int? { - let requested = node.props.getInt("auto_scroll_to", default: -1) - guard requested >= 0, !node.children.isEmpty else { return nil } + guard let requested = autoScrollRequest, + requested < node.children.count + else { return nil } - return min(requested, node.children.count - 1) + return requested } /// Brings the `auto_scroll_to` child into view.