Update accounts sheet

[dewabisma]

Changes

• Refactor accounts sheet into several widgets (Fix weird glass layer bug)
• Update button implementation in account sheet related widget
• Fix checkphrase copy message

Screenshots

• Accounts sheet

• Edit account

• Create account

[n13]

Here's my review of PR #411.

---

PR #411 — Update accounts sheet

Summary: Refactors the 792-line monolithic accounts_sheet.dart into 4 files under a new accounts/ directory, switches from showAppModalBottomSheet to the standardized BottomSheetContainer, and migrates inline icon buttons to the shared ButtonIcon component. Also fixes the checkphrase copy message.

Overall the decomposition is clean and the direction is right. I have a few concerns ranging from a potential bug to DRY violations.

---

Bug: showAccountsSheet return type is broken

The old implementation returned the modal's result:

Future<T?> showAccountsSheet<T>(BuildContext context) {
  return showAppModalBottomSheet(context: context, builder: (_) => const AccountsSheet());
}

The new version always returns null, making the generic <T> meaningless:

Future<T?> showAccountsSheet<T>(BuildContext context) async {
  BottomSheetContainer.show(context, builder: (_) => const AccountsSheet());
  return null;
}

If any caller ever depends on the return value, it will silently break. This should either await the show result and return it, or simplify the signature to Future<void>.

---

Bug: Hardcoded height of 610 with no screen-size guard

The old code clamped the height to what the screen could fit:

final maxHeight = media.size.height - media.padding.top - 20;
final sheetHeight = math.min(610.0, maxHeight);

The new code passes a raw height: 610 with no capping:

      height: 610,

On smaller devices (iPhone SE, or with large accessibility text / keyboard visible) this will overflow. The height constraint should be handled somewhere — either in BottomSheetContainer or at the call site.

---

DRY: InputDecoration duplicated verbatim

The exact same "transparent, borderless, dense" InputDecoration appears in both CreateAccountView._buildCreatedNameField and EditAccountView._buildAccountNameField:

        decoration: const InputDecoration(
          filled: true,
          fillColor: Colors.transparent,
          border: InputBorder.none,
          enabledBorder: InputBorder.none,
          focusedBorder: InputBorder.none,
          disabledBorder: InputBorder.none,
          isDense: true,
          contentPadding: EdgeInsets.zero,
        ),

        decoration: const InputDecoration(
          filled: true,
          fillColor: Colors.transparent,
          border: InputBorder.none,
          enabledBorder: InputBorder.none,
          focusedBorder: InputBorder.none,
          disabledBorder: InputBorder.none,
          isDense: true,
          contentPadding: EdgeInsets.zero,
        ),

This should be extracted to a shared constant in account_shared_components.dart (or a theme-level constant).

---

DRY: Repeated TextStyle patterns

The TextStyle(fontFamily: 'Inter', fontSize: 14, fontWeight: FontWeight.w500, color: Colors.white, height: 1.35) pattern (and its w400/accentPink variants) appears in multiple places across both views. Worth extracting to named constants or to the text theme.

---

FutureBuilder rebuilds on every setState

In _buildContent, the edit view wraps in a FutureBuilder:

      return FutureBuilder<String>(
        future: _checksumService.getHumanReadableName(editingAccount.accountId),
        builder: (context, snapshot) {
          final checksum = snapshot.connectionState == ConnectionState.done ? (snapshot.data ?? '-') : 'Loading...';
          return EditAccountView(
            // ...
          );
        },
      );

Since _checksumService.getHumanReadableName(...) is called inline inside build(), every setState creates a new Future, restarting the async call and briefly flashing "Loading...". The future should be cached when entering edit mode (in _openEdit), similar to how _draftChecksum is handled for the create flow.

---

Minor: Unnecessary comment

                  const SizedBox(width: 20), // Spacer to maintain alignment if needed

---

Minor: _buildPrimarySheetButton is trivial

  Widget _buildPrimarySheetButton({required String label, required VoidCallback onTap, bool isLoading = false}) {
    return Button(label: label, onTap: onTap, isLoading: isLoading);
  }

This is a 1:1 wrapper around Button with the same parameters. Could just call Button(...) directly and remove this method.

---

UI note: AccountGradientImage removed from header

The old accounts list had the account's gradient avatar next to "Accounts" in the header. The new version delegates the header to BottomSheetContainer which doesn't support leading icons, so the avatar is gone. This may or may not be intentional — worth confirming.

---

What looks good

• Splitting 792 lines into focused, single-responsibility files is a great improvement
• The shared AccountField, AccountCopyRow, and AccountIconActionButton components are well-extracted
• Using ButtonIcon.rounded instead of inline GlassContainer is the right call
• Enhancing BottomSheetContainer with onBack/showCloseIcon/height makes it properly reusable
• The checkphrase copy message fix is a nice touch
• Import cleanup is thorough — all 3 consumers updated

[dewabisma]

Added back the gradient!

[n13]

All previous feedback items have been addressed nicely. Here's the updated verdict:

---

Re-review of PR #411

Previously raised issues — all resolved

Issue	Status
showAccountsSheet return type broken	Fixed — now returns BottomSheetContainer.show<T>(...)
Hardcoded height 610 with no screen guard	Fixed — uses math.min(610.0, maxHeight)
InputDecoration duplicated	Fixed — extracted to accountFieldDecoration constant
Inline TextStyle duplication	Fixed — uses theme styles (context.themeText.smallParagraph, .paragraph, .detail)
FutureBuilder rebuilding on every setState	Fixed — checksum cached in _editingAccountChecksum, fetched once in _openEdit
Trivial _buildPrimarySheetButton wrapper	Fixed — removed, calls Button(...) directly
AccountGradientImage removed from header	Fixed — restored via titleBuilder on BottomSheetContainer
BottomSheetContainer.show returning void	Fixed — now returns Future<T?>

One remaining nit: missing mounted check in _openEdit

There's an await gap without a guard:

  Future<void> _openEdit(Account account) async {
    _nameController.text = account.name;

    setState(() {
      _editingAccountId = account.accountId;
      _isEditingName = false;
    });

    final checksum = await _checksumService.getHumanReadableName(account.accountId);
    setState(() {
      _editingAccountChecksum = checksum;
    });
  }

If the widget is disposed during the await, the second setState will throw. The rest of the file is careful about this pattern (e.g. _createNewAccount, _saveEditedName both check if (mounted)). Should be:

final checksum = await _checksumService.getHumanReadableName(account.accountId);
if (mounted) {
  setState(() {
    _editingAccountChecksum = checksum;
  });
}

---

Other than that, this looks good to merge. The refactoring is clean, DRY is respected, the BottomSheetContainer enhancements (titleBuilder, onBack, height) are well-designed, and the accountFieldDecoration shared constant is a nice touch.

[n13]

Fixed the mounted issue found in review below

GTG 🚀

All previous feedback items have been addressed nicely. Here's the updated verdict:

---

Re-review of PR #411

Previously raised issues — all resolved

Issue	Status
showAccountsSheet return type broken	Fixed — now returns BottomSheetContainer.show<T>(...)
Hardcoded height 610 with no screen guard	Fixed — uses math.min(610.0, maxHeight)
InputDecoration duplicated	Fixed — extracted to accountFieldDecoration constant
Inline TextStyle duplication	Fixed — uses theme styles (context.themeText.smallParagraph, .paragraph, .detail)
FutureBuilder rebuilding on every setState	Fixed — checksum cached in _editingAccountChecksum, fetched once in _openEdit
Trivial _buildPrimarySheetButton wrapper	Fixed — removed, calls Button(...) directly
AccountGradientImage removed from header	Fixed — restored via titleBuilder on BottomSheetContainer
BottomSheetContainer.show returning void	Fixed — now returns Future<T?>

One remaining nit: missing mounted check in _openEdit

There's an await gap without a guard:

  Future<void> _openEdit(Account account) async {
    _nameController.text = account.name;

    setState(() {
      _editingAccountId = account.accountId;
      _isEditingName = false;
    });

    final checksum = await _checksumService.getHumanReadableName(account.accountId);
    setState(() {
      _editingAccountChecksum = checksum;
    });
  }

If the widget is disposed during the await, the second setState will throw. The rest of the file is careful about this pattern (e.g. _createNewAccount, _saveEditedName both check if (mounted)). Should be:

final checksum = await _checksumService.getHumanReadableName(account.accountId);
if (mounted) {
  setState(() {
    _editingAccountChecksum = checksum;
  });
}

---

Other than that, this looks good to merge. The refactoring is clean, DRY is respected, the BottomSheetContainer enhancements (titleBuilder, onBack, height) are well-designed, and the accountFieldDecoration shared constant is a nice touch.