Add infinite scroll and pull to refresh

[dewabisma]

Summary

Continuation of this PR #493 , closing that one because of broken git history.

---

Note

Low Risk
UI and pagination UX changes with no auth or payment logic; load-more errors are logged instead of replacing the list.

Overview
The Activity screen now supports pull-to-refresh and infinite scroll for the active account’s transaction list (per send/receive/all filter), wired to the existing filtered pagination controller.

New Riverpod helpers expose pagination state and the notifier for the active account so the UI can call fetchMore near the list bottom and loadingRefresh on refresh. The list uses a shared ScrollController, always-scrollable physics (including empty state), a load-more footer loader, and a filter-keyed list so scroll behavior resets when the filter changes.

Pagination behavior is tightened: hasLoadedChainData distinguishes initial load failures from load-more errors—providers only surface AsyncValue.error when there is no cached chain data; otherwise they log and keep showing existing rows. PaginationState.copyWith now replaces error/stackTrace when passed (including clearing with null) instead of keeping stale errors. Analyzer excludes build/**.

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

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

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

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

[cursor]

Filter change keeps scroll offset

Medium Severity

Changing the activity filter causes the _scrollController to retain its previous scroll position. This means the new list might not start at the top, hiding recent items, and can immediately trigger fetchMore() without user interaction.

 

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

[cursor]

Premature infinite scroll fetch

Medium Severity

_onScroll treats pixels >= maxScrollExtent - 200 as “near bottom,” but when the list is shorter than the viewport maxScrollExtent is often zero or small, so that condition is true at scroll offset zero and fetchMore() runs without the user reaching the end of the list.

 

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

[n13]

I've done a thorough review of PR #499. Here's my analysis.

PR #499 Review — Add infinite scroll and pull to refresh

Overall this is a clean, well-scoped UX improvement. The core mechanics (shared ScrollController, threshold-based load-more, AlwaysScrollableScrollPhysics so the empty state still pulls, filter-keyed list, and load-more footer) are all idiomatic Flutter. The new activeAccountPaginationProvider / activeAccountPaginationNotifier helpers are a nice DRY-ing of what previously had to be re-derived ad-hoc. A few things are worth tightening before merge.

Behavior issues

1. Pull-to-refresh wipes the list to skeletons (UX regression). loadingRefresh() does state = PaginationState.initial(); before re-fetching:

    final targetAccountIds = _getAccountIds();
    if (targetAccountIds.isEmpty) {
      state = PaginationState.initial().copyWith(hasMore: false);
      return;
    }

    // Reset to initial state to show loading
    state = PaginationState.initial();
    await _fetchPage(targetAccountIds);

After that, the combined provider returns AsyncValue.loading() (isFetching && otherTransfers.isEmpty), so txAsync.when(loading: …) swaps the whole list for skeletons — and tears down the RefreshIndicator subtree mid-pull. Users will see their list blink to skeletons and the pull spinner vanish.

Pick one of:

• Use silentRefresh() from _refresh (it preserves the existing list and just replaces it on success).
• Or change loadingRefresh to not zero out otherTransfers/scheduledReversibleTransfers — only reset offsets + isFetching and let _fetchPage overwrite on success. RefreshIndicator already shows the spinner; we don't need the skeletons in addition.

2. _onScroll can fire fetchMore when content fits the viewport (Bugbot's "premature fetch"). This is a real-but-mild issue:

  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;

    activeAccountPaginationNotifier(ref, _filterOption)?.fetchMore();
  }

When the list is shorter than the viewport, maxScrollExtent == 0, so pixels (0) >= -200 is always true. The listener doesn't fire on attach, but with AlwaysScrollableScrollPhysics any drag/overscroll triggers it. Add a pos.maxScrollExtent > 0 guard:

if (pos.maxScrollExtent <= 0) return;
if (pos.pixels < pos.maxScrollExtent - _loadMoreThreshold) return;

If you actually want auto-fill-to-viewport behavior (fetch more until the list fills the screen), do it explicitly in the controller after a fetch completes, not as a side-effect of touching the screen.

3. Bugbot's "filter change keeps scroll offset" — not actually a bug, but worth confirming. You added key: ValueKey(_filterOption) on the non-empty ListView.builder, which causes the previous ScrollPosition to detach and a new one to attach. Combined with PageStorage's default keepScrollOffset: true, switching All → Send → All can actually restore the previous All offset (per-filter scroll memory). If that's the intent — great. If you instead want every filter switch to start at the top, either set keepScrollOffset: false on the controller or jumpTo(0) in _onFilterOptionChanged after setState. The empty-state ListView doesn't have the key, but since it's a different widget instance that's fine.

API / semantics

4. copyWith now replaces error/stackTrace unconditionally.

      hasMore: hasMore ?? this.hasMore,
      isFetching: isFetching ?? this.isFetching,
      error: error,
      stackTrace: stackTrace,

This is the right fix for "can't clear the error", but it changes semantics for every call site: any copyWith that doesn't mention error now wipes the existing error. In this codebase that's mostly fine (and arguably good, since state.copyWith(isFetching: true) in _fetchPage now resets stale errors at fetch start). But it's a foot-gun for future callers, including updateReversibleTransferToExecuted and addTransactionToHistory, which silently drop errors. Consider the Optional<T>/sentinel pattern (a _kUnset const) so you can distinguish "not passed" from "explicitly null", or add a focused clearError() helper and keep ?? semantics. At minimum, add a doc comment on copyWith calling out the new behavior.

5. hasLoadedChainData should probably also gate the loading branch. In both providers:

      if (pagination.isFetching && pagination.otherTransfers.isEmpty) {
        return const AsyncValue.loading();
      }

You added hasLoadedChainData (which checks both otherTransfers and scheduledReversibleTransfers) for the error gate, but the loading gate still only checks otherTransfers.isEmpty. For an account that has only scheduled reversible transfers, the screen will flash skeletons on every refetch. Use !hasLoadedChainData here too for consistency.

Minor / style

6. Logging inconsistency. Providers now use quantusDebugPrint, but unified_pagination_controller.dart still has 13 raw print(...) calls (lines 45, 105, 111, 124, 136, 141, 175, 183, 191, 224, 230, 239, 253). The "properly debug sensitive information" intent of the PR isn't really achieved until the controller (which logs txIds, fetch failures, etc.) is also gated by kDebugMode. Recommend converting them all in this PR.

7. Naming: activeAccountPaginationNotifier looks like a provider but is a top-level function taking WidgetRef. Consider renaming to make it clearly a helper, e.g. readActiveAccountPaginationNotifier(ref, filter), or define it as a Provider.family like activeAccountPaginationProvider. The current name is easy to mistake for a Riverpod provider on a quick scan.

8. Duplicated RefreshIndicator shell. The empty-state branch and the data branch both wrap in RefreshIndicator(onRefresh: _refresh, color: …, backgroundColor: …, child: …). A tiny _buildRefreshable({required Widget child}) wrapper keeps it DRY (and matches the repo's DRY rule).

9. _filterLabel lives on the State. It's pure; can be a top-level function or moved into TransactionFilter (an extension on the enum) so other screens can reuse the localized labels.

10. analysis_options.yaml excluding build/** — fine and standard. No concerns.

Things I think are good

• Splitting activeAccountFilteredParams out as the single source of params construction — clean DRY win.
• LayoutBuilder + ConstrainedBox(minHeight: constraints.maxHeight) to make the empty state pull-to-refreshable without center-collapsing — nice touch.
• key: ValueKey(_filterOption) to give each filter its own scroll state.
• Distinguishing initial-load errors from load-more errors via hasLoadedChainData is the right model; the existing list shouldn't disappear because page 5 failed.
• ScrollController is properly disposed and the listener is removed first — no leaks.

Recommendation

Approve with the following changes requested:

• Fix pull-to-refresh skeleton flash (Rust Bindings #1) — high priority, user-visible.
• Add maxScrollExtent > 0 guard in _onScroll (Feature/dilithium signatures #2) — low effort, addresses Bugbot.
• Use !hasLoadedChainData in the loading gate for parity with the error gate (Refactor screens #5).
• Convert the remaining print calls in unified_pagination_controller.dart to quantusDebugPrint (New design #6).
• Either document or harden the new copyWith semantics (Fix warnings1 #4).

The rest (naming, DRY of RefreshIndicator, _filterLabel location) are nits and can land separately.

Want me to apply any of these as edits, or open follow-up comments on the PR? Per your rules, I won't post to GitHub or push without your go-ahead.