fix: repaint UI only when needed#344
Conversation
lklimek
requested review from
QuantumExplorer and
Copilot
and removed request for
Copilot
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces a new egui-aware asynchronous message passing system, replacing previous Tokio and standard mpsc senders throughout the backend and UI layers. It adds a bundled font loader, refines developer mode and animation toggling, and modularizes the application startup process. Numerous enums are made public, and several UI components are streamlined or removed. Automated tests using Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant AppContext
participant BackendTask
participant SenderAsync
participant eguiCtx
UI->>AppContext: Trigger backend task
AppContext->>SenderAsync: Send TaskResult
SenderAsync->>eguiCtx: request_repaint()
SenderAsync->>UI: (TaskResult delivered via receiver)
sequenceDiagram
participant Main
participant AppState
participant eguiCtx
participant Fonts
Main->>AppState: AppState::new(eguiCtx)
AppState->>Fonts: Load bundled fonts
Fonts-->>AppState: FontDefinitions
AppState->>eguiCtx: set_fonts(FontDefinitions)
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
There was a problem hiding this comment.
Pull Request Overview
This pull request changes the UI repaint behavior by triggering repaints only when necessary, instead of using a fixed 50 ms interval, and refactors several channel-related APIs and internal visibility for easier testing and library integration.
- Updated all instances of tokio::sync::mpsc sender to use crate::utils::egui_mpsc::SenderAsync.
- Changed the visibility of several enums and functions to support the library structure and testing requirements.
- Adjusted the app’s channel creation and UI repaint logic accordingly.
Reviewed Changes
Copilot reviewed 66 out of 66 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/backend_task/* | Replaced mpsc::Sender with SenderAsync for task result channels. |
| src/backend_task/identity/* | Updated channel types to SenderAsync. |
| src/backend_task/contested_names/* | Updated channel types and changed enum visibility from pub(crate) to pub. |
| src/app.rs | Modified channel initialization, removed timer-based UI repaint, and changed function visibility for backend tasks. |
| Cargo.toml | Added dependency egui_kittest for additional testing support. |
Comments suppressed due to low confidence (3)
src/app.rs:473
- The visibility of handle_backend_task was changed from public to private. Confirm that this change is intentional and that no external modules rely on this function.
fn handle_backend_task(&self, task: BackendTask) {
src/app.rs:488
- The change from public to private for handle_backend_tasks might affect external usage. Verify that altering its visibility will not impact modules that expect these functions to be public.
fn handle_backend_tasks(&self, tasks: Vec<BackendTask>, mode: BackendTasksExecutionMode) {
src/backend_task/contested_names/mod.rs:17
- Changing the enum visibility from pub(crate) to pub may unintentionally expose internal details. Consider whether this change aligns with your intended API design and encapsulation strategy.
pub enum ContestedResourceTask {
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/main.rs (1)
1-1: Consider explicit imports instead of glob import.While glob imports might be convenient, they can make it harder to track symbol origins and may lead to namespace pollution. Consider importing only the specific items needed.
src/bundled.rs (1)
48-177: Consider error handling and performance implications of bundled fonts.The
fonts()function effectively loads comprehensive international font support. However, consider the following:
- Performance: Loading all font variants at startup may impact application launch time. Consider lazy loading for less commonly used scripts.
- Error handling: While the function signature returns a
Result, no actual error cases are handled within the implementation.- Bundle size: Multiple font files will significantly increase the binary size.
Consider this optimization to conditionally load fonts:
+pub fn fonts_with_options(load_all_scripts: bool) -> Result<egui::FontDefinitions, Box<dyn std::error::Error>> { + let mut fonts = FontDefinitions::default(); + + // Always load basic fonts + fonts.font_data.insert( + "noto_sans".to_owned(), + FontData::from_static(include_bytes!( + "../assets/Fonts/Noto_Sans/static/NotoSans-Light.ttf" + )).into(), + ); + + if load_all_scripts { + // Load additional script fonts only when needed + // ... existing font loading code + } + + Ok(fonts) +}src/backend_task/tokens/resume_tokens.rs (1)
26-26: Consider removing unused sender parameter.The sender parameter type change to
crate::utils::egui_mpsc::SenderAsync<TaskResult>is consistent with the broader refactor. However, since the parameter is unused (prefixed with_), consider removing it entirely to clean up the method signature.Apply this diff to remove the unused parameter:
pub async fn resume_tokens( &self, actor_identity: &QualifiedIdentity, data_contract: Arc<DataContract>, token_position: u16, signing_key: IdentityPublicKey, public_note: Option<String>, group_info: Option<GroupStateTransitionInfoStatus>, sdk: &Sdk, - _sender: crate::utils::egui_mpsc::SenderAsync<TaskResult>, ) -> Result<BackendTaskSuccessResult, String> {Alternatively, if the parameter is kept for API consistency across backend task methods, the current approach is acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockicons/dash.pngis excluded by!**/*.png
📒 Files selected for processing (64)
Cargo.toml(1 hunks)src/app.rs(8 hunks)src/backend_task/contested_names/mod.rs(2 hunks)src/backend_task/contested_names/query_dpns_contested_resources.rs(1 hunks)src/backend_task/contested_names/query_dpns_vote_contenders.rs(1 hunks)src/backend_task/contested_names/query_ending_times.rs(1 hunks)src/backend_task/contested_names/vote_on_dpns_name.rs(1 hunks)src/backend_task/contract.rs(2 hunks)src/backend_task/core/mod.rs(2 hunks)src/backend_task/document.rs(1 hunks)src/backend_task/identity/mod.rs(2 hunks)src/backend_task/identity/refresh_identity.rs(1 hunks)src/backend_task/identity/refresh_loaded_identities_dpns_names.rs(1 hunks)src/backend_task/identity/register_identity.rs(1 hunks)src/backend_task/identity/top_up_identity.rs(1 hunks)src/backend_task/mod.rs(6 hunks)src/backend_task/register_contract.rs(2 hunks)src/backend_task/system_task/mod.rs(1 hunks)src/backend_task/tokens/burn_tokens.rs(1 hunks)src/backend_task/tokens/destroy_frozen_funds.rs(1 hunks)src/backend_task/tokens/freeze_tokens.rs(1 hunks)src/backend_task/tokens/mint_tokens.rs(1 hunks)src/backend_task/tokens/mod.rs(2 hunks)src/backend_task/tokens/pause_tokens.rs(1 hunks)src/backend_task/tokens/purchase_tokens.rs(1 hunks)src/backend_task/tokens/query_my_token_balances.rs(2 hunks)src/backend_task/tokens/query_token_pricing.rs(1 hunks)src/backend_task/tokens/resume_tokens.rs(1 hunks)src/backend_task/tokens/set_token_price.rs(1 hunks)src/backend_task/tokens/transfer_tokens.rs(1 hunks)src/backend_task/tokens/unfreeze_tokens.rs(1 hunks)src/backend_task/update_data_contract.rs(2 hunks)src/bundled.rs(2 hunks)src/components/core_zmq_listener.rs(3 hunks)src/context.rs(5 hunks)src/lib.rs(1 hunks)src/main.rs(2 hunks)src/ui/components/entropy_grid.rs(1 hunks)src/ui/components/left_panel.rs(2 hunks)src/ui/components/styled.rs(6 hunks)src/ui/components/top_panel.rs(1 hunks)src/ui/helpers.rs(1 hunks)src/ui/identities/transfer_screen.rs(1 hunks)src/ui/identities/withdraw_screen.rs(3 hunks)src/ui/network_chooser_screen.rs(2 hunks)src/ui/tokens/burn_tokens_screen.rs(2 hunks)src/ui/tokens/claim_tokens_screen.rs(1 hunks)src/ui/tokens/destroy_frozen_funds_screen.rs(2 hunks)src/ui/tokens/direct_token_purchase_screen.rs(1 hunks)src/ui/tokens/freeze_tokens_screen.rs(2 hunks)src/ui/tokens/mint_tokens_screen.rs(3 hunks)src/ui/tokens/pause_tokens_screen.rs(2 hunks)src/ui/tokens/resume_tokens_screen.rs(2 hunks)src/ui/tokens/set_token_price_screen.rs(1 hunks)src/ui/tokens/tokens_screen/groups.rs(2 hunks)src/ui/tokens/tokens_screen/mod.rs(1 hunks)src/ui/tokens/tokens_screen/my_tokens.rs(2 hunks)src/ui/tokens/transfer_tokens_screen.rs(1 hunks)src/ui/tokens/unfreeze_tokens_screen.rs(2 hunks)src/ui/tokens/update_token_config.rs(2 hunks)src/utils/egui_mpsc.rs(1 hunks)src/utils/mod.rs(1 hunks)tests/kittest/main.rs(1 hunks)tests/kittest/startup.rs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (13)
tests/kittest/startup.rs (1)
src/app.rs (1)
new(143-445)
src/backend_task/identity/register_identity.rs (1)
src/utils/egui_mpsc.rs (2)
sender(37-39)sender(100-102)
src/backend_task/update_data_contract.rs (1)
src/utils/egui_mpsc.rs (2)
sender(37-39)sender(100-102)
src/backend_task/register_contract.rs (1)
src/utils/egui_mpsc.rs (2)
sender(37-39)sender(100-102)
src/backend_task/identity/top_up_identity.rs (1)
src/utils/egui_mpsc.rs (2)
sender(37-39)sender(100-102)
src/backend_task/identity/mod.rs (1)
src/utils/egui_mpsc.rs (2)
sender(37-39)sender(100-102)
src/backend_task/contested_names/query_dpns_contested_resources.rs (1)
src/utils/egui_mpsc.rs (2)
sender(37-39)sender(100-102)
src/components/core_zmq_listener.rs (1)
src/utils/egui_mpsc.rs (2)
sender(37-39)sender(100-102)
src/backend_task/identity/refresh_identity.rs (1)
src/utils/egui_mpsc.rs (2)
sender(37-39)sender(100-102)
src/backend_task/tokens/query_my_token_balances.rs (1)
src/utils/egui_mpsc.rs (2)
sender(37-39)sender(100-102)
src/backend_task/contested_names/vote_on_dpns_name.rs (1)
src/utils/egui_mpsc.rs (2)
sender(37-39)sender(100-102)
src/utils/egui_mpsc.rs (1)
src/app.rs (1)
new(143-445)
src/backend_task/mod.rs (1)
src/utils/egui_mpsc.rs (2)
sender(37-39)sender(100-102)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Clippy
🔇 Additional comments (102)
src/utils/mod.rs (1)
1-1: LGTM - Core module for egui-aware message passing.This module declaration correctly exposes the new egui_mpsc functionality that enables "repaint UI only when needed" as described in the PR objectives.
tests/kittest/main.rs (1)
1-1: LGTM - Proper test module structure.The startup module declaration correctly establishes the test infrastructure for egui_kittest, supporting the PR's goal of easier testing.
Cargo.toml (1)
70-70: LGTM - Appropriate test dependency.The egui_kittest dependency is correctly configured with matching version (0.31.1) and appropriate eframe feature for testing egui applications.
src/backend_task/document.rs (1)
32-32: LGTM - Visibility change supports library refactoring.Changing the DocumentTask enum visibility to
pubaligns with the PR objective of moving logic into a library, making this enum accessible outside the crate.src/ui/components/entropy_grid.rs (1)
9-14: LGTM - Standard Default implementation.The Default trait implementation correctly delegates to
Self::new(), following standard Rust patterns and improving API ergonomics for contexts requiring default values.src/ui/components/top_panel.rs (1)
139-139: LGTM! Excellent implementation of centralized animation repaint control.This change successfully delegates repaint requests to
AppContext, enabling conditional animation repaints and addressing the continuous repaint loop issue mentioned in the PR objectives.src/ui/tokens/unfreeze_tokens_screen.rs (2)
415-415: Good encapsulation improvement.Replacing direct atomic access with the
is_developer_mode()method improves code maintainability and provides better abstraction for developer mode state management.
543-543: Consistent implementation of developer mode encapsulation.This change maintains consistency with the encapsulation pattern established throughout the codebase.
src/ui/tokens/direct_token_purchase_screen.rs (1)
391-391: Consistent developer mode encapsulation.This change follows the established pattern of replacing direct atomic access with the encapsulated
is_developer_mode()method, improving code consistency and maintainability.src/backend_task/core/mod.rs (1)
15-15: Verify the necessity of making these enums public.The visibility change from
pub(crate)topubmakes these enums accessible outside the crate. Please confirm this is intentional and necessary for the modularization efforts mentioned in the PR summary.Does the library separation require these enums to be publicly accessible, or could they remain crate-private?
Also applies to: 41-41
src/ui/helpers.rs (1)
201-201: Excellent completion of developer mode encapsulation.This change completes the consistent pattern of replacing direct atomic access with the
is_developer_mode()method throughout the UI codebase, improving encapsulation and maintainability.src/ui/tokens/set_token_price_screen.rs (1)
723-723: LGTM: Clean encapsulation of developer mode access.The change from direct atomic field access to the
is_developer_mode()method properly encapsulates the developer mode state while maintaining identical functionality.src/ui/tokens/freeze_tokens_screen.rs (2)
416-416: LGTM: Consistent developer mode encapsulation.The refactor from direct atomic access to the
is_developer_mode()method aligns with the broader encapsulation improvements across the UI layer.
544-544: LGTM: Proper encapsulation pattern applied consistently.This change follows the same encapsulation pattern used throughout the UI refactor, replacing direct atomic field access with a clean method interface.
src/ui/tokens/transfer_tokens_screen.rs (1)
350-350: LGTM: Consistent application of developer mode encapsulation.The change maintains the existing conditional logic while properly encapsulating developer mode access through the new method interface.
src/backend_task/tokens/freeze_tokens.rs (1)
27-27: LGTM: Transition to egui-aware async channels.The switch from
tokio::sync::mpsc::SendertoSenderAsyncsupports the PR's core objective of integrating UI repaint triggers with async message passing, eliminating inefficient continuous repaints.src/backend_task/tokens/transfer_tokens.rs (1)
30-30: LGTM: Consistent async channel refactor.The transition to
SenderAsyncmaintains interface consistency across backend tasks and enables egui-integrated repaint triggering when the sender is utilized.src/ui/tokens/tokens_screen/mod.rs (1)
1197-1197: LGTM: Good encapsulation improvement.The change from direct atomic access to using the
is_developer_mode()accessor method improves encapsulation and aligns with the broader architectural improvements in this PR. This provides a cleaner API and allows for future changes to the internal implementation without affecting calling code.src/backend_task/tokens/purchase_tokens.rs (1)
27-27: LGTM: Consistent sender type refactorThe replacement of
tokio::sync::mpsc::Sender<TaskResult>withSenderAsync<TaskResult>aligns with the broader refactor to integrate egui-aware message passing. Since the parameter is unused in this method, the change maintains existing functionality while ensuring type consistency across backend tasks.src/backend_task/tokens/mint_tokens.rs (1)
31-31: LGTM: Consistent with backend task refactor patternThe sender parameter type change maintains consistency with the broader refactor to use egui-aware message passing. The functional behavior remains unchanged since the parameter is unused.
tests/kittest/startup.rs (1)
4-17: LGTM: Well-implemented startup smoke testThis test provides valuable coverage for the app initialization lifecycle using the appropriate
egui_kittestframework. The configuration choices are sound:
- Disabling animations ensures consistent test behavior
- Setting a fixed window size provides a predictable rendering context
- Running one frame is sufficient to catch basic startup issues
This complements the broader refactoring by providing automated verification of app startup behavior.
src/ui/identities/transfer_screen.rs (1)
284-284: LGTM: Excellent encapsulation of developer mode accessThe replacement of direct atomic field access with the
is_developer_mode()method improves code maintainability by:
- Hiding implementation details of developer mode storage
- Providing a cleaner, more semantic API
- Enabling future changes to developer mode handling without affecting callers
This is a textbook example of good encapsulation practice.
src/ui/tokens/tokens_screen/my_tokens.rs (1)
145-145: LGTM: Consistent application of developer mode encapsulationBoth instances correctly replace direct atomic access with the encapsulated
is_developer_mode()method. The consistent application across multiple locations in the same file demonstrates a well-coordinated refactor that improves code maintainability while preserving existing functionality.Also applies to: 193-193
src/ui/tokens/resume_tokens_screen.rs (2)
378-378: LGTM: Improved encapsulation of developer mode access.The change from direct atomic access to method call improves code maintainability and follows good encapsulation practices.
487-487: LGTM: Consistent developer mode encapsulation.Good consistency with the encapsulation pattern applied throughout the file.
src/ui/tokens/burn_tokens_screen.rs (2)
412-412: LGTM: Consistent with developer mode encapsulation refactor.The change aligns with the systematic refactor to encapsulate developer mode access across UI modules.
551-553: LGTM: Maintains consistency in developer mode access pattern.The encapsulation change is correctly applied to the button visibility logic while preserving the original conditional behavior.
src/ui/tokens/update_token_config.rs (2)
781-781: LGTM: Proper encapsulation in complex conditional logic.The developer mode encapsulation is correctly applied even within compound conditional expressions.
1054-1054: LGTM: Consistent developer mode encapsulation pattern.The systematic application of the encapsulation refactor continues to be consistent across all usage sites.
src/backend_task/contested_names/query_dpns_vote_contenders.rs (1)
20-20: LGTM: Key change supporting egui-aware messaging system.This change from
tokio::sync::mpsc::SendertoSenderAsyncis central to the PR's objective of fixing UI repaint efficiency by implementing egui-aware asynchronous messaging.src/ui/tokens/claim_tokens_screen.rs (1)
305-305: LGTM: Good encapsulation of developer mode access.The change from direct atomic access to a method call improves maintainability and consistency with the broader refactoring effort.
src/backend_task/tokens/unfreeze_tokens.rs (1)
27-27: LGTM: Improved UI integration with egui-aware sender.The change to
SenderAsync<TaskResult>aligns with the PR objective to optimize UI repaint behavior by integrating backend task communication with egui's repaint system.src/ui/tokens/mint_tokens_screen.rs (1)
451-451: LGTM: Consistent encapsulation of developer mode access.All three changes properly replace direct atomic access with the new method call, maintaining consistent behavior while improving code organization and maintainability.
Also applies to: 553-553, 616-616
src/ui/tokens/tokens_screen/groups.rs (1)
155-155: LGTM: Consistent developer mode encapsulation.Both changes properly replace direct atomic access with the encapsulated method call, maintaining the same conditional logic while improving code consistency.
Also applies to: 179-179
src/backend_task/identity/register_identity.rs (1)
109-109: LGTM: Consistent adoption of egui-aware sender type.The change to
SenderAsync<TaskResult>is consistent with the broader architectural refactoring to integrate backend task communication with egui's repaint system, supporting the PR's UI optimization goals.src/ui/tokens/pause_tokens_screen.rs (2)
378-385: LGTM: Proper encapsulation of developer mode check.The change from direct atomic access to the encapsulated
is_developer_mode()method improves code consistency and follows the broader refactor pattern. The logic for determining key availability remains unchanged.
487-489: LGTM: Consistent developer mode check for button visibility.The encapsulated method call maintains the same functionality while improving code organization. The button visibility logic correctly filters out "Test" mode buttons unless in developer mode.
src/ui/components/left_panel.rs (3)
115-121: LGTM: Improved button interaction with hover cursor.Good UX improvement to store the button response and add cursor icon on hover. This enhances user experience by providing visual feedback for interactive elements.
125-125: LGTM: Updated GradientButton constructor to accept AppContext.The signature change aligns with the broader refactor where UI components now receive AppContext for animation and repaint functionality. This enables better integration with the event-driven repaint system.
151-151: LGTM: Consistent developer mode encapsulation.The change from direct atomic access to the encapsulated method call maintains functionality while improving code organization and consistency across the codebase.
src/ui/identities/withdraw_screen.rs (3)
120-120: LGTM: Consistent developer mode encapsulation for address input.The encapsulated method call maintains the same conditional logic for showing withdrawal address input while improving code consistency with the broader refactor.
169-169: LGTM: Proper developer mode check in confirmation popup.The change maintains the same logic for handling masternode payout address scenarios while using the encapsulated method for developer mode checking.
316-316: LGTM: Consistent key availability check with encapsulation.The developer mode check for determining key availability now uses the encapsulated method, maintaining functionality while improving code organization.
src/backend_task/contested_names/query_ending_times.rs (1)
17-17: LGTM: Updated to egui-aware async sender type.The change from
tokio::sync::mpsc::Sender<TaskResult>tocrate::utils::egui_mpsc::SenderAsync<TaskResult>aligns with the PR's objective to integrate async task communication with egui's repaint system. This enables automatic UI repaint requests when backend tasks complete.Note: The parameter is currently unused (
_sender), but this is consistent with the method signature requirements and may be utilized in future implementations.src/ui/network_chooser_screen.rs (2)
64-64: LGTM: Consistent developer mode reading with encapsulation.The change from direct atomic access to the encapsulated
is_developer_mode()method maintains functionality while improving code consistency with the broader refactor pattern.
352-368: LGTM: Proper developer mode updates across all network contexts.The change from direct atomic stores to the encapsulated
enable_developer_mode()method correctly updates developer mode settings for all AppContext instances (mainnet, testnet, devnet, local). This ensures consistency across all network configurations.src/backend_task/tokens/query_token_pricing.rs (1)
13-13: LGTM: Parameter type updated consistently with the refactor.The change from
tokio::sync::mpsc::Sender<TaskResult>tocrate::utils::egui_mpsc::SenderAsync<TaskResult>is consistent with the broader refactor to use egui-aware senders. The underscore prefix correctly indicates this parameter is unused in the function.src/backend_task/contested_names/query_dpns_contested_resources.rs (2)
19-19: LGTM: Sender parameter type updated for egui integration.The parameter type change from
mpsc::Sender<TaskResult>tocrate::utils::egui_mpsc::SenderAsync<TaskResult>is consistent with the refactor to integrate message sending with egui's repaint system. The sender is actively used throughout the function for progress updates and result communication.
149-255: Sender usage patterns are correct.All
sender.send()calls are properly awaited and include appropriate error handling. The mix ofmap_err()andexpect()for error handling is acceptable, withexpect()used for internal communications where failures would indicate programming errors.src/ui/tokens/destroy_frozen_funds_screen.rs (2)
436-436: LGTM: Improved encapsulation of developer mode access.The change from
self.app_context.developer_mode.load(Ordering::Relaxed)toself.app_context.is_developer_mode()improves encapsulation by hiding atomic implementation details behind a clean method interface. This makes the code more maintainable and consistent with the broader refactor.
563-563: LGTM: Consistent developer mode access pattern.Same improvement as line 436 - replacing direct atomic access with the encapsulated
is_developer_mode()method call for better code organization and maintainability.src/backend_task/identity/top_up_identity.rs (2)
25-25: LGTM: Parameter type updated for egui integration.The sender parameter type change is consistent with the systematic refactor to use egui-aware senders throughout the backend task modules.
335-340: Sender usage is correct.The
sender.send()call is properly awaited and includes appropriate error handling withmap_err()to convert the send error to a string.src/backend_task/identity/refresh_loaded_identities_dpns_names.rs (2)
14-14: LGTM: Consistent parameter type update.The sender parameter type change aligns with the systematic refactor to use egui-aware senders across all backend task modules.
78-83: Proper sender usage with error handling.The
sender.send()call is correctly awaited and includes comprehensive error handling that formats the error message appropriately for the context.src/backend_task/tokens/set_token_price.rs (1)
28-28: LGTM!The sender type change to
SenderAsync<TaskResult>is consistent with the broader refactor. The parameter remains correctly prefixed with_since it's unused in this method.src/backend_task/tokens/mod.rs (2)
61-61: Visibility change aligns with library refactoring.Making
TokenTaskpublic is appropriate for the library refactoring mentioned in the PR objectives, allowing external usage of these task types.
213-213: Consistent sender type update.The change to
SenderAsync<TaskResult>properly integrates with the egui-aware message passing system, ensuring UI repaints are triggered only when tasks send results.src/backend_task/identity/refresh_identity.rs (1)
16-16: Proper integration with egui repaint system.The
SenderAsynctype ensures that the UI will repaint when the refresh message is sent after identity update, improving UI responsiveness.src/main.rs (1)
24-40: Clean modularization of startup logic.The extraction of initialization logic into a separate
startfunction and moving font loading elsewhere improves code organization and aligns with the library refactoring goals.src/backend_task/system_task/mod.rs (1)
7-7: Consistent refactoring of task visibility and sender type.Both changes align with the systematic refactor across all backend task modules - making task enums public for library usage and adopting the egui-aware
SenderAsynctype.Also applies to: 15-15
src/backend_task/contract.rs (2)
31-31: LGTM: Improved visibility for external crate usage.Making
ContractTaskpublic enhances the API accessibility, aligning with the library modularization goals.
46-46: LGTM: Consistent migration to egui-aware sender.The migration from
tokio::sync::mpsc::Sender<TaskResult>tocrate::utils::egui_mpsc::SenderAsync<TaskResult>is correctly implemented. The new type maintains the same async interface while integrating UI repaint requests, supporting the PR's objective to optimize UI repainting.src/backend_task/update_data_contract.rs (2)
29-29: LGTM: Clean import management.Correctly retaining only the needed
tokio::time::sleepimport while removing the no-longer-neededtokio::sync::mpscimport.
62-62: LGTM: Consistent sender type migration.The migration to
crate::utils::egui_mpsc::SenderAsync<TaskResult>is correctly implemented, maintaining the async interface while enabling UI repaint integration.src/backend_task/identity/mod.rs (2)
248-248: LGTM: Enhanced API accessibility.Making
IdentityTaskpublic improves external crate accessibility, consistent with the library modularization approach.
442-442: LGTM: Proper sender type migration.The migration to
crate::utils::egui_mpsc::SenderAsync<TaskResult>is correctly implemented and will be properly threaded through to the various identity task methods.src/lib.rs (1)
1-15: LGTM: Well-structured library modularization.The comprehensive module structure effectively organizes the codebase into logical components, supporting the PR's goal of moving logic into a library for easier testing. The public visibility enables external crate usage while maintaining clear separation of concerns.
src/backend_task/register_contract.rs (2)
8-8: LGTM: Appropriate import management.Correctly maintaining only the necessary
tokio::time::sleepimport while removing the no-longer-neededtokio::sync::mpscimport.
30-30: LGTM: Consistent egui-aware sender adoption.The migration to
crate::utils::egui_mpsc::SenderAsync<TaskResult>follows the established pattern and correctly maintains the async interface while enabling UI repaint optimization.src/backend_task/contested_names/mod.rs (2)
17-17: LGTM: Making enum public for library modularity.The visibility change from
pub(crate)topubaligns with the PR objective of refactoring the codebase to move logic into a library, allowing external access to this enum.
41-41:Details
✅ Verification successful
Verify consistency of egui-aware sender usage across the codebase.
The sender parameter type change to
crate::utils::egui_mpsc::SenderAsync<TaskResult>correctly integrates with the new egui-aware message passing system to trigger UI repaints only when needed.Run the following script to verify all backend task methods use the new sender type consistently:
🏁 Script executed:
#!/bin/bash # Description: Verify all backend task methods use the new egui-aware sender type. # Expected: All async task methods should use SenderAsync<TaskResult>. # Search for any remaining usage of tokio::sync::mpsc::Sender<TaskResult> echo "=== Checking for old tokio sender usage ===" rg -A 3 "tokio::sync::mpsc::Sender<TaskResult>" # Search for new egui-aware sender usage echo "=== Checking new egui-aware sender usage ===" rg -A 3 "crate::utils::egui_mpsc::SenderAsync<TaskResult>" # Search for potential missed conversions in backend task files echo "=== Checking for potential missed conversions ===" rg -A 5 "async.*sender.*TaskResult" src/backend_task/Length of output: 11553
All backend tasks consistently use the egui-aware sender
Verification confirms that there are no remaining uses of
tokio::sync::mpsc::Sender<TaskResult>and every async method insrc/backend_task/now declares itssenderascrate::utils::egui_mpsc::SenderAsync<TaskResult>. No further changes needed.src/components/core_zmq_listener.rs (1)
63-63: LGTM: Consistent integration with egui-aware synchronous channels.The change to
crate::utils::egui_mpsc::SenderSync<(ZMQMessage, Network)>correctly integrates the ZMQ listener with the egui repaint system. UsingSenderSync(rather thanSenderAsync) is appropriate for this synchronous context where the listener runs in a background thread.Both platform-specific implementations (#[cfg] blocks) are consistently updated, ensuring cross-platform compatibility.
Also applies to: 285-285
src/backend_task/contested_names/vote_on_dpns_name.rs (1)
26-26: LGTM: Consistent migration to egui-aware async channels.The sender parameter type change to
crate::utils::egui_mpsc::SenderAsync<TaskResult>is consistent with the broader refactor across backend task modules. The existing usage ofsender.send(TaskResult::Refresh).awaiton lines 30-33 remains compatible with the new wrapper type.src/backend_task/tokens/burn_tokens.rs (1)
30-30: LGTM: Consistent with egui-aware sender refactoring.The parameter type change from
tokio::sync::mpsc::Sender<TaskResult>tocrate::utils::egui_mpsc::SenderAsync<TaskResult>aligns with the PR's objective to use egui-aware message passing. Since the parameter is unused (indicated by the_prefix), this change is safe and maintains API compatibility.src/backend_task/tokens/query_my_token_balances.rs (1)
18-18: LGTM: Proper integration of egui-aware senders for UI refresh.The parameter type changes to
crate::utils::egui_mpsc::SenderAsync<TaskResult>are excellent here because these functions actively use the sender to sendTaskResult::Refreshmessages (lines 76-79, 127-130). With the new egui-aware senders, these refresh messages will automatically trigger UI repaints, ensuring the UI updates immediately after successful database operations.Also applies to: 98-98
src/backend_task/tokens/pause_tokens.rs (1)
26-26: LGTM: Consistent parameter type update.The change from
tokio::sync::mpsc::Sender<TaskResult>tocrate::utils::egui_mpsc::SenderAsync<TaskResult>is consistent with the broader refactoring. The unused parameter (indicated by_prefix) makes this a safe change.src/context.rs (6)
43-43: Good choice for animation timing.The 100ms refresh interval for
ANIMATION_REFRESH_TIMEprovides smooth animations while avoiding excessive CPU usage. This strikes a good balance for UI responsiveness.
48-48: Excellent encapsulation improvement.Making
developer_modeprivate and adding theanimatefield with proper encapsulation improves the API design. This prevents direct access to internal state and ensures animation control is properly coordinated with developer mode.Also applies to: 71-71
147-154: Smart inverse relationship between developer mode and animations.The logic that disables animations when developer mode is enabled is well-designed for debugging scenarios. The logging message clearly explains the behavior, which will help developers understand why animations are disabled during development.
186-194: Well-designed animation control methods.The
enable_animationsandenable_developer_modemethods provide clean APIs for controlling these related states. The inverse relationship between developer mode and animations is properly maintained inenable_developer_mode.
203-208: Efficient conditional repaint mechanism.The
repaint_animationmethod efficiently checks the animation flag before requesting repaints, preventing unnecessary repaint requests when animations are disabled. This aligns perfectly with the PR's goal of optimizing UI repaint behavior.
215-215: Proper use of encapsulated accessor method.Replacing direct atomic load with the
is_developer_mode()accessor method maintains consistency with the new encapsulation pattern.src/utils/egui_mpsc.rs (9)
7-10: Well-designed wrapper structure.The
SenderAsync<T>struct cleanly wraps the tokio sender with an egui context. The generic type parameter and field organization are appropriate for this use case.
19-25: Correct async send implementation with repaint triggering.The
sendmethod properly delegates to the underlying sender and only triggers a repaint on successful sends. This prevents unnecessary repaints when the channel is closed or full, which is exactly the behavior we want for efficient UI updates.
28-34: Proper try_send implementation.The
try_sendmethod correctly implements non-blocking send with the same repaint-on-success pattern. This provides both blocking and non-blocking send options while maintaining consistent repaint behavior.
37-44: Good utility methods for sender access.The
sender()method provides access to the underlying sender when needed, andis_closed()properly delegates to check channel status. These utility methods maintain the wrapper's transparency while providing necessary functionality.
47-54: Correct Clone implementation.The
Cloneimplementation properly clones both the sender (which is cheap for mpsc senders) and the egui context, maintaining the wrapper's functionality across clones.
55-64: Excellent trait design for channel conversion.The
EguiMpscAsynctrait provides a clean API for converting raw tokio channels into egui-aware channels. Thewith_egui_ctxmethod name is descriptive and the implementation correctly wraps only the sender while leaving the receiver unchanged.
66-75: Consistent synchronous counterpart.The
EguiMpscSynctrait mirrors the async version for standard library channels, providing consistency across both async and sync use cases. This design allows the application to use egui-aware channels throughout the codebase regardless of the underlying channel type.
79-97: Well-implemented synchronous sender wrapper.The
SenderSync<T>implementation mirrors the async version with proper error handling and repaint triggering. The synchronoussendmethod maintains the same pattern of only triggering repaints on successful sends.
105-112: Consistent Clone implementation for sync wrapper.The
Cloneimplementation forSenderSyncmaintains the same pattern as the async version, ensuring consistent behavior across both wrapper types.src/app.rs (4)
26-26: Good architectural change for event-driven UI updates.The migration from
tokio::sync::mpsc::Sendertoegui_mpsc::SenderAsyncis a solid approach to replace the inefficient 50ms repaint timer with event-driven updates. This change aligns perfectly with the PR's objective to repaint UI only when needed.Also applies to: 72-72
143-143: Breaking API change: new constructor requires egui::Context.This is a necessary breaking change to support the new event-driven repaint mechanism. The egui context is required to associate channels with the UI for automatic repaint triggers.
447-461: Well-designed animation control API.The
with_animationsmethod provides a clean builder pattern for controlling animations globally across all network contexts. This aligns perfectly with the PR objective to add a flag for disabling animated UI elements.
473-473: Good encapsulation improvement.Making
handle_backend_taskandhandle_backend_tasksprivate properly encapsulates internal task handling logic.Also applies to: 488-488
src/backend_task/mod.rs (3)
13-13: Consistent migration to egui-aware channels.The import change from
tokio::sync::mpsctocrate::utils::egui_mpsc::SenderAsyncis consistent with the architectural changes inapp.rsand supports the event-driven UI update mechanism.
97-97: Consistent sender type updates across all methods.All backend task methods have been consistently updated to use
SenderAsync<TaskResult>instead of the tokio sender. The implementation logic remains unchanged, ensuring backward compatibility while enabling the new repaint mechanism.Also applies to: 113-113, 131-131
44-44:Details
❓ Verification inconclusive
Verify the necessity of public enum exposure.
The visibility change from
pub(crate)topubforBackendTaskandBackendTaskSuccessResultexposes these enums to external crates. This aligns with the PR objective of moving logic into a library, but please confirm this is intentional and these types are meant to be part of the public API.Also applies to: 58-58
🏁 Script executed:
#!/bin/bash # Description: Check if these enums are used in public API or tests # Look for usage in lib.rs or public modules fd -e rs | xargs rg "BackendTask|BackendTaskSuccessResult" -B 2 -A 2 | grep -E "(pub\s+(fn|mod)|#\[test\])"Length of output: 232
Confirm public visibility for
BackendTaskI verified that
BackendTaskSuccessResultis returned by the public functionwipe_devnet, so making itpubis necessary. However, I didn’t find any public APIs or tests exposingBackendTask.• If
BackendTaskis not intended for external use, please revert its visibility back topub(crate).
• Otherwise, confirm it’s part of your public API and keep itpub.src/ui/components/styled.rs (3)
14-14: Good API surface reduction.Restricting
ButtonVariant,StyledButton, andButtonSizetopub(crate)reduces the public API surface, which is good practice if these components are only used internally. This change improves encapsulation and reduces maintenance burden.Also applies to: 22-22, 32-32
125-130: Good addition of Default trait.Adding a
Defaultimplementation forStyledCardimproves ergonomics and follows Rust best practices. The implementation correctly delegates to the existingnew()method.
217-217: Excellent implementation of controlled animation repaints.The
GradientButtonnow properly delegates repaint requests toAppContext::repaint_animation, which can conditionally trigger repaints based on the animation settings. This is a key improvement that addresses the PR's objective of eliminating unnecessary repaints while maintaining smooth animations when enabled.Also applies to: 221-221, 260-260
| // load fonts | ||
| ctx.set_fonts(crate::bundled::fonts().expect("failed to load fonts")); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify error handling for font loading.
The font loading uses expect("failed to load fonts") which will panic if fonts fail to load. Consider whether this should be a recoverable error or if a panic is appropriate for missing fonts.
🏁 Script executed:
#!/bin/bash
# Description: Check the bundled fonts module implementation
ast-grep --pattern 'pub fn fonts() -> $_' src/bundled.rsLength of output: 58
🏁 Script executed:
#!/bin/bash
# Search for the fonts function signature across all Rust source files
rg -n "fn fonts" -g "*.rs"Length of output: 124
Handle potential font-loading errors without panicking
The fonts() function returns a Result<egui::FontDefinitions, Box<dyn std::error::Error>>. Calling it with .expect("failed to load fonts") will abort the application on failure. Instead, decide whether missing fonts should:
- Be treated as a recoverable error, with a fallback to default fonts or a logged warning
- Or be propagated as an error from
main
For example, to log and use defaults:
// src/app.rs:174
match crate::bundled::fonts() {
Ok(fonts) => ctx.set_fonts(fonts),
Err(err) => {
log::error!("Failed to load fonts: {}", err);
ctx.set_fonts(egui::FontDefinitions::default());
}
}Or, to bubble up the error:
// Change your `main` signature to return Result<(), Box<dyn Error>>
fn main() -> Result<(), Box<dyn std::error::Error>> {
// …
let fonts = crate::bundled::fonts()?;
ctx.set_fonts(fonts);
// …
}Please update the font-loading call in src/app.rs accordingly.
🤖 Prompt for AI Agents
In src/app.rs around lines 174 to 176, the font loading uses expect which panics
on failure. Replace this with proper error handling by matching on the Result
returned from crate::bundled::fonts(). On Ok, set the fonts as usual; on Err,
either log the error and set default fonts or propagate the error by changing
main to return a Result and using the ? operator. This avoids panics and handles
font loading failures gracefully.
Current implementation forces repaint of UI every 50 ms.
This has some drawbacks, like:
This PR changes this behavior by:
It also refactors the code a bit to move majority of code to a library and allow easier testing.
This PR introduces basic test using egui kittest.