fix: merchant types info #1383
fix: merchant types info #1383
Conversation
WalkthroughThis update introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ExploreFragment
participant ExploreEntryViewModel
participant ExploreDashInfoDialog
User->>ExploreFragment: Clicks "Merchants" button
ExploreFragment->>ExploreEntryViewModel: isInfoShown()
alt Info has been shown
ExploreFragment->>ExploreFragment: Navigate to Merchants Search
else Info has not been shown
ExploreFragment->>ExploreDashInfoDialog: Show dialog with completion callback
ExploreDashInfoDialog-->>ExploreFragment: Dialog dismissed
ExploreFragment->>ExploreEntryViewModel: setIsInfoShown(true)
ExploreFragment->>ExploreFragment: Navigate to Merchants Search
end
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
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:
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 (
|
| import javax.inject.Inject | ||
|
|
||
| @HiltViewModel | ||
| class ExploreEntryViewModel @Inject constructor( |
There was a problem hiding this comment.
Moving some code out of the MainViewModel to avoid bloating it
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wallet/src/de/schildbach/wallet/ui/explore/ExploreEntryViewModel.kt (1)
77-86: Comprehensive sync status update logic.This suspend function not only updates the sync status LiveData but also calculates and updates the staking APY when the blockchain is synced. This encapsulates the sync and APY logic that was previously spread across
MainViewModel.One minor observation: while
_isBlockchainSynced.valueis used for direct assignment,_stakingAPY.postValueis used for posting values. Consider using consistent update methods for better readability.- _isBlockchainSynced.value = state.isSynced() + _isBlockchainSynced.postValue(state.isSynced())Or:
- _stakingAPY.postValue(withoutFees * blockchainStateProvider.getMasternodeAPY()) + _stakingAPY.value = withoutFees * blockchainStateProvider.getMasternodeAPY()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/explore/SearchFragment.kt(2 hunks)features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/explore/dialogs/ExploreDashInfoDialog.kt(3 hunks)features/exploredash/src/main/java/org/dash/wallet/features/exploredash/utils/ExploreConfig.kt(0 hunks)features/exploredash/src/main/res/layout/fragment_search.xml(1 hunks)features/exploredash/src/main/res/navigation/nav_explore.xml(0 hunks)wallet/src/de/schildbach/wallet/ui/explore/ExploreEntryViewModel.kt(1 hunks)wallet/src/de/schildbach/wallet/ui/explore/ExploreFragment.kt(3 hunks)wallet/src/de/schildbach/wallet/ui/main/MainViewModel.kt(2 hunks)
💤 Files with no reviewable changes (2)
- features/exploredash/src/main/java/org/dash/wallet/features/exploredash/utils/ExploreConfig.kt
- features/exploredash/src/main/res/navigation/nav_explore.xml
🧰 Additional context used
🧬 Code Graph Analysis (1)
wallet/src/de/schildbach/wallet/ui/explore/ExploreEntryViewModel.kt (1)
wallet/src/de/schildbach/wallet/ui/main/MainViewModel.kt (1)
updateSyncStatus(742-749)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (20)
features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/explore/dialogs/ExploreDashInfoDialog.kt (4)
22-22: Added import for FragmentActivity.The import of
FragmentActivityenables direct dialog presentation from the fragment, supporting the new pattern of showing the dialog directly from UI code instead of through the navigation graph.
37-37: Added callback mechanism for dialog completion.A nullable lambda property allows callers to provide actions that should occur after the dialog is dismissed, improving flexibility of the dialog's usage.
47-47: Invoke completion callback after dismissal.The dialog now executes the provided callback when the user dismisses it via the continue button, enabling post-dismissal actions like navigation or state updates.
51-54: Added helper method for showing dialog with completion callback.This new overloaded
showmethod simplifies dialog instantiation with a completion callback, providing a cleaner API for the dialog's consumers.features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/explore/SearchFragment.kt (2)
67-67: Added import for ExploreDashInfoDialog.This import enables direct use of the dialog class, supporting the transition from navigation-based dialog presentation to direct dialog instantiation.
173-175: Added info button click handler to show Merchant Types dialog.This implementation restores the missing Merchant Types dialog functionality by showing the ExploreDashInfoDialog directly when the user taps the info button, addressing the main issue in the PR.
features/exploredash/src/main/res/layout/fragment_search.xml (1)
50-59: Added info button to the toolbar.Added an info button with appropriate styling and positioning in the toolbar. The button has proper accessibility attributes and uses standard material design patterns for info buttons.
wallet/src/de/schildbach/wallet/ui/explore/ExploreFragment.kt (4)
28-28: Switched to using fragment-scoped ViewModel.Changed from
activityViewModels()toviewModels()to prepare for using a dedicated ViewModel instead of sharing the MainViewModel.
43-43: Added import for ExploreDashInfoDialog.This import enables direct instantiation of the dialog, supporting the revised dialog presentation pattern.
49-49: Using dedicated ExploreEntryViewModel instead of shared MainViewModel.This change improves separation of concerns by moving explore-specific state management to a dedicated ViewModel rather than using the shared MainViewModel.
64-73: Enhanced Merchants button click handler to conditionally show the info dialog.The updated implementation checks if the info dialog has been shown before using the ViewModel. If not, it shows the dialog with a completion callback that updates the state and navigates; otherwise, it navigates directly. This creates a better UX by only showing the info dialog once.
wallet/src/de/schildbach/wallet/ui/main/MainViewModel.kt (2)
141-142: Constructor parameter list cleanup.The
crowdNodeApiparameter has been removed from the constructor as part of moving the staking APY functionality to the newExploreEntryViewModel.
742-749: Good refactoring of sync status update logic.The
updateSyncStatusmethod has been changed from a suspend function to a regular function. The staking APY update logic that was previously here has been properly moved to the newExploreEntryViewModelclass, keeping this function focused on just updating sync status.wallet/src/de/schildbach/wallet/ui/explore/ExploreEntryViewModel.kt (7)
1-33: Good implementation of a feature-specific ViewModel.This new
ExploreEntryViewModelclass follows the single responsibility principle by encapsulating the Explore feature's logic that was previously inMainViewModel. The dependencies are properly injected and the class is correctly annotated with@HiltViewModelfor Hilt dependency injection.
35-41: Proper exposing of LiveData properties.The ViewModel correctly implements the LiveData pattern by exposing read-only LiveData properties with private mutable backing fields, following best practices for encapsulation.
43-50: Effective initialization and flow handling.Good use of
filterNotNull(),onEach(), andlaunchIn()to observe blockchain state changes. The ViewModel properly uses coroutines scoped to theviewModelScopeto ensure cleanup when the ViewModel is cleared.
52-58: Method for manual APY updates.This method allows for explicitly fetching the latest staking APY. It correctly uses
viewModelScope.launchwith the IO dispatcher for the network call, showing good coroutine usage for asynchronous operations.
60-67: Merchant types info dialog state management.These methods handle persistence of whether the merchant types info dialog has been shown to the user. The suspend function for reading is appropriate, and the writing method correctly uses a coroutine to perform the storage operation asynchronously.
69-71: Simple analytics event logging.The method correctly delegates to the injected analytics service without adding unnecessary complexity.
73-75: Network type detection.Simple utility method to determine if the wallet is connected to a test network. This is useful for conditional UI behavior based on network type.
The Merchant Types dialog is missing.
Issue being fixed or feature implemented
Related PR's and Dependencies
Screenshots / Videos
How Has This Been Tested?
Checklist:
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor