feat(ui): unified MessageBanner component for screen-level messages

[lklimek]

Summary

This is only component implementation and a few example screens. It is applied to other screens in PR #604

• Add reusable MessageBanner component following the Component Design Pattern, replacing ad-hoc error/success/warning/info message display across screens
• Add MessageType::Warning variant and corresponding DashColors theme methods for all 4 severity levels
• Store egui::Context in AppContext so banners can be set from display_task_result() without buffering
• Support multiple simultaneous global banners with text-based deduplication, replacement, and a cap of 5
• Implement Component trait for MessageBanner with BannerStatus domain type (Visible, Dismissed, TimedOut)
• Provide chainable BannerHandle API for per-banner auto-dismiss configuration
• Migrate 3 representative screens (IdentitiesScreen, MintTokensScreen, TopUpIdentityScreen) to the new system
• Add 16 kittest UI integration tests for the component

Design documents

• docs/ai-design/2026-02-17-unified-messages/message-banner-ux-spec.md — UX specification
• docs/ai-design/2026-02-17-unified-messages/architecture.md — Technical architecture
• docs/ai-design/2026-02-17-unified-messages/message-banner-mockup.html — Interactive HTML mockup

API

// Global banners (stored in egui context data, rendered by island_central_panel)
MessageBanner::set_global(ctx, "Success!", MessageType::Success);
MessageBanner::set_global(ctx, "Temp error", MessageType::Error)
    .with_auto_dismiss(Duration::from_secs(8));
MessageBanner::replace_global(ctx, "old text", "new text", MessageType::Success);
MessageBanner::clear_global_message(ctx, "specific message");

// Per-instance banners (owned by individual screens)
let mut banner = MessageBanner::new();
banner.set_message("Error occurred", MessageType::Error);
let response = banner.show(ui); // Component trait

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit

* **New Features**
* Unified MessageBanner system: global and per-screen banners displayed above content for backend feedback, refresh/status, successes and errors; central rendering with deduplication and auto-dismiss/countdown.
* Warning severity added with distinct styling and behavior; many screens now surface runtime messages via the unified banner.

* **Documentation**
* UX spec, architecture design, and visual mockup for the banner system.

* **Tests**
* Comprehensive banner test suite covering lifecycle and edge cases.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• ✅ Review completed - (🔄 Check again to review again)

📝 Walkthrough

Walkthrough

Adds a unified MessageBanner system (global and per-instance) with lifecycle, deduplication, auto-dismiss, central rendering via island_central_panel, MessageType::Warning, AppContext egui context plumbing, widespread screen migrations to global banners, DashColors message helpers, docs, mockups, and extensive kittest coverage.

Changes

Cohort / File(s)	Summary
Message Banner Core src/ui/components/message_banner.rs, src/ui/components/mod.rs	New MessageBanner, BannerHandle, BannerState/BannerStatus, MessageBannerResponse; global & per-instance APIs (set_global, replace_global, clear_global, has_global, show_global); deduplication, auto-dismiss, MAX_BANNERS cap; re-exports.
Central Rendering & Integration src/ui/components/styled.rs, src/app.rs, src/ui/components/message_banner.rs	island_central_panel now invokes MessageBanner::show_global(ui); App emits global banners on backend task results; AppContext construction updated to accept/store egui::Context and call sites adjusted.
App Context src/context/mod.rs	AppContext gains egui_ctx: egui::Context, constructor signature updated, and egui_ctx() accessor added.
Theme & Message Colors src/ui/theme.rs, src/ui/mod.rs	Added DashColors helpers: info_color, message_color, message_background_color; moved message color logic to DashColors; added MessageType::Warning.
Screen Migrations to Banners src/ui/identities/..., src/ui/identities/top_up_identity_screen/..., src/ui/tokens/..., src/ui/many_screens/*	Many screens removed local error/refresh state and now use global MessageBanner for errors/warnings; display_message/display_task_result updated; some enums/fields removed or simplified.
Warning Support / Color Updates src/ui/dashpay/..., src/ui/wallets/..., src/ui/contracts_documents/..., src/ui/tools/..., src/ui/tokens/...	Added MessageType::Warning handling across UI files—either mapped to DashColors::warning_color for visuals or treated like Error in display_message branches.
Docs & Mockups CLAUDE.md, doc/COMPONENT_DESIGN_PATTERN.md, docs/ai-design/.../architecture.md, .../message-banner-mockup.html, .../message-banner-ux-spec.md	New architecture/design/UX docs, mockup HTML, and component pattern docs explaining global vs per-instance banner usage, APIs, and migration notes. Duplicate insertion noted in CLAUDE.md.
Tests (kittest) tests/kittest/main.rs, tests/kittest/message_banner.rs	New egui kittest suite exercising global/per-instance lifecycle, rendering assertions, deduplication, capacity/eviction, replace/clear semantics, elapsed/auto-dismiss behaviors.

Sequence Diagram(s)

sequenceDiagram
    participant Backend as Backend Task
    participant AppState as AppState::update
    participant AppCtx as AppContext (egui_ctx)
    participant Banner as MessageBanner
    participant Central as island_central_panel
    participant Screen as Screen Render

    Backend->>AppState: send task result (success/error)
    activate AppState
    AppState->>AppCtx: access egui_ctx
    AppState->>Banner: MessageBanner::set_global(ctx, text, type)
    AppState->>Screen: call display_message() for side-effects
    deactivate AppState

    Screen->>Central: start frame render
    Central->>Banner: MessageBanner::show_global(ui)
    activate Banner
    Banner->>AppCtx: read banner state from ctx (get_banners)
    Banner->>Banner: process_banner (expiry/annotation)
    Banner->>Screen: render_banner (icon, text, dismiss)
    deactivate Banner

    Screen->>Banner: user dismiss or auto-dismiss triggers
    activate Banner
    Banner->>AppCtx: update/clear banner state in ctx
    deactivate Banner

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

• test: E2E Testing Roadmap — End-Goal State & Phased Plan #599 — PR adds MessageBanner implementation, kittest suite, and AppContext::new signature changes that align with the E2E testing and kittest concerns described in the issue.

Poem

🐰 A banner hops across the screen,

Errors, warnings — tidy and keen,
Global drum or local scene,
I twitch my whiskers, soft and green,
Hooray — messages neat and seen!

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat(ui): unified MessageBanner component for screen-level messages' clearly and concisely summarizes the main change: introducing a new reusable MessageBanner component for managing screen-level UI messages, which is the primary objective of this PR.
Docstring Coverage	✅ Passed	Docstring coverage is 95.16% which is sufficient. The required threshold is 80.00%.

_{✏️ 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
• Commit unit tests in branch design/unified-message-display

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[lklimek]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[Copilot]

Pull request overview

This PR introduces the MessageBanner component as a unified system for displaying screen-level messages, replacing ad-hoc error/success/warning/info message displays across the codebase. It follows the Component Design Pattern and provides both global (app-wide) and per-instance usage modes.

Changes:

• Adds MessageBanner component with global banner API and BannerHandle lifecycle management
• Adds MessageType::Warning variant to the existing 3-variant enum
• Extends DashColors with banner-specific color methods for all 4 severity levels
• Stores egui::Context in AppContext for non-UI code paths to set global banners
• Migrates 3 example screens (IdentitiesScreen, MintTokensScreen, TopUpIdentityScreen) to demonstrate the pattern
• Adds 16 kittest UI integration tests for the component
• Includes comprehensive documentation (architecture.md, UX spec, HTML mockup)

Reviewed changes

Copilot reviewed 46 out of 46 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/ui/components/message_banner.rs	New 469-line component implementing both global and per-instance banner APIs with Component trait
tests/kittest/message_banner.rs	Comprehensive test suite with 16 tests covering lifecycle, handles, deduplication, eviction, and rendering
src/ui/mod.rs	Adds MessageType::Warning variant to existing enum
src/ui/theme.rs	Removes old dead MessageType enum, adds info_color(), message_color(), message_background_color() methods
src/context/mod.rs	Stores egui::Context in AppContext for banner access from display_task_result()
src/app.rs	Sets global banners automatically for all TaskResult variants in update()
src/ui/components/styled.rs	Calls MessageBanner::show_global() in island_central_panel() before content
src/ui/identities/identities_screen.rs	Migrates from custom refreshing status to BannerHandle with elapsed time tracking
src/ui/tokens/mint_tokens_screen.rs	Migrates from error_message field and status enum to global banners
src/ui/identities/top_up_identity_screen/mod.rs	Removes 20-line custom error banner frame, uses global banner API
20+ screen files	Adds MessageType::Warning case to message color match statements
docs/ai-design/2026-02-17-unified-messages/	Comprehensive architecture, UX spec, and interactive HTML mockup documentation
doc/COMPONENT_DESIGN_PATTERN.md	Documents display-only components and global state pattern
CLAUDE.md	Adds MessageBanner usage guidelines to coding standards

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/ui/identities/top_up_identity_screen/by_wallet_qr_code.rs (1)

126-132: ⚠️ Potential issue | 🟡 Minor

Clear the global error banner when QR rendering succeeds or input becomes valid.

MessageBanner::set_global() returns a BannerHandle that can be cleared, but the current code discards it. In the render loop, once an error message is displayed, it persists visually even after the user fixes their input, since no code explicitly clears the banner on success.

Store the banner handle on TopUpIdentityScreen (e.g., qr_error_banner: Option<BannerHandle>) and clear it when rendering succeeds or when the input becomes valid again. This ensures stale error messages don't linger after the underlying issue is resolved.

Suggested implementation

-                    if let Err(e) = self.render_qr_code(ui, amount_dash) {
-                        MessageBanner::set_global(ui.ctx(), &e, MessageType::Error);
-                    }
+                    if let Err(e) = self.render_qr_code(ui, amount_dash) {
+                        if let Some(handle) = self.qr_error_banner.take() {
+                            handle.clear();
+                        }
+                        self.qr_error_banner =
+                            Some(MessageBanner::set_global(ui.ctx(), &e, MessageType::Error));
+                    } else if let Some(handle) = self.qr_error_banner.take() {
+                        handle.clear();
+                    }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/identities/top_up_identity_screen/by_wallet_qr_code.rs` around lines
126 - 132, The QR error banner is never cleared because the BannerHandle
returned by MessageBanner::set_global(...) is discarded; update
TopUpIdentityScreen to hold an Option<BannerHandle> field (e.g.,
qr_error_banner) and when calling MessageBanner::set_global in the render path
(inside render_qr_code error handling) store the returned handle in
self.qr_error_banner; when render_qr_code succeeds or when parsing
self.funding_amount yields Ok(amount_dash) && amount_dash > 0.0, check
self.qr_error_banner and call .clear() on the handle and set
self.qr_error_banner = None so the global banner is removed when the input or
rendering becomes valid.

🧹 Nitpick comments (12)

src/ui/mod.rs (1)

829-834: Warning severity added to MessageType — good addition.
Please remember to run cargo clippy and cargo +nightly fmt before finalizing.
As per coding guidelines: "**/*.rs: Always run cargo clippy and cargo +nightly fmt when finalizing work".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/mod.rs` around lines 829 - 834, You added a new variant Warning to the
MessageType enum (MessageType::Warning) but forgot to run the project
linters/formatters; run cargo clippy and cargo +nightly fmt across the repo to
ensure code style and clippy lints are clean before finalizing, then commit the
resulting formatting and any small clippy-driven fixes (e.g., matching arms,
unused imports) that may be required in modules that pattern-match on
MessageType.

src/ui/dashpay/add_contact_screen.rs (1)

242-246: Use the dark‑mode warning color helper for consistency.

DashColors::WARNING bypasses dark‑mode theming used elsewhere. Prefer the helper so warning colors stay consistent across themes.

🔧 Suggested update

-                let color = match message_type {
+                let dark_mode = ui.ctx().style().visuals.dark_mode;
+                let color = match message_type {
                     MessageType::Success => egui::Color32::DARK_GREEN,
                     MessageType::Error => egui::Color32::DARK_RED,
-                    MessageType::Warning => DashColors::WARNING,
+                    MessageType::Warning => DashColors::warning_color(dark_mode),
                     MessageType::Info => egui::Color32::LIGHT_BLUE,
                 };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/dashpay/add_contact_screen.rs` around lines 242 - 246, The match arm
for MessageType::Warning currently uses the constant DashColors::WARNING which
bypasses dark-mode theming; update the match in the message rendering (the match
on message_type / MessageType::Warning) to call the dark‑mode aware helper
(e.g., DashColors::warning_color() or the project’s designated warning_color
helper) instead of the raw constant so warning text follows theme-aware colors.

src/ui/wallets/asset_lock_detail_screen.rs (1)

365-370: Run clippy/fmt before finalizing.

Quick reminder to run cargo clippy and cargo +nightly fmt before finalizing.

As per coding guidelines, "Always run cargo clippy and cargo +nightly fmt when finalizing your work".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/wallets/asset_lock_detail_screen.rs` around lines 365 - 370, This
change needs formatting and lint fixes before merge: run cargo clippy and cargo
+nightly fmt, address any clippy warnings/errors and formatting changes (focus
on the code around message_color, the MessageType match arm, and uses of
DashColors::warning_color/text_primary) so the match expression and imports
conform to project lint/style rules, then re-run the checks to ensure the file
compiles cleanly.

src/app.rs (1)

190-229: Reminder: run clippy and fmt before finalizing.

As per repo guidelines, please run cargo clippy and cargo +nightly fmt before finalizing.

As per coding guidelines: **/*.rs: Always run cargo clippy and cargo +nightly fmt when finalizing work.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/app.rs` around lines 190 - 229, Run the required linter and formatter on
this change: execute `cargo clippy` and then `cargo +nightly fmt` before
finalizing; ensure the file changes around AppContext::new usages
(mainnet_app_context, testnet_app_context, devnet_app_context,
local_app_context) are compliant with clippy suggestions and formatted by
rustfmt, addressing any warnings or style issues reported for those identifiers
and their surrounding code.

src/ui/tokens/mint_tokens_screen.rs (2)

366-372: display_message ignores non-Error types — verify intentional.

Only MessageType::Error triggers the status change. If the global system ever sends Warning or Info to this screen, the status won't update. This is probably fine since the global banner handles display, but it's worth confirming no caller relies on display_message for non-error side-effects on this screen.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/tokens/mint_tokens_screen.rs` around lines 366 - 372, display_message
on MintTokensScreen only updates status for MessageType::Error, so non-Error
messages (Warning/Info) are ignored; either make this explicit or implement
handling: open the impl of ScreenLike for MintTokensScreen and update
display_message (or add a clarifying comment) to handle
MessageType::Warning/Info by setting self.status to appropriate MintTokensStatus
variants (or leave as-is but document intent), referencing the display_message
method, the MessageType enum, the MintTokensStatus enum, and the
MintTokensScreen::status field so callers/readers understand expected
side-effects.

---

499-514: MessageBanner::set_global called on every frame while wallet is locked.

try_open_wallet_no_password is invoked each frame. When it fails, line 501 calls set_global with the error string. Deduplication prevents duplicate banners, but this re-reads and re-writes the global banner vec every frame for a no-op. Not a functional bug, but mildly wasteful on a hot path. Consider guarding with a status flag or moving the check to a one-shot path.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/tokens/mint_tokens_screen.rs` around lines 499 - 514, The code calls
try_open_wallet_no_password every frame and repeatedly invokes
MessageBanner::set_global when it fails; add a one-shot guard field on the
screen state (e.g., wallet_open_error_shown or wallet_try_opened_once) to ensure
MessageBanner::set_global is only called once per failing wallet attempt: call
try_open_wallet_no_password and set the guard and banner only when the guard is
false and the call returns Err, set the guard true after setting the banner, and
clear/reset that guard when selected_wallet changes or when
wallet_needs_unlock/wallet_unlock_popup.open triggers a state change (or when
unlock succeeds) so future attempts can re-run; reference selected_wallet,
try_open_wallet_no_password, MessageBanner::set_global, wallet_needs_unlock, and
wallet_unlock_popup.open.

tests/kittest/message_banner.rs (1)

488-493: Potential flakiness on slow CI.

Line 493 asserts elapsed < Duration::from_secs(1). On an overloaded CI machine, the time between replace_global (line 487) and elapsed() (line 493) could exceed 1 second. Consider using a more generous threshold (e.g., 5 seconds) for robustness.

🛡️ Optional: widen threshold

-    assert!(replaced.elapsed().unwrap() < Duration::from_secs(1));
+    assert!(replaced.elapsed().unwrap() < Duration::from_secs(5));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/kittest/message_banner.rs` around lines 488 - 493, The assertion in the
test using replaced.elapsed().unwrap() currently expects <
Duration::from_secs(1), which can be flaky on slow CI; update the threshold to a
more generous value (e.g., Duration::from_secs(5)) in the assertion that follows
the call to replace_global so the check becomes
assert!(replaced.elapsed().unwrap() < Duration::from_secs(5)); keep the same
replaced and replace_global usage and only change the Duration literal to avoid
timing-related flakes.

src/ui/theme.rs (1)

346-385: Consider deriving background base colors from message_color() to reduce duplication.

The RGB tuples in message_background_color are manually specified per variant and mostly mirror the foreground colors from message_color() — except Success dark mode uses (80, 200, 120) here vs (80, 160, 80) in success_color(). If this divergence is intentional (e.g., brighter tint for better background visibility at low alpha), a brief comment explaining the choice would help future maintainers. If not, you could derive the base from message_color() to keep them in sync:

♻️ Optional: derive from foreground color

     pub fn message_background_color(
         message_type: crate::ui::MessageType,
         dark_mode: bool,
     ) -> Color32 {
         let alpha = if dark_mode { 30 } else { 20 };
-        match message_type {
-            crate::ui::MessageType::Error => {
-                let c = if dark_mode { (255, 100, 100) } else { (235, 87, 87) };
-                Color32::from_rgba_unmultiplied(c.0, c.1, c.2, alpha)
-            }
-            // ... repeated for each variant
-        }
+        let fg = Self::message_color(message_type, dark_mode);
+        Color32::from_rgba_unmultiplied(fg.r(), fg.g(), fg.b(), alpha)
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/theme.rs` around lines 346 - 385, The RGB tuples in
message_background_color are duplicated from the foreground colors; change
message_background_color to call the existing message_color (or the specific
color helpers like success_color) to obtain the base RGB and then apply the
lower alpha for the background so colors stay in sync with message_color() and
avoid duplication; also address the Success dark-mode mismatch by either
intentionally using the brighter (80,200,120) with a short comment explaining
why, or align it to the foreground value (80,160,80) so both functions match.

src/ui/identities/identities_screen.rs (2)

1186-1198: AppAction::BackendTasks(_, _) arm is overly broad — currently safe but fragile.

This wildcard matches any BackendTasks action, not just identity refreshes. Today that's fine because the only BackendTasks generated in this screen are refresh tasks (lines 1135–1148), but if future work adds different bulk tasks to this screen the banner will incorrectly fire. A narrower guard or a comment would make the intent explicit.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/identities/identities_screen.rs` around lines 1186 - 1198, The arm
matching AppAction::BackendTasks(_, _) is too broad and should only trigger for
identity-refresh tasks; change the pattern to explicitly detect identity
refreshes (for example, match AppAction::BackendTasks(meta, tasks) and check
tasks contains BackendTask::IdentityTask(IdentityTask::RefreshIdentity(_)) or
add a guard like if matches!(...)) so the banner is only set when an identity
refresh occurs, then keep the existing logic using self.refresh_banner and
MessageBanner::set_global/with_elapsed; alternatively add a clear explanatory
comment if you intentionally want to keep the broad match.

---

1097-1108: Banner clears prematurely when multiple identities refresh concurrently.

Lines 1144–1146 dispatch one RefreshIdentity task per identity with BackendTasksExecutionMode::Concurrent. Since display_task_result() is called once per completed task, the refresh banner created at line 1193 gets cleared (via .take() at line 1100) on the first successful result, even though other refresh tasks remain in-flight. Users will see "Refreshing identities..." disappear while some identities are still being refreshed, creating misleading feedback.

Consider tracking the number of pending refreshes to clear the banner only after all tasks complete, or switch to per-identity progress indicators.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/identities/identities_screen.rs` around lines 1097 - 1108, The refresh
banner is cleared on the first successful RefreshIdentity task because
display_task_result() calls self.refresh_banner.take() (clearing the global
banner) for each BackendTaskSuccessResult::RefreshedIdentity; change the logic
so the banner is only removed when all concurrent refresh tasks complete — e.g.,
add a pending_refresh_count field on the identities screen, increment it when
dispatching RefreshIdentity tasks (BackendTasksExecutionMode::Concurrent) and
decrement it in display_task_result() for each complete result, and only call
self.refresh_banner.clear()/MessageBanner removal (currently via
self.refresh_banner.take() and MessageBanner::set_global) when the count reaches
zero; alternatively implement per-identity progress indicators instead of a
single global banner.

src/ui/components/message_banner.rs (2)

317-342: Component::show() allocates a zero-size hover response after rendering — works but unconventional.

process_banner (via render_banner) renders the actual UI through Frame::show, but Component::show() returns InnerResponse wrapping a dummy allocate_response(Vec2::ZERO, Sense::hover()). The banner's real interaction (dismiss button) is handled inside render_banner. This means the returned egui::Response doesn't reflect the banner's actual rect or interactions — callers relying on the outer Response for layout/hit-testing would get incorrect info. If this is only used for MessageBannerResponse, it's fine.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/components/message_banner.rs` around lines 317 - 342, The show()
method currently wraps a dummy zero-size hover response
(allocate_response(Vec2::ZERO, Sense::hover())), which misrepresents the
banner's real rect and interactions; change the rendering pipeline so
process_banner (and/or render_banner/Frame::show) returns the actual
egui::Response alongside BannerStatus, then in MessageBanner::show() use that
real Response when constructing InnerResponse::new instead of the dummy
allocate_response; update signatures/returns for process_banner/render_banner to
return (BannerStatus, egui::Response) (or a struct) and propagate that Response
into InnerResponse, keeping MessageBannerResponse creation and current_value()
behavior unchanged.

---

203-231: set_global dedup ignores message_type mismatch.

When an existing banner with the same text is found (line 205), the handle to that banner is returned without updating its message_type. If a caller sets "Operation failed" as Error and later calls set_global("Operation failed", Warning), the banner stays Error-colored. This may be the desired behavior (text identity = same banner), but it's worth documenting explicitly since replace_global does update the type.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/components/message_banner.rs` around lines 203 - 231, The current
set_global in function set_global returns an existing BannerHandle when a banner
with matching text is found but does not update its message_type; modify
set_global so that when get_banners() finds an existing BannerState with the
same text and existing.message_type != message_type, you update that
BannerState's message_type (and update related fields like auto_dismiss_after =
default_auto_dismiss(message_type) and optionally created_at or show_elapsed if
desired), call set_banners(ctx, banners) to persist the change, and then return
the BannerHandle; reference the BannerState struct fields, the
get_banners/set_banners helpers, and BannerHandle/replace_global for locating
related logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/ai-design/2026-02-17-unified-messages/architecture.md`:
- Around line 127-146: Add explicit language specifiers to the three fenced code
blocks so markdownlint MD040 is fixed: update the block containing the function
signature for render_banner(ui, text, message_type, annotation: Option<&str>) to
start with ```text, change the banner ASCII-art block beginning with
+-----------------------------------------------------------------------+ to
```text, and change the line showing TaskResult::Error(message) to ```text; this
preserves rendering and lint compliance while keeping the exact content shown by
render_banner, process_banner, and TaskResult::Error.

In `@docs/ai-design/2026-02-17-unified-messages/message-banner-ux-spec.md`:
- Around line 31-35: The fenced code blocks in the message-banner-ux-spec
markdown use plain ``` fences and trigger markdownlint MD040; update each
triple-backtick fence that encloses the ASCII message banner (e.g., the block
starting with
"+-----------------------------------------------------------------------+" and
similar block at lines ~81-91) to include a language identifier such as "text"
(change ``` to ```text at the opening fence and ensure the closing fence
matches), so the code blocks read ```text ... ```text.

In `@src/app.rs`:
- Around line 861-864: The fallback arm for BackendTaskSuccessResult currently
calls MessageBanner::set_global("Success", MessageType::Success) which creates
noisy, generic global banners; remove that call and leave the fallback arm to
only forward the unboxed_message to
self.visible_screen_mut().display_task_result(unboxed_message) so individual
screens can decide whether to show a banner via their
display_task_result()/display_message() logic; if you want to keep a trace,
replace the global banner call with a debug log instead, but do not create a
global MessageBanner in the default branch.

---

Outside diff comments:
In `@src/ui/identities/top_up_identity_screen/by_wallet_qr_code.rs`:
- Around line 126-132: The QR error banner is never cleared because the
BannerHandle returned by MessageBanner::set_global(...) is discarded; update
TopUpIdentityScreen to hold an Option<BannerHandle> field (e.g.,
qr_error_banner) and when calling MessageBanner::set_global in the render path
(inside render_qr_code error handling) store the returned handle in
self.qr_error_banner; when render_qr_code succeeds or when parsing
self.funding_amount yields Ok(amount_dash) && amount_dash > 0.0, check
self.qr_error_banner and call .clear() on the handle and set
self.qr_error_banner = None so the global banner is removed when the input or
rendering becomes valid.

---

Nitpick comments:
In `@src/app.rs`:
- Around line 190-229: Run the required linter and formatter on this change:
execute `cargo clippy` and then `cargo +nightly fmt` before finalizing; ensure
the file changes around AppContext::new usages (mainnet_app_context,
testnet_app_context, devnet_app_context, local_app_context) are compliant with
clippy suggestions and formatted by rustfmt, addressing any warnings or style
issues reported for those identifiers and their surrounding code.

In `@src/ui/components/message_banner.rs`:
- Around line 317-342: The show() method currently wraps a dummy zero-size hover
response (allocate_response(Vec2::ZERO, Sense::hover())), which misrepresents
the banner's real rect and interactions; change the rendering pipeline so
process_banner (and/or render_banner/Frame::show) returns the actual
egui::Response alongside BannerStatus, then in MessageBanner::show() use that
real Response when constructing InnerResponse::new instead of the dummy
allocate_response; update signatures/returns for process_banner/render_banner to
return (BannerStatus, egui::Response) (or a struct) and propagate that Response
into InnerResponse, keeping MessageBannerResponse creation and current_value()
behavior unchanged.
- Around line 203-231: The current set_global in function set_global returns an
existing BannerHandle when a banner with matching text is found but does not
update its message_type; modify set_global so that when get_banners() finds an
existing BannerState with the same text and existing.message_type !=
message_type, you update that BannerState's message_type (and update related
fields like auto_dismiss_after = default_auto_dismiss(message_type) and
optionally created_at or show_elapsed if desired), call set_banners(ctx,
banners) to persist the change, and then return the BannerHandle; reference the
BannerState struct fields, the get_banners/set_banners helpers, and
BannerHandle/replace_global for locating related logic.

In `@src/ui/dashpay/add_contact_screen.rs`:
- Around line 242-246: The match arm for MessageType::Warning currently uses the
constant DashColors::WARNING which bypasses dark-mode theming; update the match
in the message rendering (the match on message_type / MessageType::Warning) to
call the dark‑mode aware helper (e.g., DashColors::warning_color() or the
project’s designated warning_color helper) instead of the raw constant so
warning text follows theme-aware colors.

In `@src/ui/identities/identities_screen.rs`:
- Around line 1186-1198: The arm matching AppAction::BackendTasks(_, _) is too
broad and should only trigger for identity-refresh tasks; change the pattern to
explicitly detect identity refreshes (for example, match
AppAction::BackendTasks(meta, tasks) and check tasks contains
BackendTask::IdentityTask(IdentityTask::RefreshIdentity(_)) or add a guard like
if matches!(...)) so the banner is only set when an identity refresh occurs,
then keep the existing logic using self.refresh_banner and
MessageBanner::set_global/with_elapsed; alternatively add a clear explanatory
comment if you intentionally want to keep the broad match.
- Around line 1097-1108: The refresh banner is cleared on the first successful
RefreshIdentity task because display_task_result() calls
self.refresh_banner.take() (clearing the global banner) for each
BackendTaskSuccessResult::RefreshedIdentity; change the logic so the banner is
only removed when all concurrent refresh tasks complete — e.g., add a
pending_refresh_count field on the identities screen, increment it when
dispatching RefreshIdentity tasks (BackendTasksExecutionMode::Concurrent) and
decrement it in display_task_result() for each complete result, and only call
self.refresh_banner.clear()/MessageBanner removal (currently via
self.refresh_banner.take() and MessageBanner::set_global) when the count reaches
zero; alternatively implement per-identity progress indicators instead of a
single global banner.

In `@src/ui/mod.rs`:
- Around line 829-834: You added a new variant Warning to the MessageType enum
(MessageType::Warning) but forgot to run the project linters/formatters; run
cargo clippy and cargo +nightly fmt across the repo to ensure code style and
clippy lints are clean before finalizing, then commit the resulting formatting
and any small clippy-driven fixes (e.g., matching arms, unused imports) that may
be required in modules that pattern-match on MessageType.

In `@src/ui/theme.rs`:
- Around line 346-385: The RGB tuples in message_background_color are duplicated
from the foreground colors; change message_background_color to call the existing
message_color (or the specific color helpers like success_color) to obtain the
base RGB and then apply the lower alpha for the background so colors stay in
sync with message_color() and avoid duplication; also address the Success
dark-mode mismatch by either intentionally using the brighter (80,200,120) with
a short comment explaining why, or align it to the foreground value (80,160,80)
so both functions match.

In `@src/ui/tokens/mint_tokens_screen.rs`:
- Around line 366-372: display_message on MintTokensScreen only updates status
for MessageType::Error, so non-Error messages (Warning/Info) are ignored; either
make this explicit or implement handling: open the impl of ScreenLike for
MintTokensScreen and update display_message (or add a clarifying comment) to
handle MessageType::Warning/Info by setting self.status to appropriate
MintTokensStatus variants (or leave as-is but document intent), referencing the
display_message method, the MessageType enum, the MintTokensStatus enum, and the
MintTokensScreen::status field so callers/readers understand expected
side-effects.
- Around line 499-514: The code calls try_open_wallet_no_password every frame
and repeatedly invokes MessageBanner::set_global when it fails; add a one-shot
guard field on the screen state (e.g., wallet_open_error_shown or
wallet_try_opened_once) to ensure MessageBanner::set_global is only called once
per failing wallet attempt: call try_open_wallet_no_password and set the guard
and banner only when the guard is false and the call returns Err, set the guard
true after setting the banner, and clear/reset that guard when selected_wallet
changes or when wallet_needs_unlock/wallet_unlock_popup.open triggers a state
change (or when unlock succeeds) so future attempts can re-run; reference
selected_wallet, try_open_wallet_no_password, MessageBanner::set_global,
wallet_needs_unlock, and wallet_unlock_popup.open.

In `@src/ui/wallets/asset_lock_detail_screen.rs`:
- Around line 365-370: This change needs formatting and lint fixes before merge:
run cargo clippy and cargo +nightly fmt, address any clippy warnings/errors and
formatting changes (focus on the code around message_color, the MessageType
match arm, and uses of DashColors::warning_color/text_primary) so the match
expression and imports conform to project lint/style rules, then re-run the
checks to ensure the file compiles cleanly.

In `@tests/kittest/message_banner.rs`:
- Around line 488-493: The assertion in the test using
replaced.elapsed().unwrap() currently expects < Duration::from_secs(1), which
can be flaky on slow CI; update the threshold to a more generous value (e.g.,
Duration::from_secs(5)) in the assertion that follows the call to replace_global
so the check becomes assert!(replaced.elapsed().unwrap() <
Duration::from_secs(5)); keep the same replaced and replace_global usage and
only change the Duration literal to avoid timing-related flakes.

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/ui/identities/identities_screen.rs (1)

55-89: ⚠️ Potential issue | 🟡 Minor

Refresh banner clears after the first identity result while others may still be running.

When a refresh action spawns multiple RefreshIdentity tasks, the banner is cleared on the first RefreshedIdentity, which can prematurely end the elapsed indicator and show success while other refreshes are still in flight. Consider tracking a pending count and only clearing when the last refresh completes.

🔧 One way to track pending refreshes

 pub struct IdentitiesScreen {
     pub identities: Arc<Mutex<IndexMap<Identifier, QualifiedIdentity>>>,
     pub app_context: Arc<AppContext>,
@@
     sort_column: IdentitiesSortColumn,
     sort_order: IdentitiesSortOrder,
     use_custom_order: bool,
     refresh_banner: Option<BannerHandle>,
+    refresh_in_flight: usize,
     // Alias editing state
     editing_alias_identity: Option<Identifier>,
     editing_alias_value: String,
 }
@@
             use_custom_order: true,
             refresh_banner: None,
+            refresh_in_flight: 0,
             editing_alias_identity: None,
             editing_alias_value: String::new(),
         };
@@
     fn display_message(&mut self, _message: &str, message_type: MessageType) {
         // Banner display is handled globally by AppState; this is only for side-effects.
-        if matches!(message_type, MessageType::Error | MessageType::Warning)
-            && let Some(handle) = self.refresh_banner.take()
-        {
-            handle.clear();
-        }
+        if matches!(message_type, MessageType::Error | MessageType::Warning) {
+            if self.refresh_in_flight > 0 {
+                self.refresh_in_flight = self.refresh_in_flight.saturating_sub(1);
+            }
+            if self.refresh_in_flight == 0 {
+                if let Some(handle) = self.refresh_banner.take() {
+                    handle.clear();
+                }
+            }
+        }
     }
@@
         if let crate::ui::BackendTaskSuccessResult::RefreshedIdentity(_) =
             backend_task_success_result
         {
-            if let Some(handle) = self.refresh_banner.take() {
-                handle.clear();
-            }
+            if self.refresh_in_flight > 0 {
+                self.refresh_in_flight = self.refresh_in_flight.saturating_sub(1);
+            }
+            if self.refresh_in_flight == 0 {
+                if let Some(handle) = self.refresh_banner.take() {
+                    handle.clear();
+                }
+            }
             MessageBanner::set_global(
                 self.app_context.egui_ctx(),
                 "Successfully refreshed identity",
                 MessageType::Success,
             );
         }
@@
-        match action {
-            AppAction::BackendTask(BackendTask::IdentityTask(IdentityTask::RefreshIdentity(_)))
-            | AppAction::BackendTasks(_, _) => {
+        match action {
+            AppAction::BackendTask(BackendTask::IdentityTask(IdentityTask::RefreshIdentity(_))) => {
+                self.refresh_in_flight = 1;
                 if let Some(handle) = self.refresh_banner.take() {
                     handle.clear();
                 }
                 let handle =
                     MessageBanner::set_global(ctx, "Refreshing identities...", MessageType::Info);
                 handle.with_elapsed();
                 self.refresh_banner = Some(handle);
             }
+            AppAction::BackendTasks(ref tasks, _) => {
+                self.refresh_in_flight = tasks.len();
+                if let Some(handle) = self.refresh_banner.take() {
+                    handle.clear();
+                }
+                let handle =
+                    MessageBanner::set_global(ctx, "Refreshing identities...", MessageType::Info);
+                handle.with_elapsed();
+                self.refresh_banner = Some(handle);
+            }
             _ => {}
         }

Also applies to: 1084-1107, 1186-1196

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/identities/identities_screen.rs` around lines 55 - 89, The refresh
banner is cleared as soon as the first RefreshedIdentity arrives; add a pending
refresh counter (e.g., pending_refresh_count: usize or AtomicUsize) to
IdentitiesScreen, increment it whenever you spawn RefreshIdentity tasks, and
decrement it when handling RefreshedIdentity; only clear refresh_banner when the
counter reaches zero. Update the spawn logic that emits RefreshIdentity to bump
the counter, and update the message-handling code that processes
RefreshedIdentity to decrement the counter and conditionally clear
refresh_banner (and reset any elapsed indicator) only when pending_refresh_count
== 0. Use the existing refresh_banner field and the
RefreshIdentity/RefreshedIdentity message handlers to locate the changes.

🧹 Nitpick comments (3)

src/ui/components/message_banner.rs (2)

451-463: Route banner colors through ComponentStyles for consistency.

render_banner pulls colors directly from DashColors. If ComponentStyles is the standard theming hook for components, consider routing banner styles through it to align with the component conventions.
As per coding guidelines: “src/ui/components/**/*.rs: UI components must follow the lazy initialization pattern … and support for both light and dark mode via ComponentStyles.”

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/components/message_banner.rs` around lines 451 - 463, render_banner
currently fetches colors directly from DashColors (fg_color, bg_color,
secondary_color); switch it to use the centralized ComponentStyles to ensure
lazy initialization and proper light/dark theming. Update render_banner to
accept or obtain a &ComponentStyles (or call ComponentStyles::get for lazy
init), then replace DashColors::message_color, ::message_background_color and
::text_secondary calls with the corresponding ComponentStyles API (e.g.,
message_text_color, message_bg_color, text_secondary) so banner colors follow
the component theming conventions while preserving message_type/dark_mode
behavior.

---

280-326: Avoid carrying stale details/suggestions on replace.

replace_global updates text/type but leaves details/suggestion intact, which can leak old context into the new message. Consider clearing those fields when replacing.

♻️ Suggested tweak

         if let Some(b) = banners.iter_mut().find(|b| b.text == old_text) {
             key = b.key;
             b.text = new_text.to_string();
             b.message_type = message_type;
             b.created_at = Instant::now();
             b.auto_dismiss_after = default_auto_dismiss(message_type);
             b.show_elapsed = false;
+            b.details = None;
+            b.suggestion = None;
+            b.details_expanded = false;
         } else if let Some(existing) = banners.iter().find(|b| b.text == new_text) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/components/message_banner.rs` around lines 280 - 326, In
replace_global, when you find and mutate an existing banner (the let Some(b) =
banners.iter_mut().find(|b| b.text == old_text) branch), clear any stale
contextual fields by setting b.details = None, b.suggestion = None, and
b.details_expanded = false (and keep the other updates you already do); also
ensure any code path that reuses an existing banner (the else if let
Some(existing) = banners.iter().find(|b| b.text == new_text) branch) either
clears those fields or returns a fresh BannerState so old details/suggestion
aren’t leaked into the new message.

src/app.rs (1)

1-45: Run clippy and fmt before finalizing.

Quick reminder to run cargo clippy and cargo +nightly fmt for these Rust changes.
As per coding guidelines: “**/*.rs: Always run cargo clippy and cargo +nightly fmt when finalizing work”.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/app.rs` around lines 1 - 45, Run cargo clippy and cargo +nightly fmt and
address any lints/formatting issues they report; focus on the symbols
introduced/used in this diff (e.g. App/egui usage, AppContext, TaskManager,
CoreZMQListener, ContestedResourceTask, BackendTask, and the various Screen
types) to resolve warnings (unused imports, style, or complexity) and reformat
the file to match project style before finalizing.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/ai-design/2026-02-17-unified-messages/architecture.md`:
- Around line 22-31: The BannerState struct docs are missing the new internal
fields; update the BannerState declaration and prose to include the fields
details: Option<String>, suggestion: Option<String>, and details_expanded: bool
so the documentation matches the runtime state; reference the BannerState struct
and ensure each new field is described (purpose and type) alongside existing
fields like key, text, message_type, created_at, auto_dismiss_after, and
show_elapsed.
- Around line 154-164: Update the AppState::update() documentation to remove the
now-removed generic "Success" global banner: replace the description that shows
TaskResult::Success calling MessageBanner::set_global with a note that the
default success path delegates to the screen
(screen.display_task_result(result)) and does not emit a global
MessageBanner::set_global(ctx, "Success", MessageType::Success); keep the error
path example (TaskResult::Error → MessageBanner::set_global(ctx, &message,
MessageType::Error) → screen.display_message(&message, MessageType::Error))
intact.
- Around line 127-129: The documented signature for render_banner is out of
date: the implementation now accepts additional parameters suggestion, details,
and details_expanded besides annotation. Update the docs to list the full,
current signature (render_banner(ui, text, message_type, annotation:
Option<&str>, suggestion: Option<&str>, details: Option<&str>, details_expanded:
bool) -> bool or equivalent Rust-style types) and briefly describe what each new
parameter does (suggestion, details, details_expanded) so readers can match the
doc to the actual render_banner function.
- Around line 238-245: Update the compound "Pre-Migration" to the hyphenated
form "Pre-Migration" (or "pre-migration" where used inline) in the document
header and any inline occurrences; specifically change the section heading "##
10. Pre-Migration Analysis" to use the hyphenated compound and scan the
surrounding text for other instances of pre-migration to apply the same
hyphenation for consistency.

In `@src/ui/components/message_banner.rs`:
- Around line 239-274: The dedup branch in set_global currently returns a
BannerHandle without refreshing the existing BannerState; modify the branch
(where banners.iter().find(|b| b.text == text) is handled) to locate the mutable
banner in the banners vector, update its message_type, created_at =
Instant::now(), auto_dismiss_after = default_auto_dismiss(message_type) (and
reset show_elapsed if desired), then call set_banners(ctx, banners) before
returning the BannerHandle (use next_banner_key only for new banners). This
ensures styling and timers on the existing BannerState are refreshed when
reusing by text.

---

Outside diff comments:
In `@src/ui/identities/identities_screen.rs`:
- Around line 55-89: The refresh banner is cleared as soon as the first
RefreshedIdentity arrives; add a pending refresh counter (e.g.,
pending_refresh_count: usize or AtomicUsize) to IdentitiesScreen, increment it
whenever you spawn RefreshIdentity tasks, and decrement it when handling
RefreshedIdentity; only clear refresh_banner when the counter reaches zero.
Update the spawn logic that emits RefreshIdentity to bump the counter, and
update the message-handling code that processes RefreshedIdentity to decrement
the counter and conditionally clear refresh_banner (and reset any elapsed
indicator) only when pending_refresh_count == 0. Use the existing refresh_banner
field and the RefreshIdentity/RefreshedIdentity message handlers to locate the
changes.

---

Nitpick comments:
In `@src/app.rs`:
- Around line 1-45: Run cargo clippy and cargo +nightly fmt and address any
lints/formatting issues they report; focus on the symbols introduced/used in
this diff (e.g. App/egui usage, AppContext, TaskManager, CoreZMQListener,
ContestedResourceTask, BackendTask, and the various Screen types) to resolve
warnings (unused imports, style, or complexity) and reformat the file to match
project style before finalizing.

In `@src/ui/components/message_banner.rs`:
- Around line 451-463: render_banner currently fetches colors directly from
DashColors (fg_color, bg_color, secondary_color); switch it to use the
centralized ComponentStyles to ensure lazy initialization and proper light/dark
theming. Update render_banner to accept or obtain a &ComponentStyles (or call
ComponentStyles::get for lazy init), then replace DashColors::message_color,
::message_background_color and ::text_secondary calls with the corresponding
ComponentStyles API (e.g., message_text_color, message_bg_color, text_secondary)
so banner colors follow the component theming conventions while preserving
message_type/dark_mode behavior.
- Around line 280-326: In replace_global, when you find and mutate an existing
banner (the let Some(b) = banners.iter_mut().find(|b| b.text == old_text)
branch), clear any stale contextual fields by setting b.details = None,
b.suggestion = None, and b.details_expanded = false (and keep the other updates
you already do); also ensure any code path that reuses an existing banner (the
else if let Some(existing) = banners.iter().find(|b| b.text == new_text) branch)
either clears those fields or returns a fresh BannerState so old
details/suggestion aren’t leaked into the new message.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/ui/components/message_banner.rs (1)

239-276: set_global with empty text still consumes a key from the counter.

When text is empty, next_banner_key() on line 254 increments the atomic counter but the key is never stored. The returned BannerHandle is a dead handle. This is harmless given u64 range, but calling set_global(ctx, "", ...) could be guarded earlier to avoid the wasted allocation and unnecessary get_banners clone.

♻️ Optional early return for empty text

     pub fn set_global(ctx: &egui::Context, text: &str, message_type: MessageType) -> BannerHandle {
+        if text.is_empty() {
+            return BannerHandle {
+                ctx: ctx.clone(),
+                key: next_banner_key(),
+            };
+        }
         let mut banners = get_banners(ctx);
         if let Some(existing) = banners.iter_mut().find(|b| b.text == text) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/components/message_banner.rs` around lines 239 - 276, set_global
currently consumes a banner key even when text is empty; move the empty-text
guard to the top of set_global (before calling next_banner_key and ideally
before get_banners) to avoid incrementing the atomic and cloning banners
unnecessarily. If text.is_empty() return early with a no-op BannerHandle (e.g.,
BannerHandle { ctx: ctx.clone(), key: 0 }) and otherwise proceed with the
existing logic that calls get_banners, checks for duplicates, calls
next_banner_key, and uses set_banners.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/ui/components/message_banner.rs`:
- Around line 297-325: The dedup branch in replace_global currently uses
banners.iter().find and returns the existing banner without updating its state,
so requested message_type is ignored; change the else-if to use
banners.iter_mut().find on new_text and update the found BannerState's
message_type (and mirror set_global's behavior—e.g., update created_at =
Instant::now(), auto_dismiss_after = default_auto_dismiss(message_type),
show_elapsed = false, details = None, suggestion = None, details_expanded =
false) before grabbing its key so replace_global matches set_global's dedup
semantics.

---

Duplicate comments:
In `@docs/ai-design/2026-02-17-unified-messages/architecture.md`:
- Around line 165-177: The docs claim AppState::update in src/app.rs sets a
generic "Success" global banner via MessageBanner::set_global(ctx, "Success",
MessageType::Success) for TaskResult::Success, but the implementation no longer
emits that banner; update the text in
docs/ai-design/2026-02-17-unified-messages/architecture.md (around the
TaskResult::Success section) to reflect the current behavior: state that the
success path delegates to screens (e.g., screen.display_task_result) and does
not call MessageBanner::set_global, or, if the code actually still sets the
banner, adjust src/app.rs to either call MessageBanner::set_global or restore
that behavior in AppState::update so the doc matches the implementation—verify
by checking AppState::update, TaskResult::Success handling, and
MessageBanner::set_global usages in src/app.rs before editing the doc.

---

Nitpick comments:
In `@src/ui/components/message_banner.rs`:
- Around line 239-276: set_global currently consumes a banner key even when text
is empty; move the empty-text guard to the top of set_global (before calling
next_banner_key and ideally before get_banners) to avoid incrementing the atomic
and cloning banners unnecessarily. If text.is_empty() return early with a no-op
BannerHandle (e.g., BannerHandle { ctx: ctx.clone(), key: 0 }) and otherwise
proceed with the existing logic that calls get_banners, checks for duplicates,
calls next_banner_key, and uses set_banners.

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/ui/tokens/update_token_config.rs (1)

1104-1121: ⚠️ Potential issue | 🟡 Minor

MessageType::Warning rendering arm is dead code — display_message never stores warnings

display_message (lines 920–931) only stores MessageType::Error and MessageType::Info into backend_message; MessageType::Warning falls through to _ => {} and is silently dropped. Because backend_message can therefore never carry a Warning type at runtime, the MessageType::Warning branch added here at line 1104 is permanently unreachable.

To make this change functional, display_message also needs to handle Warning:

 fn display_message(&mut self, message: &str, message_type: MessageType) {
     match message_type {
         MessageType::Error => {
             self.backend_message = Some((message.to_string(), MessageType::Error, Utc::now()));
             self.update_status = UpdateTokenConfigStatus::NotUpdating;
         }
+        MessageType::Warning => {
+            self.backend_message = Some((message.to_string(), MessageType::Warning, Utc::now()));
+        }
         MessageType::Info => {
             self.backend_message = Some((message.to_string(), MessageType::Info, Utc::now()));
         }
         _ => {}
     }
 }

Additionally, if/when the Warning path is reachable, line 1114 will display "Error: <warning text>" because the prefix is hardcoded — it should be conditional on the message type:

-ui.label(RichText::new(format!("Error: {}", msg)).color(error_color));
+let prefix = if matches!(msg_type, MessageType::Warning) { "Warning" } else { "Error" };
+ui.label(RichText::new(format!("{}: {}", prefix, msg)).color(error_color));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/tokens/update_token_config.rs` around lines 1104 - 1121,
display_message currently only stores MessageType::Error and MessageType::Info
into backend_message, so the MessageType::Warning branch in the rendering code
(the match arm that builds the Frame and labels "Error: <msg>") is dead; update
display_message to also store MessageType::Warning into self.backend_message so
warnings are propagated, and change the rendering code that builds the label
(inside the MessageType::Error | MessageType::Warning arm) to use a dynamic
prefix based on the actual MessageType (e.g., "Error: " vs "Warning: " or
similar) rather than the hardcoded "Error: " string so warnings display with the
correct prefix; refer to the display_message function, the backend_message
field, and the MessageType::Warning match arm to locate the changes.

🧹 Nitpick comments (2)

src/ui/wallets/send_screen.rs (1)

2673-2685: Warnings render with full error styling — consider a SendStatus::Warning variant for visual distinction

MessageType::Warning now routes to SendStatus::Error, so warning messages are rendered with the same red DashColors::ERROR frame, border, and text color as hard errors (render_send_status lines 1142–1165). While intentional under the PR's unification strategy, a Warning variant in SendStatus would let the screen use DashColors::WARNING (amber) for warnings, preserving the semantic distinction that MessageType::Warning was designed to express.

🎨 Optional: add a Warning variant to SendStatus

 pub enum SendStatus {
     NotStarted,
     WaitingForResult(u64),
     Complete(String),
     Error(String),
+    Warning(String),
 }

-    MessageType::Error | MessageType::Warning => {
-        self.send_status = SendStatus::Error(message.to_string());
-    }
+    MessageType::Error => {
+        self.send_status = SendStatus::Error(message.to_string());
+    }
+    MessageType::Warning => {
+        self.send_status = SendStatus::Warning(message.to_string());
+    }

Then add a SendStatus::Warning arm in render_send_status using DashColors::WARNING styling.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/wallets/send_screen.rs` around lines 2673 - 2685, display_message
currently maps MessageType::Warning to SendStatus::Error so warnings render with
error styling; add a SendStatus::Warning variant to the SendStatus enum and
change display_message to set SendStatus::Warning for MessageType::Warning. Then
update render_send_status to handle SendStatus::Warning and use
DashColors::WARNING (amber) for frame, border, and text styling (parallel to the
existing Error and Complete arms). Ensure pattern matches and any downstream
matches/exhaustiveness are adjusted accordingly.

src/context/mod.rs (1)

405-408: Defer signature refactoring to follow-up PR—this is a move-only refactor.

While it's true that repaint_animation() could drop the redundant ctx: &Context parameter (since self.egui_ctx is now available), this change extends beyond the scope of PR #543, which is a move-only refactor splitting context.rs into submodules. The refactoring would require updating call sites in src/ui/components/top_panel.rs and src/ui/components/styled.rs. Please address this in a follow-up PR.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/context/mod.rs` around lines 405 - 408, The current change exposes
egui_ctx via pub fn egui_ctx(&self) -> &egui::Context but you must NOT remove
the now-redundant ctx parameter from repaint_animation() as that refactor is out
of scope; leave repaint_animation(ctx: &Context, ...) signature and all call
sites in src/ui/components/top_panel.rs and src/ui/components/styled.rs
untouched, keep egui_ctx() as-is for now, and defer any signature cleanup to a
follow-up PR that updates repaint_animation and its callers.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/ui/identities/identities_screen.rs`:
- Around line 1118-1129: display_task_result currently clears
self.refresh_banner on the first per-identity success and posts a singular
"Successfully refreshed identity" even while other refresh tasks are still
running; change this by tracking a pending-refresh counter (e.g., add a field
like self.refresh_pending_count set when the refresh banner is created) and in
display_task_result, decrement that counter for each RefreshedIdentity result,
only clear/take self.refresh_banner and remove the "Refreshing identities…"
banner when the counter hits zero, and call MessageBanner::set_global with a
pluralized message (choose "identity" vs "identities" based on the
remaining/initial count) so the success message accurately reflects bulk vs
single refresh; update references to self.refresh_banner,
backend_task_success_result handling, and MessageBanner::set_global accordingly.
- Around line 1208-1217: Remove the unreachable singular arm
AppAction::BackendTask(BackendTask::IdentityTask(IdentityTask::RefreshIdentity(_)))
and instead only handle AppAction::BackendTasks when the payload actually
contains identity tasks; replace the broad AppAction::BackendTasks(_, _) arm
with a more specific pattern or a guard that inspects the tasks payload (e.g.,
match on the tasks list or use tasks.iter().all(|t| matches!(t,
BackendTask::IdentityTask(_)))) and only call
MessageBanner::set_global("Refreshing identities...", ...) and update
self.refresh_banner when all relevant BackendTask entries are IdentityTask
variants; reference ensure_identities_status() and the Refresh button flows to
confirm which dispatches produce BackendTasks so non-identity backend actions do
not overwrite self.refresh_banner.

In `@src/ui/tokens/mint_tokens_screen.rs`:
- Around line 380-385: display_message currently discards message text for
Success/Info causing silent no-ops; update the function (display_message) to
handle all MessageType variants consistently by either mapping Success and Info
to explicit local statuses (e.g., MintTokensStatus::Success /
MintTokensStatus::Info) or logging/asserting when those types are received and
the global banner isn't set, and ensure the message text is forwarded to the
global AppState banner or stored locally so callers get visible feedback;
reference MessageType, MintTokensStatus, and AppState to locate where to set the
banner/state and add a short comment documenting the chosen behavior.

---

Outside diff comments:
In `@src/ui/tokens/update_token_config.rs`:
- Around line 1104-1121: display_message currently only stores
MessageType::Error and MessageType::Info into backend_message, so the
MessageType::Warning branch in the rendering code (the match arm that builds the
Frame and labels "Error: <msg>") is dead; update display_message to also store
MessageType::Warning into self.backend_message so warnings are propagated, and
change the rendering code that builds the label (inside the MessageType::Error |
MessageType::Warning arm) to use a dynamic prefix based on the actual
MessageType (e.g., "Error: " vs "Warning: " or similar) rather than the
hardcoded "Error: " string so warnings display with the correct prefix; refer to
the display_message function, the backend_message field, and the
MessageType::Warning match arm to locate the changes.

---

Duplicate comments:
In `@src/app.rs`:
- Around line 923-956: The explicit success arms
(BackendTaskSuccessResult::UpdatedThemePreference and
BackendTaskSuccessResult::CastScheduledVote) currently call both
MessageBanner::set_global and the screen-level
visible_screen_mut().display_message, producing duplicate text for screens that
still render inline messages (e.g., SendPaymentScreen which uses self.message);
to fix, choose one display path per case—either remove the screen-level
display_message call from those arms so only the global banner is shown, or
remove the MessageBanner::set_global call so only the screen handles the
message—and ensure display_task_result continues to be used for the fallback;
update the branches in the match handling those variants (and any
TaskResult::Error branch mentioned) accordingly so only a single notifier is
invoked.

---

Nitpick comments:
In `@src/context/mod.rs`:
- Around line 405-408: The current change exposes egui_ctx via pub fn
egui_ctx(&self) -> &egui::Context but you must NOT remove the now-redundant ctx
parameter from repaint_animation() as that refactor is out of scope; leave
repaint_animation(ctx: &Context, ...) signature and all call sites in
src/ui/components/top_panel.rs and src/ui/components/styled.rs untouched, keep
egui_ctx() as-is for now, and defer any signature cleanup to a follow-up PR that
updates repaint_animation and its callers.

In `@src/ui/wallets/send_screen.rs`:
- Around line 2673-2685: display_message currently maps MessageType::Warning to
SendStatus::Error so warnings render with error styling; add a
SendStatus::Warning variant to the SendStatus enum and change display_message to
set SendStatus::Warning for MessageType::Warning. Then update render_send_status
to handle SendStatus::Warning and use DashColors::WARNING (amber) for frame,
border, and text styling (parallel to the existing Error and Complete arms).
Ensure pattern matches and any downstream matches/exhaustiveness are adjusted
accordingly.

[coderabbitai]

🧹 Nitpick comments (1)

src/ui/components/message_banner.rs (1)

612-618: Error and Warning share the same icon — only color distinguishes them.

Both Error and Warning emit \u{26A0} (⚠). A distinct icon for Error (e.g. \u{274C} ✖ or \u{1F6AB} 🚫) would let colorblind users differentiate severity levels without relying on hue alone.

♻️ Suggested tweak

 fn icon_for_type(message_type: MessageType) -> &'static str {
     match message_type {
-        MessageType::Error => "\u{26A0}",   // warning sign
+        MessageType::Error => "\u{274C}",   // cross mark
         MessageType::Warning => "\u{26A0}", // warning sign (differentiated by color)
         MessageType::Success => "\u{2713}", // check mark
         MessageType::Info => "\u{2139}",    // info
     }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/components/message_banner.rs` around lines 612 - 618, icon_for_type
currently returns the same glyph for MessageType::Error and
MessageType::Warning, which relies on color to convey severity; update
icon_for_type to return a distinct symbol for Error (for example "\u{274C}" or
"\u{1F6AB}" instead of "\u{26A0}") while keeping Warning as "\u{26A0}", ensuring
the function signature fn icon_for_type(message_type: MessageType) -> &'static
str and match arms for MessageType::Error, MessageType::Warning,
MessageType::Success, and MessageType::Info are updated accordingly so
colorblind users can distinguish severity by shape as well.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/ui/components/message_banner.rs`:
- Around line 328-329: In replace_global, when old_text is None but new_text
matches an existing banner the code uses banners.iter().find(...) which prevents
mutating the found banner and silently drops the caller-supplied message_type;
change this branch to search with a mutable iterator (e.g.,
banners.iter_mut().find(...)) so you can update the existing banner's
message_type to the provided message_type and then use its key (the
existing.key) — ensure you set the banner's severity/styling to the caller's
message_type before assigning key.
- Around line 264-272: The dedup branch in the set_global flow only updates
message_type and leaves timer fields stale; update the matched banner (found via
banners.iter_mut().find(|b| b.text == text)) to also refresh auto_dismiss_after,
created_at, and show_elapsed to match how replace_global refreshes timers, then
call set_banners(ctx, banners) and return the same BannerHandle (preserving
existing.key); ensure the same field names (auto_dismiss_after, created_at,
show_elapsed, message_type) on the existing banner are set to the new values
used when creating a fresh banner.

---

Nitpick comments:
In `@src/ui/components/message_banner.rs`:
- Around line 612-618: icon_for_type currently returns the same glyph for
MessageType::Error and MessageType::Warning, which relies on color to convey
severity; update icon_for_type to return a distinct symbol for Error (for
example "\u{274C}" or "\u{1F6AB}" instead of "\u{26A0}") while keeping Warning
as "\u{26A0}", ensuring the function signature fn icon_for_type(message_type:
MessageType) -> &'static str and match arms for MessageType::Error,
MessageType::Warning, MessageType::Success, and MessageType::Info are updated
accordingly so colorblind users can distinguish severity by shape as well.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

docs/ai-design/2026-02-17-unified-messages/architecture.md (1)

167-168: ⚠️ Potential issue | 🟡 Minor

Add language specifier to fenced code block.

The code block at line 167 lacks a language identifier, which was flagged in a previous review for similar blocks.

📝 Suggested fix

-```
+```text
 TaskResult::Error(message)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/ai-design/2026-02-17-unified-messages/architecture.md` around lines 167
- 168, The fenced code block showing TaskResult::Error(message) is missing a
language specifier; update the block that contains the symbol TaskResult::Error
to include a language identifier (e.g., change ``` to ```text) so the block
becomes ```text followed by TaskResult::Error(message) and the closing ```; this
mirrors other blocks and satisfies the linting rule.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/ai-design/2026-02-17-unified-messages/architecture.md`:
- Around line 152-154: Update the icon mapping in the documentation to match the
implementation: replace the combined "⚠ for Error/Warning" entry with distinct
icons — use ❌ for Error and ⚠ for Warning — and keep ✓ for Success and ℹ for
Info; ensure the surrounding style notes still reference
DashColors::message_background_color() for fill, DashColors::message_color() for
border, and DashColors::message_color() for text so the doc reflects the actual
icon choices used in the code.

---

Duplicate comments:
In `@docs/ai-design/2026-02-17-unified-messages/architecture.md`:
- Around line 167-168: The fenced code block showing TaskResult::Error(message)
is missing a language specifier; update the block that contains the symbol
TaskResult::Error to include a language identifier (e.g., change ``` to ```text)
so the block becomes ```text followed by TaskResult::Error(message) and the
closing ```; this mirrors other blocks and satisfies the linting rule.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8d9fbc1 and 83f13e8.

📒 Files selected for processing (4)

• docs/ai-design/2026-02-17-unified-messages/architecture.md
• src/ui/components/message_banner.rs
• src/ui/identities/identities_screen.rs
• src/ui/tokens/mint_tokens_screen.rs