feat: shortcuts info panel #1392
Conversation
WalkthroughThis update introduces a Compose-based informational panel to guide users in customizing shortcut bars, managed by new state logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WalletFragment
participant ShortcutsViewModel
participant UI
User->>WalletFragment: Opens wallet screen
WalletFragment->>ShortcutsViewModel: Observe showShortcutInfo
ShortcutsViewModel->>ShortcutsViewModel: Determine if info panel should show (upgrade/install/flag)
ShortcutsViewModel-->>WalletFragment: showShortcutInfo = true/false
WalletFragment->>UI: Display InfoPanel if showShortcutInfo = true
User->>UI: Clicks close on InfoPanel
UI->>WalletFragment: onAction callback
WalletFragment->>ShortcutsViewModel: hideShortcutInfo()
ShortcutsViewModel->>ShortcutsViewModel: Set showShortcutInfo = false, persist flag
ShortcutsViewModel-->>WalletFragment: showShortcutInfo = false
WalletFragment->>UI: Hide InfoPanel
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ 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:
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 (
|
|
@CodeRabbit review |
✅ 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: 0
🧹 Nitpick comments (4)
wallet/src/de/schildbach/wallet/ui/main/shortcuts/ShortcutProvider.kt (1)
50-56: Enhanced null handling for shortcut data.The code now explicitly handles null shortcut sets by resetting to an empty list and returning early, providing clearer control flow compared to the previous implicit filtering approach.
Consider adding a log statement when this occurs to help with debugging if users experience unexpected shortcut resets:
if (shortcuts == null) { // DataStore was cleared out, probably due to a reset + Log.d("ShortcutProvider", "Shortcut data was null, resetting to empty list") _customShortcuts.value = emptyList() return@onEach }common/src/main/java/org/dash/wallet/common/ui/components/InfoPanel.kt (1)
43-111: Well-structured InfoPanel composable with good UI practicesThe InfoPanel composable is well-implemented with clean organization and flexible parameters. It effectively combines layout, theming, and interactive elements.
Consider making the "Close" content description in the action icon (line 104) configurable through a parameter rather than hardcoded, to support different action types and improve accessibility.
@Composable fun InfoPanel( title: String, description: String, modifier: Modifier = Modifier, @DrawableRes leftIconRes: Int? = null, @DrawableRes actionIconRes: Int? = null, + actionContentDescription: String? = "Close", onAction: (() -> Unit)? = null ) { // ... Icon( painter = painterResource(id = actionIconRes), - contentDescription = "Close", + contentDescription = actionContentDescription, tint = MyTheme.Colors.gray ) // ... }wallet/src/de/schildbach/wallet/ui/main/shortcuts/ShortcutsViewModel.kt (2)
83-83: Consider limiting logging to debug buildsWhile logging the shortcuts size is helpful for debugging, consider wrapping it in a condition to only log in debug builds to avoid unnecessary logging in production.
- .onEach { Log.i("SHORTCUTS", "size: ${it.size}") } + .onEach { + if (BuildConfig.DEBUG) { + Log.i("SHORTCUTS", "size: ${it.size}") + } + }
88-103: Complex visibility logic could benefit from more commentsThe logic for determining the initial visibility of the shortcut info panel handles different scenarios well, but could benefit from more detailed inline comments explaining each conditional branch.
// Check if need to show the shortcut info panel viewModelScope.launch { val isHidden = walletUIConfig.get(WalletUIConfig.IS_SHORTCUT_INFO_HIDDEN) + // Determine whether to show the shortcut info panel based on app state + // and user preferences showShortcutInfo = if (config.wasUpgraded()) { // If upgraded, show immediately if not already hidden isHidden != true } else if (isHidden == null) { // New install and the first time opening the app - don't show // Update IS_SHORTCUT_INFO_HIDDEN to detect non-first launch next time walletUIConfig.set(WalletUIConfig.IS_SHORTCUT_INFO_HIDDEN, false) false } else { // New install, not the first time opening the app, show if not hidden !isHidden } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
build.gradle(1 hunks)common/src/main/java/org/dash/wallet/common/data/WalletUIConfig.kt(3 hunks)common/src/main/java/org/dash/wallet/common/ui/components/InfoPanel.kt(1 hunks)common/src/main/java/org/dash/wallet/common/ui/components/MenuItem.kt(1 hunks)common/src/main/java/org/dash/wallet/common/ui/components/MyTheme.kt(2 hunks)wallet/build.gradle(1 hunks)wallet/res/drawable/ic_shortcuts.xml(1 hunks)wallet/res/layout/home_content.xml(2 hunks)wallet/res/values/strings.xml(1 hunks)wallet/src/de/schildbach/wallet/ui/main/HeaderBalanceFragment.kt(2 hunks)wallet/src/de/schildbach/wallet/ui/main/MainViewModel.kt(6 hunks)wallet/src/de/schildbach/wallet/ui/main/WalletFragment.kt(6 hunks)wallet/src/de/schildbach/wallet/ui/main/shortcuts/ShortcutProvider.kt(2 hunks)wallet/src/de/schildbach/wallet/ui/main/shortcuts/ShortcutsViewModel.kt(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
wallet/src/de/schildbach/wallet/ui/main/shortcuts/ShortcutProvider.kt (1)
Learnt from: Syn-McJ
PR: dashpay/dash-wallet#1389
File: features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/explore/SearchFragment.kt:45-46
Timestamp: 2025-05-07T14:18:11.161Z
Learning: In the ExploreViewModel of the dash-wallet application, `appliedFilters` is a StateFlow (not LiveData), so Flow operators like `distinctUntilChangedBy` can be used with it.
🧬 Code Graph Analysis (1)
wallet/src/de/schildbach/wallet/ui/main/WalletFragment.kt (1)
common/src/main/java/org/dash/wallet/common/ui/components/InfoPanel.kt (1)
InfoPanel(43-111)
🔇 Additional comments (41)
build.gradle (1)
31-31: Dependency version update looks good.Updated ConstraintLayout version from 2.1.4 to 2.2.1 which aligns with the UI updates in this PR.
wallet/res/drawable/ic_shortcuts.xml (1)
1-22: Well-structured vector drawable for shortcuts icon.The icon is properly defined with appropriate size, viewport, and properly structured paths. The use of
fillType="evenOdd"for the first path is appropriate for the compound shape.common/src/main/java/org/dash/wallet/common/ui/components/MenuItem.kt (1)
59-59: Good theme integration for background color.Replacing the hardcoded
Color.WhitewithMyTheme.Colors.backgroundSecondaryimproves theme consistency and supports dynamic theming (like dark mode).wallet/src/de/schildbach/wallet/ui/main/HeaderBalanceFragment.kt (2)
34-34: Added import for observe utility.The import addition is necessary for the refined state observation logic.
70-70: Improved null handling for boolean state.Changed from using Elvis operator with a default (
showHint ?: true) to an explicit check (showHint != false). This makes the intent clearer - the hint is visible unless explicitly set to false.wallet/build.gradle (1)
103-103: Added necessary dependency for Compose lifecycle integration.This dependency is essential for the new Compose-based info panel, enabling lifecycle-aware state collection from the associated ViewModel.
wallet/res/layout/home_content.xml (2)
59-59: Reduced top padding for better vertical spacing.This adjustment from 54dp to 40dp provides better vertical spacing within the sync_status_pane_parent container, accounting for the new info panel element.
78-82: Added ComposeView for the shortcuts information panel.The new ComposeView will host the Compose-based InfoPanel component, positioned below the sync and mixing status panels with consistent margin treatment.
wallet/src/de/schildbach/wallet/ui/main/shortcuts/ShortcutProvider.kt (1)
38-39: Improved documentation for minimum shortcuts requirement.The comment clarifies the logic behind the MINIMUM_SHORTCUTS constant, providing better context for the shortcut management code.
wallet/res/values/strings.xml (1)
426-427: Added strings for shortcut customization guidance.These strings provide clear instructions to users on how to customize the shortcut bar, enhancing the user experience with intuitive guidance.
common/src/main/java/org/dash/wallet/common/data/WalletUIConfig.kt (3)
30-30: Good addition of the Kotlin extension function for SharedPreferencesUsing
androidx.core.content.editimport enables a more concise and elegant SharedPreferences edit pattern which is utilized later in the file.
70-70: LGTM: Clear and consistent preference key nameThe new preference key
IS_SHORTCUT_INFO_HIDDENfollows the existing naming convention and clearly describes its purpose.
117-117: Good refactor to use the Kotlin DSL for SharedPreferencesYou've improved code readability by replacing the traditional
edit().putString(...).apply()pattern with the more concise Kotlin extension functionedit { putString(...) }.common/src/main/java/org/dash/wallet/common/ui/components/MyTheme.kt (3)
31-34: Good enhancement to font managementBreaking down the generic font family into specific font weight variants improves typography control throughout the app.
59-71: Good addition of Caption text stylesThe new
CaptionandCaptionMediumtext styles provide appropriate typography options for the info panel component.
97-97: LGTM: New color for enhanced UIThe new
graycolor enhances the color palette and will be used for the close icon in the info panel.common/src/main/java/org/dash/wallet/common/ui/components/InfoPanel.kt (1)
113-123: Good use of Preview composableIncluding a preview composable helps with development and QA, providing a sample visualization without running the full app.
wallet/src/de/schildbach/wallet/ui/main/shortcuts/ShortcutsViewModel.kt (2)
76-76: LGTM: Clear state variable for info panel visibilityThe new mutable state variable
showShortcutInfoclearly represents whether the shortcut info panel should be displayed.
143-148: Clear and concise method to hide info panelThe
hideShortcutInfo()method effectively updates both the UI state and persists the preference.wallet/src/de/schildbach/wallet/ui/main/WalletFragment.kt (13)
25-31: Added necessary Compose UI imports for the new InfoPanel component.The new imports support the implementation of the Compose-based InfoPanel component that will display shortcut customization guidance.
100-100: Good change: Scoping ShortcutsViewModel to the activity level.Changing from
by viewModels()toby activityViewModels()ensures the shortcut view model state is preserved across navigation events and shared between fragments within the same activity.
162-181: Well-implemented Compose integration for the shortcuts info panel.The InfoPanel implementation follows best practices by:
- Setting a proper composition strategy with
ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed- Using conditional rendering based on the view model state
- Proper resource string references instead of hardcoded text
- Well-structured layout with appropriate modifiers
- Including a close action that updates the view model state
466-466: Fixed explicit NavOptions.Builder() qualification.This change ensures proper resolution of the NavOptions.Builder class, avoiding potential ambiguity with other Builder classes.
511-511: Good UX: Hiding info panel after shortcut customization interaction.Calling
shortcutViewModel.hideShortcutInfo()after a long-press on a shortcut shows good attention to user experience - once the user demonstrates awareness of the customization feature, there's no need to keep showing the guidance panel.
25-31: Appropriate Compose imports added for the new InfoPanel integration.The necessary imports for Compose UI components have been properly added to support the new InfoPanel implementation.
37-37: Good switch to activityViewModels() for improved lifecycle management.Changing to
activityViewModels()is appropriate for the ShortcutsViewModel since it ensures the shortcut configuration state persists across fragment recreations and is shared within the activity scope.
55-59: Correctly imported shortcut-related classes.The necessary imports for the shortcut customization feature have been added properly.
81-81: InfoPanel component successfully imported.The Compose-based InfoPanel component is correctly imported from the common UI components package.
100-100: Appropriate scope change for shortcutViewModel.Changing the ShortcutsViewModel to be scoped to the activity is beneficial for maintaining state across fragment lifecycle changes, ensuring the shortcut configuration persists appropriately.
162-181: Well-implemented InfoPanel integration with proper lifecycle handling.The InfoPanel has been properly integrated with:
- Correct composition strategy to handle fragment lifecycle
- Conditional display based on the viewModel state
- Appropriate styling and layout parameters
- Correct action handling to hide the panel when closed
The implementation properly follows Compose best practices.
466-466: Fixed NavOptions.Builder() qualification.Explicitly qualifying
NavOptions.Builder()ensures proper resolution of the class, preventing potential ambiguity with other Builder classes.
511-512: Proper handling of shortcut info when user interacts with shortcuts.Hiding the shortcut info panel when a user performs a long tap on a shortcut is a good UX decision - it assumes the user has discovered the feature and no longer needs guidance.
wallet/src/de/schildbach/wallet/ui/main/MainViewModel.kt (9)
227-231: Improved state handling using StateFlow.Converting LiveData to StateFlow with proper configuration parameters:
- Uses
SharingStarted.WhileSubscribed(5000)for efficient resource management- Provides appropriate scoping with
viewModelScope- Sets a sensible default value
This modernizes the codebase by leveraging Kotlin Flow's reactive capabilities.
233-233: Simplified property declaration using Flow.Converting from LiveData to Flow for the tap-to-hide hint improves consistency with other reactive state properties in the class.
429-429: Simplified balance visibility toggle logic.The previous implementation likely had unnecessary null-checking. This change directly toggles the current state value, making the code more concise and readable.
478-478: Improved exception handling with discard pattern.Using the underscore in
catch (_: Exception)clearly communicates the intention to ignore exceptions without assigning them to an unused variable.
68-71: Proper Flow-related imports added.The necessary imports for Kotlin Flow APIs have been added to support the state management improvements.
227-231: Improved state management with StateFlow.Converting
hideBalancefrom LiveData to StateFlow with proper configuration:
- Uses
stateIn()with appropriate scoping and caching strategy- Sets a sensible timeout (5 seconds) for the WhileSubscribed strategy
- Provides a proper initial value
This modernization aligns with Kotlin coroutines best practices.
233-234: Simplified tap-to-hide hint observation.Converting to a direct Flow observation simplifies the code and removes unnecessary LiveData wrapping.
429-430: Simplified balance visibility toggle logic.The implementation now directly toggles the current state using the negation operator, making the code more concise and readable.
478-479: Improved exception handling with Kotlin idioms.Using the Kotlin underscore syntax to ignore the caught exception is cleaner and more idiomatic. This approach clearly communicates that the exception is intentionally ignored while maintaining the necessary catch block.
We want to show an info panel informing users about the shortcut customization feature.
Issue being fixed or feature implemented
Related PR's and Dependencies
This should be merged before #1385
Screenshots / Videos
How Has This Been Tested?
Checklist:
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores