Scroll forward immediately on approve/pay/submit instead of after button animation by MelvinBot · Pull Request #98515 · Expensify/App

MelvinBot · 2026-08-13T00:43:05Z

Explanation of Change

When a user approves/pays/submits an older report preview from the workspace chat, the auto-scroll that follows the action badge to the next actionable preview was waiting for the resolve button animation (submit/approve/pay) to finish before scrolling. That delay made the forward-scroll feel laggy.

This change makes the forward-scroll fire immediately on click (on the next frame, driven by the optimistic badge-target advance) instead of waiting for the animation. The resolved preview keeps playing its success animation in place while the list scrolls to the next actionable preview.

Since the animation wait was the only reason getActionBadgeScrollDelay and the actionBadge/prevActionBadge plumbing existed, they are removed and useFollowActionBadgeTarget now always schedules the scroll on the next requestAnimationFrame. The direction/should-follow logic (shouldFollowActionBadgeTarget) is unchanged, so the scroll still only follows the badge downward to newer actionable previews.

Files changed:

src/pages/inbox/report/useFollowActionBadgeTarget.ts — always scroll on the next frame; dropped the animation-delay branch and the now-unused actionBadge param.
src/pages/inbox/report/getActionBadgeScrollDelay.ts — deleted (only existed to compute the animation wait).
tests/unit/getActionBadgeScrollDelayTest.ts — deleted alongside the helper.
src/pages/inbox/report/ReportActionsList.tsx — stopped passing the removed actionBadge param.

Fixed Issues

$ #96427

Tests

Open a workspace chat that has multiple report previews requiring action (e.g. several reports needing Approve/Pay), with the oldest actionable one scrolled above the current viewport.
Approve (or Pay/Submit) the oldest actionable report preview.
Verify the chat scrolls forward to the next actionable report preview.

Verify that no errors appear in the JS console

Offline tests

Same as Tests

QA Steps

Same as Tests

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

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

aimane-chnaif · 2026-08-13T00:45:03Z

+ * Once the current action-badge target is resolved (e.g. the user approves/pays/submits an older report preview), the badge target
+ * advances to the next report preview that requires action. This hook follows it by scrolling down to the new target immediately, so
+ * the scroll starts as soon as the action is taken (on click / optimistically) rather than waiting for the resolve animation to
+ * finish.


@MelvinBot simplify comment

Simplified the hook's JSDoc down to two lines in 6e5b187 — same meaning, less verbose.

aimane-chnaif · 2026-08-13T00:45:12Z

+    // Keep the latest scroll callback in a ref so a scroll scheduled on the next frame still targets the current badge index.
+    // The effect below only re-runs when the target id changes, so without this the scheduled callback would close over a stale
+    // target index if the list shifts (new message, pagination, resolved preview collapsing) before the frame runs.


@MelvinBot simplify comment

Trimmed the ref comment from three lines to two in 6e5b187, keeping the "why" (stale index if the list shifts before the frame runs).

aimane-chnaif · 2026-08-13T00:45:20Z

+        // Scroll to the next target immediately (on the next frame) so the forward-scroll starts as soon as the user acts, rather
+        // than waiting for the submit/approve/pay resolve animation to finish. The resolved preview keeps playing its success
+        // animation in place while the list scrolls to the next actionable preview.


@MelvinBot simplify comment

Condensed the scroll comment to two lines in 6e5b187.

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

MelvinBot · 2026-08-13T00:49:41Z

The one real check failure here is unrelated to this PR — I'm not pushing a fix for it.

Failing test: tests/actions/AppTest.ts › actions/App › trigger full reconnect (AppTest.ts:127) in test (job 8).

Why it's unrelated: this PR only touches four inbox-report auto-scroll files (ReportActionsList.tsx, useFollowActionBadgeTarget.ts, and the deleted getActionBadgeScrollDelay.ts + its test). The failing test exercises app reconnect logic that lives entirely in App.ts / FullReconnectUtils.ts — none of which this PR modifies.

Why it's flaky: the test seeds LAST_FULL_RECONNECT_TIME during openApp(), then immediately sets serverReconnectCutoff = DateUtils.getDBTime() and expects a reconnect to fire. The reconnect only triggers when the cutoff is after the recorded reconnect time, so when both land in the same millisecond the reconnect is (correctly) suppressed and triggerFullReconnect is called 0 times instead of 1 — a timing race, not a logic error introduced here.

The remaining red checks (checklist / PR Author Checklist, Check independent approval) are the human-completed checklist and approval gates, which I intentionally don't fill out.

I'd recommend re-running the failed test (job 8) job to clear the flake (I attempted the re-run but don't have permission to trigger it in this environment).

MelvinBot · 2026-08-13T00:50:03Z

🤖 Addressed the review feedback — simplified all three comments in src/pages/inbox/report/useFollowActionBadgeTarget.ts in 6e5b187:

The hook JSDoc: 4 lines → 2 lines
The scroll-callback ref comment: 3 lines → 2 lines
The requestAnimationFrame scroll comment: 3 lines → 2 lines

Each kept its meaning and its "why", just tightened. Formatted and lint-clean. Replied in each thread as well.

codecov · 2026-08-13T00:53:36Z

Codecov Report

❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.

Files with missing lines	Coverage Δ
src/pages/inbox/report/ReportActionsList.tsx	`88.40% <ø> (-0.09%)`	⬇️
...c/pages/inbox/report/useFollowActionBadgeTarget.ts	`69.23% <0.00%> (+13.67%)`	⬆️
... and 260 files with indirect coverage changes

MelvinBot · 2026-08-13T00:53:42Z

Update: the flaky test failure has cleared on its own. test (job 8) — which earlier failed on the unrelated, timing-sensitive tests/actions/AppTest.ts › trigger full reconnect — now passes on the fresh CI run, along with typecheck and the rest of the test suite.

The only remaining red checks are checklist (PR Author Checklist) and Check independent approval — the human-completed checklist and approval gates, which I intentionally don't fill out. No code fix was needed.

MelvinBot · 2026-08-13T00:54:42Z

🤖 Already handled — all three comments in src/pages/inbox/report/useFollowActionBadgeTarget.ts were simplified in 6e5b187 (each trimmed to two lines, keeping the "why"). If you'd like them shorter still, or want a specific one reworded, point me at it and I'll take another pass.

MelvinBot · 2026-08-13T00:56:37Z

Good news — the failure cleared on its own. The latest CI run has all real checks green, including test (job 8), which previously failed on the flaky tests/actions/AppTest.ts › trigger full reconnect timing test (AppTest.ts:127). It passed on a fresh run with no code change, confirming it was a flake unrelated to this PR (which only touches inbox-report auto-scroll files).

No fix was needed. The only remaining red checks are the human-gated PR Author Checklist and Check independent approval, which I intentionally leave for a person to complete.

aimane-chnaif · 2026-08-13T08:50:19Z

@MelvinBot pull main

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

MelvinBot · 2026-08-13T08:57:52Z

🤖 Merged main into claude-scrollForwardImmediatelyOnApprove and pushed.

Merged 63 new commits from main (up to 4adc694) — no conflicts.
Merge commit: 9c70f80

The branch is now up to date with main.

MelvinBot · 2026-08-13T09:05:24Z

The failing test (job 1) check is unrelated to this PR — it's a pre-existing failure in a sign-in test, not in the auto-scroll code this PR changes. I'm not pushing any changes.

Failing test: tests/unit/SignInBackButtonTest.tsx → TypeError: _Navigation.default.dismissModal is not a function (case: "registers no back listener that consumes the press without navigating").

Why it's unrelated:

This PR only touches report auto-scroll files (src/pages/inbox/report/* plus a deleted helper/test). It doesn't touch sign-in, Navigation, or this test.
SignInBackButtonTest.tsx and SignInModal.tsx are byte-identical between this branch and origin/main, so the failure reproduces on main independently of these changes.

Root cause (for whoever owns this test)

The test's Navigation mock stubs only goBack and omits dismissModal (SignInBackButtonTest.tsx:12-19), but SignInModal calls Navigation.dismissModal() on render (SignInModal.tsx:60). Adding dismissModal: () => {} to the mock would fix it — but that belongs in a separate PR since it's a main issue, not caused by this change.

The other two red checks — Check independent approval and checklist (PR Author Checklist) — are process/checklist checks, not code failures.

MelvinBot · 2026-08-13T09:12:00Z

Refining my earlier analysis now that I understand why test (job 1) fails only on this PR: it's a pre-existing, order-dependent flaky test that this PR's changes expose through test re-sharding. The failing file isn't in this PR's diff, so per policy I'm reporting rather than fixing it here — but the fix is small and I've pinpointed it below.

Failing test: tests/unit/SignInBackButtonTest.tsx → TypeError: _Navigation.default.dismissModal is not a function.

Why it fails only here (verified):

The same test passes on other main-based PRs (e.g. Fix flash of disabled feature page before redirect to More features #98549, test (job 1) green), and SignInBackButtonTest.tsx + SignInModal.tsx are byte-identical between this branch and origin/main. So the PR's auto-scroll code doesn't touch anything involved.
CI runs jest --shard=${chunk}/8 (test.yml:46). Jest's --shard partitions the full sorted list of test files, so this PR deleting tests/unit/getActionBadgeScrollDelayTest.ts shifts the partition boundaries and moves SignInBackButtonTest.tsx into shard 1, alongside a suite that leaves global state set.

Root cause of the latent flake (the actual bug in the sign-in test)

SignInModal calls Navigation.dismissModal() only inside a useEffect gated on isLoadingApp === false (SignInModal.tsx:53-64), where isLoadingApp comes from the global Onyx key IS_LOADING_APP. The test's Navigation mock only stubs goBack and omits dismissModal (SignInBackButtonTest.tsx:12-19), and the suite never resets IS_LOADING_APP.

So when a prior suite in the same shard/worker leaves IS_LOADING_APP=false in Onyx, that effect fires and hits the missing dismissModal stub → crash. When IS_LOADING_APP is unset (the ordering on other PRs), the effect early-returns and the test passes. Hence the order dependence.

Fix (belongs in a separate PR to the sign-in test, not this one): add dismissModal: () => {} to the Navigation mock, and/or clear IS_LOADING_APP (e.g. Onyx.clear() / Onyx.set(ONYXKEYS.IS_LOADING_APP, undefined)) in beforeEach. Once that lands on main and this branch merges main, test (job 1) will go green regardless of sharding.

The other two red checks — Check independent approval and checklist (PR Author Checklist) — are process checks, not code failures.

I can open that separate PR to harden SignInBackButtonTest.tsx on main if you'd like — just say the word.

aimane-chnaif · 2026-08-13T11:04:51Z

@MelvinBot no scroll animation on native

MelvinBot · 2026-08-13T11:15:55Z

Native already scrolls without animation — the badge-follow scroll relies on the platform-split scroll manager, which jumps instantly on native and animates only on web. So no code change is needed for that behavior, but I can make it explicit if you'd prefer.

Why native doesn't animate today

The forward-scroll calls scrollToActionBadgeTarget, which calls reportScrollManager.scrollToIndex(...) without an animated option.
useReportScrollManager is platform-split, so animated falls back to a different default per platform:
- Native → index.native.ts:16 defaults animated = false → instant jump, no animation.
- Web → index.ts:12 defaults animated = true → smooth scroll.
This is asserted by useReportScrollManagerTest.tsx:90-97: "defaults to a non-animated (instant) scroll on native".
Note the same scrollToActionBadgeTarget also backs the manual badge tap (ReportActionsList.tsx:453), so keeping web animated is intentional there — disabling animation unconditionally would also remove it on web.

If you'd still like it stated explicitly in code (e.g. an explicit animated: false for native at the call site rather than leaning on the platform default), or if you actually observed a scroll animation on a native build, let me know and I'll push that change and adjust the tests.

aimane-chnaif · 2026-08-13T11:26:23Z

Details

Explanation of Change

When a user approves/pays/submits an older report preview from the workspace chat, the auto-scroll that follows the action badge to the next actionable preview was waiting for the resolve button animation (submit/approve/pay) to finish before scrolling. That delay made the forward-scroll feel laggy.

This change makes the forward-scroll fire immediately on click (on the next frame, driven by the optimistic badge-target advance) instead of waiting for the animation. The resolved preview keeps playing its success animation in place while the list scrolls to the next actionable preview.

Since the animation wait was the only reason getActionBadgeScrollDelay and the actionBadge/prevActionBadge plumbing existed, they are removed and useFollowActionBadgeTarget now always schedules the scroll on the next requestAnimationFrame. The direction/should-follow logic (shouldFollowActionBadgeTarget) is unchanged, so the scroll still only follows the badge downward to newer actionable previews.

Files changed:

src/pages/inbox/report/useFollowActionBadgeTarget.ts — always scroll on the next frame; dropped the animation-delay branch and the now-unused actionBadge param.
src/pages/inbox/report/getActionBadgeScrollDelay.ts — deleted (only existed to compute the animation wait).
tests/unit/getActionBadgeScrollDelayTest.ts — deleted alongside the helper.
src/pages/inbox/report/ReportActionsList.tsx — stopped passing the removed actionBadge param.

Fixed Issues

$ #96427

Tests

Open a workspace chat that has multiple report previews requiring action (e.g. several reports needing Approve/Pay), with the oldest actionable one scrolled above the current viewport.
Approve (or Pay/Submit) the oldest actionable report preview.
Verify the chat scrolls forward to the next actionable report preview.

Verify that no errors appear in the JS console

Offline tests

Same as Tests

QA Steps

Same as Tests

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

@MelvinBot please set the PR body to the content in the details section above

MelvinBot · 2026-08-13T11:34:34Z

🤖 Updated the PR body to the content you provided.

aimane-chnaif · 2026-08-13T11:36:54Z

@MelvinBot mark as ready for review

melvin-bot · 2026-08-13T11:41:52Z

@aimane-chnaif Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

MelvinBot · 2026-08-13T11:41:54Z

🤖 Marked this PR as ready for review.

Note: the Tests and QA Steps sections in the PR description are still placeholders (// TODO) and the author checklist is unchecked — those need to be filled out/confirmed by the human co-author before merge.

aimane-chnaif · 2026-08-13T11:42:57Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
I verified tests pass on all platforms & I tested again on:
- Android: HybridApp
- Android: mWeb Chrome
- iOS: HybridApp
- iOS: mWeb Safari
- MacOS: Chrome / Safari
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp

android.mov

Android: mWeb Chrome

iOS: HybridApp

ios.mov

iOS: mWeb Safari

MacOS: Chrome / Safari

web.mov

melvin-bot · 2026-08-13T11:43:54Z

We did not find an internal engineer to review this PR, trying to assign a random engineer to #96427 as well as to this PR... Please reach out for help on Slack if no one gets assigned!

MelvinBot · 2026-08-13T12:30:40Z

The failing check is unrelated to this PR — it's a pre-existing breakage on main, so I have not pushed any change.

Failing check: test (job 1) → tests/unit/SignInBackButtonTest.tsx — TypeError: _Navigation.default.dismissModal is not a function

Root cause: A merge collision between two PRs that each passed independently on main:

SignInModal.tsx:60 calls Navigation.dismissModal().
SignInBackButtonTest.tsx:12-25 mocks @libs/Navigation/Navigation with only goBack and navigationRef — no dismissModal — so the call throws.

Both the dismissModal() call and the incomplete mock are present on origin/main (SHA 4adc694), so this test fails on main on its own — it is not introduced by this PR. This PR only touches useFollowActionBadgeTarget.ts, getActionBadgeScrollDelay.ts, ReportActionsList.tsx, and getActionBadgeScrollDelayTest.ts, none of which are related to sign-in or navigation.

The fix belongs on main (add dismissModal: jest.fn() to the Navigation mock in SignInBackButtonTest.tsx), not in this PR. Merging latest main here won't help until that fix lands.

The other failing check, Check independent approval, is a checklist/approval gate and not a code issue.

MelvinBot · 2026-08-13T12:46:40Z

The failing checks are not related to this PR — there's nothing to fix on this branch.

test (job 1) — tests/unit/SignInBackButtonTest.tsx fails with TypeError: _Navigation.default.dismissModal is not a function. Its @libs/Navigation/Navigation mock only stubs goBack/navigationRef and omits dismissModal, which SignInModal calls on mount. That test and both files live entirely on main — this PR only touches report-preview scroll code (useFollowActionBadgeTarget.ts, getActionBadgeScrollDelay.ts, ReportActionsList.tsx), so it can't affect it. It's a pre-existing issue in the test's own mock and should be fixed separately.

validate — infrastructure flake, not a code failure: "The self-hosted runner lost communication with the server." The contributor-authorization step passed before the runner died; re-running the job should clear it.

Check independent approval — expected process check, not a code failure.

Since none of these stem from this PR's changes, I haven't pushed anything.

MelvinBot · 2026-08-13T12:54:01Z

I re-checked all four failing checks — none are caused by this PR, so there's no fix to push here. (The branch head is unchanged since my last comment, so these are the same runs.)

test (job 1) — tests/unit/SignInBackButtonTest.tsx fails with TypeError: _Navigation.default.dismissModal is not a function. I verified against main directly: the test's @libs/Navigation/Navigation mock stubs only goBack and omits dismissModal, which SignInModal calls on mount. Both files, and the incomplete mock, exist unchanged on main — this is a pre-existing broken test on main, not something this PR touches (this PR only changes report-preview scroll code). It needs a separate fix on main (add dismissModal to that mock); fixing it here would be out of scope for this PR.

validate and review — both infrastructure flakes, not code failures: "The self-hosted runner lost communication with the server." Every real step in each job passed (authorization, and for review the code-review steps too) before the runner died. Re-running the jobs should clear them.

Check independent approval — expected process check, not a code failure.

Since none of these stem from this PR's changes, I haven't pushed anything.

MelvinBot · 2026-08-13T14:20:39Z

I re-analyzed every failing check (five now, including the new review/checklist runs). None are caused by this PR, so there is no fix to push to this branch. The branch head is unchanged (9c70f80), so the code-test run is the same one as before.

Three are infrastructure flakes — validate, review, and checklist all failed with the same annotation: "The self-hosted runner lost communication with the server." In each, every functional step passed (authorization, reviewerChecklist.js, Claude review + "Post code review results") and the job only died at "Complete runner." These need a re-run, not a code change.

Check independent approval — expected process gate, not a code failure.

test (job 1) — the only real test failure, and it's a pre-existing breakage on main, unrelated to this PR:

It fails in tests/unit/SignInBackButtonTest.tsx with TypeError: _Navigation.default.dismissModal is not a function.
That test's @libs/Navigation/Navigation mock stubs only goBack/navigationRef and omits dismissModal, which SignInModal calls on mount once the app-loading state settles.
The test and both files live entirely on main (the incomplete mock is still present on the latest main, 6410a7a, so merging main won't clear it). This PR only touches report-preview scroll code (useFollowActionBadgeTarget.ts, getActionBadgeScrollDelay.ts, ReportActionsList.tsx). Jest runs each test file in an isolated module registry, so those changes cannot affect this signin test's mock.

Per policy I don't push unrelated/pre-existing test fixes into an unrelated PR — doing so would mix a signin-navigation change into a scroll PR. The actual fix is a one-liner on main (add dismissModal: () => {} to that test's Navigation mock) and should go in a separate PR targeting main. If you'd like, reply and I can open that separate PR against main.

garrettmknight

Looks good from a product perspective.

aimane-chnaif · 2026-08-13T15:38:44Z

@MelvinBot pull main

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

MelvinBot · 2026-08-13T15:50:49Z

🤖 Pulled main into this branch — merged cleanly with no conflicts and pushed.

Merge commit: dfdc14f

melvin-bot · 2026-08-13T15:50:50Z

@aimane-chnaif Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

chuckdries

Big fan of this change 🤩

Just have some nit-picks about the comments

chuckdries · 2026-08-13T16:24:27Z

- * advances to the next report preview that requires action. This hook follows it by scrolling down to the new target, waiting for
- * the resolve animation to finish first when the resolved badge was an animated (submit/approve/pay) button.
+ * When the action-badge target is resolved (e.g. the user approves/pays/submits an older report preview), it advances to the next
+ * preview requiring action. This hook scrolls down to follow it immediately on action, rather than waiting for the resolve animation.


NIT: generally we don't want comments to describe historical behaviors that aren't relevant anymore

Done in 7728e10 — dropped the "rather than waiting for the resolve animation" historical framing so the doc comment just describes current behavior.

chuckdries · 2026-08-13T16:24:38Z

-        const scrollTimeoutID = setTimeout(() => scrollToActionBadgeTargetRef.current(), scrollDelay);
-        return () => clearTimeout(scrollTimeoutID);
+        // Scroll to the next target on the next frame so the forward-scroll starts as soon as the user acts, rather than waiting for
+        // the resolve animation. The resolved preview keeps animating in place while the list scrolls.


Done in 7728e10 — same fix here: removed the reference to the old wait-for-animation behavior, keeping only the description of what the code does now.

aimane-chnaif · 2026-08-13T16:26:57Z

@MelvinBot address #98515 (comment) and #98515 (comment)

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

MelvinBot · 2026-08-13T16:35:59Z

⚠️ MelvinBot CI Fix Loop Detected

MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR.

Next steps: A human should review the recent commits and resolve the conflicting CI errors manually.

chuckdries · 2026-08-13T17:00:15Z

Hmm, for some reason in my testing it scrolls, then jumps to the wrong location

scroll.then.jump.mp4

aimane-chnaif · 2026-08-13T17:02:58Z

Hmm, for some reason in my testing it scrolls, then jumps to the wrong location

scroll.then.jump.mp4

Can you please check if it happening on staging as well?

aimane-chnaif · 2026-08-13T17:03:18Z

@MelvinBot investigate #98515 (comment)

chuckdries · 2026-08-13T17:08:14Z

It does not happen when I check out main

scroll.works.fine.mp4

MelvinBot · 2026-08-13T17:15:44Z

🤖 Investigated the "scrolls, then jumps to the wrong location" report.

Root cause (medium confidence): This PR's core change — scrolling on the very next frame instead of waiting out the resolve animation — is what causes the misland. The old getActionBadgeScrollDelay wait wasn't purely cosmetic: it also guaranteed the layout was static before scrolling. The resolved preview's Submit/Pay/Approve button animates its own height → 0 over ~200–300 ms (AnimatedSubmitButton/index.tsx:93, AnimatedSettlementButton.tsx:120), so that preview item collapses while the new scroll is running.

The scroll is index-based — scrollToIndex(actionBadgeTargetIndex, {viewPosition: 1, viewOffset: 40}) — and the list has maintainVisibleContentPosition active (you've scrolled up, so hasScrolledOverThreshold is true) plus an onContentSizeChange handler that re-runs scroll tracking (ReportActionsList.tsx:494-498). So the sequence is: scroll lands → the in-flight collapse changes content size → MVCP / onContentSizeChange re-adjust the offset → the list visibly re-settles to a different spot. That's the "scroll, then jump."

Why it lands wrong and fix options

The just-resolved preview sits above the new (newer, lower-index) target in the inverted list, so it doesn't corrupt the target's own scrollToIndex offset directly — the misland is a settling/compensation effect from the height collapse happening concurrently with the scroll and MVCP. This is why a bare index recompute (which the ref in useFollowActionBadgeTarget.ts already does) is not enough to fix it.

Fix directions, roughly in order of least-invasive:

Scroll immediately for responsiveness, then re-issue a corrective scrollToActionBadgeTarget() once layout settles — e.g. after the resolve-animation duration, or debounced off onContentSizeChange. Keeps the snappy feel the PR is after while guaranteeing a correct final resting position.
Don't collapse the resolved preview's height until the scroll has settled (reserve/hold its height during the scroll), so the layout the scroll targets stays valid.
Fall back to the old animation-timeline wait only for the animated (submit/pay/approve) badges, keeping the immediate-scroll path for non-animating badges — i.e. the misland only happens when there's a height collapse to fight.

Option 1 is likely the smallest change that preserves this PR's goal.

I didn't run a live repro (the flow needs a workspace chat with several stacked actionable previews), but your video already demonstrates it. If you want, I can try to reproduce on web to confirm which compensator (MVCP vs FlashList off-screen self-correction) dominates before committing to a fix.

Scroll forward immediately on approve/pay/submit instead of waiting f…

a36aefd

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

MelvinBot assigned aimane-chnaif Aug 13, 2026

MelvinBot requested a review from a team August 13, 2026 00:43

MelvinBot mentioned this pull request Aug 13, 2026

[$250] Scroll forward immediately upon approving a report in the workspace chat #96427

Open

aimane-chnaif suggested changes Aug 13, 2026

View reviewed changes

Simplify comments in useFollowActionBadgeTarget

6e5b187

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

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

9c70f80

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

MelvinBot marked this pull request as ready for review August 13, 2026 11:41

MelvinBot requested a review from a team as a code owner August 13, 2026 11:41

melvin-bot Bot requested review from aimane-chnaif and garrettmknight and removed request for a team August 13, 2026 11:41

melvin-bot Bot removed the request for review from a team August 13, 2026 11:41

aimane-chnaif approved these changes Aug 13, 2026

View reviewed changes

melvin-bot Bot requested a review from chuckdries August 13, 2026 11:44

garrettmknight previously approved these changes Aug 13, 2026

View reviewed changes

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

dfdc14f

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

MelvinBot requested a review from a team as a code owner August 13, 2026 15:50

melvin-bot Bot requested a review from aimane-chnaif August 13, 2026 15:50

melvin-bot Bot removed the request for review from a team August 13, 2026 15:50

aimane-chnaif approved these changes Aug 13, 2026

View reviewed changes

chuckdries reviewed Aug 13, 2026

View reviewed changes

Remove historical-behavior references from useFollowActionBadgeTarget…

7728e10

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

MelvinBot dismissed garrettmknight’s stale review via 7728e10 August 13, 2026 16:34

Uh oh!

Conversation

MelvinBot commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MelvinBot commented Aug 13, 2026

Uh oh!

MelvinBot commented Aug 13, 2026

Uh oh!

codecov Bot commented Aug 13, 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!

MelvinBot commented Aug 13, 2026

Uh oh!

MelvinBot commented Aug 13, 2026

Uh oh!

MelvinBot commented Aug 13, 2026

Uh oh!

aimane-chnaif commented Aug 13, 2026

Uh oh!

MelvinBot commented Aug 13, 2026

Uh oh!

MelvinBot commented Aug 13, 2026

Uh oh!

MelvinBot commented Aug 13, 2026

Uh oh!

aimane-chnaif commented Aug 13, 2026

Uh oh!

MelvinBot commented Aug 13, 2026

Uh oh!

aimane-chnaif commented Aug 13, 2026

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

MelvinBot commented Aug 13, 2026

Uh oh!

aimane-chnaif commented Aug 13, 2026

Uh oh!

melvin-bot Bot commented Aug 13, 2026

Uh oh!

MelvinBot commented Aug 13, 2026

Uh oh!

aimane-chnaif commented Aug 13, 2026

Reviewer Checklist

Screenshots/Videos

Uh oh!

melvin-bot Bot commented Aug 13, 2026

Uh oh!

MelvinBot commented Aug 13, 2026

Uh oh!

MelvinBot commented Aug 13, 2026

Uh oh!

MelvinBot commented Aug 13, 2026

Uh oh!

MelvinBot commented Aug 13, 2026

MelvinBot commented Aug 13, 2026 •

edited

Loading

codecov Bot commented Aug 13, 2026 •

edited by github-actions Bot

Loading