Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[enhancement]: Migrate codebase to jetpack compose #5422

Open
itsPronay opened this issue Jun 7, 2024 · 6 comments
Open

[enhancement]: Migrate codebase to jetpack compose #5422

itsPronay opened this issue Jun 7, 2024 · 6 comments
Labels
enhancement End user-perceivable enhancements. Impact: Low Low perceived user impact (e.g. edge cases). Work: High It's not clear what the solution is.

Comments

@itsPronay
Copy link

itsPronay commented Jun 7, 2024

Is your feature request related to a problem? Please describe.

Migrate codebase to jetpack compose.
Jetpack Compose is the modern toolkit for building native Android UIs. By adopting Compose, Kiwix can simplify UI development, improve maintainability, and ensure the app aligns with the latest Android development practices.

Describe the solution you'd like

Migration can be carried out in 2 phrases,
In the first migration, we can migrate screens, and then use them with views
In the second migration, we can remove Fragments and Navigation components

To get a full idea of how the migration will take place please view official docs on Migration strategy

image

Describe alternatives you've considered

No response

Additional context

Benefits of Jetpack Compose

  • Simplified UI Development: Compose allows developers to write less code, reducing boilerplate and making the UI code more intuitive and easier to maintain.

  • Improved Performance: Compose is designed to be more efficient, which can result in better app performance and responsiveness.

  • Better Integration with Kotlin: Compose is fully Kotlin-based, providing a seamless development experience with Kotlin features like coroutines and flows.

  • Future-Proof: Google is actively developing and promoting Compose as the future of Android development, ensuring long-term support and innovation.

@itsPronay itsPronay added enhancement End user-perceivable enhancements. triage needed labels Jun 7, 2024
@itsPronay
Copy link
Author

@BenHenning Hey, what do you think about migrating codebase to jetpack compose ? since everyone is adopting it.

@adhiamboperes adhiamboperes added Impact: Low Low perceived user impact (e.g. edge cases). Work: Low Solution is clear and broken into good-first-issue-sized chunks. and removed triage needed labels Jun 11, 2024
@itsPronay itsPronay changed the title [Feature Request]: Migrate codebase to jetpack compose [enhancement]: Migrate codebase to jetpack compose Jun 12, 2024
@BenHenning
Copy link
Sponsor Member

Thanks for filing this @itsPronay! This has been on our list for many years, but we never actually filed an issue, so this is a great one to use as the main feature request.

At a high-level, we definitely want to move to Jetpack Compose in the long-term. Besides the benefits you listed, Jetpack Compose has very specific benefits that I think we're interested in:

  • It's a more stable replacement for Android databinding.
  • It consolidates code to a single language for most of the app (Kotlin).
  • It has a much smaller coding footprint over the much-more-verbose XML.

However, there are some cascading challenges that I see with migrating to Compose:

  • We've noticed some compatibility issues with newer Compose libraries and the old version of AGP that we use. For reasons a bit too detailed to go into here, migrating to a newer version of AGP is very likely not an option. This means we'll likely need to finish resolving Migrate from Gradle to Bazel #59 before we can dedicate an effort to moving to Jetpack Compose.
  • Since this migration would be touching the entire app UI, we would want to ensure that we can effectively verify the changes. Since it's so visually heavy, we'd very much likely want screenshot testing in place (Intoduce screenshot-testing framework #1815).
  • Screenshot testing itself will likely require some heavier test infrastructure changes to get Espresso consistently working in the Bazel world, including parts of Implement strategy to ensure test device breadth & testing consistency between Robolectric & Espresso #2047 and probably some more Bazel-specific elements.

To summarize, both the migration and prepping for it will be a large undertaking. :) However, we are interested in doing this. In fact, we're prototyping our first UI using Compose in #5344 but we'll probably not go past that new UI for now without more of the above addressed.

In terms of timeline, I can't really say when we'll be able to start on these efforts as it greatly depends on contributor availability and willingness. At the moment, we're planning on finishing the Bazel migration in completion this year, but there aren't yet plans for screenshot testing or test uniformity fixing.

@adhiamboperes adhiamboperes added Work: High It's not clear what the solution is. and removed Work: Low Solution is clear and broken into good-first-issue-sized chunks. labels Jun 20, 2024
adhiamboperes added a commit that referenced this issue Jun 30, 2024
… Classroom List Screen with Jetpack Compose (#5437)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

Fixes part of #5344
Fixes part of #5422
Fixes part of #5012

This PR introduces the Classroom List Screen, which will replace the
existing Home Screen. The new screen features a classroom carousel that
remains sticky when the screen is scrolled. Various approaches were
considered for implementing the sticky header, as detailed in [Decision
3: How to implement the sticky classroom
carousel?](https://docs.google.com/document/d/1wGnoDS-8zHS5QSHufvTf9SXKi8N3lodirs51Xnehdvo/edit#heading=h.fv51u13soqp4).
Ultimately, it was decided to use the `stickyHeader` component of
Jetpack Compose.

- **Introduce Jetpack Compose** - Sets the groundwork for migrating the
codebase to Jetpack Compose. The Classroom List Screen uses a
`ComposeView` to host the composable components. Appropriate tests have
been set up to verify the functionality.
- **Migrate away from `onActivityResult`** - Deprecates the use of
`onActivityResult` and transitions to using `ActivityResultContracts`
for handling activity results.
- **Initiate Deprecation of Android KitKat** - The selected versions of
Jetpack Compose dependencies require the `minSdkVersion` to be atleast
21. This PR initiates the process of deprecating support for Android 19
(KitKat).


https://github.com/oppia/oppia-android/assets/84731134/2a2145e5-5bd0-4d80-9741-ff6baf20fd12

## Screenshots
### Phone Light Mode
|Potrait|Landscape|
|--|--|

|![image](https://github.com/oppia/oppia-android/assets/84731134/367fe033-833a-4c44-b9ae-05a79c872271)|![image](https://github.com/oppia/oppia-android/assets/84731134/771f3681-884b-45d4-91af-880b775a5937)|

|![image](https://github.com/oppia/oppia-android/assets/84731134/921a136c-d84c-479f-a7e6-f40aa55e9b5b)|![image](https://github.com/oppia/oppia-android/assets/84731134/8783f5f8-3b09-4899-b286-09f1d9cfae2f)|

### Phone Dark Mode
|Potrait|Landscape|
|--|--|

|![image](https://github.com/oppia/oppia-android/assets/84731134/2fd50b98-cd5e-45bb-aeff-8690886ddf28)|![image](https://github.com/oppia/oppia-android/assets/84731134/64053fd1-0c22-414a-9cbf-b6fcb615fe38)|

|![image](https://github.com/oppia/oppia-android/assets/84731134/2d2160ba-c886-48c7-8832-f1b8327e4daa)|![image](https://github.com/oppia/oppia-android/assets/84731134/7c3974a0-0c4a-4de2-a39b-2239f27bdd64)|

### Tablet Light Mode
|Potrait|Landscape|
|--|--|

|![image](https://github.com/oppia/oppia-android/assets/84731134/459907ce-4bdf-44dd-a84b-e9e50baaa53b)|![image](https://github.com/oppia/oppia-android/assets/84731134/fc3076de-35bf-493e-8b88-e74658285f61)|

||![image](https://github.com/oppia/oppia-android/assets/84731134/34d34a58-e173-488b-9ba4-5de28c2d1342)|

### Tablet Dark Mode
|Potrait|Landscape|
|--|--|

|![image](https://github.com/oppia/oppia-android/assets/84731134/8eff1354-9faa-4717-93df-ae643db0131b)|![image](https://github.com/oppia/oppia-android/assets/84731134/1fe8232d-399f-4268-a889-854e9680529c)|

||![image](https://github.com/oppia/oppia-android/assets/84731134/c8b09dfe-00d9-4d1b-a991-dd2c049d4de1)|

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

---------

Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>
@adhiamboperes
Copy link
Collaborator

Just to add, aside from AGP compatibility issues, I ran into Glide compatibility issues as well. Since we heavily rely on Glide for image loading, I checked the Glide Compose Dependency, and it requires Kotlin 1.7.0, and it is not yet stable.

@BenHenning
Copy link
Sponsor Member

I think at this point we definitely want to continue migrating the codebase to Jetpack Compose, but I don't think we're ready to actually commit to starting a migration quite yet. Beginning a migration without getting the proper arrangements in place is likely going to slow down development and complicate testing in the long run. To try and minimize the impact this migration has on the broader team, I think we should freeze Compose support at its current place (the home screen experience) and work toward preparing for a broader migration before any other code is updated.

To that end, I think the following needs to be done before we can actually begin to migrate the codebase:

  • Gradle needs to be removed (Move off of Gradle [Blocked: #2746] #2747 and Migrate from Gradle to Bazel #59). We're very unlikely to comfortably migrate to Compose with such an old version of Gradle, AGP, and Android Studio.
  • A migration plan needs to be drafted, reviewed, and approved that covers all of the following:
    • A code guide detailing how to migrate each of the low-level components (e.g. recycler views, text views, layouts, etc.) from XML to Compose.
    • A plan breaking down all of the affected screens and files that will need to be migrated, and an order in which they should be migrated.
    • A plan for how to safely stage this migration work behind a feature flag so that the changes can be tested.
    • A plan for how to manage the complexity of features being implemented in both XML and Compose during the testing phase.
    • A plan for launching the new functionality, including identifying a list of metrics to monitor during production to ensure no major user experience degradation occurs when the feature is being enabled for users.
    • A plan for cleaning up the old XML code once we've shipped and finalized the work, and removing all databinding artifacts and configurations.
    • A list of risks and mitigations, including long-term considerations moving to Compose and any changes that we may need to do for testing.
    • Any infrastructural changes needed (such as when interacting with data providers).
  • A designated pair (lead and co-lead) of contributors who can start organizing an effort around this migration, including filing detailed issues for each of the constituent pieces and reviewing PRs as they come in.
  • All accompanying wiki documentation for writing new features and migrating existing code should be written and checked in.

Unlike most projects, this one is likely to have a schedule around it once we are finished with planning and begin actual coding work. A migration like this can have significant impacts on the day-to-day development experience, and we can't pause bug fixes or feature work while this is happening. Thus, it's especially important to have a clear plan in place, and a planned schedule around completing it so that the project doesn't operate in perpetuity.

While the actual timeline will be highly dependent on the final plan, I expect that the project will take at minimum 1 full calendar year from start to finish for code completion, and 1-2 more quarters to actually test, finalize, and ship the code. Another 1-2 quarters should be considered for unanticipated complexities, and for cleaning up code post-migration.

@pratistha-05
Copy link

Hey @BenHenning I'm interested in migrating more code to jetpack compose, be it screen wise or component wise. Do we have any draft/schedule in place so that we can have at least arrangements ready? Is anyone interested to collaborate for the same?

@adhiamboperes
Copy link
Collaborator

Hi @pratistha-05, as per previous comments, we are still working on getting started on this. Meanwhile, you can get started with somw of our starter issues here: https://github.com/oppia/oppia-android/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement End user-perceivable enhancements. Impact: Low Low perceived user impact (e.g. edge cases). Work: High It's not clear what the solution is.
Development

No branches or pull requests

4 participants