Skip to content

fix: preserve native MVCP position on item resize against a bottom inset #481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
trcoffman wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

fix: preserve native MVCP position on item resize against a bottom inset #481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
109ab29
Make AI Messages expand/contract in fixture
trcoffman Jun 18, 2026
325df8b
fix: preserve native MVCP position on item resize against a bottom inset
trcoffman Jun 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions __tests__/core/doMaintainScrollAtEnd.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ describe("doMaintainScrollAtEnd", () => {
mockState.pendingNativeMVCPAdjust = {
amount: -40,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: 0,
startScroll: 100,
};
Expand All @@ -199,6 +200,7 @@ describe("doMaintainScrollAtEnd", () => {
mockState.pendingNativeMVCPAdjust = {
amount: -40,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: 0,
startScroll: 100,
};
Expand Down
200 changes: 200 additions & 0 deletions __tests__/core/mvcp.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,158 @@ describe("mvcp helpers", () => {
}
});

// Builds a native list whose viewport overlaps the bottom inset zone. The anchor item
// (item-1) is the first item in view; moving its position up (simulating an above-viewport
// shrink) makes MVCP want a negative adjust. With a bottom inset this is the geometry where
// the spacer-only adjust fights the native end-clamp and must be routed through the handoff.
//
// NOTE: these unit mocks do not shrink `totalSize`/`sizes` when `positions` is hand-edited,
// so `getContentSize`/maxScroll stay at their pre-shrink values. That faithfully exercises the
// ARMING decision and the isResize routing, but not the partial native-clamp split (where
// predictedNativeClamp is a non-zero fraction of the amount) — that split is covered by the
// on-device verification and the resolve-path tests below that set the pending directly.
const buildNativeResizeAgainstInsetContext = (anchoredEndSpaceVisible: boolean) => {
const SCROLL_LENGTH = 300;
const ANCHORED_END_SPACE = 250;
const RAW_CONTENT = 580;

const mockCtx = createMockContext(
{
anchoredEndSpaceSize: anchoredEndSpaceVisible ? ANCHORED_END_SPACE : 0,
readyToRender: true,
totalSize: RAW_CONTENT,
},
{
didContainersLayout: true,
didFinishInitialScroll: true,
hasScrolled: true,
idCache: ["item-0", "item-1", "item-2"],
idsInView: ["item-1", "item-2"],
indexByKey: new Map([
["item-0", 0],
["item-1", 1],
["item-2", 2],
]),
positions: [0, 400, 500],
props: {
anchoredEndSpace: anchoredEndSpaceVisible ? { anchorIndex: 1, includeInEndInset: true } : undefined,
data: [{ id: 0 }, { id: 1 }, { id: 2 }],
keyExtractor: (item: { id: number }) => `item-${item.id}`,
maintainVisibleContentPosition: normalizeMaintainVisibleContentPosition(true),
},
scrollLength: SCROLL_LENGTH,
sizes: new Map([
["item-0", 400],
["item-1", 100],
["item-2", 80],
]),
},
);

// Scroll to the end (content size includes the blank inset).
const contentSize = RAW_CONTENT + (anchoredEndSpaceVisible ? ANCHORED_END_SPACE : 0);
mockCtx.state.scroll = Math.max(0, contentSize - SCROLL_LENGTH);
return mockCtx;
};

it("routes a resize in the bottom-inset zone through the native-clamp handoff", () => {
Platform.OS = "ios";
const mockCtx = buildNativeResizeAgainstInsetContext(/* anchoredEndSpaceVisible */ true);

const requestAdjustSpy = spyOn(requestAdjustModule, "requestAdjust");
try {
const adjustFunction = prepareMVCP(mockCtx);
// Item-0 above the viewport shrank by 200, so the anchor (item-1) recomputes up by 200.
mockCtx.state.positions[1] = 200;

adjustFunction?.();

// The resize against a bottom inset is routed through the native-clamp handoff (a pending
// adjust is queued with isResize) rather than a plain spacer adjust, so it can reconcile
// against native instead of being eaten by the end-clamp.
expect(mockCtx.state.pendingNativeMVCPAdjust).toBeDefined();
expect(mockCtx.state.pendingNativeMVCPAdjust?.amount).toBeCloseTo(-200, 1);
expect(mockCtx.state.pendingNativeMVCPAdjust?.isResize).toBe(true);
} finally {
requestAdjustSpy.mockRestore();
}
});

it("does not queue a handoff for a resize with no bottom inset", () => {
Platform.OS = "ios";
const mockCtx = buildNativeResizeAgainstInsetContext(/* anchoredEndSpaceVisible */ false);

const requestAdjustSpy = spyOn(requestAdjustModule, "requestAdjust");
try {
const adjustFunction = prepareMVCP(mockCtx);
mockCtx.state.positions[1] = 200;

adjustFunction?.();

// Without a bottom inset there is no end-clamp to fight, so the plain adjust path runs.
expect(mockCtx.state.pendingNativeMVCPAdjust).toBeUndefined();
expect(requestAdjustSpy).toHaveBeenCalledWith(mockCtx, -200, undefined);
} finally {
requestAdjustSpy.mockRestore();
}
});

it("does not queue a handoff for a mid-list resize against a bottom inset", () => {
Platform.OS = "ios";
const mockCtx = buildNativeResizeAgainstInsetContext(/* anchoredEndSpaceVisible */ true);
// Move well away from the end (viewport entirely above the inset zone) so the native clamp
// would not eat the adjustment and the plain spacer path is correct.
mockCtx.state.scroll = 100;

const requestAdjustSpy = spyOn(requestAdjustModule, "requestAdjust");
try {
const adjustFunction = prepareMVCP(mockCtx);
mockCtx.state.positions[1] = 200;

adjustFunction?.();

expect(mockCtx.state.pendingNativeMVCPAdjust).toBeUndefined();
expect(requestAdjustSpy).toHaveBeenCalledWith(mockCtx, -200, undefined);
} finally {
requestAdjustSpy.mockRestore();
}
});

it("arms the handoff for a resize while only PARTIALLY into the inset zone", () => {
// Regression for the partial-inset upward-shift bug: the viewport overlaps the bottom inset
// but is not pinned hard at the end, so the handoff must arm even though the shrink does not
// exceed the distance to the end. Pre-fix this fell through to a plain requestAdjust(-200)
// which over-compensated upward (native had room and absorbed nothing).
Platform.OS = "ios";
const mockCtx = buildNativeResizeAgainstInsetContext(/* anchoredEndSpaceVisible */ true);
// contentSize=830, realContentEnd=830-250=580. Park partway into the inset zone: the
// viewport [430,730] covers 150px of real content + 150px of inset.
mockCtx.state.scroll = 430;

const requestAdjustSpy = spyOn(requestAdjustModule, "requestAdjust");
try {
const adjustFunction = prepareMVCP(mockCtx);
mockCtx.state.positions[1] = 200;

adjustFunction?.();

// It arms the handoff (isResize) instead of taking the plain spacer path.
expect(mockCtx.state.pendingNativeMVCPAdjust).toBeDefined();
expect(mockCtx.state.pendingNativeMVCPAdjust?.amount).toBeCloseTo(-200, 1);
expect(mockCtx.state.pendingNativeMVCPAdjust?.isResize).toBe(true);
} finally {
requestAdjustSpy.mockRestore();
}
});

it("settles immediately when only the manual native MVCP adjustment remained", () => {
const mockCtx = createMockContext(
{ totalSize: 300 },
{
pendingNativeMVCPAdjust: {
amount: -80,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: -80,
startScroll: 420,
},
Expand All @@ -117,4 +262,59 @@ describe("mvcp helpers", () => {
requestAdjustSpy.mockRestore();
}
});

// When native reaches its true max with a large remaining amount (a big shrink scrolled deep
// into the now-gone content), a resize must settle WITHOUT a further spacer adjust — applying
// the leftover would force native to re-clamp and overshoot the visible position. A data change
// in the same situation keeps its tuned behavior of applying the remainder. These two tests pin
// that divergence (the hardest-won part of the fix); reverting the resize branch fails the first.
const buildClampSettleContext = (isResize: boolean) =>
createMockContext(
// totalSize 300 + scrollLength 100 => native max scroll is 200.
{ totalSize: 300 },
{
pendingNativeMVCPAdjust: {
amount: -300,
furthestProgressTowardAmount: 0,
isResize,
manualApplied: -80,
startScroll: 420,
},
scrollLength: 100,
},
);

it("settles a resize at the native clamp without applying a further spacer adjust", () => {
Platform.OS = "ios";
const mockCtx = buildClampSettleContext(/* isResize */ true);
const requestAdjustSpy = spyOn(requestAdjustModule, "requestAdjust");
try {
// newScroll === expectedNativeClampScroll (200) => native has clamped to its true max.
const didSettle = resolvePendingNativeMVCPAdjust(mockCtx as StateContext, 200);

expect(didSettle).toBe(true);
expect(mockCtx.state.pendingNativeMVCPAdjust).toBeUndefined();
// The resize must NOT nudge the spacer further once native is pinned at the clamp.
expect(requestAdjustSpy).not.toHaveBeenCalled();
} finally {
requestAdjustSpy.mockRestore();
}
});

it("applies the remaining amount for a data change at the native clamp (unchanged behavior)", () => {
Platform.OS = "ios";
const mockCtx = buildClampSettleContext(/* isResize */ false);
const requestAdjustSpy = spyOn(requestAdjustModule, "requestAdjust");
try {
// Same geometry, but a data change still applies the leftover remainder via settle.
const didSettle = resolvePendingNativeMVCPAdjust(mockCtx as StateContext, 200);

expect(didSettle).toBe(true);
expect(mockCtx.state.pendingNativeMVCPAdjust).toBeUndefined();
// remainingAfterManual(-220) - nativeDelta(200 - (420 + -80) = -140) = -80 applied.
expect(requestAdjustSpy).toHaveBeenCalledWith(mockCtx, -80, true);
} finally {
requestAdjustSpy.mockRestore();
}
});
});
3 changes: 3 additions & 0 deletions __tests__/core/prepareMVCP.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ describe("prepareMVCP", () => {
mockState.pendingNativeMVCPAdjust = {
amount: -300,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: 0,
startScroll: 420,
};
Expand Down Expand Up @@ -349,6 +350,7 @@ describe("prepareMVCP", () => {
mockState.pendingNativeMVCPAdjust = {
amount: -300,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: 0,
startScroll: 420,
};
Expand Down Expand Up @@ -376,6 +378,7 @@ describe("prepareMVCP", () => {
mockState.pendingNativeMVCPAdjust = {
amount: -300,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: 0,
startScroll: 420,
};
Expand Down
14 changes: 14 additions & 0 deletions __tests__/core/updateScroll.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ describe("updateScroll large user jumps", () => {
mockCtx.state.pendingNativeMVCPAdjust = {
amount: -500,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: 0,
startScroll: 0,
};
Expand Down Expand Up @@ -84,6 +85,7 @@ describe("updateScroll large user jumps", () => {
mockCtx.state.pendingNativeMVCPAdjust = {
amount: 500,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: 0,
startScroll: 0,
};
Expand Down Expand Up @@ -177,6 +179,7 @@ describe("updateScroll mvcp active mode", () => {
mockCtx.state.pendingNativeMVCPAdjust = {
amount: -300,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: 0,
startScroll: 420,
};
Expand All @@ -196,6 +199,7 @@ describe("updateScroll mvcp active mode", () => {
mockCtx.state.pendingNativeMVCPAdjust = {
amount: -300,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: 0,
startScroll: 420,
};
Expand All @@ -215,6 +219,7 @@ describe("updateScroll mvcp active mode", () => {
mockCtx.state.pendingNativeMVCPAdjust = {
amount: -300,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: 0,
startScroll: 420,
};
Expand All @@ -241,6 +246,7 @@ describe("updateScroll mvcp active mode", () => {
mockCtx.state.pendingNativeMVCPAdjust = {
amount: -300,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: 0,
startScroll: 420,
};
Expand All @@ -260,6 +266,7 @@ describe("updateScroll mvcp active mode", () => {
mockCtx.state.pendingNativeMVCPAdjust = {
amount: -300,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: -80,
startScroll: 420,
};
Expand All @@ -286,6 +293,7 @@ describe("updateScroll mvcp active mode", () => {
mockCtx.state.pendingNativeMVCPAdjust = {
amount: -300,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: -80,
startScroll: 420,
};
Expand All @@ -305,6 +313,7 @@ describe("updateScroll mvcp active mode", () => {
mockCtx.state.pendingNativeMVCPAdjust = {
amount: -300,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: -80,
startScroll: 420,
};
Expand All @@ -324,6 +333,7 @@ describe("updateScroll mvcp active mode", () => {
mockCtx.state.pendingNativeMVCPAdjust = {
amount: -300,
furthestProgressTowardAmount: 120,
isResize: false,
manualApplied: 0,
startScroll: 420,
};
Expand All @@ -346,6 +356,7 @@ describe("updateScroll mvcp active mode", () => {
pendingNativeMVCPAdjust: {
amount: -20,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: 0,
startScroll: 100,
},
Expand Down Expand Up @@ -385,6 +396,7 @@ describe("updateScroll mvcp active mode", () => {
pendingNativeMVCPAdjust: {
amount: -92.25,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: -37.91664632161462,
startScroll: 984.6666666666666,
},
Expand Down Expand Up @@ -427,6 +439,7 @@ describe("updateScroll mvcp active mode", () => {
pendingNativeMVCPAdjust: {
amount: -100,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: -38.16664632161451,
startScroll: 1813.6666666666667,
},
Expand Down Expand Up @@ -468,6 +481,7 @@ describe("updateScroll mvcp active mode", () => {
pendingNativeMVCPAdjust: {
amount: -20,
furthestProgressTowardAmount: 0,
isResize: false,
manualApplied: 0,
startScroll: 100,
},
Expand Down
Loading