Integrate infinite scroll and refresh to activity screen

[dewabisma]

Summary

• Support infinite scroll in activity screen for all filters
• Add pull to push in activity screen for all filters

---

Note

High Risk
Activity pagination UX is moderate risk; the PR also contains extensive duplicate/broken code in pos_qr_screen.dart that will fail analysis/compile unless reverted.

Overview
Adds pull-to-refresh and infinite scroll on the activity screen for each send/receive/all filter, wired through new activeAccountPaginationProvider helpers that expose the filtered UnifiedPaginationController for fetchMore and loadingRefresh.

Transaction list providers now keep showing loaded chain data when pagination fails after the first page (initial load still errors if nothing was fetched); filtered loads log load-more errors separately.

analysis_options.yaml excludes build/** from analysis.

^{Reviewed by Cursor Bugbot for commit 0b4c4d8. Bugbot is set up for automated code reviews on this repo. Configure here.}

[v12-auditor]

Warning

No auditable source files found in this PR's diff.

[n13]

I am actually talking to Sejal about removing the filters for now
It's a bit much and not much benefit for a pretty intrusive UX that makes this whole screen much more complicated (for the user I mean)

[n13]

I've read the diff plus the surrounding code (UnifiedPaginationController, PaginationState, the two transactions providers, home_screen.dart, and existing bugbot/author threads). Here's my review of PR #493 — strictly from the diff, no checkouts.

Overall

Solid implementation of pagination + pull-to-refresh wired to a new family-scoped activeAccountPaginationProvider. The DRY helpers in active_account_transactions_provider.dart are nice. The key: ValueKey(_filterOption) fix for filter switching is the right call. Most of the bugbot threads have reasonable resolutions from the author.

A few real issues remain — one of which I think actually warrants a fix.

Findings

1. Silent failure in all_transactions_provider.dart (should fix)

The filtered provider was updated to log load-more errors, but the global one wasn't — the two branches diverged.

  final pagination = ref.watch(paginationControllerProvider);

  final hasLoadedChainData = pagination.otherTransfers.isNotEmpty || pagination.scheduledReversibleTransfers.isNotEmpty;
  if (pagination.error != null && !hasLoadedChainData) {
    return AsyncValue.error(pagination.error!, pagination.stackTrace!);
  }
  if (pagination.isFetching && pagination.otherTransfers.isEmpty) {

Compare with the filtered version which does add a log:

      if (pagination.error != null && !hasLoadedChainData) {
        print('FilteredTransactionsProvider: Error: ${pagination.error}');
        return AsyncValue.error(pagination.error!, pagination.stackTrace!);
      }
      if (pagination.error != null) {
        print('FilteredTransactionsProvider: Load-more error: ${pagination.error}');
      }

Recommend adding the matching print('AllTransactionsProvider: Load-more error: ...') to keep the two paths symmetric and avoid swallowing errors silently.

2. DRY opportunity for the masking logic

The exact same hasLoadedChainData computation + suppression pattern is now duplicated across both providers. Consider a tiny extension on PaginationState:

extension on PaginationState {
  bool get hasLoadedChainData => otherTransfers.isNotEmpty || scheduledReversibleTransfers.isNotEmpty;
}

Small thing, but the rule about no duplicate code applies and this is going to drift the next time someone touches it.

3. Pre-existing bug exposed by the new behavior — PaginationState.copyWith can't clear error

  PaginationState copyWith({
    ...
    Object? error,
    StackTrace? stackTrace,
  }) {
    return PaginationState(
      ...
      error: error ?? this.error,
      stackTrace: stackTrace ?? this.stackTrace,
    );
  }

error: error ?? this.error means passing null is a no-op. So in _fetchPage (line 94–103 of unified_pagination_controller.dart), the success path's error: null, stackTrace: null is silently ignored. Combined with this PR's new "log load-more errors" logic, a single transient error followed by many successful fetchMores will keep logging Load-more error: <stale error> on every rebuild forever.

Not introduced by this PR, but this PR is the first place where the stale value becomes user-visible (via logs). Worth a follow-up — either error: error, or use a sentinel.

4. Pull-to-refresh drops the RefreshIndicator (bugbot raised, author closed)

This was the bugbot "Pull refresh drops RefreshIndicator" thread. Author response was "we care more about new data than the scrollable child", which I get pragmatically — but the actual UX consequence is: the spinner disappears mid-gesture and the list collapses into skeletons. That's a noticeable visual hiccup for pull-to-refresh.

Two cleaner alternatives that wouldn't change the design intent:

• Use silentRefresh() from inside _refresh instead of loadingRefresh(). The RefreshIndicator already owns its own spinner; the existing data stays visible, then swaps in-place when the new page arrives.
• Or, in the providers, keep returning AsyncValue.data(previous) while isFetching && hasLoadedChainData is true, so the data branch is preserved during refresh.

Up to the team whether to push back — flagging because it matches what bugbot caught and is an easy win.

5. _onScroll short-list edge case (bugbot raised, author closed)

  void _onScroll() {
    if (!_scrollController.hasClients) return;
    final pos = _scrollController.position;
    if (pos.pixels < pos.maxScrollExtent - _loadMoreThreshold) return;

    final pagination = ref.read(activeAccountPaginationProvider(_filterOption));
    if (pagination == null || pagination.isFetching || !pagination.hasMore) return;

The author tested it and it works because:

• A short list still doesn't fire _onScroll without user input;
• The isFetching guard catches repeated calls during fetch.

But one residual concern: after the first fetchMore returns, if there are still hasMore: true items but the new page didn't grow the list past the viewport (server returned a small page), the next scroll event will immediately trigger another fetchMore, and so on, with no user intent. The guard correctly serializes them — it's not a correctness bug — but it can lead to "chained" auto-loading of every remaining page on a short list. May be desired behavior; just calling it out.

6. activeAccountPaginationNotifier API shape

UnifiedPaginationController? activeAccountPaginationNotifier(WidgetRef ref, TransactionFilter filter) {
  final params = activeAccountFilteredParams(ref.read(activeAccountProvider).value, filter);
  if (params == null) return null;
  return ref.read(filteredPaginationControllerProviderFamily(params).notifier);
}

Free function taking WidgetRef while activeAccountPaginationProvider (just below) uses regular Ref — mixing scopes feels a bit off. Functionally fine. Minor style: I'd take a Ref and have the screen do activeAccountPaginationNotifier(ref, filter) via WidgetRef-as-Ref if Riverpod allows (it does for ref.read). Not a blocker.

7. Empty-state height heuristic

                      child: ListView(
                        controller: _scrollController,
                        physics: const AlwaysScrollableScrollPhysics(),
                        children: [
                          SizedBox(
                            height: MediaQuery.sizeOf(context).height * 0.3,
                            child: Center(
                              child: Text(

* 0.3 is arbitrary and won't center reliably on different device sizes. Consider wrapping in LayoutBuilder + ConstrainedBox(minHeight: constraints.maxHeight) so the empty text actually centers in the available viewport. Same pattern is used by RefreshIndicator docs for empty pull-to-refresh lists.

8. Bugbot "Filter switch triggers spurious load-more" — verified fixed

Adding key: ValueKey(_filterOption) to the ListView.builder causes the controller to re-attach with the new ListView at offset 0, so _onScroll won't immediately satisfy the threshold after filter change. Good fix.

9. Bugbot "Refresh races pagination fetches" — looks fixed

_refresh now early-returns when pagination.isFetching, and _onScroll already does the same. Since Dart is single-threaded the two callbacks can't truly overlap, and the guard catches the "scroll triggered → fetch in flight → user pulls down" sequence. Good.

10. analysis_options.yaml — excluding build/**

    - "**/*.g.dart"
    - "**/*.freezed.dart"
    - "lib/generated/**"
    - "build/**"

Sensible. The build/ folder contains transitively-fetched packages (the firebase_messaging-16.2.2 content you've been seeing in recently viewed files is from there) and shouldn't be analyzed.

Summary

Must-fix (per your rules):

• Add the matching load-more error log to all_transactions_provider.dart (silent failure violates the fail-early rule).

Nice-to-fix:

• Extract the hasLoadedChainData masking into a shared helper (DRY).
• Either switch _refresh to silentRefresh() or keep the data branch alive during refresh, so the RefreshIndicator doesn't get unmounted mid-gesture.

Worth a follow-up issue, not this PR:

• PaginationState.copyWith can't clear error/stackTrace to null. This PR exposes the consequence (potentially spammy "Load-more error" logs after a single transient failure).

Approved-with-comments otherwise. Nice work; the family-scoped pagination factoring is a clean improvement.

[n13]

3. is interesting - we should be able to clear error... there must be a normal way to do this kind of thing.
   I think this should be fixed

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 0b4c4d8. Configure here.}

[cursor]

Paid POS shows QR content

High Severity

After a payment is detected, mainContent invokes _buildPaidContent as a discarded expression but always returns _buildQrContent. Users still see the QR flow in the body while the app bar and bottom actions reflect a completed payment.

 

^{Reviewed by Cursor Bugbot for commit 0b4c4d8. Configure here.}

[dewabisma]

Closing this PR because of broken git history, continuation with proper history is in this one #499