Feedback fixes for home screen and setting screen

[dewabisma]

Summary

Fix all issues mentioned in home screen and setting screen. Also fix trailing divider on last tx item.
Fixed on iPhone 8 simulator.

Screenshots

• Home screen

• Home screen (hide mode on)

• View all txs

• Setting screen

[n13]

Here's my review of PR #383: Feedback fixes for home screen and setting screen.

---

Overall

Clean UI polish PR -- font size bumps in settings, balance-hidden persistence, trailing divider fix, and layout adjustments. One bug to fix and some DRY opportunities.

---

Bug

isLastItem is wrong when fewer than 5 transactions exist

In activity_section.dart, the diff introduces:

...all.take(5).mapIndexed((index, tx) {
  final data = TxItemData.from(tx, activeAccount.accountId);
  final isLastItem = index == 4;

If there are fewer than 5 transactions (e.g., 2), index == 4 will never be true, so the last item will still render a trailing divider -- the exact problem this PR is trying to fix.

Compare with activity_screen.dart, which does it correctly:

...group.transactions.mapIndexed((index, tx) {
  final itemData = TxItemData.from(tx, active.account.accountId);
  final isLastItem = index == group.transactions.length - 1;

Fix: Convert to a list first and compare against its actual length:

final visible = all.take(5).toList();
...visible.mapIndexed((index, tx) {
  final isLastItem = index == visible.length - 1;

---

DRY Violations

1. Settings screen: title/subtitle Column repeated 4 times

The exact same Column structure appears in _toggleItem, _chevronItem, _externalItem, and _comingSoonItem:

              Text(title, style: text.smallParagraph?.copyWith(color: colors.textPrimary)),
              const SizedBox(height: 4),
              Text(subtitle, style: text.detail?.copyWith(color: colors.textTertiary)),

This was pre-existing, but the PR applies the same text style change to all 4 copies (bumping smallParagraph -> paragraph and detail -> smallParagraph), which underscores the duplication. A small helper would consolidate all 4:

Widget _itemContent(String title, String? subtitle, AppColorsV2 colors, AppTextTheme text) {
  return Column(
    crossAxisAlignment: CrossAxisAlignment.start,
    children: [
      Text(title, style: text.paragraph?.copyWith(color: colors.textPrimary)),
      if (subtitle != null) ...[
        const SizedBox(height: 4),
        Text(subtitle, style: text.smallParagraph?.copyWith(color: colors.textTertiary)),
      ],
    ],
  );
}

Then _toggleItem, _chevronItem, _externalItem, and _comingSoonItem all call _itemContent(...) instead of repeating the Column. Fewer places to update when styles change again.

2. isBalanceHidden read pattern duplicated

Both activity_screen.dart and activity_section.dart add the same two lines:

final settingsService = ref.watch(settingsServiceProvider);
final isBalanceHidden = settingsService.isBalanceHidden();

Minor, but if this grows to more screens, consider a dedicated isBalanceHiddenProvider that wraps this.

---

Other Notes

3. toggleBalanceHidden doesn't await the async write

void toggleBalanceHidden() {
  _settingsService.setBalanceHidden(!_balanceHidden);
  setState(() {});
}

setBalanceHidden returns Future<void>, but the result is fire-and-forgotten. This works because SharedPreferencesWithCache updates the in-memory cache synchronously, but it's fragile. If SharedPreferences implementation changes or the write fails, the UI will show a toggled state that wasn't persisted. Consider async/await:

Future<void> toggleBalanceHidden() async {
  await _settingsService.setBalanceHidden(!_balanceHidden);
  setState(() {});
}

4. txItemSeparator at 2% opacity

txItemSeparator: const Color(0x05FFFFFF),

0x05 is ~2% opacity white -- essentially invisible on most screens. This might be intentional to make dividers nearly vanish, but worth confirming it's not a typo (e.g., 0x15 for ~8% might be more in line with the existing separator at 0x1A / ~10%).

5. GradientBackground scope change

The gradient moved from wrapping just _buildContent to wrapping the entire CustomScrollView (including ActivitySection). This is a meaningful layout change -- the gradient now extends behind the activity list. Looks intentional based on the screenshots, just flagging it's not just a refactor.

6. Fixed-height balance container

return SizedBox(
  height: 96,
  child: Column(

Hard-coding height: 96 prevents layout shift when toggling between showing/hiding the ~$0.00 subtitle. Smart approach, but could break if text size or accessibility scaling changes. Consider keeping it flexible or at minimum using a ConstrainedBox(constraints: BoxConstraints(minHeight: 96)) to be safer.

---

Summary

Category	Items
Bug	isLastItem uses index == 4 instead of comparing to actual list length
DRY	Title/subtitle Column duplicated across 4 settings item builders
Minor	Unawaited async in toggleBalanceHidden; near-invisible separator opacity; fixed-height balance box
Layout	GradientBackground scope intentionally expanded (looks correct)

The only blocking item is the isLastItem bug -- the rest are improvements that could be addressed now or in a follow-up.

[n13]

Great, the latest version has addressed several of the previous issues. Here's the updated review:

---

PR #383 Re-Review (latest)

Nice improvements since the last version! The isLastItem bug is fixed, SettingsService access is now consistent via a proper Riverpod provider, the async is properly awaited, and the settings screen DRYs up the repeated title/subtitle pattern with _itemContent. Well done.

A few remaining items:

---

Bug: IsBalanceHiddenNotifier doesn't load persisted state on init

The notifier initializes with super(false):

class IsBalanceHiddenNotifier extends StateNotifier<bool> {
  final SettingsService _settingsService;

  IsBalanceHiddenNotifier(this._settingsService) : super(false);

If a user hid their balance in a previous session, on app restart ref.watch(isBalanceHiddenProvider) returns false (the hardcoded initial value) instead of true (the persisted value). The balance will briefly show as visible before any toggle.

Fix is straightforward - initialize from the service:

IsBalanceHiddenNotifier(this._settingsService) 
    : super(_settingsService.isBalanceHidden());

---

Minor: ref.watch(isBalanceHiddenProvider) called twice in the same build

In home_screen.dart, the provider is watched separately in both _buildTopBar and _buildBalance. Since both are called from the same build cycle, this is functionally fine but slightly redundant. Could read it once in build (or _buildContent) and pass it down, keeping things cleaner.

---

Minor: Positional booleans in buildTxItem

Still two consecutive positional booleans:

Widget buildTxItem(
  TransactionEvent tx,
  TxItemData data,
  AppColorsV2 colors,
  AppTextTheme text,
  bool isBalanceHidden,
  bool isLastItem, {
  VoidCallback? onTap,
})

Named parameters would be friendlier at call sites, but this is a minor readability preference.

---

Nits

• SizedBox(height: 96) for the balance area - works to prevent layout shift, but is a magic number tied to the current text style. A named constant would make intent clearer.
• txItemSeparator: const Color(0x05FFFFFF) - 2% opacity is very faint. Presumably intentional, just worth a sanity check on-device.

---

Summary

Severity	Item	Status
Bug	IsBalanceHiddenNotifier ignores persisted value on init	New finding
Minor	isBalanceHiddenProvider watched twice in same build	Nitpick
Minor	Positional booleans in buildTxItem	Carried over
Nit	Magic number 96, very faint separator	Carried over
Bug	isLastItem hardcoded to 4	Fixed
Should fix	Inconsistent SettingsService access	Fixed
Should fix	Unawaited async	Fixed
Should fix	DRY in settings item builders	Fixed (extracted _itemContent)

The super(false) bug is the main actionable item - a user who hides their balance will see it flash visible on every app restart. Everything else looks good!

[n13]

See above

[n13]

PR #383 Review — Feedback fixes for home screen and setting screen

Branch: beast/fix-home-screen → beast/remote-notification
Files changed: 8 (7 modified + 1 new provider)
Reviewed: 2026-02-16

---

Summary

This PR addresses feedback on the home screen and settings screen. The changes include:

• Persisting balance hidden state via SettingsService + a Riverpod StateNotifier
• Hiding tx amounts/addresses in activity lists when balance is hidden
• Removing the trailing divider on the last transaction item
• Moving GradientBackground up the tree so it covers the activity section
• Fixed-height balance container to prevent layout shift on toggle
• Font size bump across all settings screen item builders
• DRY extraction of _itemContent in settings screen
• New txItemSeparator theme color

---

Verdict: Approve with minor suggestions

Everything looks solid. Previous issues (init bug, positional booleans, DRY, async handling, isLastItem logic) have all been addressed. Only minor suggestions remain.

---

Minor suggestions

1. Redundant isBalanceHidden getter on IsBalanceHiddenNotifier

The notifier exposes both state (via ref.watch) and a separate isBalanceHidden getter that reads directly from SettingsService. Since super(_settingsService.isBalanceHidden()) now correctly initializes state, the getter is only used in one place — toggleBalanceHidden:

await isBalanceHiddenNotifier.setIsBalanceHidden(!isBalanceHiddenNotifier.isBalanceHidden);

This could use state directly instead:

await isBalanceHiddenNotifier.setIsBalanceHidden(!isBalanceHiddenNotifier.state);

Then the getter can be removed, keeping the notifier's API surface minimal — state is the single source of truth.

2. SizedBox(height: 96) for balance area

This fixed height prevents layout shift when toggling the ≈ $0.00 line — good idea. Worth considering making it a named constant (e.g. static const _balanceAreaHeight = 96.0) so it's clear this is intentional and easy to find if text styles change later.

3. txItemSeparator opacity

Color(0x05FFFFFF) is ~2% white opacity — extremely subtle. Likely intentional for a very soft divider, but worth a quick sanity check on a physical device to make sure it's actually visible and not effectively invisible.

4. "View All" underline gap technique

The transparent-color-with-shadow approach is a well-known Flutter workaround for controlling underline offset. It works, but is non-obvious to someone reading the code for the first time. A one-line comment like // Shadow trick to create gap between text and underline would help.

---

What was fixed since last review

Item	Status
IsBalanceHiddenNotifier ignoring persisted value on init (super(false))	Fixed — now super(_settingsService.isBalanceHidden())
isLastItem hardcoded to index == 4 in activity_section	Fixed — now uses recentTransactions.length - 1
Positional booleans in buildTxItem	Fixed — now named parameters (isBalanceHidden:, isLastItem:)
Inconsistent SettingsService access (direct vs provider)	Fixed — uses isBalanceHiddenProvider everywhere
Unawaited async setBalanceHidden	Fixed — toggleBalanceHidden is now async/await
DRY violation in settings item builders	Fixed — extracted _itemContent helper
ref.watch called twice for same provider in one build	Fixed — read once in build, passed down to _buildTopBar and _buildBalance

[n13]

Approved with minor suggestions up there - pick and choose