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

Deprecate support for Android KitKat #5012

Open
seanlip opened this issue Jun 4, 2023 · 3 comments
Open

Deprecate support for Android KitKat #5012

seanlip opened this issue Jun 4, 2023 · 3 comments
Labels
enhancement End user-perceivable enhancements. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.

Comments

@seanlip
Copy link
Member

seanlip commented Jun 4, 2023

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

We have decided not to pursue support for Android KitKat. This was an aspirational goal that we had wanted to attempt because we knew of tablets owned by some partners that still run on KitKat, but as the years go by we expect that the number of such devices will dwindle.

Additionally, there are significant barriers to supporting KitKat, due to this requiring third-party dependencies that cannot be upgraded and have security issues. So we have decided to not attempt to support it, and remove any extra code that is special-cased for KitKat in order to simplify the codebase and development experience.

Describe the solution you'd like

Remove any code in the Android codebase that is there only to provide support for KitKat.

Describe alternatives you've considered
We could leave the KitKat code in, but this does not seem useful since we aren't planning to support it in the long run. It seems better to clean up the code in order to reduce the maintenance burden and make it easier for developers to follow it.

@seanlip seanlip added the enhancement End user-perceivable enhancements. label Jun 4, 2023
@MohitGupta121 MohitGupta121 added Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. labels Jun 16, 2023
@MohitGupta121 MohitGupta121 added Work: High It's not clear what the solution is. and removed Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. labels Jun 27, 2023
@theMr17
Copy link
Collaborator

theMr17 commented Sep 30, 2023

Hi @adhiamboperes, I would like to work on this issue. My approach to solve this will be as following:

  • Search the codebase for any checks for KITKAT and remove any unnecessary code.
  • Search the codebase for any checks for Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP or Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP and modify the code accordingly.
  • Upgrade the minSdkVersion to 21.

Please let me know, if anything else needs to be done.

@adhiamboperes
Copy link
Collaborator

Hi @adhiamboperes, I would like to work on this issue. My approach to solve this will be as following:

  • Search the codebase for any checks for KITKAT and remove any unnecessary code.
  • Search the codebase for any checks for Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP or Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP and modify the code accordingly.
  • Upgrade the minSdkVersion to 21.

Please let me know, if anything else needs to be done.

@the-mr17 I don't recommend directly working on this issue at the moment since there are blocking issues that have not been addressed, e.g when you search for existing usages of the tems you mention, you will see TODOs like #4751, which need to be fixed first.

adhiamboperes added a commit that referenced this issue Jun 12, 2024
## Explanation
Fixes #4119
Fixes #4120
Fixes part of #59

This PR finishes the migration of the codebase to Kotlin 1.6 (addressing
both #4119 and #4120).

Kotlin 1.6 is needed as part of moving rules_kotlin to 1.7.x (which is,
in turn, needed in conjunction with Bazel 6.x to enable strict
dependency checking which significantly simplifies modularization which
is planned for downstream PRs). This PR doesn't actually finish the
movement to that version of rules_kotlin, but it does finish moving the
codebase to a new enough (and no longer pre-release) version of
rules_kotlin to allow using Kotlin 1.6 (over Kotlin 1.4 that the
codebase currently uses): version 1.5.0.

Previous PRs (#5400 and #5402) prepared for the changes here by
addressing large categories of build warnings that have either arisen
from this migration, or from past work. Note that another large category
of warnings have also been addressed in this PR: by moving to Kotlin
1.6, there's no longer a runtime incompatibility between the Kotlin SDK
and the reflection APIs (which was causing a _lot_ of warning output
previously). Between all three PRs, the output is now very clean and
free of nearly all build warnings.

To try and keep the warnings clean long-term, this PR introduces
warnings-as-errors for both Java and Kotlin code. However, please note
some caveats:
- Dagger generated code doesn't follow the Java warnings-as-error flag,
so those warnings were cleaned up manually (and will need to be
generally watched for, unfortunately).
- The version of rules_kotlin used in this PR doesn't directly support
turning on the functionality, but does internally (so a small patch file
has been added to augment rules_kotlin). When the codebase is updated to
rules_kotlin 1.7.x this patch will no longer be needed.
- To ease development, a build configuration flag was added to disable
failure upon encountering build warnings (per
https://bazel.build/run/bazelrc and
https://bazel.build/docs/configurable-attributes#query-and-cquery as an
example), though this needs to be opted into:
  ```sh
  bazel build --config=ignore_build_warnings <target>
  ```

Some other details to note:
- Version 1.6.10 is specifically picked in order to ensure Jetpack
Compose compatibility (for preparation of the work being prototyped in
#5401 to be compatible with the Oppia Android build environment).
- The vast majority of code in this PR is updating parameterized tests
to use a cleaner repeatable annotation pattern that wasn't available in
Kotlin 1.4.
- This upgrade absolutely does have runtime implications, but we're
relying very heavily on existing automated tests to ensure correctness
and no regressions.
- This PR doesn't make an effort to move toward newer Kotlin language
features except where forced (API deprecations) or largely wanted (the
repeatable annotation change).
- android-spotlight and kotlitex have been updated to support newer
versions of Kotlin (as both are custom forks managed in the broader
Oppia GitHub organization).
- Gradle files have been updated to match the same dependency versions
as Bazel (where it was obvious to make changes; some might still be a
bit off).
- The Gradle build configuration was also updated to use Kotlin 1.6.x
(otherwise there would be build incompatibilities with Bazel). I think
this is the last upgrade we can do for Gradle without upgrading AGP
(which will cause us significant issues with the model module, so we're
planning on instead dropping Gradle support).
- API changes that needed to be addressed in this PR due to deprecations
include: ``String.captialize``, ``String.toLowerCase``,
``String.toUpperCase``, ``SendChannel.offer``, and ``Char.toInt``.
- New API changes that have been leveraged in this PR:
``Flow.lastOrNull`` and ``Deferred.asListenableFuture`` (to replace
``SettableFuture`` for safety; this also resulted in nice
simplifications in ``CoroutineExecutorService``).
- The JVM coroutines dependency needed to be split out from Maven and
manually imported with some empty internal Java class files since it
otherwise has some issues being desugared:
bazelbuild/bazel#13553. This is a problem with
the Desugarer used in Bazel 4.x (and maybe later versions, so this
solution will probably need to kept for a while).
- Some Proguard rule updates were needed due to Kotlin SDK changes--see
the Proguard file & comments for specifics.
- Due to dependency changes, the KitKat main dex file was also trimmed
down. I'm fairly certain that it's already crashing on startup, so I
don't care much about this change--it just needs to build. We plan to
remove KitKat entirely eventually, anyway: #5012.
- Jetifier (that is, automatic conversion from support libraries to
Jetpack/AndroidX) support was disabled in Gradle. We don't have it
enabled in Bazel, and it could potentially encourage strange one-version
violations if it was ever actually needed. This is a safer (and likely
more performant) change to make.
- Moshi was updated to 1.13 to support the upgrade in Kotlin. This did
result in a small configuration change due to its annotation processor
being moved. Note that Moshi 1.14 couldn't be supported since it
requires Kotlin 1.7+ which requires rules_kotlin 1.7+. This will be an
option to upgrade in the future.
- Some improvements and fixes were made in
``FilterPerLanguageResources`` (I think it was outputting something
incorrectly before and that's now been fixed as part of a broader
logical reworking of the filtering logic).
- ``com.android.support:support-annotation`` was removed as a dependency
since it was never used in Bazel, and shouldn't be used (since it's
support library and not AndroidX).
- The updates to Moshi and Kotlin dependencies resulted in a bunch of
other transitive dependency updates.
- Note that Gradle doesn't have ``allWarningsAsErrors`` enabled since it
would require fixing more warnings than is exposed in Bazel, and we're
using Bazel builds as the general source of truth for code quality.

## Essential Checklist
- [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
N/A -- This is an infrastructural change. While it could inadvertently
affect user-facing code, it shouldn't based on the current passing state
of automated tests.

---------

Co-authored-by: Adhiambo Peres <[email protected]>
Co-authored-by: Sean Lip <[email protected]>
@adhiamboperes adhiamboperes added Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. and removed Work: High It's not clear what the solution is. labels Jun 26, 2024
@adhiamboperes
Copy link
Collaborator

adhiamboperes commented Jun 26, 2024

We need to prioritize this issue since #5437 raises minSdkVersion to 21, due to

Error: Exception in thread "main" com.google.devtools.build.android.AndroidManifestProcessor$ManifestProcessingException: Manifest merger failed : uses-sdk:minSdkVersion 19 cannot be smaller than version 21 declared in library [@maven//:androidx_activity_activity_compose] /tmp/manifest_merge_tmp9428924506616588346/AndroidManifest17206523561529774253.xml
	Suggestion: use tools:overrideLibrary="androidx.activity.compose" to force usage
	at com.google.devtools.build.android.AndroidManifestProcessor.mergeManifest(AndroidManifestProcessor.java:186)
	at com.google.devtools.build.android.ManifestMergerAction.main(ManifestMergerAction.java:228)
	at com.google.devtools.build.android.ResourceProcessorBusyBox$Tool$5.call(ResourceProcessorBusyBox.java:97)
	at com.google.devtools.build.android.ResourceProcessorBusyBox.processRequest(ResourceProcessorBusyBox.java:251)
	at com.google.devtools.build.android.ResourceProcessorBusyBox.main(ResourceProcessorBusyBox.java:181)
Target //:oppia_dev_kitkat failed to build

And #5442

@kkmurerwa, could you PTAL?

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 <[email protected]>
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: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.
Development

No branches or pull requests

6 participants