Reveal a sliver of later content when deep linking to an unread/linked action by MelvinBot · Pull Request #92469 · Expensify/App

MelvinBot · 2026-06-03T03:36:48Z

Explanation of Change

When deep linking to a report action (or jumping to the unread marker), the app scrolls the chat so the linked/oldest-unread action lands at the very bottom of the inverted list. Because the anchor was pinned flush against the bottom, it looked like the newest message in the conversation — there was no visual hint that more (newer) content existed below it.

This PR keeps the existing deep-link behaviour but, after the scroll handoff settles, nudges the list by a small fixed offset so a sliver (~100px) of later content peeks out underneath the anchored action, hinting "there is more here".

How it works:

The proven slice + two-frame RAF handoff in useFlashListScrollKey is preserved unchanged, so there is no flicker regression in the deep-link landing itself.
A new opt-in initialScrollOffset prop is threaded ReportActionsList → InvertedFlashList → useFlashListScrollKey. ReportActionsList passes the new constant CONST.REPORT.ACTIONS.INITIAL_LINKED_ACTION_SCROLL_OFFSET (100). The offset applies to both the linked-action deep link and the unread-marker anchor (the existing initialScrollKey = linkedReportActionID ?? unreadMarkerReportActionID).
On the second RAF (after maintainVisibleContentPosition is disabled), if an offset is set and the anchor is not already the newest action (anchorIndex > 0), we issue a single non-animated scrollToIndex({index, viewOffset: -offset}). A negative viewOffset on the inverted list scrolls toward newer content, revealing the offset px below the anchor. A useRef guard ensures this runs at most once.
When the anchor already is the newest action (anchorIndex <= 0), it no-ops, so the bottom is reached exactly as today.
The offset is opt-in, so the other InvertedFlashList consumer (BaseVideoPlayer) is unaffected.

The offset value lives in a single CONST, so its magnitude (and sign, if a platform needs tuning) is trivial to adjust.

Fixed Issues

$ #92152
PROPOSAL:

Tests

Open a chat/report that has unread messages with newer messages below the unread marker (e.g. mark an older message as unread, then have/receive newer messages after it).
Deep link to (or navigate so the app scrolls to) the oldest unread action / unread marker.
Verify the list still scrolls to the oldest unread action / unread marker.
Verify the anchored action is not flush against the bottom — roughly 100px of later (newer) content is visible underneath it.
Repeat with a direct deep link to a specific report action (linked action) that is not the newest message; verify the same ~100px peek of later content underneath it.
Deep link to / anchor on the newest action in a report; verify the bottom is reached as before (no extra offset, no empty gap).
Verify there is no visible flicker or jump during the landing animation.
Verify that no errors appear in the JS console.

Verify that no errors appear in the JS console

Offline tests

Same as Tests.

QA Steps

Same as Tests. Pay particular attention to verifying the ~100px peek of later content and the absence of flicker across all platforms (web, desktop, iOS native, iOS mWeb Safari, Android native, Android mWeb Chrome), since scroll/layout timing can differ per platform.

Verify that no errors appear in the JS console

PR Author Checklist

AI Tests

MelvinBot ran the following required-local checks for Expensify/App and all passed:

npm run typecheck (tsc, CI merge gate) — passed, no errors
npm run typecheck-tsgo — passed, no errors
npx prettier --check on all changed files — passed (one file auto-formatted)
eslint on the changed files — 0 errors (only pre-existing grandfathered eslint-seatbelt warnings in ReportActionsList.tsx, none from the added line)
npm run react-compiler-compliance-check check — the two logic files (InvertedFlashList/index.tsx, useFlashListScrollKey.ts) COMPILED. ReportActionsList.tsx fails compilation, but this is pre-existing — the unmodified main version fails with the identical 9 errors, so this PR does not regress it.
npm test for the relevant suites: FlashListTest, ReportActionsListHeaderTest, ReportActionsListPaddingViewTest — passed. (No existing unit test covers the slice/RAF/layout scroll-key path; that behaviour requires cross-platform manual QA.)

Could not be verified automatically (reviewer/QA must confirm): the actual ~100px peek and its direction, and the absence of flicker, on web, desktop, iOS native, iOS mWeb, Android native, Android mWeb. The single post-handoff scrollToIndex is non-animated; if any platform shows a flicker or the offset reveals the wrong side, the magnitude/sign in CONST.REPORT.ACTIONS.INITIAL_LINKED_ACTION_SCROLL_OFFSET is the single tuning point.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

…d action When deep linking (or jumping to the unread marker) the anchored action was pinned flush against the bottom of the inverted chat list, making it look like the newest message. After the deep-link handoff settles, nudge the list by a fixed offset so ~100px of later content peeks out underneath the anchor, hinting there is more below. No-ops when the anchor is already the newest action. Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

MelvinBot · 2026-06-03T03:37:15Z

🤖 I created this PR from an upstream branch (claude-deepLinkScrollOffset) since I don't have push access to a fork of yours.

To take ownership of this branch and be able to push updates from your own fork, run:

git remote add upstream https://github.com/Expensify/App.git
git fetch upstream claude-deepLinkScrollOffset
git checkout -b claude-deepLinkScrollOffset upstream/claude-deepLinkScrollOffset
git push -u origin claude-deepLinkScrollOffset

Then you can close this PR and open a new one from your fork, or just push directly to this branch if you have write access.

codecov · 2026-06-03T03:48:08Z

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines	Coverage Δ
src/CONST/index.ts	`93.79% <ø> (ø)`
...c/components/FlashList/InvertedFlashList/index.tsx	`100.00% <100.00%> (ø)`
src/pages/inbox/report/ReportActionsList.tsx	`72.54% <85.71%> (+0.43%)`	⬆️
... and 12 files with indirect coverage changes

aimane-chnaif · 2026-06-08T12:11:10Z

@MelvinBot pull main

…Offset Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

MelvinBot · 2026-06-08T12:17:17Z

🤖 Merged main into claude-deepLinkScrollOffset (no conflicts) and pushed. The branch is now up to date with main.

aimane-chnaif · 2026-06-08T12:25:03Z

+                if (!initialScrollOffset || hasAppliedInitialScrollOffset.current) {
+                    return;
+                }
+                const anchorIndex = data.findIndex((item, index) => keyExtractor(item, index) === initialScrollKey);


@MelvinBot can we reuse targetIndex?

Good catch — both spots computed the identical findIndex. I couldn't literally reuse the targetIndex variable: it's declared after the early returns in the render body (line 94), so it isn't in scope inside the effect's RAF closure. Instead I extracted a shared getAnchorIndex() helper and call it from both places: useFlashListScrollKey.ts:44.

It's a function rather than a precomputed const so we don't run findIndex on every render — it's only evaluated where it was before (the initial-render slice path and the post-handoff RAF). Pushed in d56afcc. Lint, typecheck, react-compiler, and FlashListTest all pass.

aimane-chnaif · 2026-06-08T12:25:44Z

+                ref?.current?.scrollToIndex({index: anchorIndex, viewOffset: -initialScrollOffset, animated: false});
+            });
        });
+        // eslint-disable-next-line react-hooks/exhaustive-deps


@MelvinBot is it possible to get rid of this disable comment without causing regressions?

@/tmp/reply-3373160660.md

aimane-chnaif · 2026-06-08T12:28:48Z

@MelvinBot your current solution doesn't work on native at all

Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

MelvinBot · 2026-06-08T12:44:11Z

You're right — the current approach works on web but is a no-op on native. Here's why, and the fix I'd recommend.

Root cause: The post-handoff ref.current.scrollToIndex({viewOffset: -100}) in useFlashListScrollKey.ts:82 loses a race against maintainVisibleContentPosition (MVCP) on native. FlashList v2 only disables offset correction on web (supportsOffsetCorrection: false); on iOS/Android the native ScrollView keeps re-pinning the anchor flush to the bottom, so the 100px nudge is immediately undone. Web has no such pinning, so it sticks.

Recommended fix: Drop the imperative scrollToIndex nudge entirely and instead shift the existing slice anchor so a few newer items render below the anchor. The deep-link slice at useFlashListScrollKey.ts:101 currently does data.slice(targetIndex), which lands the anchor flush at the visual bottom. Slicing from Math.max(0, targetIndex - N) instead includes N newer items below the anchor — using the same proven slice + MVCP handoff that already lands deep links correctly on every platform. No manual scroll, no MVCP race, no inverted-coordinate sign ambiguity.

The tradeoff: the reveal becomes item-count based (N newer messages peeking) rather than a fixed 100px. That's a small design change from the PR description, but it's the only approach that behaves identically on web and native here.

Why the pixel approach can't easily be made to work on native

scrollToIndex runs inside the same RAF2 callback that calls setHasLinkingSettled(true) — so MVCP is still enabled (disabled flips only on the next render). Deferring it to a useEffect keyed on hasLinkingSettled still races the native re-pin: FlashList's scrollToIndex runs a multi-step animation loop + a setTimeout(…, 200), during which applyOffsetCorrection fires on layout commits and re-pins the anchor.
On native, the un-sliced items above the anchor aren't measured within 2 RAFs, so scrollToIndex computes against estimated offsets and gets corrected once real measurements land.
FlashList v2 doesn't normalize viewOffset sign for inverted lists, so -100 is also ambiguous in inverted coordinates.

Evidence: node_modules/@shopify/flash-list/src/recyclerview/hooks/useRecyclerViewController.tsx (scrollToIndex L310-491, applyOffsetCorrection L119-213) and src/native/config/PlatformHelper.{ios,android,web}.ts (supportsOffsetCorrection).

I can implement the slice-shift fix, but two caveats: (1) it changes the offset from 100px to N items — let me know your preferred N (1 feels like a natural "sliver"); and (2) I can only verify web in my environment, so the native peek would still need your QA confirmation. Want me to go ahead with the slice-shift approach?

aimane-chnaif · 2026-06-08T12:48:14Z

I can implement the slice-shift fix, but two caveats: (1) it changes the offset from 100px to N items — let me know your preferred N (1 feels like a natural "sliver"); and (2) I can only verify web in my environment, so the native peek would still need your QA confirmation. Want me to go ahead with the slice-shift approach?

@MelvinBot Even 1 item can cause regression in case its height is large like report preview.

See:

Is there any better solution?

MelvinBot · 2026-06-08T12:54:43Z

You're right — item-granular slicing can't give a controlled sliver when the next item is tall. There is a fixed-pixel approach that works on native: use FlashList v2's built-in initialScrollIndex + initialScrollIndexParams.viewOffset instead of the slice + post-handoff scrollToIndex nudge.

Recommended approach

Point initialScrollIndex at the anchor's index in the full (unsliced) data, and pass initialScrollIndexParams={{viewOffset: -100}}.
Drop the data slice + double-RAF handoff + post-handoff scrollToIndex in useFlashListScrollKey.ts:65-101.

This gives a true ~100px reveal of real newer content (not empty space, not a whole item), applied at the initial scroll position (so no flicker — matches the issue's "ideally part of the initial scroll" criterion), with no per-row height measurement (so it doesn't reintroduce the render-delay problem that closed #90375).

Why this survives MVCP on native (where `scrollToIndex` didn't)

When initialScrollIndex is set, FlashList bakes the viewOffset into the very first scrollToOffset call, before MVCP captures its first-visible anchor. MVCP then locks onto the item at the already-offset position and maintains that gap — so the 100px sticks on iOS/Android, not just web. The post-handoff scrollToIndex loses because by the time it fires, native MVCP has already re-pinned the anchor flush and immediately undoes the nudge.

Evidence

initialScrollIndex / initialScrollIndexParams declared at @shopify/flash-list/src/FlashListProps.ts:188-198.
applyInitialScrollIndex applies viewOffset into the first scrollToOffset and sets pauseOffsetCorrection = true during that window: @shopify/flash-list/src/recyclerview/hooks/useRecyclerViewController.tsx:565-605.
MVCP's first-visible anchor is captured only after initial scroll completes (isInitialScrollComplete gated by getInitialScrollIndex() === undefined in RecyclerViewManager.ts:75; first-visible computed at useRecyclerViewController.tsx:92-112).
On native, applyOffsetCorrection → scrollAnchorRef.scrollBy(diff) re-pins the tracked item, which is exactly why the post-handoff nudge is a no-op (useRecyclerViewController.tsx:119-213; supportsOffsetCorrection is true on iOS/Android, false on web — PlatformHelper.{ios,android,web}.ts).
Inverted lists: viewOffset: -100 shifts the visible window toward newer content, revealing ~100px below the anchor.

Note: initialScrollIndex is already wired for the shouldFocusToTopOnMount branch in ReportActionsList.tsx — the deep-link path currently bypasses it in favor of slicing. The change is to drive initialScrollIndex/initialScrollIndexParams from the deep-link target instead.

Caveat: I can only verify web locally, so the native ~100px peek would still need your QA confirmation. Want me to implement this approach?

aimane-chnaif · 2026-06-08T12:55:33Z

@MelvinBot implement this after fully reverting your current changes.

…sitioning Replace the data-slice + double-RAF handoff + post-handoff scrollToIndex nudge with FlashList v2's native initialScrollIndex and initialScrollIndexParams driven from the deep-link/unread anchor. A small negative viewOffset reveals a ~100px sliver of later content beneath the anchor so it doesn't look like the last message. MVCP is kept enabled while deep linking so it locks onto the offset initial scroll position. Drops the useFlashListScrollKey slice machinery entirely and inlines MVCP in InvertedFlashList. Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

MelvinBot · 2026-06-08T13:12:40Z

🤖 Done. I fully reverted the previous approach and reimplemented per this comment.

What changed (`c5aa0bdab76`)

Removed the data-slice + double-RAF handoff + post-handoff scrollToIndex nudge:

Deleted src/components/FlashList/useFlashListScrollKey.ts (the slice machinery).
InvertedFlashList/index.tsx: dropped the hook and inlined maintainVisibleContentPosition.

Added FlashList v2's native positioning in ReportActionsList.tsx:

initialScrollIndex now points at the deep-link/unread anchor's real index in the full data.
initialScrollIndexParams={{viewOffset: -CONST.REPORT.ACTIONS.INITIAL_LINKED_ACTION_SCROLL_OFFSET}} — a small negative viewOffset on the inverted list reveals a ~100px sliver of later content beneath the anchor so it doesn't look like the last message.
MVCP is kept enabled while deep linking (|| !!initialScrollKey) so it locks onto that offset position and holds the revealed sliver.
New constant INITIAL_LINKED_ACTION_SCROLL_OFFSET: 100 in CONST/index.ts.

AI Tests

Check	Result
`prettier --log-level error`	✅ clean
`lint-changed`	✅ exit 0
`typecheck` (tsc)	✅ exit 0
`jest` (FlashListTest, ReportActionsListHeaderTest, ReportActionsListPaddingViewTest)	✅ 8 passed
`react-compiler-compliance-check` — InvertedFlashList	✅ COMPILED
`react-compiler-compliance-check` — ReportActionsList	⚠️ FAILED (`149:0: Cannot access refs during render`) — pre-existing on main; my edits start at line 301, lines 1–300 are byte-identical to main

Needs QA

The native peek (~100px sliver) and absence of scroll flicker are web-verifiable behaviors I can't exercise here — please confirm the deep-link/unread scroll lands correctly and reveals the sliver beneath the anchor.

aimane-chnaif · 2026-06-08T13:29:00Z

@MelvinBot the new solution causes regression.

Older/Newer adjacent messages don't appear immediately like production app does.

Can you fix this?

MelvinBot · 2026-06-08T13:39:51Z

Root cause

The blank below the anchor is a structural limitation of FlashList v2.3.0's initialScrollIndex, not a tunable bug.

On the first paint, RecyclerViewManager.applyInitialScrollAdjustment() sets the render window's scrollOffset to exactly the anchor's y — without the viewOffset:

// node_modules/@shopify/flash-list/.../RecyclerViewManager.ts:391-397
this.layoutManager.recomputeLayouts(0, initialScrollIndex);
const initialItemOffset = this.layoutManager.getLayout(initialScrollIndex).y;
this.engagedIndicesTracker.scrollOffset = initialItemOffset; // viewOffset NOT included

renderProgressively() then computes the visible window as [anchorY, anchorY + viewportHeight] with no draw-distance buffer on this first frame. In an inverted list that window contains the anchor + older messages (higher indices, visually above). The newer messages (lower indices, visually below the anchor — the ~100px we want to reveal) have y < anchorY, so they fall outside the first window and paint blank.

The viewOffset: -100 is applied a moment later via an imperative scrollToOffset (applyInitialScrollIndex), which only re-renders the below-anchor items after the resulting onScroll round-trips. That round-trip is the visible blank frame you captured.

Why we keep playing whack-a-mole

Each approach trades one regression for another because of this same limitation:

Slice (production): anchor becomes index 0 → first window is [0, viewport] → neighbors paint immediately, works on native. But it slices off all newer items, so it structurally cannot reveal content below the anchor (that's the flush-bottom behavior Add ~100 pixels of scroll offset when deep linking to unread/linked report action #92152 is trying to fix).
Slice + post-handoff scrollToIndex: no-op on native (MVCP re-pins).
Slice-shift by N items: tall items overshoot.
initialScrollIndex + viewOffset (current): reveals the 100px and sticks on native, but blanks the below-anchor neighbors on first paint — this regression.

The realistic options

Option A — Patch flash-list to bake viewOffset into the first-paint window. Make applyInitialScrollAdjustment start the initial window at anchorY + viewOffset (i.e. anchorY - 100) so the 100px sliver below the anchor is inside the first render window and paints immediately — no imperative-scroll round-trip, no blank. Patching is already an established pattern here (see patches/@shopify/flash-list/...+004+fix-inverted-first-item-offset.patch). This is the only way to get the exact ~100px reveal and immediate neighbors.

⚠️ Blast radius: that function also runs for the shouldFocusToTopOnMount path (which passes viewOffset: windowHeight) and any other initialScrollIndex consumer, so the patch must guard so it only shifts the window for the deep-link case. And it's a core scroll path I can only verify on web — native (iOS/Android) behavior would need your QA, same as before.

Option B — Keep the proven slice landing, drop the pixel reveal. Revert to the slice approach (immediate neighbors, all platforms, zero new risk) and signal "there's more below" with a non-scroll affordance instead — e.g. the existing floating new-messages counter — rather than a 100px peek. Lower risk, but it changes the UX the issue asked for.

My recommendation

Given that the last three native regressions were all caught by you (I can't exercise iOS/Android here), I don't want to push a 4th blind change to a core scroll path. Option A is the only path that satisfies the issue exactly, and patching flash-list is in-pattern for this repo — but it touches a hot path and needs native QA.

Do you want me to implement Option A (guarded flash-list patch + revert the app-side viewOffset plumbing to drive it), or fall back to Option B? I'll verify web on whichever you pick; native QA would be on you either way.

MelvinBot assigned aimane-chnaif Jun 3, 2026

MelvinBot requested a review from a team June 3, 2026 03:36

MelvinBot mentioned this pull request Jun 3, 2026

Add ~100 pixels of scroll offset when deep linking to unread/linked report action #92152

Open

Merge remote-tracking branch 'origin/main' into claude-deepLinkScroll…

bbdd617

…Offset Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

aimane-chnaif reviewed Jun 8, 2026

View reviewed changes

Dedupe anchor index lookup into a shared helper

d56afcc

Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

Conversation

MelvinBot commented Jun 3, 2026

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

AI Tests

Screenshots/Videos

Uh oh!

MelvinBot commented Jun 3, 2026

Uh oh!

codecov Bot commented Jun 3, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aimane-chnaif commented Jun 8, 2026

Uh oh!

MelvinBot commented Jun 8, 2026

Uh oh!

aimane-chnaif Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

MelvinBot Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

aimane-chnaif Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

MelvinBot Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

aimane-chnaif commented Jun 8, 2026

Uh oh!

MelvinBot commented Jun 8, 2026

Uh oh!

aimane-chnaif commented Jun 8, 2026

Uh oh!

MelvinBot commented Jun 8, 2026

Recommended approach

Why this survives MVCP on native (where scrollToIndex didn't)

Uh oh!

aimane-chnaif commented Jun 8, 2026

Uh oh!

MelvinBot commented Jun 8, 2026

What changed (c5aa0bdab76)

AI Tests

Needs QA

Uh oh!

aimane-chnaif commented Jun 8, 2026

Uh oh!

MelvinBot commented Jun 8, 2026

Root cause

Why we keep playing whack-a-mole

The realistic options

My recommendation

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Jun 3, 2026 •

edited by github-actions Bot

Loading

Why this survives MVCP on native (where `scrollToIndex` didn't)

What changed (`c5aa0bdab76`)