Skip to content

refactor(settings): restyle wallet deletion prompts as bottom sheets - #2691

Merged
ethicnology merged 1 commit into
developfrom
fix/wallet-deletion-dialog-style
Aug 18, 2026
Merged

refactor(settings): restyle wallet deletion prompts as bottom sheets#2691
ethicnology merged 1 commit into
developfrom
fix/wallet-deletion-dialog-style

Conversation

@wired-pasteque

Copy link
Copy Markdown
Collaborator

Fixes: #2690

Replaces the raw Material AlertDialog's for the delete-wallet confirmation and failure prompts with BullBottomSheets, matching the app's other prompts (delete account, log out).

Simulator Screenshot - iPhone 17e - 2026-08-18 at 13 25 08

@wired-pasteque wired-pasteque self-assigned this Aug 18, 2026
@claude

This comment was marked as resolved.

@wired-pasteque
wired-pasteque force-pushed the fix/wallet-deletion-dialog-style branch from 562cc63 to b412bf6 Compare August 18, 2026 17:48
@wired-pasteque

Copy link
Copy Markdown
Collaborator Author

@claude, check the fixes

@claude

claude Bot commented Aug 18, 2026

Copy link
Copy Markdown

Claude finished @wired-pasteque's task in 3m 12s —— View job


Review: wallet deletion prompts as bottom sheets

  • Gather context (PR diff, BullBottomSheet, comparable sheets, bloc/theme APIs)
  • Review the new bottom sheet widgets and wiring
  • Post review feedback

Verdict: LGTM ✅ — the refactor is correct and well-wired. No blocking issues found.

What I verified

  • All APIs exist and match: BullButton.big supports outlined/borderColor/bgColor/textColor (packages/bull_ui/lib/src/buttons/bull_button.dart:22), BullIcon takes a positional glyph with size/color, and the theme colors used (transparent, onError, outlineVariant, text, error) are all defined in BullTheme.
  • Error lifecycle is sound: WalletBloc clears walletDeletionError at the start of each deletion (lib/features/wallet/presentation/bloc/wallet_bloc.dart:337), so the router's listenWhen null→non-null transition fires again on repeat failures. Passing state.walletDeletionError! into the sheet at show-time is actually an improvement over the old dialog's reactive context.select — the sheet content can't go blank if the bloc state changes while it's open.
  • Context handling is correct: showModalBottomSheet builds the sheet under the root navigator, where WalletBloc isn't in scope. Passing onConfirm as a closure that captures the screen's context (wallet_details_screen.dart) sidesteps that correctly — the old dialog relied on the provider being reachable from the dialog context.
  • No dangling references: nothing left points to the deleted *_alert_dialog.dart files; all reused localization keys exist in every .arb file; the new Navigator export in bull_ui.dart keeps the list alphabetized and is consistent with the package's re-export philosophy.
  • Pattern consistency: the sheets closely mirror the existing DeleteAccountConfirmationBottomSheet / logout prompts (drag handle, icon, title, body, stacked actions).

Minor, non-blocking suggestions

  1. Duplicated drag handle — the 40×4 handle Container is now copy-pasted in both new sheets, and the same block already exists in ~8 other files. Since BullBottomSheet is the shared chrome, it'd be a nice follow-up to move the handle into BullBottomSheet (or a small BullSheetHandle widget) so callers stop duplicating it. Not required here since it matches the prevailing pattern. Fix this →
  2. Icon size — the new sheets use size: 36 while the delete-account sheet uses 48. If the screenshot reflects the intended design this is fine; just flagging the small inconsistency across destructive prompts.

Note: I couldn't run flutter analyze in this environment (the Flutter CLI invocation isn't in my allowed tools), so the verification above is from reading the code paths directly. CI should confirm the build.

@ethicnology
ethicnology merged commit e32adc2 into develop Aug 18, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wallet deletion prompts don't match the app style

2 participants