From b93c6de0a55ed4501916b64777d1eb2a7bfd93d9 Mon Sep 17 00:00:00 2001 From: Bao Nguyen Date: Thu, 13 Aug 2026 18:31:08 +0700 Subject: [PATCH] fix: compensate header size changes for MVCP on native `shouldAdjustForHeaderSizeChange` opened with `Platform.OS === "web"`, so `setHeaderSize` only requested an MVCP scroll adjustment on web. On iOS and Android a `ListHeaderComponent` that changed size while the user was scrolled below it went uncompensated and the viewport jumped by the header's size delta. Nothing else covered this on native: MVCP anchors on `positions[]`, which exclude `headerSize` (the header is folded in separately as `topPad` in `calculateItemsInView` and via `getContentSize`), so a header-only size change produces no item position diff for `prepareMVCP` to act on. The web gate came from #468, which was reported and tested on web only, so it was scope of testing rather than a deliberate native exclusion. Removing the clause routes native through the same `requestAdjust` path already used for item size changes, which has native-specific handling of its own (the Android old-architecture `doScrollTo` workaround and the `ignoreScrollFromMVCP` threshold window). The remaining guards are unchanged, so the initial header measurement and changes made while the header is still visible stay uncompensated on every platform. Covers the core behavior across web/ios/android, including the resulting settle position through the real `ScrollAdjustHandler`, and adds a native `header-mvcp` fixture mirroring the existing web `HeaderMvcpExample`. Fixes #515 --- __tests__/core/contentMetrics.test.ts | 182 ++++++++++++++--------- example/screens/fixtures/header-mvcp.tsx | 135 +++++++++++++++++ example/screens/routes.tsx | 10 ++ src/core/updateContentMetrics.ts | 2 - 4 files changed, 260 insertions(+), 69 deletions(-) create mode 100644 example/screens/fixtures/header-mvcp.tsx diff --git a/__tests__/core/contentMetrics.test.ts b/__tests__/core/contentMetrics.test.ts index 001937ec..743a8ec9 100644 --- a/__tests__/core/contentMetrics.test.ts +++ b/__tests__/core/contentMetrics.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, spyOn } from "bun:test"; import { clampScrollOffset } from "../../src/core/clampScrollOffset"; +import { ScrollAdjustHandler } from "../../src/core/ScrollAdjustHandler"; import { setContentInsetOverride, setFooterSize, setHeaderSize } from "../../src/core/updateContentMetrics"; import { updateContentMetricsState } from "../../src/core/updateContentMetricsState"; import { Platform } from "../../src/platform/Platform"; @@ -201,42 +202,87 @@ describe("updateContentMetrics", () => { expect(ctx.values.get("footerSize")).toBe(12); }); - it("compensates web MVCP when a measured header changes above the viewport", () => { - const prevPlatform = Platform.OS; - Platform.OS = "web"; - const requestAdjustSpy = spyOn(requestAdjustModule, "requestAdjust"); - const ctx = createMockContext( - { - headerSize: 60, - readyToRender: true, - totalSize: 1000, - }, - { - didContainersLayout: true, - didFinishInitialScroll: true, - props: { - data: [1], - maintainVisibleContentPosition: { data: false, size: true }, + for (const platformOS of ["web", "ios", "android"] as const) { + it(`compensates MVCP on ${platformOS} when a measured header changes above the viewport`, () => { + const prevPlatform = Platform.OS; + Platform.OS = platformOS; + const requestAdjustSpy = spyOn(requestAdjustModule, "requestAdjust"); + const ctx = createMockContext( + { + headerSize: 60, + readyToRender: true, + totalSize: 1000, }, - scroll: 200, - scrollLength: 500, - totalSize: 1000, - }, - ); - - try { - setHeaderSize(ctx, 60); - expect(requestAdjustSpy).not.toHaveBeenCalled(); - - requestAdjustSpy.mockClear(); - setHeaderSize(ctx, 120); - - expect(requestAdjustSpy).toHaveBeenCalledWith(ctx, 60); - } finally { - requestAdjustSpy.mockRestore(); - Platform.OS = prevPlatform; - } - }); + { + didContainersLayout: true, + didFinishInitialScroll: true, + props: { + data: [1], + maintainVisibleContentPosition: { data: false, size: true }, + }, + scroll: 200, + scrollLength: 500, + totalSize: 1000, + }, + ); + + try { + setHeaderSize(ctx, 60); + expect(requestAdjustSpy).not.toHaveBeenCalled(); + + requestAdjustSpy.mockClear(); + setHeaderSize(ctx, 120); + + expect(requestAdjustSpy).toHaveBeenCalledWith(ctx, 60); + + requestAdjustSpy.mockClear(); + setHeaderSize(ctx, 60); + + expect(requestAdjustSpy).toHaveBeenCalledWith(ctx, -60); + } finally { + requestAdjustSpy.mockRestore(); + Platform.OS = prevPlatform; + } + }); + + it(`settles the scroll position on ${platformOS} when a measured header grows above the viewport`, () => { + const prevPlatform = Platform.OS; + Platform.OS = platformOS; + const ctx = createMockContext( + { + headerSize: 60, + readyToRender: true, + totalSize: 1000, + }, + { + didContainersLayout: true, + didFinishInitialScroll: true, + props: { + data: [1], + maintainVisibleContentPosition: { data: false, size: true }, + }, + scroll: 200, + scrollLength: 500, + totalSize: 1000, + }, + ); + // Drive the real adjust handler so the assertion is about the resulting scroll + // position the user sees, not just that an adjustment was requested. + ctx.state.scrollAdjustHandler = new ScrollAdjustHandler(ctx); + + try { + setHeaderSize(ctx, 60); + setHeaderSize(ctx, 120); + + // The header grew by 60 above the viewport, so the same content stays put. + expect(ctx.state.scroll).toBe(260); + expect(ctx.values.get("scrollAdjust")).toBe(60); + } finally { + ctx.state.scheduledWork.dispose(); + Platform.OS = prevPlatform; + } + }); + } it("does not compensate the initial web MVCP header measurement", () => { const prevPlatform = Platform.OS; @@ -339,37 +385,39 @@ describe("updateContentMetrics", () => { } }); - it("does not compensate web MVCP header changes while the header is visible", () => { - const prevPlatform = Platform.OS; - Platform.OS = "web"; - const requestAdjustSpy = spyOn(requestAdjustModule, "requestAdjust"); - const ctx = createMockContext( - { - headerSize: 60, - readyToRender: true, - totalSize: 1000, - }, - { - didContainersLayout: true, - didFinishInitialScroll: true, - didMeasureHeader: true, - props: { - data: [1], - maintainVisibleContentPosition: { data: false, size: true }, + for (const platformOS of ["web", "ios", "android"] as const) { + it(`does not compensate MVCP header changes on ${platformOS} while the header is visible`, () => { + const prevPlatform = Platform.OS; + Platform.OS = platformOS; + const requestAdjustSpy = spyOn(requestAdjustModule, "requestAdjust"); + const ctx = createMockContext( + { + headerSize: 60, + readyToRender: true, + totalSize: 1000, }, - scroll: 20, - scrollLength: 500, - totalSize: 1000, - }, - ); - - try { - setHeaderSize(ctx, 120); - - expect(requestAdjustSpy).not.toHaveBeenCalled(); - } finally { - requestAdjustSpy.mockRestore(); - Platform.OS = prevPlatform; - } - }); + { + didContainersLayout: true, + didFinishInitialScroll: true, + didMeasureHeader: true, + props: { + data: [1], + maintainVisibleContentPosition: { data: false, size: true }, + }, + scroll: 20, + scrollLength: 500, + totalSize: 1000, + }, + ); + + try { + setHeaderSize(ctx, 120); + + expect(requestAdjustSpy).not.toHaveBeenCalled(); + } finally { + requestAdjustSpy.mockRestore(); + Platform.OS = prevPlatform; + } + }); + } }); diff --git a/example/screens/fixtures/header-mvcp.tsx b/example/screens/fixtures/header-mvcp.tsx new file mode 100644 index 00000000..f2f07b6d --- /dev/null +++ b/example/screens/fixtures/header-mvcp.tsx @@ -0,0 +1,135 @@ +import { useRef, useState } from "react"; +import { Pressable, StyleSheet, Text, View } from "react-native"; + +import { LegendList, type LegendListRef } from "@legendapp/list/react-native"; + +const ROW_HEIGHT = 72; +const INITIAL_HEADER_HEIGHT = 96; +const ANCHOR_ROW_INDEX = 20; +const DATA = Array.from({ length: 80 }, (_, index) => ({ + id: String(index), + title: `Row ${index}`, +})); + +type RowItem = (typeof DATA)[number]; + +function Header({ height }: { height: number }) { + return ( + + Measured ListHeaderComponent + height: {height} + + ); +} + +export default function HeaderMvcpFixture() { + const listRef = useRef(null); + const [headerHeight, setHeaderHeight] = useState(INITIAL_HEADER_HEIGHT); + const [scrollOffset, setScrollOffset] = useState(0); + + return ( + + + data={DATA} + estimatedItemSize={ROW_HEIGHT} + initialScrollIndex={ANCHOR_ROW_INDEX} + keyExtractor={(item) => item.id} + ListHeaderComponent={
} + maintainVisibleContentPosition={{ data: false, size: true }} + onScroll={(event) => setScrollOffset(event.nativeEvent.contentOffset.y)} + recycleItems + ref={listRef} + renderItem={({ item }) => ( + + {item.title} + + )} + style={styles.list} + /> + + + scrollOffset: {Math.round(scrollOffset)} + + setHeaderHeight((value) => value + 80)} style={styles.button}> + Grow header + + setHeaderHeight((value) => Math.max(24, value - 80))} + style={styles.button} + > + Shrink header + + listRef.current?.scrollToOffset({ animated: false, offset: 0 })} + style={styles.button} + > + Show header + + + + + ); +} + +const styles = StyleSheet.create({ + button: { + backgroundColor: "#1e3a8a", + borderRadius: 6, + paddingHorizontal: 12, + paddingVertical: 8, + }, + buttonRow: { + flexDirection: "row", + gap: 8, + }, + buttonText: { + color: "#ffffff", + fontSize: 13, + }, + container: { + backgroundColor: "#ffffff", + flex: 1, + }, + controls: { + backgroundColor: "#f8fafc", + borderTopColor: "#cbd5e1", + borderTopWidth: 1, + gap: 8, + padding: 12, + }, + header: { + backgroundColor: "#dbeafe", + borderBottomColor: "#bfdbfe", + borderBottomWidth: 1, + justifyContent: "center", + paddingHorizontal: 20, + }, + headerSubtitle: { + color: "#1e3a8a", + fontSize: 12, + }, + headerTitle: { + color: "#1e3a8a", + fontSize: 14, + fontWeight: "600", + }, + list: { + flex: 1, + }, + readout: { + color: "#0f172a", + fontSize: 13, + fontVariant: ["tabular-nums"], + }, + row: { + borderBottomColor: "#dbe3ef", + borderBottomWidth: 1, + height: ROW_HEIGHT, + justifyContent: "center", + paddingHorizontal: 20, + }, + rowText: { + color: "#0f172a", + fontSize: 15, + }, +}); diff --git a/example/screens/routes.tsx b/example/screens/routes.tsx index e9d86267..80c96b40 100644 --- a/example/screens/routes.tsx +++ b/example/screens/routes.tsx @@ -54,6 +54,7 @@ import CountriesWithHeadersFixedFixture from "~/screens/fixtures/countries-with- import CountriesWithHeadersStickyFixture from "~/screens/fixtures/countries-with-headers-sticky"; import ExtraDataFixture from "~/screens/fixtures/extra-data"; import FilterElementsFixture from "~/screens/fixtures/filter-elements"; +import HeaderMvcpFixture from "~/screens/fixtures/header-mvcp"; import HorizontalAlignItemsFixture from "~/screens/fixtures/horizontal-align-items"; import HorizontalCrossAxisFixture from "~/screens/fixtures/horizontal-cross-axis"; import InitialScrollAtEndEmptyFixture from "~/screens/fixtures/initial-scroll-at-end-empty"; @@ -259,6 +260,15 @@ export const FIXTURE_ROUTES: FixtureRouteDefinition[] = [ slug: "mvcp-test", title: "MVCP Test", }, + { + component: HeaderMvcpFixture, + description: "Grows and shrinks a measured header while it is above the viewport.", + groupKey: "scroll", + groupTitle: "Scroll & Position", + kind: "fixture", + slug: "header-mvcp", + title: "Header MVCP", + }, { component: AlwaysRenderFixture, description: "Keeps nearby cells mounted to inspect render-window behavior.", diff --git a/src/core/updateContentMetrics.ts b/src/core/updateContentMetrics.ts index 717b2aeb..a011e0f3 100644 --- a/src/core/updateContentMetrics.ts +++ b/src/core/updateContentMetrics.ts @@ -1,4 +1,3 @@ -import { Platform } from "@/platform/Platform"; import { peek$, type StateContext, set$ } from "@/state/state"; import type { Insets } from "@/types.base"; import { requestAdjust } from "@/utils/requestAdjust"; @@ -25,7 +24,6 @@ function shouldAdjustForHeaderSizeChange(ctx: StateContext, previousHeaderSize: const previousHeaderEnd = (leadingPadding || 0) + previousHeaderSize; return ( - Platform.OS === "web" && props.maintainVisibleContentPosition.size && didContainersLayout && didFinishInitialScroll &&