Skip to content

fix(ui): let the key-rotation confirmation be arrowed to NO - #144

Open
amuntri wants to merge 1 commit into
MostroP2P:mainfrom
amuntri:fix/generate-keys-confirm-arrows
Open

fix(ui): let the key-rotation confirmation be arrowed to NO#144
amuntri wants to merge 1 commit into
MostroP2P:mainfrom
amuntri:fix/generate-keys-confirm-arrows

Conversation

@amuntri

@amuntri amuntri commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Problem

UiMode::ConfirmGenerateNewKeys is missing from both Left/Right groups in src/ui/key_handler/navigation.rs. On the Generate New Keys confirmation the arrow keys therefore do nothing: the popup opens on YES, and Enter rotates the keys regardless of what the user tried to select. Only Esc cancels.

This is present on current main (shipped in 0.2.8), and it is the one confirmation in the app where the arrows silently do not work — every other confirm popup (Add Relay, Change Mostro Pubkey, Add Currency Filter, Delete History, Clean Up History, Exit, …) is in those groups. The muscle memory built on all of them leads straight into rotating keys.

And the action is destructive: spawn_key_rotation_task runs User::replace_all_in_tx (DELETE FROM users + insert) and Order::delete_all_in_tx in one transaction. A user who arrows to NO and presses Enter loses their identity and their whole local order history, having done what the UI told them was a cancel.

Fix

Add ConfirmGenerateNewKeys to both groups in handle_left_key / handle_right_key, plus a regression test asserting Right selects NO and Left selects back to YES.

Two lines of behaviour change; no other confirmation is touched.

cargo test --all-features → 426 passed. Clippy -D warnings and fmt clean.

Note

Found while working on #114 — adding ConfirmRestoreSession right next to it in those match groups is what made the omission visible. #114 currently carries the same fix in its own branch; whichever lands first, I will drop it from the other so there is no duplicate.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Arrow keys now toggle between YES and NO in the generate-new-keys confirmation popup.
  • Tests

    • Added coverage to verify button selection changes correctly with left and right navigation.

UiMode::ConfirmGenerateNewKeys was missing from both Left/Right groups in
navigation.rs, so on the "Generate New Keys" confirmation the arrow keys
did nothing: the popup opens on YES and Enter rotated the keys regardless
of what the user tried to select. Only Esc cancelled.

That is a destructive action — spawn_key_rotation_task replaces the user
row and clears the orders table in the same transaction — and it is the
one confirmation in the app where the arrows silently did not work, so
the muscle memory built on every other confirm popup (Add Relay, Change
Mostro Pubkey, Delete History, Exit...) leads straight to rotating keys.

Adds it to both groups, with a regression test.

Found while working on MostroP2P#114, where adding ConfirmRestoreSession next to it
made the omission visible.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d2505c9b-6144-40db-b26d-75cee530d9eb

📥 Commits

Reviewing files that changed from the base of the PR and between c4f72b6 and e7416f9.

📒 Files selected for processing (1)
  • src/ui/key_handler/navigation.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The generate-new-keys confirmation mode now handles Left and Right navigation. Left selects YES, Right selects NO, and a test verifies the toggle behavior.

Changes

Generate-new-keys navigation

Layer / File(s) Summary
Toggle confirmation selection
src/ui/key_handler/navigation.rs
Left selects YES and Right selects NO in ConfirmGenerateNewKeys. A test verifies both transitions.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to e7416

This enables arrow-key selection of NO for the key-rotation confirmation and adds regression coverage; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: arkanoider

Poem

A rabbit taps Left: “YES!”
Right makes “NO” appear.
The keys now guide the choice,
While tests keep watch nearby.
Fresh confirmation hops along.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: enabling arrow-key navigation to select NO in the key-rotation confirmation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant