Migrated the whole app to Jetpack compose #281
Conversation
…f settings screen
Thank you for the pull request!The Scribe team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :) If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Android rooms once you're in. Also consider joining our bi-weekly Saturday dev syncs. It'd be great to have you! Maintainer checklist |
|
Will surely make the necessary changes @andrewtavis :) |
|
@angrezichatterbox @andrewtavis if possible then can you please share some screenshots of the bugs/inconsistencies as it is working all fine on my debugging machine it would help me a lot :-) |
# Conflicts: # app/src/main/java/be/scri/adapters/ViewPagerAdapter.kt # app/src/main/java/be/scri/fragments/AboutFragment.kt # app/src/main/java/be/scri/fragments/LanguageSettingsFragment.kt # app/src/main/java/be/scri/fragments/MainFragment.kt # app/src/main/java/be/scri/fragments/PrivacyPolicyFragment.kt # app/src/main/java/be/scri/fragments/ScribeFragment.kt # app/src/main/java/be/scri/fragments/SettingsFragment.kt # app/src/main/java/be/scri/fragments/ThirdPartyFragment.kt # app/src/main/java/be/scri/fragments/WikimediaScribeFragment.kt # app/src/main/java/be/scri/ui/screens/AboutScreen.kt # app/src/main/java/be/scri/ui/screens/SettingsScreen.kt
|
I'll let @angrezichatterbox send along the screenshots. In the meantime, would you be able to check the linting errors and fix those? :) Seems to be some indentation issues. |
|
Wishing you a happy new year ✨🎉@andrewtavis @angrezichatterbox I've implemented the requested changes—please let me know if there's anything else to address! |
|
Happy new year to you too, @Saifuddin53! 🎉 Thanks for the changes and looking forward to the review 😊 |
|
Happy new year @Saifuddin53 In Medium Phone API 35 and Pixel 9 Pro the screen looks like this. 
The navigation bar seems to be like this for some reason. 
Could u fix the dark mode toggle to be like the usual toggle instead of the one you have used. 
The entire page seems to have centered a bit. Also the individual page app bar title should have a little top padding. |
|
Thanks so much for the detail in your review, @angrezichatterbox! |
|
Thanks a lot @angrezichatterbox for sharing the screenshots, this will help me a lot in solving the inconsistencies 🫡 |
|
@angrezichatterbox I have made the required changes ready for review :) |
There was a problem hiding this comment.
All looks Great @Saifuddin53. Just a few changes and we would be good for merging.
| .testTag("backgroundContainer") | ||
| .verticalScroll(scrollState), | ||
| horizontalAlignment = Alignment.CenterHorizontally, | ||
| verticalArrangement = Arrangement.Center, |
There was a problem hiding this comment.
Could you remove the verticalArrangement and add a small top padding to the Image. Currently this is how the Installation Page looks for Scribe-iOS.
| @@ -1,251 +0,0 @@ | |||
| /** | |||
There was a problem hiding this comment.
Could you add this LICENSE TEXT to the SettingsScreen.kt that you have created. Could you check the same for the other pages as well. If it was already existing was it removed.
| contentDescription = "Keyboard", | ||
| modifier = | ||
| Modifier | ||
| .clip(MaterialTheme.shapes.medium) |
There was a problem hiding this comment.
Could you remove this clip. I don't think it would be necessary. It add a circle to the Installation icon.
|
@angrezichatterbox pushed the review changes |
|
Everything looks good to me now. Thanks for the quick changes @Saifuddin53 |
|
Feel free to accept and merge this, @angrezichatterbox, or I can get to it tonight after work as well :) |
Contributor checklist
./gradlew lintKotlin detekt testcommand as directed in the testing section of the contributing guideDescription
Complete Migration to Jetpack Compose
Removed all fragment dependencies.
Ensured composables are fully independent of XML.
Replaced the traditional ViewPager with Jetpack Compose's Pager layout.
Migrated all individual pages from fragments to composables.
Adopted Material theming specifically for managing colors across the app.
Replaced fragment-based navigation with Jetpack Navigation.
Designed a new navigation graph for seamless navigation within the app.
Added a custom dark mode switch, controlled via shared preferences.
Made dark mode independent of system settings for a better user experience.
Improved the settings screen to dynamically update the list of installed keyboards whenever a change occurs.
Refined layouts and components to ensure consistency across the app.
Adjusted design elements to align with a unified theme.
Installation Page Responsiveness Fix
Screenshots
Before
before.mp4
After
after.mp4
Manual Testing
Automated Test Status
Currently, two tests are failing:
- Run Coverage
- Tests for XML-based Screens
These tests are for the XML-based screens migrated to Jetpack Compose. Addressing these failures will require updating or rewriting the tests, which can be done in a subsequent PR.
Related issue
Additional notes