diff --git a/resources/android/ListRenderer.kt b/resources/android/ListRenderer.kt index 2a65e39..9a487d6 100644 --- a/resources/android/ListRenderer.kt +++ b/resources/android/ListRenderer.kt @@ -50,6 +50,7 @@ import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import com.nativephp.mobile.ui.nativerender.NativeElementBridge +import com.nativephp.mobile.ui.nativerender.NativeScrollTargetRegistry import com.nativephp.mobile.ui.nativerender.NativeUINode import com.nativephp.mobile.ui.nativerender.NodeView import kotlinx.coroutines.delay @@ -78,6 +79,13 @@ object ListRenderer { // Set when this list is a page of a pager with a header, which the // list draws above its rows (see PagerHeader). val pagerHeader = if (horizontal) null else LocalPagerHeader.current + // The key a Tab::onReselect() names to act on this list from a + // bottom-tab reselect. Absent by default: nothing registers and + // the list behaves exactly as before the seam. See ListScrollTarget. + val scrollTarget = node.props.getString("scroll_target", "") + if (scrollTarget.isNotEmpty()) { + ListScrollTarget(scrollTarget, scrollState, LocalPagerPage.current) + } // Rows are measured while the list reports its visible range: that's // what a list putting placeholders in rows' places goes by. val rowHeights = remember { ListRowHeights() } @@ -301,6 +309,78 @@ object ListRenderer { /** How long a list waits between reports of its visible range while scrolling. */ private const val VISIBLE_RANGE_INTERVAL_MS = 150L +/** + * Publishes [state] under [key] in the core [NativeScrollTargetRegistry] + * while the list that owns it is composed — the list's half of the + * bottom-tab reselect scroll seam. A reselect that names [key] scrolls + * the list back to its top from native (no PHP round trip), or fires + * its refresh method when the list already sits at exact top; see the + * registry's docs for the seam. Android only — iOS ignores the prop. + * + * The effect is keyed on [state] as well as [key]: when a re-keyed or + * remounted list gets a fresh `rememberLazyListState`, the entry is + * re-registered against the new one, so no entry ever points at a + * disposed state. `owner` is a fresh token per composition and the + * registry unregisters by identity, so the stale `onDispose` of a + * composition a newer one already replaced (a pager page recomposed + * under a different list key) can never remove the live entry — only + * its own. + * + * A list on a page of a pager ([page] from [LocalPagerPage]) only + * publishes while its page is the one the pager is settled on: the pages + * beside the one showing stay composed and would otherwise each answer + * the same reselect, and PHP names the new page only a round trip after + * the swipe. Two layers keep that true, one at each end of the registry: + * + * - The map: a list whose page is not the one showing holds no entry at + * all. Nothing registers while `active` is false, the dispose a page + * going away triggers unregisters, and a register under a key replaces + * whatever entry was there (the newest composition wins). + * - The read: the registry's `get` consults the live `isAvailable` + * predicate at read time — a bottom-tab tap, or a scroll's settle gate — + * never at composition time, so an entry still present in the map + * during the frame or two between a swipe and this effect applying is + * filtered out all the same. To a caller an absent entry and an + * unavailable one are indistinguishable: both read as nothing to act + * on. + * + * Keying the effect on [active] also invalidates the entry's identity + * when a page goes away and returns: the re-registered entry is a fresh + * one, so the core settle gate's identity check cannot match a scroll + * that started before the round trip. + * + * A list outside any pager passes `page = null` and registers exactly as + * before the seam: always, with availability that reads true. + * + * Both orientations register, and "top" is the start of the list: item + * 0 at a zero pixel offset, vertically or horizontally. + */ +@Composable +private fun ListScrollTarget(key: String, state: LazyListState, page: PagerPageContext?) { + // Read here, in composition, so the pager's page flips are observed + // and the effect below restarts when this page stops or starts being + // the one showing. The boolean only flips on page changes — never + // per frame — so the entry is not churned while the pager rests. + val active = page?.isAvailable() ?: true + val owner = remember { Any() } + // `page` is keyed too: the context is remembered per pager and page, + // so its identity only changes when the pager or the pager around it + // does — without the key the registered predicate could go on reading + // a context from a pager that has been replaced. + DisposableEffect(key, state, page, active) { + if (active) { + NativeScrollTargetRegistry.register( + key = key, + owner = owner, + isAtTop = { state.firstVisibleItemIndex == 0 && state.firstVisibleItemScrollOffset == 0 }, + scrollToTop = { state.animateScrollToItem(0) }, + isAvailable = { page?.isAvailable() ?: true }, + ) + } + onDispose { NativeScrollTargetRegistry.unregister(key, owner) } + } +} + /** * The height each row of a list last drew at, in pixels, by node id. A * node's id comes from its key, so a placeholder with a row's key finds the diff --git a/resources/android/PagerRenderer.kt b/resources/android/PagerRenderer.kt index 35d9806..72af81d 100644 --- a/resources/android/PagerRenderer.kt +++ b/resources/android/PagerRenderer.kt @@ -132,6 +132,11 @@ object PagerRenderer { val pagerState = rememberPagerState(initialPage = requestedPage) { pageCount } + // The pager this one sits in a page of, if any: a page of a page + // is only available while every pager around it is settled on its + // page (see PagerPageContext). + val parentPage = LocalPagerPage.current + // Pages reported to PHP that it has not echoed back yet, oldest // first. A `page` prop matching one of these is an echo of our own // report, not a programmatic jump. @@ -218,18 +223,28 @@ object PagerRenderer { val page: @Composable (Int) -> Unit = { index -> Box(modifier = Modifier.fillMaxSize()) { - val child = pageByIndex[index] - if (child != null && header != null) { - // The page's list draws the header and the tabs (see PagerHeader). - CompositionLocalProvider(LocalPagerHeader provides headerScroll.header(index, header, tabs, pagerState, selectTab, background)) { + // Where the pager is, live, for whatever the page + // contains: the pages beside the one showing stay + // composed, so a page's contents must not act as the + // page showing until the pager settles on it (see + // PagerPageContext). Remembered, so the context survives + // recomposition and its identity only changes when the + // pager or the page around it does. + val pageContext = remember(pagerState, index, parentPage) { PagerPageContext(index, pagerState, parentPage) } + CompositionLocalProvider(LocalPagerPage provides pageContext) { + val child = pageByIndex[index] + if (child != null && header != null) { + // The page's list draws the header and the tabs (see PagerHeader). + CompositionLocalProvider(LocalPagerHeader provides headerScroll.header(index, header, tabs, pagerState, selectTab, background)) { + NodeView(node = child) + } + } else if (child != null) { NodeView(node = child) + } else if (hasMore && index >= count) { + PagerLoadingPage() + } else { + PagerPlaceholder(placeholders.getOrNull(index).orEmpty()) } - } else if (child != null) { - NodeView(node = child) - } else if (hasMore && index >= count) { - PagerLoadingPage() - } else { - PagerPlaceholder(placeholders.getOrNull(index).orEmpty()) } } } @@ -514,6 +529,52 @@ class PagerHeader internal constructor( /** The header a page's list draws, if the page is in a pager with one. */ val LocalPagerHeader = compositionLocalOf { null } +/** + * The pager a page is in, for what the page contains to read: which page + * this is ([page]) of which pager ([pagerState]), and the same for the + * pager around that one, if any ([parent]). + * + * The pages beside the one showing stay composed + * (`beyondViewportPageCount = 1`), so a page can be composed without being + * the page showing. A list on such a page publishes a scroll target + * (`scroll_target`) that a bottom-tab reselect must not act on until the + * pager settles back on that page: PHP names the new page a round trip + * after the swipe, and only the pager itself knows where it is in the + * meantime. [isAvailable] answers that, read live from the pager's own + * state — never from a prop PHP sends, which is exactly what lags. + */ +@Stable +class PagerPageContext( + val page: Int, + val pagerState: PagerState, + val parent: PagerPageContext? = null, +) { + /** + * The pager is at rest on this page, and every pager around it is at + * rest on its own: no drag and no settle still running, this page both + * the nearest one and the one a running gesture targets. + * + * All three of the pager's own clauses are needed. [PagerState.targetPage] + * alone commits as soon as a swipe leans toward a page, so the page + * being swiped to would count as available while it is still moving; + * [PagerState.currentPage] adds that this page is the nearest one; and + * `isScrollInProgress` waits out the whole swipe and settle, so that + * neither the page being left nor the one arriving is available + * mid-flight — a reselect during that window must find nothing to act + * on. Reads are live at whatever moment asks (a tap, a scroll's settle + * gate), not at composition time: composition can lag the swipe by a + * frame or two, which is precisely the gap this closes. + */ + fun isAvailable(): Boolean = + !pagerState.isScrollInProgress && + pagerState.currentPage == page && + pagerState.targetPage == page && + (parent?.isAvailable() ?: true) +} + +/** The page a composable is on, if it is in a pager at all. Null outside one. */ +val LocalPagerPage = compositionLocalOf { null } + /** * How far the header has scrolled, from the page showing, and each page's * list, so the pages beside it can be kept level with it. diff --git a/src/Elements/NativeList.php b/src/Elements/NativeList.php index 66aee63..2eaa4f5 100644 --- a/src/Elements/NativeList.php +++ b/src/Elements/NativeList.php @@ -20,6 +20,8 @@ class NativeList extends Element protected ?string $visibleRangeCallback = null; + protected ?string $scrollTarget = null; + protected ?string $scrollEndCallback = null; public static function make(Element ...$children): static @@ -66,6 +68,9 @@ public function applyAttributes(array $attrs): void if (isset($attrs['visible-range-step']) || isset($attrs['visibleRangeStep'])) { $this->visibleRangeStep((int) ($attrs['visible-range-step'] ?? $attrs['visibleRangeStep'])); } + if (isset($attrs['scroll-target']) || isset($attrs['scrollTarget'])) { + $this->scrollTarget((string) ($attrs['scroll-target'] ?? $attrs['scrollTarget'])); + } $autoScrollTo = $attrs['auto-scroll-to'] ?? $attrs['autoScrollTo'] ?? null; if ($autoScrollTo !== null && $autoScrollTo !== '') { $this->autoScrollTo((int) $autoScrollTo); @@ -187,6 +192,28 @@ public function visibleRangeStep(int $rows): static return $this; } + /** + * Publish this list under $key in the bottom-tab reselect scroll + * seam (Android only — iOS ignores the prop): while the list is on + * screen its scroll state is registered under the key, so a + * `Tab::onReselect($method, $key)` naming the same key scrolls this + * list back to its top when the tab is re-tapped mid-scroll, or + * fires $method once the list already sits at exact top. "Top" is + * the start of the list, whichever way it scrolls. + * + * A pager keeps the pages beside the one showing composed, so give + * every simultaneously-composed list its own key and point the + * tab's reselect at the currently selected one (a Log pager's + * `log-feed-following` / `log-feed-latest` pages, say) — the newest + * list registered under a key is the one a reselect scrolls. + */ + public function scrollTarget(string $key): static + { + $this->scrollTarget = $key; + + return $this; + } + /** * Open the list with row $index at the top: a screen restored after a * relaunch, a deep link to a record, or the row just added. Rows count @@ -243,6 +270,12 @@ protected function resolveProps(CallbackRegistry $registry): array $props['on_visible_range'] = $registry->register($this->visibleRangeCallback, 'virtual_window'); } + if ($this->scrollTarget !== null) { + // Not a callback id: the literal key the renderer registers its + // scroll state under for the bottom-tab reselect seam. + $props['scroll_target'] = $this->scrollTarget; + } + if ($this->scrollEndCallback !== null) { // "first,last" too, decoded the same way. $props['on_scroll_end'] = $registry->register($this->scrollEndCallback, 'virtual_window'); diff --git a/tests/ListRendererScrollTargetTest.php b/tests/ListRendererScrollTargetTest.php new file mode 100644 index 0000000..6f98c74 --- /dev/null +++ b/tests/ListRendererScrollTargetTest.php @@ -0,0 +1,86 @@ +toContain('import com.nativephp.mobile.ui.nativerender.NativeScrollTargetRegistry'); + + // The DisposableEffect block, mirroring the contract documented on + // NativeScrollTargetRegistry: exact top (index 0 AND zero pixel + // offset), native scroll to item 0, unregister on dispose with the + // composition's own owner token, and the live availability predicate + // the registry's get() consults at tap time and settle-gate time. + $effect = '/DisposableEffect\(key, state, page, active\) \{\s*' + .'if \(active\) \{\s*' + .'NativeScrollTargetRegistry\.register\(\s*' + .'key = key,\s*' + .'owner = owner,\s*' + .'isAtTop = \{ state\.firstVisibleItemIndex == 0 && state\.firstVisibleItemScrollOffset == 0 \},\s*' + .'scrollToTop = \{ state\.animateScrollToItem\(0\) \},\s*' + .'isAvailable = \{ page\?\.isAvailable\(\) \?: true \},\s*' + .'\)\s*' + .'\}\s*' + .'onDispose \{ NativeScrollTargetRegistry\.unregister\(key, owner\) \}\s*' + .'\}/s'; + + // A fresh owner token per composition, so a stale onDispose from a + // superseded composition can never unregister the newer entry (the + // registry matches by identity); keying the effect on the state + // re-registers when rememberLazyListState produces a new one, so no + // entry ever points at a disposed state. + expect(preg_match($effect, $source))->toBe(1) + ->and(preg_match('/val owner = remember \{ Any\(\) \}/', $source))->toBe(1); +}); + +test('registers only while a scroll target key is declared', function () { + $source = file_get_contents(dirname(__DIR__).'/resources/android/ListRenderer.kt'); + + // Default lists (no prop) register nothing: same props, same + // behavior as before the seam existed. + expect(preg_match('/node\.props\.getString\("scroll_target", ""\)/', $source))->toBe(1) + ->and(preg_match('/isNotEmpty\(\)\) \{\s*ListScrollTarget\(/', $source))->toBe(1); +}); + +test('waits for the pager page it is on to be the one showing', function () { + $source = file_get_contents(dirname(__DIR__).'/resources/android/ListRenderer.kt'); + + // A pager keeps the pages beside the one showing composed, so every + // composed page's list would otherwise answer the same reselect while + // PHP is still a round trip behind the swipe. The list reads the page + // context it is on and only publishes while that page is the one the + // pager is settled on. + expect($source) + ->toContain('ListScrollTarget(scrollTarget, scrollState, LocalPagerPage.current)') + ->toContain('val active = page?.isAvailable() ?: true'); + + // `active` is observed in composition (the pager's page flips restart + // the effect) and keys it alongside key, state and page. `page` keys it + // too, so the registered predicate never keeps reading a context from + // a pager that has been replaced. + expect(preg_match('/DisposableEffect\(key, state, page, active\) \{/', $source))->toBe(1); +}); + +test('re-registers a fresh entry when a page goes away and returns', function () { + $source = file_get_contents(dirname(__DIR__).'/resources/android/ListRenderer.kt'); + + // The registration sits inside `if (active)` and the effect is keyed + // on it, so a page leaving the one showing disposes its entry and the + // page coming back registers a new one. Each register() call builds a + // fresh Entry, so the core settle gate — which compares the entry it + // tapped with the one live after the scroll — cannot match a scroll + // that started before the page went away and fire late into the page + // that came back. Between the flip and this effect applying, the live + // predicate is already filtering, closing that frame or two. + expect(preg_match('/if \(active\) \{\s*NativeScrollTargetRegistry\.register\(/s', $source))->toBe(1) + ->and(preg_match('/DisposableEffect\(key, state, page, active\) \{/', $source))->toBe(1); +}); diff --git a/tests/ListScrollTargetTest.php b/tests/ListScrollTargetTest.php new file mode 100644 index 0000000..3ad08e8 --- /dev/null +++ b/tests/ListScrollTargetTest.php @@ -0,0 +1,65 @@ +toArray(new CallbackRegistry)['props'] ?? []; + + NativeElementCollector::open('list', ['separator' => true, 'on-refresh' => 'reload']); + NativeElementCollector::close(); + $fromAttrs = NativeElementCollector::collect()->toArray(new CallbackRegistry)['props'] ?? []; + + expect($fluent)->not->toHaveKey('scroll_target') + ->and($fromAttrs)->not->toHaveKey('scroll_target'); +}); + +it('serializes the scroll target from attributes', function () { + NativeElementCollector::open('list', ['scroll-target' => 'log-feed-following']); + NativeElementCollector::close(); + + $props = NativeElementCollector::collect()->toArray(new CallbackRegistry)['props']; + + expect($props['scroll_target'])->toBe('log-feed-following'); +}); + +it('accepts the scroll target attribute in its camelCase spelling', function () { + NativeElementCollector::open('list', ['scrollTarget' => 'log-feed-trending']); + NativeElementCollector::close(); + + $props = NativeElementCollector::collect()->toArray(new CallbackRegistry)['props']; + + expect($props['scroll_target'])->toBe('log-feed-trending'); +}); + +it('exposes the scroll target on the fluent builder', function () { + $list = NativeList::make()->scrollTarget('log-feed-latest'); + + expect($list->toArray(new CallbackRegistry)['props']['scroll_target'])->toBe('log-feed-latest'); +}); diff --git a/tests/ListTest.php b/tests/ListTest.php index 9805b7f..c58b307 100644 --- a/tests/ListTest.php +++ b/tests/ListTest.php @@ -172,9 +172,10 @@ function collectList(array $attrs): array }); it('opens a list at a row with auto-scroll-to', function () { - $tree = collectList(['auto-scroll-to' => '12']); + $tree = collectList(['auto-scroll-to' => '12', 'scroll-target' => 'log-following']); - expect($tree['props']['auto_scroll_to'])->toBe(12); + expect($tree['props']['auto_scroll_to'])->toBe(12) + ->and($tree['props']['scroll_target'])->toBe('log-following'); }); it('sends no row to scroll to unless one is asked for', function () { diff --git a/tests/PagerPageContextTest.php b/tests/PagerPageContextTest.php new file mode 100644 index 0000000..2e8bc41 --- /dev/null +++ b/tests/PagerPageContextTest.php @@ -0,0 +1,66 @@ += count\) \{\s*' + .'PagerLoadingPage\(\).*?' + .'\} else \{\s*' + .'PagerPlaceholder\(/s'; + + expect(preg_match($provider, $source))->toBe(1) + ->and($source)->toContain('val LocalPagerPage = compositionLocalOf { null }'); +}); + +test('counts a page available only once the pager has settled on it', function () { + $source = file_get_contents(dirname(__DIR__).'/resources/android/PagerRenderer.kt'); + + // Conservative on purpose: targetPage alone commits as soon as a + // swipe leans toward a page (so the page arriving would count as + // available while still moving), currentPage alone still names the + // page being left until the swipe passes halfway, and neither says + // whether a settle is still running. All three together mean the + // pager is at rest on this page, read live from the pager's own + // state — never from a prop PHP sends, which is what lags the swipe. + $predicate = '/fun isAvailable\(\): Boolean =\s*' + .'!pagerState\.isScrollInProgress &&\s*' + .'pagerState\.currentPage == page &&\s*' + .'pagerState\.targetPage == page &&\s*' + .'\(parent\?\.isAvailable\(\) \?: true\)/s'; + + expect(preg_match($predicate, $source))->toBe(1) + ->and(preg_match('/class PagerPageContext\(\s*val page: Int,\s*val pagerState: PagerState,\s*val parent: PagerPageContext\? = null,?\s*\)/s', $source))->toBe(1); +}); + +test('combines nested pagers through the parent context', function () { + $source = file_get_contents(dirname(__DIR__).'/resources/android/PagerRenderer.kt'); + + // A pager that is itself a page of a pager reads the outer context + // at its own scope and threads it into each page's context, so a list + // deep inside is available only while every pager around it is + // settled on its page. The context is remembered per pager, page and + // parent, so recomposition neither churns its identity nor leaves a + // stale capture behind. + expect($source) + ->toContain('val parentPage = LocalPagerPage.current') + ->toContain('remember(pagerState, index, parentPage) { PagerPageContext(index, pagerState, parentPage) }'); +}); diff --git a/tests/TabRowChromeCompatibilityTest.php b/tests/TabRowChromeCompatibilityTest.php new file mode 100644 index 0000000..dfc18fd --- /dev/null +++ b/tests/TabRowChromeCompatibilityTest.php @@ -0,0 +1,17 @@ +not->toContain('LocalTopBarColor', 'LocalTopTabRow'); +}); + +test('keeps the tab strip transparent without an optional core chrome hook', function () { + $source = file_get_contents(dirname(__DIR__).'/resources/android/TabRowRenderer.kt'); + + expect($source)->toContain('containerColor = Color.Transparent,'); +});