Skip to content

fix(ui): prevent stacking banners during wallet identity search#734

Closed
thepastaclaw wants to merge 1 commit into
dashpay:v1.0-devfrom
thepastaclaw:fix/stacking-banners
Closed

fix(ui): prevent stacking banners during wallet identity search#734
thepastaclaw wants to merge 1 commit into
dashpay:v1.0-devfrom
thepastaclaw:fix/stacking-banners

Conversation

@thepastaclaw
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw commented Mar 12, 2026

Summary

Fixes #713 — "Load identity: load from wallet displays messy messages."

When searching a wallet for identities up to a given index, each progress update ("Searching index X of Y...") was sent as a BackendTaskSuccessResult::Message, which triggered MessageBanner::set_global() in app.rs — creating a new green success banner for every iteration. These banners stacked up, cluttering the UI.

Changes

  • Add BackendTaskSuccessResult::Progress(String) — a new variant for progress messages that should NOT create global banners
  • app.rs — handle Progress by forwarding to the screen's display_task_result only (no MessageBanner::set_global)
  • load_identity_from_wallet.rs — change the loop's progress sends from Message to Progress
  • add_existing_identity_screen.rs — handle Progress by updating backend_message (rendered inline as a label, not a stacking banner)

Validation

  • cargo check passes
  • cargo fmt passes
  • Verified that the Progress variant skips MessageBanner::set_global in app.rs (line 1115)
  • Verified that backend_message is rendered inline at line 1143 as "{progress_msg} ({elapsed})" — single updating label, no stacking

Summary by CodeRabbit

  • New Features
    • Added progress tracking for long-running backend operations, enabling real-time feedback to be displayed during extended tasks such as identity loading.
    • Progress information is now displayed separately from general messages, providing users with clearer, more granular updates throughout multi-step operations and improving overall application responsiveness.

…pay#713)

Add BackendTaskSuccessResult::Progress variant for progress messages
that should update the existing banner instead of creating new global
banners. Change load_user_identities_up_to_index to send Progress
instead of Message for "Searching index X of Y..." updates.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@thepastaclaw
Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 12, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6674730b-359d-4b55-b806-e75d37b3df35

📥 Commits

Reviewing files that changed from the base of the PR and between 91abb2c and 4febba7.

📒 Files selected for processing (4)
  • src/app.rs
  • src/backend_task/identity/load_identity_from_wallet.rs
  • src/backend_task/mod.rs
  • src/ui/identities/add_existing_identity_screen.rs

📝 Walkthrough

Walkthrough

A new Progress(String) variant is introduced to the BackendTaskSuccessResult enum to distinguish progress updates from general messages. Wallet identity loading now emits progress updates instead of messages, with corresponding handling added to app state routing and UI display logic.

Changes

Cohort / File(s) Summary
Enum Definition
src/backend_task/mod.rs
Adds Progress(String) variant to BackendTaskSuccessResult enum for long-operation progress updates; shortens comment on existing Message(String) variant.
Task Implementation
src/backend_task/identity/load_identity_from_wallet.rs
Replaces BackendTaskSuccessResult::Message with BackendTaskSuccessResult::Progress for per-index progress messages during identity loading.
App State & UI Handling
src/app.rs, src/ui/identities/add_existing_identity_screen.rs
Routes BackendTaskSuccessResult::Progress through display_task_result pathway in app state; UI screen now explicitly handles progress updates and stores them in backend_message.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Progress hops along the screen,
No longer mixed with message's gleam,
A cleaner path from task to view,
One variant makes the messages true!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. 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 PR title clearly and concisely describes the main change: preventing multiple banner stacks during wallet identity search, which directly addresses the core issue being fixed.
Linked Issues check ✅ Passed The PR fully addresses issue #713 by introducing BackendTaskSuccessResult::Progress to prevent stacked banners during wallet identity search, with progress displayed inline instead.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing issue #713: Progress enum variant, AppState routing, wallet identity loader, and progress display in add_existing_identity_screen—no unrelated modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 12, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@thepastaclaw
Copy link
Copy Markdown
Collaborator Author

Closing as duplicate of #735, which has the more targeted fix. Sorry for the duplicate — improving my dedup guards to prevent this.

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.

Load identity: load from wallet displays messy messages

1 participant