feat: redesign about screen#1498
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughReplaces ChangesAbout Screen: Activity → Compose Fragment
Design System: ListItemVariants, TopIntro, MenuItem, MyTheme
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsFragment
participant NavController
participant AboutFragment
participant AboutViewModel
participant AboutScreen
User->>SettingsFragment: tap "About Dash"
SettingsFragment->>NavController: navigate(R.id.settings_to_about)
NavController->>AboutFragment: instantiate & attach
AboutFragment->>AboutViewModel: collect uiState StateFlow
AboutViewModel-->>AboutFragment: AboutViewState emissions
AboutFragment->>AboutFragment: formatDeviceSyncStatus() / formatServerUpdate()
AboutFragment->>AboutScreen: render(AboutUIState, callbacks)
User->>AboutScreen: tap force sync or contact support
AboutScreen-->>AboutFragment: onForceSyncClick() / onContactSupportClick()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
common/src/main/java/org/dash/wallet/common/ui/components/TopIntro.kt (1)
33-63: ⚡ Quick winRemove the commented legacy
TopIntrocode path.The old implementation and commented-out parameter should be deleted now that the consolidated composable is in place; keeping both increases drift and review noise.
✂️ Suggested cleanup
-//@Composable -//fun TopIntro( -// heading: String, -// text: String? = null, -// modifier: Modifier = Modifier.padding(top = 10.dp, start = 20.dp, end = 20.dp, bottom = 20.dp) -//) { -// ... -//} @@ -// modifier: Modifier = Modifier.padding(top = 10.dp, start = 20.dp, end = 20.dp, bottom = 20.dp),Also applies to: 70-70
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@common/src/main/java/org/dash/wallet/common/ui/components/TopIntro.kt` around lines 33 - 63, Remove the entire commented-out `TopIntro` composable function implementation. Delete all lines containing the old `TopIntro` function definition along with its parameters (heading, text, modifier) and implementation details including the Column layout and Text composables. This includes any related commented code on line 70 as well. The consolidated composable implementation should remain active while all legacy commented code paths are completely removed to reduce code drift and review noise.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/agents/DEVELOPMENT-PATTERNS.md:
- Around line 275-276: The documentation for the ListItem component's title
property in DEVELOPMENT-PATTERNS.md incorrectly states that title uses Body M
Regular typography, but the actual ListItem implementation in
common/src/main/java/org/dash/wallet/common/ui/components/ListItem.kt at line
188 renders it with MyTheme.Typography.LabelLarge. Update the title typography
description in the documentation table at lines 275-276 to correctly reflect
LabelLarge instead of Body M Regular, and apply the same correction to the other
occurrence mentioned at lines 298-299 to ensure consistency between
documentation and implementation.
- Around line 217-223: Add language identifiers to the fenced code blocks in the
DEVELOPMENT-PATTERNS.md file that are triggering markdownlint MD040.
Specifically, locate the two code blocks containing the file paths for
ListItemVariants.kt and ListItem.kt, and modify their opening fence markers from
triple backticks to include the `text` language identifier (e.g., ```text
instead of ```). This applies to both the
common/src/main/java/org/dash/wallet/common/ui/components/ListItemVariants.kt
block and the
common/src/main/java/org/dash/wallet/common/ui/components/ListItem.kt block.
In
`@common/src/main/java/org/dash/wallet/common/ui/components/ListItemVariants.kt`:
- Around line 257-264: The Icon composable for the trailing icon in ListItem7
has contentDescription set to null but is clickable when onTrailingIconClick is
provided. Make the contentDescription parameter conditional based on whether
onTrailingIconClick is null: provide a meaningful accessibility label describing
the action when the icon is clickable, and only use null when the icon is not
clickable. This ensures screen readers can properly announce the action to
users.
- Around line 354-355: The secondaryText in the ListItem10 composable is
currently using MyTheme.Colors.textPrimary for its color, which removes the
visual hierarchy between primary and secondary text. Change the color parameter
of the Text composable rendering secondaryText from MyTheme.Colors.textPrimary
to the appropriate secondary color token (likely MyTheme.Colors.textSecondary)
to maintain proper visual hierarchy and distinguish it from the primaryText
element.
In `@common/src/main/java/org/dash/wallet/common/ui/components/MenuItem.kt`:
- Line 148: The contentDescription for the info icon in the MenuItem component
is hardcoded as the English string "Info" rather than using a localized string
resource. Replace the hardcoded "Info" string with a call to stringResource()
that references a string resource identifier from your strings.xml file,
ensuring the accessibility label can be properly localized for different
languages. Define or reuse an appropriate string resource key in your res/values
directory structure that contains the "Info" text so it can be translated.
In `@wallet/res/navigation/nav_home.xml`:
- Around line 390-393: The aboutFragment navigation destination in nav_home.xml
has a hardcoded android:label value of "About" which prevents proper
localization. Replace the hardcoded label text with a string resource reference
using android:label="`@string/about_title`" (or another appropriate resource
name). If the string resource does not already exist in your strings.xml files,
add the about_title string resource with the value "About" to ensure it can be
properly translated across different locales.
In `@wallet/src/de/schildbach/wallet/ui/more/AboutFragment.kt`:
- Around line 149-153: The openGithubLink() function calls startActivity(intent)
unconditionally without verifying if any application can handle the ACTION_VIEW
intent, which will crash the app with ActivityNotFoundException if no handler
exists. Guard the startActivity call by first checking if
intent.resolveActivity(packageManager) returns a non-null value before launching
it; if it returns null, handle the case gracefully such as displaying a
user-friendly message or skipping the action.
- Around line 116-129: The code can format invalid or uninitialized timestamps
(such as 0L) into epoch-era dates, which provides poor user experience. Before
each call to DateUtils.formatDateTime() in the when block, add explicit checks
to ensure the timestamp values are valid (greater than 0). For
dbPrefs.lastSyncTimestamp used in the second branch and else branch, and
dbPrefs.preloadedOnTimestamp used in the third branch, wrap each
DateUtils.formatDateTime() call with a conditional check that returns a
user-friendly fallback string like "never" when the timestamp is zero or
negative, otherwise call DateUtils.formatDateTime() with the valid timestamp.
In `@wallet/src/de/schildbach/wallet/ui/more/AboutViewModel.kt`:
- Around line 52-66: Consolidate the separate state flows
(_exploreRemoteTimestamp, _exploreIsSyncing, _firebaseInstallationId,
_firebaseCloudMessagingToken) into a single UIState data class to prevent
stale/mixed snapshots. Create a new data class that contains all these fields,
replace the individual MutableStateFlow declarations with a single _uiState
MutableStateFlow<UIState>, and expose it as a public immutable uiState property
using asStateFlow(). Update all places where these individual fields are mutated
to instead update the corresponding field within the _uiState.value copy
operation. This ensures the UI observes a consistent, atomic state snapshot
rather than individual flows that can go out of sync.
---
Nitpick comments:
In `@common/src/main/java/org/dash/wallet/common/ui/components/TopIntro.kt`:
- Around line 33-63: Remove the entire commented-out `TopIntro` composable
function implementation. Delete all lines containing the old `TopIntro` function
definition along with its parameters (heading, text, modifier) and
implementation details including the Column layout and Text composables. This
includes any related commented code on line 70 as well. The consolidated
composable implementation should remain active while all legacy commented code
paths are completely removed to reduce code drift and review noise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4c946a5-feb4-410e-9315-1ab0a2edf815
📒 Files selected for processing (46)
.claude/agents/DEVELOPMENT-PATTERNS.md.claude/agents/figma-to-compose.mdcommon/src/main/java/org/dash/wallet/common/ui/components/ListItem.ktcommon/src/main/java/org/dash/wallet/common/ui/components/ListItemVariants.ktcommon/src/main/java/org/dash/wallet/common/ui/components/MenuItem.ktcommon/src/main/java/org/dash/wallet/common/ui/components/MyTheme.ktcommon/src/main/java/org/dash/wallet/common/ui/components/TopIntro.ktcommon/src/main/res/drawable/ic_menu_info.xmlwallet/AndroidManifest.xmlwallet/build.gradlewallet/res/drawable/ic_contact_support.xmlwallet/res/drawable/ic_contact_support_filled.xmlwallet/res/drawable/ic_rate.xmlwallet/res/drawable/ic_rate_app_filled.xmlwallet/res/drawable/ic_refresh_blue.xmlwallet/res/layout/activity_about.xmlwallet/res/layout/fragment_more.xmlwallet/res/navigation/nav_home.xmlwallet/res/values-de/strings-extra.xmlwallet/res/values-el/strings-extra.xmlwallet/res/values-es/strings-extra.xmlwallet/res/values-fa/strings-extra.xmlwallet/res/values-fil/strings-extra.xmlwallet/res/values-fr/strings-extra.xmlwallet/res/values-id/strings-extra.xmlwallet/res/values-it/strings-extra.xmlwallet/res/values-ja/strings-extra.xmlwallet/res/values-ko/strings-extra.xmlwallet/res/values-nl/strings-extra.xmlwallet/res/values-pl/strings-extra.xmlwallet/res/values-pt/strings-extra.xmlwallet/res/values-ru/strings-extra.xmlwallet/res/values-sk/strings-extra.xmlwallet/res/values-uk/strings-extra.xmlwallet/res/values-zh-rTW/strings-extra.xmlwallet/res/values-zh/strings-extra.xmlwallet/res/values/strings-extra.xmlwallet/src/de/schildbach/wallet/ui/SettingsFragment.ktwallet/src/de/schildbach/wallet/ui/more/AboutActivity.ktwallet/src/de/schildbach/wallet/ui/more/AboutFragment.ktwallet/src/de/schildbach/wallet/ui/more/AboutScreen.ktwallet/src/de/schildbach/wallet/ui/more/AboutViewModel.ktwallet/src/de/schildbach/wallet/ui/more/SettingsScreen.ktwallet/src/de/schildbach/wallet/ui/more/ToolsScreen.ktwallet/src/de/schildbach/wallet/ui/more/masternode_keys/MasternodeKeyChainScreen.ktwallet/src/de/schildbach/wallet/ui/more/masternode_keys/MasternodeKeysScreen.kt
💤 Files with no reviewable changes (9)
- wallet/res/drawable/ic_contact_support.xml
- wallet/res/layout/activity_about.xml
- wallet/src/de/schildbach/wallet/ui/more/masternode_keys/MasternodeKeyChainScreen.kt
- wallet/src/de/schildbach/wallet/ui/more/SettingsScreen.kt
- wallet/src/de/schildbach/wallet/ui/more/ToolsScreen.kt
- wallet/src/de/schildbach/wallet/ui/more/masternode_keys/MasternodeKeysScreen.kt
- wallet/res/drawable/ic_rate.xml
- wallet/src/de/schildbach/wallet/ui/more/AboutActivity.kt
- wallet/AndroidManifest.xml
Issue being fixed or feature implemented
Related PR's and Dependencies
Screenshots / Videos
How Has This Been Tested?
Checklist:
Summary by CodeRabbit