Android
Android copied to clipboard
Migration to Jetpack Compose
Catima uses a lot of "legacy" tech. Java, Android XML. It's become very obvious Google is no longer committed to these technologies and is definitely not giving them the love they need anymore.
To ensure we can use Jetpack Compose, we will have to first migrate things to Kotlin and then Compose them slowly. We can, however, do this per-activity.
Help is very much appreciated here! If there's any part you are willing to do, please help! This is a huge undertaking!
Global changes
- [ ] Get rid of custom colour support in Catima (this feature is rarely used and will add to the load of creating themes for the Compose code)
- [ ] Create a Compose theme as similar as possible to the current theme (so the Compose and non-compose parts feel similar)
Per activity changes
MainActivity
- [ ] Convert to Kotlin (#2830)
- [ ] Convert to Compose
AboutActivity
- [X] Convert to Kotlin (#2360)
- [ ] Convert to Compose (claimed by @TheLastProject in #2489)
ManageGroupsActivity
- [X] Convert to Kotlin (#2763)
- [ ] Convert to Compose
ManageGroupActivity
- [X] Convert to Kotlin (#2760)
- [ ] Convert to Compose
LoyaltyCardViewActivity
- [ ] Split into LoyaltyCardViewActivity and LoyaltyCardFullscreenViewActivity (to simplify the UI and card code)
- [ ] Convert each of the new activities to Kotlin
- [ ] Convert each of the new activities to Compose
LoyaltyCardEditActivity
- [ ] Convert to Kotlin (claimed by @davkenn, https://github.com/CatimaLoyalty/Android/issues/2483#issuecomment-3172912078)
- [ ] Convert to Compose
ScanActivity
- [X] Convert to Kotlin (#2779)
- [ ] Convert to Compose
(May be quite a challenge with the zxing code)
BarcodeSelectorActivity
- [X] Convert to Kotlin (#2585)
- [ ] Convert to Compose
preferences.SettingsActivity
- [x] Convert to Kotlin (#2744)
- [ ] Convert to Compose
(This settings activity works strangely, do we need anything else to make this work?)
ImportExportActivity
- [X] Convert to Kotlin (#2755)
- [ ] Convert to Compose
CardShortcutConfigure
- [X] Convert to Kotlin (#2585)
- [ ] Convert to Compose
UCropWrapper
- [X] Convert to Kotlin (#2782)
- [ ] Convert to Compose
(Is this viable? Consider switching library)
Marking this as good first issue because at least the Kotlin rewrites should be fairly straightforward to anyone with Kotlin knowledge.
For Compose, maybe someone can help with the testing for #2489, but compose is step 2 anyway :)
I do not recommend new contributors try to rewrite to Compose at this time (we first want to finish #2489 so we have the baseline for that).
hi i want to contribute to this
Feel free to do any of the Kotlin rewrites. Or, if you know Compose, setting up Compose tests for #2489 would be very helpful (then that can finally be finished 😃)
Could I work on converting LoyaltyCardEditActivity to Kotlin? I've been playing around with it and so far I've been fixing nullability, moving state to LoyaltyCardEditActivityViewModel and adding tests to more fully cover LoyaltyCardEditActivity. ETA: probably about a week.
Sure @davkenn, thanks a lot for taking that on. That activity is quite a beast so looking forward to seeing the result (will probably take me a while to review though) :)
I'm new to kotlin ( may be a bit too new ) can I try migrating small individual files ?
Like this class here:- https://github.com/CatimaLoyalty/Android/blob/main/app/src/main/java/protect/card_locker/ThirdPartyInfo.java
@PrathameshBhagat Sure! The activities are the most important part as Google is caring less and less for Android XML and we need Kotlin to use Jetpack Compose, but the end goal is 100% Kotlin. So rewriting other Java files is helpful too!
And here I feel for Actions if we specify even one permissions all others default to none/restricted so we can reduce the extra lines under permissions cause they are not needed anymore.
So all of this :- https://github.com/CatimaLoyalty/Android/blob/5f33679ddda1fe28ffcf583c5a3a9e3074be04f9/.github/workflows/changelog-to-fastlane.yml#L9-L22
could be just
premissions:
contents: write
pull-requests: write
All other will default to none. By default
Although I can see you have it in all your YAML files, indicating you really wanna avoid misuse, so might not consider removing it.
Refrence to Github docs
Refrence :- https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#permissions
For each of the available permissions, shown in the table below, you can assign one of the access levels: read (if applicable), write, or none. write includes read. If you specify the access for any of these permissions, all of those that are not specified are set to. none.
Might help readability, feel free to make a MR for that, but in the future please try to keep each issue on-topic :)
Sure, Am (Was) I supposed to create a new issue for this ?
@PrathameshBhagat Generally one issue per... issue is best yeah, it makes it easier to keep track of everything :)
Hi, would it be okay if I worked on the Theme part? Do you follow any folder structure conventions?
@0spol The compose theme should already be working in https://github.com/CatimaLoyalty/Android/pull/2489. However, this MR is blocked because I haven't figured out how to write proper tests for Jetpack Compose.
A lack of tests is basically the main block of Jetpack Compose adoption. As soon as we have tests, we can start migrating stuff to compose. If you can help with tests or know someone who can that would be great.
Ahh, I see.
I could try to make some test, do we need to change the test we already have to kotlin? Any suggestions?
Hi @TheLastProject i created PR (#2744) for preferences.SettingsActivity for kotlin migration
@0spol sorry, I missed your message, my mailbox kinda exploded the day Hacktoberfest started, I am still not done with all the notifications I got 😅
Changing the tests is not required, but would be nice. The main priority is the app itself, as the Kotlin rewrite is needed for Jetpack compose :)
@TheLastProject Np, hope you had fun in Hacktoberfest. I will try to help with the migration 👍🏻
@TheLastProject , thz for last PR.
Here i create a new PR(#2755) for Convert ImportExportActivity to Kotlin
@TheLastProject Would it be okay if my group and I work on converting the ManageGroupActivity classes to Kotlin?
Hello. Would we be able to work on converting ManageGroupsActivity?
@u7683648 @Henry-ANU Sorry for the late responses, very busy days (so I'm quite behind on messages). It seems someone else already picked those up (still need to review), so I guess those aren't really helpful anymore. But feel free to work on any Java file you find, no need to really ask permission (I understand wanting to announce it, but I guess it doesn't really stop others from working on the same files anyway)
@TheLastProject #2830
- [ ]