feat: migrate ConfigureScreen to Jetpack Compose in purchase tester module#2991
feat: migrate ConfigureScreen to Jetpack Compose in purchase tester module#2991nimi0112 wants to merge 4 commits intoRevenueCat:mainfrom
Conversation
nimi0112
force-pushed
the
feature/compose-migrate-configure-screen
branch
from
f3faef4 to
21f04cf
Compare
JayShortway
left a comment
JayShortway
left a comment
There was a problem hiding this comment.
Hey thanks for your contribution! Lots of good stuff in here. Had some initial comments.
| import com.revenuecat.purchasetester.ui.screens.configure.ConfigureScreen | ||
| import com.revenuecat.purchasetester.ui.theme.PurchaseTesterTheme | ||
|
|
||
| class ConfigureActivity : ComponentActivity() { |
There was a problem hiding this comment.
Instead of adding a ConfigureActivity, could you make ConfigureFragment host the new composable ConfigureScreen? That way we can migrate each fragment separately without having to add activities.
There was a problem hiding this comment.
Hi @JayShortway, thanks a lot for the review.
I can consider the Fragment wrapper approach, but here is the current scenerio:
Current approach:
• ConfigureFragment has been completely removed and replaced with a pure Compose implementation
• ConfigureActivity serves as the entry point (LAUNCHER intent) while other screens remain in MainActivity
If we go with the Fragment wrapper approach:
Creating a ConfigureFragment that hosts ComposeView would require:
- Recreating the Fragment I just deleted
- Adding temporary scaffolding (Fragment lifecycle wrapping Compose)
- A second cleanup phase to remove the wrapper after all screens are migrated
This leads to unnecessary rework - writing code we know will be deleted soon.
Migration plan:
I'm migrating all 6 remaining fragments (Login, Overview, Offering, Logs, etc.) to Compose following the same pattern. Once complete, I'll consolidate into a single
ComponentActivity with Compose Navigation and remove MainActivity entirely.
Does this approach work, or is there a strong preference for the incremental wrapper pattern?
There was a problem hiding this comment.
So what's the plan with ConfigureActivity? Wouldn't that need to be deleted too?
In this in-between phase we'd have 2 launcher activities, if I understand you correctly. That's going to be confusing for whoever is using this app.
There was a problem hiding this comment.
Hi @JayShortway , I think there might be some confusion about the architecture which I might not be able to explain clearly, my bad. Apologies for that. Let me try to clarify the intent and migration plan more explicitly.
Current state:
ConfigureActivityis now the LAUNCHERMainActivityis NOT a launcher anymore (only accessible via navigation)
Migration path:
ConfigureActivitystays and grows (adds more Compose screens)MainActivityshrinks and gets deleted (as Fragment screens are migrated)
End state:
- A single ConfigureActivity hosting all Compose screens
- Navigation handled entirely via Compose Navigation
- MainActivity deleted entirely
nav_graph.xmldeleted (replaced by Compose Navigation)
Why this approach:
- ConfigureActivity is permanent, not temporary
- No need to recreate ConfigureFragment (which I just deleted)
- No intermediate Fragment wrappers that would need cleanup later
- Clean migration: Fragment screens move from MainActivity → ConfigureActivity one by one.
That said, this is just my proposed approach based on how I see the migration evolving. I’m completely open to feedback, and of course the final decision is yours. Happy to adjust this if you feel there’s a better or simpler direction to take.
...src/main/java/com/revenuecat/purchasetester/ui/screens/configure/ConfigureScreenViewModel.kt
Outdated
Show resolved
Hide resolved
nimi0112
force-pushed
the
feature/compose-migrate-configure-screen
branch
from
21f04cf to
42b2d6f
Compare
nimi0112
force-pushed
the
feature/compose-migrate-configure-screen
branch
from
f0901f6 to
380b3af
Compare
|
Hi @JayShortway @fire-at-will @tonidero
If these changes looks good. I can also update the configure screen with respect to the changes done in the samsung-dev branch with regards to supporting Galaxy Store in the Purchase Tester App. |
…odule Migrate ConfigureScreen to Jetpack Compose - Add Compose theme system (Color, Type, Theme) - Create ConfigureActivity as Compose entry point - Add Compose dependencies to build config - Add test cases for ConfigureScreenViewModel
Refactor ConfigureScreenViewModelImpl to use a reactive flow-based approach: - Remove init block and loadData() method to eliminate side effects - Transform _state from MutableStateFlow to derived StateFlow using combine() - Add userEdits flow to track user modifications before persistence - Use SharingStarted.WhileSubscribed() for lazy, lifecycle-aware data loading - Extract subscription timeout as SUBSCRIPTION_TIMEOUT_MILLIS constant Benefits: - No side effects in constructor/init - Data loads only when UI observes the state - Better testability and lifecycle awareness - Automatic cleanup when no observers - Follows modern Android ViewModel best practices All existing tests pass without modifications.
nimi0112
force-pushed
the
feature/compose-migrate-configure-screen
branch
from
380b3af to
908e387
Compare
|
Hey @JayShortway @skydoves I’d appreciate any feedback. If this contribution aligns with your roadmap, I’m happy to iterate further. Otherwise, please let me know the next steps or if there are any planned changes in this area. Thanks |
2 participants
Migrate ConfigureScreen to Jetpack Compose
Checklist
purchases-iosand hybridsMotivation
This PR modernizes the purchase-tester UI to Jetpack Compose for better maintainability and consistency with the paywall-tester module. This is the first step in migrating from Fragments/XML to Compose, which reduces boilerplate and improves the developer experience.
Description
• Migrated ConfigureFragment to ConfigureScreen composable with ViewModel and state management
• Created Compose theme system (Color, Type, Theme) following Material3 patterns
• Added ConfigureActivity as the Compose entry point
• Updated build.gradle.kts with Compose dependencies (BOM, UI, Material3, Navigation Compose, etc.)
• Added test cases for ConfigureScreenViewModel
• Maintained feature parity with the original Fragment implementation
I have verified the behaviour by comparing it with the previous version of the app, especially around input validation and navigation flows. The functionality matches the existing implementation based on my testing.
I’ve also attached a comparison video (new vs. old) for easier review. Please let me know if you'd like me to validate additional edge cases or specific scenarios.
I have also worked on migrating other screens as well, but I wanted to get feedback on this screen first before proceeding further.
Comparison.Edit.mp4