Redesign activity screen and tx detail sheet

[dewabisma]

Summary

• Updated activity screen with filtering buttons (all, sent, received)
• Add transaction filter capability to chain history service
• Update tx details sheet styling and layout
• Add skeleton loading to activity screen

Screenshots

• Activity screen loading

• Activity screen filter all

• Activity screen filter sent

• Activity screen filter received

• Tx details

• Tx details flipped currency

• Tx details hidden amount

[n13]

All cleaned up. Here's my review of PR #453:

PR Review — Redesign activity screen and tx detail sheet

Nice work overall — the per-filter GraphQL queries, skeleton loading, themed text styles, and FilteredTransactionsParams as a typed Riverpod family key are all solid moves. A few things I'd want to land before merging though.

---

Critical / correctness

1. Send/Receive filters never auto-refresh and there's no pull-to-refresh

Every callsite that updates pagination controllers hardcodes TransactionFilter.All:

• global_history_polling_service.dart lines 96–110, 140–149 — 1-min polling only refreshes the All controller.
• reversible_transfer_monitoring_service.dart lines 159–169, 175–184, 216–225 — execution monitor only updates the All controller.
• transaction_submission_service.dart lines 345–367 — newly broadcast txs only land in the All controller.
• home_screen.dart _refresh() and transaction_submission_service.dart use TransactionFilter.All.

Combined with the activity screen using ScaffoldBase (no RefreshIndicator), if a user is viewing the Send or Receive filter:

• Polling never refreshes their view
• Newly submitted transactions never appear
• Reversible transfers never update inline
• They have no way to manually refresh — switching filters back/forth reuses the cached controller (per family key), so the data stays stale until the screen is destroyed.

Two options I'd consider:

• Iterate over TransactionFilter.values in those service updates so all live controllers get refreshed.
• Add ScaffoldBase.refreshable to the activity screen with an onRefresh that invalidates/loadingRefresh()s the controller for the currently selected filter.

2. Formatting check fails on currency_display_provider.dart

Despite the chore: formatting commit, this file isn't formatted. Running dart format produces a diff:

-    Provider<CurrencyDisplayState Function(BigInt, {required bool isSend,  bool withQuanSymbol, String? customHiddenText})>((ref) {
+    Provider<
+      CurrencyDisplayState Function(BigInt, {required bool isSend, bool withQuanSymbol, String? customHiddenText})
+    >((ref) {

Also note the double space isSend, bool. CI will reject this.

---

Significant

3. DRY violation — _listEquals reimplements Flutter's built-in

  static bool _listEquals(List<String> a, List<String> b) {
    if (a.length != b.length) return false;
    for (var i = 0; i < a.length; i++) {
      if (a[i] != b[i]) return false;
    }
    return true;
  }

package:flutter/foundation.dart already exports listEquals<T>(List<T>? a, List<T>? b). Use it.

Also: since AccountIdListCache.get already returns identity-stable lists and every caller passes through the cache (and filteredTransactionsProviderFamily re-normalizes), you could probably just use identity-based equality (identical(other.accountIds, accountIds)) and avoid the deep compare entirely.

4. Tx detail sheet UX regressions

The redesign removes:

• Address copy button (_copyButton(... value: _counterparty))
• Human-readable checkphrase display (HumanReadableChecksumService.getHumanReadableName)
• Checkphrase copy button

The checkphrase is a security/verification feature elsewhere in the codebase (used in recent_address_list_item.dart, shared_address_action_sheet.dart, etc.). Was its removal here intentional? At minimum the user can no longer copy the address from the sheet — a clear regression. The screenshots match, so I assume it's deliberate, but worth confirming.

5. txAmountFormatterProvider API shape and lost DI

final txAmountFormatterProvider =
    Provider<CurrencyDisplayState Function(BigInt, {required bool isSend,  bool withQuanSymbol, String? customHiddenText})>((ref) {
      ...
      if (!isHidden) {
        data = data.copyWith(primaryAmount: '$prefix${data.primaryAmount}', secondaryAmount: data.secondaryAmount);
      }

      if (withQuanSymbol && !isFlipped) {
        data = data.copyWith(primaryAmount: '${data.primaryAmount} ${AppConstants.tokenSymbol}', secondaryAmount: data.secondaryAmount);
      }

      return data;
    });

Three smells:

• The provider is named txAmount**Formatter** but now returns the full CurrencyDisplayState. Most callers immediately .primaryAmount. Either rename or split into two providers — one returning String, one returning CurrencyDisplayState.

• secondaryAmount: data.secondaryAmount in copyWith is redundant — that's the default when omitted.

• _toFiatDisplayState instantiates NumberFormattingService() directly:

    final quanFormatted = NumberFormattingService().formatBalance(amount, addSymbol: isFlipped);
    final fiatFormatted = selectedFiat.format(_toFiatNumeric(amount, selectedFiat, xRate));

  The original code went through numberFormattingServiceProvider. The DI hook is gone — passes the service via the helper signature instead.

---

Design / code quality

6. GraphQL dynamic-string queries are fragile

    switch (filter) {
      case TransactionFilter.Send:
        whereClause =
            '{AND: [{account: {id_in: \$accounts}, $baseCondition}, $transferGuard, {OR: [{transfer: {from: {id_in: \$accounts}}}, {executedReversibleTransfer: {scheduledTransfer: {from: {id_in: \$accounts}}}}, {cancelledReversibleTransfer: {scheduledTransfer: {from: {id_in: \$accounts}}}}]}]}';
        connectionWhereClause =
            '{AND: [{account: {id_in: \$accounts}, $baseCondition}, $transferGuard, {OR: [{transfer: {from: {id_in: \$accounts}}}, {executedReversibleTransfer: {scheduledTransfer: {from: {id_in: \$accounts}}}}, {cancelledReversibleTransfer: {scheduledTransfer: {from: {id_in: \$accounts}}}}]}]}';

whereClause and connectionWhereClause are identical strings duplicated for every case — extract to a single variable. The whole approach is brittle: if the schema renames a field, six string literals need to change in lockstep.

Worth investigating whether the GraphQL backend supports passing the OR clause as a variable so you can keep one static query. If not, at least factor the per-direction OR clause into a helper (_directionOr(filter)) so the AND wrapper stays in one place.

Also doc-comment drift — refers to lowercase variants:

When [filter] is [TransactionFilter.send] or [TransactionFilter.receive]

But the enum is Send/Receive.

7. TransactionFilter enum naming

// ignore: constant_identifier_names
enum TransactionFilter { All, Send, Receive }

Standard Dart style is camelCase, which is why the lint had to be suppressed. The activity screen also uses e.name for button labels:

    final filterButtons = TransactionFilter.values
        .map(
          (e) => _buildFilterButton(
            e.name,
            ...

That couples enum values to UI strings — bad for i18n later. Suggest enum TransactionFilter { all, send, receive } with an extension adding displayName (or a localized lookup).

8. Hardcoded button widths per label

    final filterButtonWidthMap = {
      TransactionFilter.All: 80.0,
      TransactionFilter.Receive: 130.0,
      TransactionFilter.Send: 90.0,
    };

Brittle to translations or label changes. Use intrinsic sizing or symmetric horizontal padding.

9. _DetailRow falls back to a hardcoded color

          Text(
            value,
            style: text.transactionDetailRowValue?.copyWith(color: valueColor ?? Colors.white.withValues(alpha: 0.8)),
          ),

That Colors.white.withValues(alpha: 0.8) should be a theme color (e.g. colors.textPrimary.useOpacity(0.8)) for consistency.

10. 'View in Explorer ↗' literal arrow

Use an Icon(Icons.arrow_outward, size: 14) (already used in activity_section.dart) so the symbol respects theme/font scaling.

---

Minor

• No tests. The new FilteredTransactionsParams equality and the dynamic query builder are exactly the kind of self-contained logic that benefits from unit tests. There's already mobile-app/test/unit/ set up.
• pendingTransactions filter logic in filtered_all_transactions_provider.dart — relying on normalizedParams.filter != TransactionFilter.Receive works but reads as "negative filter". Consider being explicit: if (filter == TransactionFilter.Receive) [] else pending.where(...).
• Skeleton ListView count — loading: () => ListView.builder(itemCount: 3, ...) renders 3 groups of 3 = 9 skeleton items. Confirm with design that's the intent.
• ref.invalidate(activeAccountTransactionsProvider) in home_screen.dart and activity_section.dart is mostly a no-op — the wrapper Provider re-runs but the underlying controller state is preserved. The loadingRefresh() call right after is what actually fetches. Worth a comment, or just remove the invalidate.

---

What I liked

• Per-direction GraphQL queries — real performance win for active accounts
• Skeleton loading and the Skeleton.txItem named constructor (clean DRY)
• Extracting tx-detail text styles into AppTextTheme
• ButtonVariant.outline cleanly extends the existing button system
• FilteredTransactionsParams as a typed family key is the right Riverpod pattern
• New formatTxDateTime helper in DatetimeFormattingService
• Status row + dotted separator structure is much cleaner than the previous bespoke _amountCard/_addressSection/_feeRow

Happy to dive deeper into any of these — issue #1 (stale data on filtered views) is the one I'd consider a blocker.

[n13]

Re-review of PR #453 — Redesign activity screen and tx detail sheet

2 new commits since my last review (53b81c2 merge + 9fb57b2 "feat: resolve PR review issues"). Ran CI-equivalent format check + code read.

Previous review follow-up

Resolved cleanly

• Critical — Send/Receive filters never auto-refresh — fixed via a new helper mobile-app/lib/shared/utils/tx_filter_family_provider.dart:

  void refreshPaginationFiltersFor(Reader read, List<String> targetIds, void Function(UnifiedPaginationController) action) {
    final cachedIds = AccountIdListCache.get(targetIds);
    for (final filter in TransactionFilter.values) {
      final notifier = read(filteredPaginationControllerProviderFamily(FilteredTransactionsParams(accountIds: cachedIds, filter: filter)).notifier);
      action(notifier);
    }
  }

  All three call-sites now use it: global_history_polling_service (1-min polling), reversible_transfer_monitoring_service (execution monitor), and transaction_submission_service (newly broadcast txs). Iterating TransactionFilter.values is exactly the right fix — adding a new filter case will auto-participate.

• DRY _listEquals — replaced with identical(other.accountIds, accountIds). I verified every caller routes through AccountIdListCache.get, so identity equality is sound.

• GraphQL duplicate connectionWhereClause — consolidated to reuse whereClause. The broader fragility of string-built queries is still there, but the immediate "6 places to update" problem is now 3.

• TransactionFilter naming — all/send/receive (camelCase) + a DisplayName extension with displayName getter. The activity screen now uses e.displayName instead of e.name, so enum values are properly decoupled from UI strings.

• Hardcoded filter button widths — replaced with IntrinsicWidth.

• txAmountFormatterProvider → txAmountDisplayProvider — good rename (it returns CurrencyDisplayState, not a String). DI restored for NumberFormattingService via numberFormattingServiceProvider. The redundant secondaryAmount: data.secondaryAmount noise in copyWith calls went away too.

• Doc-comment drift — docs now correctly reference TransactionFilter.send / TransactionFilter.receive.

Still open

1. CI formatting failures (blocker for merge to main)

   dart format . --line-length=120 --set-exit-if-changed reports 2 files changed:

   • mobile-app/lib/shared/utils/tx_filter_family_provider.dart — the refreshPaginationFiltersFor signature goes past 120 cols and needs wrapping
   • quantus_sdk/lib/src/models/transaction_filter.dart — missing trailing newline (\ No newline at end of file)

   Just run melos exec -- dart format . --line-length=120 and commit.

2. _DetailRow still hardcodes fallback color

   color: valueColor ?? Colors.white.withValues(alpha: 0.8)

   Should be a theme color (e.g. colors.textPrimary.useOpacity(0.8)) — same reasoning as the original centralization philosophy of the home-screen PR.

3. View in Explorer ↗ literal arrow — still a string-embedded glyph. Icon(Icons.arrow_outward, size: 14) is already used in activity_section.dart, so reusing it keeps the symbol consistent and theme/font-scaling-friendly.

4. Tx detail sheet still missing the address copy button and human-readable checkphrase — previously I flagged this as "confirm intentional". The screenshots match, so I'll take that as intentional, but it's a genuine functional regression vs the old sheet (users can't copy the address from the detail view anymore). Worth a brief confirmation in a comment or a follow-up issue.

5. addTransactionToHistory only goes to the all controller
   In transaction_submission_service.dart, the silent refresh now covers all three filters (good), but the pending tx is still only added to the TransactionFilter.all controller's in-memory history. For newly-broadcast send txs, the Send filter will show the tx only after the next silent refresh pulls it from the chain — there's a short window where it's missing from the Send view. Low-severity because the refresh fires immediately after, but refreshPaginationFiltersFor(...) { n.addTransactionToHistory(newTransaction); } in the same loop would close the gap.

6. No tests — previously flagged, still no unit tests for the FilteredTransactionsParams identity equality, the tx_filter_family_provider helper, or the dynamic GraphQL where-clause builder. Not a blocker, but these are exactly the self-contained, pure-logic pieces that most benefit from a test.

Nits

• transaction_filter.dart: the extension name DisplayName is very generic — something like TransactionFilterX or TransactionFilterDisplay reduces future name clashes.
• chain_history_service.dart: whereClause is now only assigned inside the switch and immediately used — could be inlined, but keeping it named is also fine for readability.

What's strong

• refreshPaginationFiltersFor is a genuinely good abstraction — one place to add new filters, all call-sites get them for free.
• The txAmountDisplayProvider API cleanup (dropping redundant copyWith args, restoring DI) shows real attention to detail.
• Identity-based equality for FilteredTransactionsParams is the right call given the cache invariant — that's a nice 7-line simplification.

Verdict

Not approving yet — blocked on CI format. The critical correctness issue (stale Send/Receive filters) is properly fixed with a clean abstraction, enum naming is cleaned up, DRY is in good shape. Items 2–5 above are quality-of-life and can land in a follow-up, but the format check must be green before this stack lands on main. Once dart format runs I'm happy to approve.