fix: Confirm Delete stays disabled after a successful wallet delete, so a second delete needs a popup reopen #335

Closed
opened 2026-08-20 14:42:14 +02:00 by clawbot · 1 comment
Collaborator

Found while implementing #312. Pre-existing on the password route, not introduced by that change.

btn-delete-wallet-confirm is disabled before the decrypt and never re-enabled on success. Delete one wallet, then open Delete Wallet for a second one in the same popup session: the "Confirm Delete" button is dead until the popup is closed and reopened. Nothing explains why, so the screen looks broken rather than busy.

Note the asymmetry this leaves behind: the "I have lost my password" route added by #312 re-enables its own button via the leave hook, so the two routes on the same screen now behave differently. That inconsistency is the strongest argument for fixing this — a reader comparing them will assume one of the two is wrong, and be unable to tell which.

Also relevant to the fix: src/popup/views/deleteWallet.js now has a shared finishDelete() used by both routes, so there is one obvious place for the button-state reset to live.

Definition of done

  • A second wallet can be deleted in the same popup session without reopening the popup.
  • Both routes reset their control state the same way, through the shared path rather than each handling it separately.
  • Test covering the password route's button state after a successful delete, alongside the existing coverage in tests/deleteWalletLostPassword.test.js.
  • The test fails against current head — state the mutation and observed result.
  • make check green.
Found while implementing https://git.eeqj.de/sneak/AutistMask/issues/312. Pre-existing on the password route, not introduced by that change. `btn-delete-wallet-confirm` is disabled before the decrypt and never re-enabled on success. Delete one wallet, then open Delete Wallet for a second one in the same popup session: the "Confirm Delete" button is dead until the popup is closed and reopened. Nothing explains why, so the screen looks broken rather than busy. Note the asymmetry this leaves behind: the "I have lost my password" route added by #312 re-enables its own button via the leave hook, so the two routes on the same screen now behave differently. That inconsistency is the strongest argument for fixing this — a reader comparing them will assume one of the two is wrong, and be unable to tell which. Also relevant to the fix: `src/popup/views/deleteWallet.js` now has a shared `finishDelete()` used by both routes, so there is one obvious place for the button-state reset to live. ## Definition of done - [ ] A second wallet can be deleted in the same popup session without reopening the popup. - [ ] Both routes reset their control state the same way, through the shared path rather than each handling it separately. - [ ] Test covering the password route's button state after a successful delete, alongside the existing coverage in `tests/deleteWalletLostPassword.test.js`. - [ ] The test fails against current head — state the mutation and observed result. - [ ] `make check` green.
clawbot added this to the 1.0.0 milestone 2026-09-21 09:20:00 +02:00
Author
Collaborator

Fixed in #392.

Both delete routes now re-enable the Confirm Delete button through the shared
finishDelete(), so a second wallet can be deleted in the same popup session
without reopening, and the two routes reset the same way instead of one doing it
in its leave hook.

To be exact about the lost-password route: in the running popup it already
re-enabled its button through its leave hook, since completing a delete switches
views and runs that view's leave hook. This change does not fix a broken
re-enable there; it routes both resets through finishDelete() so there is the
single reset path this issue asks for, and the re-enable no longer depends on
the leave hook firing.

New tests drive a password-route delete and a second delete in one session; they
fail against current head, where the button stays disabled after the first
delete.

Model: opus-4-8

Fixed in https://git.eeqj.de/sneak/AutistMask/pulls/392. Both delete routes now re-enable the Confirm Delete button through the shared `finishDelete()`, so a second wallet can be deleted in the same popup session without reopening, and the two routes reset the same way instead of one doing it in its leave hook. To be exact about the lost-password route: in the running popup it already re-enabled its button through its leave hook, since completing a delete switches views and runs that view's leave hook. This change does not fix a broken re-enable there; it routes both resets through `finishDelete()` so there is the single reset path this issue asks for, and the re-enable no longer depends on the leave hook firing. New tests drive a password-route delete and a second delete in one session; they fail against current head, where the button stays disabled after the first delete. Model: opus-4-8
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#335