Implemented Account Filters in Bottom Sheet (Saving Account, Loan Account, Share Account) #2994
Conversation
WalkthroughThis PR centralizes filter expansion state into AccountsState with new toggle actions, moves SavingsAccountFilters from a dialog to a MifosBottomSheet, adds two localized filter labels, and replaces an explicit white bottom sheet container color with the MaterialTheme surface color. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as AccountsScreen
participant VM as AccountsViewModel
participant State as AccountsState
rect rgb(220,235,255)
note left of UI: Prior (local state)
User->>UI: Tap filter header
UI->>UI: Toggle local rememberSaveable flag
UI->>UI: Recompose
end
rect rgb(220,255,225)
note left of VM: Now (centralized state)
User->>UI: Tap filter header
UI->>VM: Dispatch ToggleTypeExpanded / ToggleStatusExpanded
VM->>VM: Toggle flag
VM->>State: Emit updated state
State->>UI: New state observed
UI->>UI: Recompose with updated expansion
end
rect rgb(255,245,210)
note left of UI: Presentation change
UI->>UI: Render SavingsAccountFilters inside MifosBottomSheet
UI->>UI: Resolve labels via stringResource(...)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 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. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/component/MifosBottomSheet.kt (1)
57-57: Use theme colors instead of hardcoded Color.Black.Hardcoding
Color.Blackdoesn't respect the app's theme and will appear the same in both light and dark modes. Consider usingMaterialTheme.colorScheme.surfaceor another appropriate semantic color from the theme to ensure proper appearance across different theme configurations.Apply this diff to use a theme-based color:
- containerColor = Color.Black, + containerColor = MaterialTheme.colorScheme.surface,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cmp-android/prodRelease-badging.txt(3 hunks)core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/component/MifosBottomSheet.kt(1 hunks)feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accounts/AccountsScreen.kt(4 hunks)feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accounts/AccountsViewModel.kt(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accounts/AccountsScreen.kt (3)
core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/component/MifosBottomSheet.kt (1)
MifosBottomSheet(31-68)core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/component/FiterTopSection.kt (1)
FilterTopSection(33-125)feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/component/FilterSection.kt (1)
FilterSection(61-159)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR Checks KMP / Static Analysis Check
🔇 Additional comments (3)
feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accounts/AccountsViewModel.kt (1)
86-87: LGTM! Clean state management implementation.The expansion state management is well-structured:
- Actions are properly defined in the sealed interface
- Handlers follow the established pattern
- State fields have sensible defaults (expanded by default)
- Documentation is clear and helpful
Also applies to: 102-114, 250-251, 279-280
feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accounts/AccountsScreen.kt (1)
197-226: Bottom sheet refactor looks good.The migration from dialog-based filters to bottom sheet presentation is well-executed:
- State management properly moved to ViewModel
- Layout is clean and scrollable
- Actions are dispatched correctly for expansion toggles
- Component reuse (FilterSection) follows DRY principles
Note: The
AccountScreenContentsignature no longer accepts amodifierparameter. Since this is an internal API, this change is acceptable and provides consistency in the bottom bar presentation.cmp-android/prodRelease-badging.txt (1)
1-126: Auto-generated manifest metadata - no concerns.This file appears to be auto-generated Android manifest metadata (aapt badging output). The changes reflect standard build configuration updates including version bumps, activity name changes, and locale additions. No review action needed.
.../accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accounts/AccountsScreen.kt
Outdated
Show resolved
Hide resolved
Screen_recording_20251101_065843.webm |
|
Loan Account, Savings Account And Share Account All are Done in this one pr. |
WizCoderr
changed the title
...ystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/component/MifosBottomSheet.kt
Show resolved
Hide resolved
2 participants
Fixes - Jira-#MM-449
Fixes - Jira-#MM-456
Fixes - Jira-#MM-457
Didn't create a Jira ticket, click here to create new.
Please Add Screenshots If there are any UI changes.
Screen_recording_20251101_062243.webm
Please make sure these boxes are checked before submitting your pull request - thanks!
Run the static analysis check
./gradlew checkorci-prepush.shto make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.
Summary by CodeRabbit
New Features
Refactor
Style