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.

kelson42 · 2025-03-03T12:17:16Z

@SOUMEN-PAL What is going on here? Do you still work on this PR?

SOUMEN-PAL · 2025-03-03T14:06:11Z

@SOUMEN-PAL What is going on here? Do you still work on this PR?

Yes it's done I gave an update to check up on the review comment. There i also commented on the difference between the XML based dp and dp in jetpack compose so I asked up there should I manually use required sizes that will look the same?

MohitMaliFtechiz · 2025-03-03T15:51:57Z

Yes it's done I gave an update to check up on the review comment. There i also commented on the difference between the XML based dp and dp in jetpack compose so I asked up there should I manually use required sizes that will look the same?

Where you have commented and asked the question? I do not see your comment in the review comments. However, we have set up the theme(sizes, shapes, etc) for our textView, button, etc in #4253, you can take reference from that PR.

SOUMEN-PAL · 2025-03-05T04:16:15Z

Yes it's done I gave an update to check up on the review comment. There i also commented on the difference between the XML based dp and dp in jetpack compose so I asked up there should I manually use required sizes that will look the same?

Where you have commented and asked the question? I do not see your comment in the review comments. However, we have set up the theme(sizes, shapes, etc) for our textView, button, etc in #4253, you can take reference from that PR.

@MohitMaliFtechiz sir,
In Jetpack Compose you should set icon and image sizes directly using Compose’s dp values (e.g. using Modifier.size(24.dp)), rather than relying on XML-based R.dimen values. The Compose image pipeline handles scaling differently, so defining sizes explicitly for images and icons ensures consistent results, while text sizes generally remain unaffected.

MohitMaliFtechiz · 2025-03-05T05:03:02Z

In Jetpack Compose you should set icon and image sizes directly using Compose’s dp values (e.g. using Modifier.size(24.dp)) rather than relying on XML-based R.dimen values.

@SOUMEN-PAL Yes correct, we are not using the XML_base R.dimen file for setting the sizes for images and icons. But hardcoding this dp everywhere is not a good idea. In #4253, we have set up the theme for compose, and using the text sizes from MaterialTheme. For the normal dp sizes we have created a ComposeDimen file to avoid hardcoding these dp, sp everywhere, and managing the sizes for common views.

… the requested changes

SOUMEN-PAL · 2025-03-07T17:38:18Z

@MohitMaliFtechiz
Hello sir
I have completed the required changes.
Fixed the Font Sizes,
Resolve the Lint Issues like Magic Number Issue and Long method Issue
Used R.Dimensions for assigning sizes to different elements.

Major changes:

Requested deletion of HelpAdpter as after migration it was not needed
Added navigating host id in customHelpFragment entry
The topAppbar in Scaffold is a little bit bigger than the XML topBar which is 56dp to make the bar same through out I took the values from the theme ans assigned it.

MohitMaliFtechiz

@SOUMEN-PAL Sorry for late review, please see the review changes.

Edited
Now we have migrated the helpFragment in compose, so there is no need for item_help.xml, and fragment_help.xml so please remove them.

Can you please refactor the HelpFragmentTest as now it will fail because of migration. you can take reference from already written test cases in #4258, and #4253.

MohitMaliFtechiz · 2025-03-12T04:33:06Z

buildSrc/src/main/kotlin/Libs.kt

+  const val androidx_compose_ui: String =
+    "androidx.compose.ui:ui:" + Versions.androidx_compose_ui_version
+
+  const val androidx_compose_bom: String =


@SOUMEN-PAL Why are we adding both the bom and the normal version of dependencies? We should only add and manage the dependencies. See how we done in https://github.com/kiwix/kiwix-android/pull/4253/files#diff-df8b4af8cccf28e19228317a74d3fba23fed6c067665f72670b7d4986515ad39.

MohitMaliFtechiz · 2025-03-12T05:24:33Z

core/build.gradle.kts

@@ -13,6 +13,8 @@ buildscript {
 }
 plugins {
  `android-library`
+  id("org.jetbrains.kotlin.android")


Please rebase on the main branch, there we have added the support for compose in a proper way.

MohitMaliFtechiz · 2025-03-12T05:53:43Z

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

+          rawTitleDescriptionMap()
+        )
+        // Retrieve the NavController if your composable needs it.
+        val navController = Navigation.findNavController(requireActivity(), navHostFragmentId)


Please use the already defined navController in CoreMainActivity. Like:

val navController = (requireActivity() as CoreMainActivity).navController

After using this, you don't need to define this:

protected abstract val navHostFragmentId: Int

So remove this from HelpFragment as well as from the child fragments.

MohitMaliFtechiz · 2025-03-12T05:58:26Z

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

+    // Already Applied ad TopAppBAr in scaffold in composable
+    null
+  }
+  override val fragmentTitle: String? by lazy { getString(R.string.menu_help) }


Remove these fragmentToolbar and fragmentTitle, as well as the onViewCreated method as they are unused now.

MohitMaliFtechiz · 2025-03-12T06:05:19Z

custom/src/main/java/org/kiwix/kiwixmobile/custom/help/CustomHelpFragment.kt

+  override val navHostFragmentId: Int
+    get() = org.kiwix.kiwixmobile.custom.R.id.custom_nav_controller
+
+  override fun rawTitleDescriptionMap() =


in custom apps we are not showing these rows, so please don't add these.

MohitMaliFtechiz · 2025-03-12T06:25:42Z

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

+        modifier = Modifier.fillMaxWidth(),
+        verticalAlignment = Alignment.CenterVertically
+      ) {
+        HelpTopAppBar(navController)


Please use KiwixAppBar for the toolbar it has the all capabilities e.g. handling the menu button, title, back button, etc with the proper theme of our application for both night/day modes.

MohitMaliFtechiz · 2025-03-12T06:30:44Z

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

+fun HelpContent(
+  data: List<HelpScreenItemDataClass>,
+  dividerColor: Color,
+  innerPadding: androidx.compose.foundation.layout.PaddingValues


Instead of writing full packageName just import this package and use the PaddingValues directly.

MohitMaliFtechiz · 2025-03-12T06:43:15Z

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

+    Text(
+      text = stringResource(R.string.send_report),
+      color = if (isDarkTheme) Color.LightGray else Color.DarkGray,
+      fontSize = SendDiagnosticReportFontSize


Instead of hardcoding the size of text use style = MaterialTheme.typography.titleMedium from the KiwixTheme or any better which fits in this.

Also, add the divider here at the last which will show the line at the bottom of "Send diagnostic report" to make the row completed.

MohitMaliFtechiz · 2025-03-12T06:45:49Z

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

+      .fillMaxWidth()
+  ) {
+    itemsIndexed(data, key = { _, item -> item.title }) { _, item ->
+      HorizontalDivider(


You should add this divider after the HelpScreenItem so that it will be below the content, and remove the last extra item added here(The below code) to simplify the design and code.

item { HorizontalDivider( color = dividerColor ) }

MohitMaliFtechiz · 2025-03-12T06:55:20Z

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

+private const val HelpItemArrowRotationClosed = 0f
+
+@Composable
+fun HelpScreenItem(


The design is very different from the previous one. please fix this. See the images:

XML design Compose design

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

Fixed Linted issuse related to Magic numbers and long methods and did…

9f22bb4

… the requested changes

MohitMaliFtechiz requested changes Mar 12, 2025

View reviewed changes

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

kelson42 commented Mar 3, 2025

SOUMEN-PAL commented Mar 3, 2025

MohitMaliFtechiz commented Mar 3, 2025

SOUMEN-PAL commented Mar 5, 2025 • edited Loading

MohitMaliFtechiz commented Mar 5, 2025

SOUMEN-PAL commented Mar 7, 2025 • edited Loading

MohitMaliFtechiz left a comment • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MohitMaliFtechiz Feb 18, 2025 •

edited

Loading

SOUMEN-PAL commented Mar 5, 2025 •

edited

Loading

SOUMEN-PAL commented Mar 7, 2025 •

edited

Loading

MohitMaliFtechiz left a comment •

edited

Loading