4159 migrate help fragment to compose #4217

SOUMEN-PAL · 2025-02-07T17:28:58Z

Fixes #4159

📌 Summary

Migrated HelpFragment from XML-based UI to Jetpack Compose.

🔄 Changes Made

Created new Composable components
- Added HomeScreen.kt and HomeScreenItem.kt to handle the UI logic in Jetpack Compose.
Updated HelpFragment.kt
- Converted HelpFragment to return a Composable UI instead of using View Binding.
Updated dependencies
- Added necessary Jetpack Compose dependencies and versions in Libs.kt and Versions.kt in buildSrc.
Handled Navigation
- Introduced a variable for navigation, which is overridden in KiwixHelpFragment.kt, as the host fragment ID is required for Composable-to-Fragment and vice versa navigation.

🖼️ Screenshots / GIFs

SOUMEN-PAL · 2025-02-07T17:33:51Z

@MohitMaliFtechiz sir I would like to put a thing in notice that whenever i sync the gradel using sync now after sync some string.xml files are automatically changed or updated. I am not able to know why.

The above image shows before sync and after sync status of the files.

MohitMaliFtechiz · 2025-02-08T06:30:46Z

@SOUMEN-PAL This is due to our gradle task, which we have added to fix #3176. Please do not include this automate change in commit.

kiwix-android/app/build.gradle.kts

Line 144 in 687fbe0

task("renameTarakFile") {

kelson42 · 2025-02-16T08:43:25Z

@SOUMEN-PAL Any update?

SOUMEN-PAL · 2025-02-18T05:34:32Z

@SOUMEN-PAL Any update?

The ui is working I ensured the version and gradel dependencies the only problem is the strings that are getting updated in gradel sync and I am not able to fix the changes the jetpack compose code is working seemlesly. I am not able to figure out the Tarak file thing

MohitMaliFtechiz · 2025-02-18T05:50:54Z

The ui is working I ensured the version and gradel dependencies the only problem is the strings that are getting updated in gradel sync and I am not able to fix the changes the jetpack compose code is working seemlesly. I am not able to figure out the Tarak file thing

@SOUMEN-PAL Apart from the tarask file-related issue, the PR is ready for review? I will have a look at the tarask file-related issue and will push the fix for this.

SOUMEN-PAL · 2025-02-18T05:52:10Z

The ui is working I ensured the version and gradel dependencies the only problem is the strings that are getting updated in gradel sync and I am not able to fix the changes the jetpack compose code is working seemlesly. I am not able to figure out the Tarak file thing

@SOUMEN-PAL Apart from the tarask file-related issue, the PR is ready for review? I will have a look at the tarask file-related issue and will push the fix for this.

Yes The UI is ready for review.

MohitMaliFtechiz

@SOUMEN-PAL Thanks for your changes. Please see the review comments.

MohitMaliFtechiz · 2025-02-18T11:35:54Z

core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreenItem.kt

+  val isDarkTheme = isSystemInDarkTheme()
+  val itemColor = if (isDarkTheme) Color.White else Color.Black
+  val arrowRotation by animateFloatAsState(
+    targetValue = if (isOpen) 180f else 0f,


@SOUMEN-PAL This is giving the MagicNumber lint error. Please fix it.

MohitMaliFtechiz · 2025-02-18T11:48:18Z

core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreen.kt

+
+@OptIn(ExperimentalMaterial3Api::class)
+@Composable
+fun HelpScreen(


Here lint is giving the LongMethod error, please fix it.

MohitMaliFtechiz · 2025-02-18T12:00:10Z

core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpFragment.kt

-          throw IllegalArgumentException("Invalid description resource type for title: $title")
-        }
-      }
+  protected abstract val navHostFragmentId: Int


This is abstract so why we are not implementing it in CustomHelpFragment like we are doing it in KiwixHelpFragment? It will not allow us to build the Custom module without implementing this in CustomHelpFragment.

MohitMaliFtechiz · 2025-02-18T12:15:59Z

core/build.gradle.kts

+  implementation(Libs.androidx_compose_material3)
+  implementation(Libs.androidx_activity_compose)
+
+  implementation(Libs.androidx_compose_ui)


Since we have added androidx.compose.ui:ui-android:1.7.7 in our project. Due to this, we are facing a PrivateResouce lint issue for our close_drawer string. Can you please fix that? IMO add this in in lintConfig file since our app can have the same resource name, and we are getting this name from our R package so it will not conflict with compose internal string.

<issue id="PrivateResource" severity="warning" />

MohitMaliFtechiz · 2025-02-18T12:21:29Z

core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreen.kt

+        navigationIcon = {
+          IconButton(onClick = navController::popBackStack) {
+            Icon(
+              imageVector = Icons.Filled.ArrowBack,


Icons.Filled.ArrowBack is deprecated we should use Icons.AutoMirrored.Filled.ArrowBack instead.

MohitMaliFtechiz · 2025-02-18T12:25:35Z

core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreen.kt

+    topBar = {
+      TopAppBar(
+        title = {
+          Text(


This text should be a little bold to align with the toolbar's default title. See the difference in below images.

Toolbar's title This title

MohitMaliFtechiz · 2025-02-18T12:29:45Z

core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreen.kt

+      ) {
+        Image(
+          painter = painterResource(R.drawable.ic_feedback_orange_24dp),
+          contentDescription = "Feedback",


The contentDescription should be Send diagnostic report as it is for "Send diagnostic report" we have a string for this send_report which we are using in the fragment_help.xml so we should use that.

MohitMaliFtechiz · 2025-02-18T12:34:24Z

core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpFragment.kt

-    fragmentHelpBinding?.activityHelpRecyclerView?.addItemDecoration(
-      DividerItemDecoration(activity, DividerItemDecoration.VERTICAL)
-    )
-    fragmentHelpBinding?.activityHelpRecyclerView?.adapter = HelpAdapter(titleDescriptionMap)


After moving HelpScreen to compose our HelpAdapter is unused so we should remove the unused code.

MohitMaliFtechiz · 2025-02-18T12:53:21Z

core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreenItem.kt

+          .padding(start = 16.dp, end = 16.dp)
+      ) {
+        Text(
+          text = data.description,


The internal text is showing bigger and looks different from the previous one. Also, the side image for opening and closing the menu items is bigger which does not look good. See the difference:

Previous design Current design

You can take the reference from xml design how we have used sizes, margins, paddings, etc, for text and images.

MohitMaliFtechiz · 2025-02-18T12:56:02Z

the only problem is the strings that are getting updated in gradel sync

@SOUMEN-PAL I have rebased the PR and resolved the tarask file conflicts. Now only add the files in which you have made the changes.

Yes The UI is ready for review.

I have reviewed your PR, please have a look at the review changes.

SOUMEN-PAL · 2025-02-18T13:26:14Z

the only problem is the strings that are getting updated in gradel sync

@SOUMEN-PAL I have rebased the PR and resolved the tarask file conflicts. Now only add the files in which you have made the changes.

Yes The UI is ready for review.

I have reviewed your PR, please have a look at the review changes.

I more question regarding the workflow I mean I was thinking for getting the upstream then updating my branch to the branch that is added after rebase from https://github.com/kiwix/kiwix-android/tree/4159-migrate-help-fragment-to-compose

Other wise i wont be able to see the changes in my forked repository.

I just want to confirm my workflow for the post rebase step
1st step
git remote add upstream https://github.com/kiwix/kiwix-android.git
2d Step
git fetch upstream
3rd step
git checkout 4159-migrate-help-fragment-to-compose
git reset --hard upstream/4159-migrate-help-fragment-to-compose

MohitMaliFtechiz · 2025-02-19T05:10:50Z

@SOUMEN-PAL Your branch has the latest code, so you can directly get it via running git pull in your current branch(4159-migrate-help-fragment-to-compose).

I just want to confirm my workflow for the post rebase step
1st step
git remote add upstream https://github.com/kiwix/kiwix-android.git
2d Step
git fetch upstream
3rd step
git checkout 4159-migrate-help-fragment-to-compose
git reset --hard upstream/4159-migrate-help-fragment-to-compose

If your local branch does not have the origin, you can do it.

MohitMaliFtechiz self-requested a review February 18, 2025 05:55

kiwix#4159 migrated Help Fragment to jetpack Compose

6873bd5

MohitMaliFtechiz force-pushed the 4159-migrate-help-fragment-to-compose branch from b81ad3c to 6873bd5 Compare February 18, 2025 11:08

MohitMaliFtechiz requested changes Feb 18, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

4159 migrate help fragment to compose #4217

4159 migrate help fragment to compose #4217

SOUMEN-PAL commented Feb 7, 2025

SOUMEN-PAL commented Feb 7, 2025

MohitMaliFtechiz commented Feb 8, 2025

kelson42 commented Feb 16, 2025

SOUMEN-PAL commented Feb 18, 2025

MohitMaliFtechiz commented Feb 18, 2025

SOUMEN-PAL commented Feb 18, 2025

MohitMaliFtechiz left a comment

MohitMaliFtechiz Feb 18, 2025

MohitMaliFtechiz Feb 18, 2025

MohitMaliFtechiz Feb 18, 2025

MohitMaliFtechiz Feb 18, 2025

MohitMaliFtechiz Feb 18, 2025

MohitMaliFtechiz Feb 18, 2025

MohitMaliFtechiz Feb 18, 2025

MohitMaliFtechiz Feb 18, 2025

MohitMaliFtechiz Feb 18, 2025 •

edited

Loading

MohitMaliFtechiz commented Feb 18, 2025

SOUMEN-PAL commented Feb 18, 2025

MohitMaliFtechiz commented Feb 19, 2025

4159 migrate help fragment to compose #4217

Are you sure you want to change the base?

4159 migrate help fragment to compose #4217

Conversation

SOUMEN-PAL commented Feb 7, 2025

📌 Summary

🔄 Changes Made

🖼️ Screenshots / GIFs

SOUMEN-PAL commented Feb 7, 2025

MohitMaliFtechiz commented Feb 8, 2025

kelson42 commented Feb 16, 2025

SOUMEN-PAL commented Feb 18, 2025

MohitMaliFtechiz commented Feb 18, 2025

SOUMEN-PAL commented Feb 18, 2025

MohitMaliFtechiz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MohitMaliFtechiz Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

MohitMaliFtechiz commented Feb 18, 2025

SOUMEN-PAL commented Feb 18, 2025

MohitMaliFtechiz commented Feb 19, 2025

MohitMaliFtechiz Feb 18, 2025 •

edited

Loading