Migrated SearchFragment -> Search Composable by atharvyadav22 · Pull Request #4703 · kiwix/kiwix-android

atharvyadav22 · 2026-02-18T15:11:29Z

Fixes: #4616 - Migrated Search Fragment to Pure Composable with basic test functionality.

Fixes: #4662 - Search keyword travel was broke due to Fragment lifecycle error and intent action issue.
It now works Within app and also outside the app

Fixes #4722

WhatsApp.Video.2026-02-18.at.9.09.27.PM.1.mp4

Fixes: #4702 - Voice to Search was broken due to Regression and Fragment changes.

WhatsApp.Video.2026-02-18.at.9.09.27.PM.mp4

Fixes: #4722 - Fixed Keyboard not closing when back button press on search screen.

WhatsApp.Video.2026-02-26.at.9.51.05.PM.mp4

atharvyadav22 · 2026-02-18T15:13:50Z

@MohitMaliFtechiz @gouri-panda
For this commit please review only the code files and provide feedback for the changes. Later i'll add Test cases.
Sorry for the delay caused as this migration was little big and keeping every logic was difficult.
Also i have deleted my earlier PR as it was bit messy Apologies for it.

MohitMaliFtechiz

What is this SEARCH text here? It is making the UI very odd. If you are talking about the voice feature of search. It should not be like this. There should be a mic icon instead of this text. It's confusing the user here.

The second issue is when we open any item in the reader by clicking the searched item. Then again, open the searchScreen, that item is not showing(in the previous code that shows). But, if I searched for anything and removed the query from searchBox then those items show that we have opened in a previous search.

MohitMaliFtechiz · 2026-02-20T09:21:46Z

app/src/main/java/org/kiwix/kiwixmobile/ui/KiwixNavGraph.kt

+      val context = LocalActivity.current
+      val coreMainActivity = context as CoreMainActivity
+
+      SearchContainer(


The name should be SearchScreenRoute.

MohitMaliFtechiz · 2026-02-20T09:39:59Z

core/src/main/java/org/kiwix/kiwixmobile/core/main/reader/CoreReaderFragment.kt

  ): FragmentActivityExtensions.Super {
    pendingIntent = intent
+
+    if (!isWebViewHistoryRestoring) {


Why are we adding this here? It has no effect here. Because when the onNewIntent runs, then isWebViewHistoryRestoring will always be true, so this condition will never run. Also, we are storing the intent in pendingIntent which will run after the history is restored.

Okay see for that issue no #4662 whats was happening earlier when we did OnSearch Kiwix it was immediately cancelling intent actions here we had 2 possible ways which kiwix should pass intent one was outside the app and other within reader screen

I don't think that this code #4662 will fix. The search travel is a different issue.

The search travel is only possible in the visible area of the EditTextView.

Ops looks like an mistake sorry that fix is for different purpose i didn't raise an Issue for that. actually that SearchKiwix didn't work when in readerScreen that fix is for it sorry for that i referenced it with wrong one.

core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreMainActivity.kt

MohitMaliFtechiz · 2026-02-20T09:57:05Z

core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchScreen.kt

  actionMenuItemList: List<ActionMenuItem>,
-  isLoadingMoreResult: Boolean
+  isLoadingMoreResult: Boolean,
+  lazyListState: LazyListState


Why are we passing this from the outside? What is the use of this there?

pagination was affected and i thought it was recomposing so i changed but later realized my code was not the issue left to revert it thanks for pointing

MohitMaliFtechiz · 2026-02-20T09:59:42Z

core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchContainer.kt

+    lazyListState = listState
+  )
+
+  DialogHost(dialogShower as AlertDialogShower)


We don't need to set this here, as we already set the DialogHost in mainActivity for this dialogShower.

okay as Fragment had it so i added up ill revert it

MohitMaliFtechiz · 2026-02-20T10:05:57Z

core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchContainer.kt

+  val showFindInPage by viewModel.showFindInPage.collectAsStateWithLifecycle(initialValue = false)
+
+  // Handles SideEffects
+  LaunchedEffect(viewModel) {


The key should not be viewModel since it only needs to be attached once. Use the CollectSideEffectWithActivity instead of this.

MohitMaliFtechiz · 2026-02-20T10:26:07Z

core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchContainer.kt

+
+  SearchScreen(
+    screenState,
+    listOfNotNull(


We should split this into a function that returns the actionMenuList.

MohitMaliFtechiz · 2026-02-20T10:41:43Z

core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModel.kt

-      )
-    ).isSuccess
-    _effects.trySend(ShowToast(R.string.delete_specific_search_toast)).isSuccess
+      DeleteRecentSearch(it.searchListItem, recentSearchRoomDao, viewModelScope)


When I am deleting the SearchedItem, then the toast shows the success of deleting. But the item remains shown on the screen.

MohitMaliFtechiz · 2026-02-20T10:45:43Z

core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModel.kt

+      .collect {
+        state.value = it
+
+        if (it.searchTerm != lastQuery && !_isLoadingMore.value) {


Why are we checking it.searchTerm != lastQuery when we are using the distinctUntilChanged?

yes actually when i migrated i thought pagination and other stuff like read was affected by this reason so i was checking and i left mistakenly there bad code thanks for pointing.

MohitMaliFtechiz · 2026-02-23T08:01:39Z

@atharvyadav22 Please rebase this PR as well with the review changes.

atharvyadav22 · 2026-02-23T08:11:19Z

@atharvyadav22 Please rebase this PR as well with the review changes.

I'll also add that test coverage here should i reference this PR? or only work for migration and create new issue for it.

MohitMaliFtechiz · 2026-02-23T09:28:58Z

@atharvyadav22 One ticket/PR per thing. First we should atleast achive the existing fucntionality and than we should add testing. Migration is a different thing, and code coverage is different. Doing both together can introduce regressions here.

MohitMaliFtechiz · 2026-02-23T11:53:09Z

@atharvyadav22 Mark all your PRs along with this one, as draft, on which work is currently in progress.

atharvyadav22 · 2026-02-26T16:13:35Z

@MohitMaliFtechiz @kelson42
Done with all the changes

Could you verify if there are any more functionalities other than these? as we have seen migration have caused us an trouble with many functionality.

Debounce search
Search suggestions
Recent search
Voice search
Find in page
Pagination
Kiwix Search Feature
Delete dialogs, Toasts

atharvyadav22 · 2026-02-26T16:14:06Z

@MohitMaliFtechiz we have Fragment Tests too what do with them now ?

codecov · 2026-02-26T17:01:51Z

Codecov Report

❌ Patch coverage is 70.62937% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.34%. Comparing base (02520f2) to head (f4c42ee).

Files with missing lines	Patch %	Lines
...kiwix/kiwixmobile/core/search/SearchScreenRoute.kt	70.27%	16 Missing and 6 partials ⚠️
...wixmobile/core/search/viewmodel/SearchViewModel.kt	70.17%	8 Missing and 9 partials ⚠️
...ain/java/org/kiwix/kiwixmobile/ui/KiwixNavGraph.kt	87.50%	1 Missing ⚠️
...rg/kiwix/kiwixmobile/core/main/CoreMainActivity.kt	50.00%	0 Missing and 1 partial ⚠️
...kiwixmobile/core/main/reader/CoreReaderFragment.kt	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #4703      +/-   ##
============================================
- Coverage     60.70%   60.34%   -0.36%     
+ Complexity     1625     1588      -37     
============================================
  Files           330      330              
  Lines         15726    15668      -58     
  Branches       2162     2158       -4     
============================================
- Hits           9546     9455      -91     
- Misses         4724     4759      +35     
+ Partials       1456     1454       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

MohitMaliFtechiz · 2026-02-27T06:57:08Z

@MohitMaliFtechiz we have Fragment Tests too what do with them now ?

They are not for testing the fragment. They are for testing the functionality of that screen. We should rename the test cases, as now we have composables, not fragments.

MohitMaliFtechiz · 2026-02-27T07:03:50Z

@MohitMaliFtechiz @kelson42 Done with all the changes

Could you verify if there are any more functionalities other than these? as we have seen migration have caused us an trouble with many functionality.

Debounce search

Search suggestions

Recent search

Voice search

Find in page

Pagination

Kiwix Search Feature

Delete dialogs, Toasts

There is one more functionality where we can directly search from browsers as well via the Search Kiwix option menu.

atharvyadav22 · 2026-02-27T07:53:51Z

Yes i have already covered that too mentioned it in description as it was also an Issue it now works for both within and and outside the app

atharvyadav22 · 2026-02-27T07:55:20Z

@MohitMaliFtechiz we have Fragment Tests too what do with them now ?

They are not for testing the fragment. They are for testing the functionality of that screen. We should rename the test cases, as now we have composables, not fragments.

Okay now it should be aligned with SearchScreenRoutes right? so should i raise an another PR for it as you said tests are different
Or should i continue with other migration then after migrations these cases like we can reference in coverage PR

MohitMaliFtechiz · 2026-02-27T09:23:58Z

@atharvyadav22 Renaming the test cases should happen here. Since we are migrating to composables, related to that should be improved here.

atharvyadav22 · 2026-02-28T16:11:00Z

@atharvyadav22 Renaming the test cases should happen here. Since we are migrating to composables, related to that should be improved here.

I thought we should add tests too got confused.

MohitMaliFtechiz · 2026-03-02T07:06:53Z

I thought we should add tests too got confused.

@atharvyadav22 If you have found something that needs to be improved in testing for the current migration screen. Then you can add test cases for that in your current PR. @harshsomankar123-tech did this in his PRs. You can take reference from his PRs. But no unrelated changes should be made in PRs.

… lifecycle

MohitMaliFtechiz · 2026-03-05T09:29:47Z

@atharvyadav22 Thank you for making the changes. I will review this shortly :)

MohitMaliFtechiz

@atharvyadav22, I don't understand why we are always changing the UI behaviour when this PR is just for removing the fragment and migrating it to pure composables.

Here FIND_IN_PAGE option is not visible when there is no word typed. We already have a ticket for this #1782. But this type of UI change made the review harder(needs more testing/caring, and they have no effect. It only takes a long time to merge any PR).

MohitMaliFtechiz · 2026-03-06T10:00:43Z

core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchScreenRoute.kt

+  }
+
+  // Search Results
+  LaunchedEffect(viewModel) {


My previous comment has not been taken. Here we should use Unit instead of viewModel. However, the viewModel will not change, but it is not ideal to use the viewModel as a key here. Because it only requires once, so it should be Unit. In the future, it might be possible that the viewModel is changed due to some condition.

okay i'll keep in mind like i assumed that ViewModel() is attached with lifecycle so it wont change but i agree with you.

MohitMaliFtechiz · 2026-03-06T10:11:42Z

core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchScreenRoute.kt

+  showFindInPage: Boolean
+): List<ActionMenuItem> {
+  return listOfNotNull(
+    ActionMenuItem(


I appreciate this one, that you have found this issue, and placed a fix for it 👏 .

MohitMaliFtechiz · 2026-03-06T10:20:56Z

core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchScreenRoute.kt

+    navigationIcon = {
+      NavigationIcon(
+        onClick = {
+          coreMainActivity.onBackPressedDispatcher.onBackPressed()


The keyboard is closing after the reader screen is visible in my Google Pixel 7a. But the keyboard is not closing in the emualtor. So I guess my device autoamtically handling the keyboard hiding. We should hide the keyboard first if the navigation icon is clicked. Because it is taking us to the reader screen, which is not an ideal UX.

When the user presses the device back button then first keyboard hides, and on the second back pressed screen comes to the reader. This is the ideal UX for a user. We should keep this same for navigationIcon click.

Okay as i only tested it one Real Device i'll keep that in mind.

MohitMaliFtechiz · 2026-03-06T10:25:54Z

core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModel.kt

+        val firstPage = it.getVisibleResults(0).orEmpty()
+        _visibleResults.value = firstPage
+
+        if (it.searchTerm.isNotBlank()) {


This is a huge memory consumption code. It everytime ask to libkiwix to give suggestions; however, the result already exists. The suggestion API is just for giving suggestions if the search API did not return a result.

If you see the fragment code, it is clearly mentioned there.

Okay i'll update 👍

MohitMaliFtechiz · 2026-03-06T10:48:15Z

core/src/main/java/org/kiwix/kiwixmobile/core/main/reader/CoreReaderFragment.kt

  ): FragmentActivityExtensions.Super {
    pendingIntent = intent
+
+    if (!isWebViewHistoryRestoring) {


I don't think that this code #4662 will fix. The search travel is a different issue.

The search travel is only possible in the visible area of the EditTextView.

MohitMaliFtechiz · 2026-03-06T11:04:13Z

core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchViewModel.kt

  }

-  suspend fun getSuggestedSpelledWords(word: String, maxCount: Int): List<String> =
+  private fun getSuggestedSpelledWords(word: String, maxCount: Int): List<String> =


This operation should not be performed on the main thread, so it should be suspend function to make its usage clear.

Yes ! my bad

MohitMaliFtechiz self-requested a review February 19, 2026 10:36

MohitMaliFtechiz requested changes Feb 20, 2026

View reviewed changes

atharvyadav22 marked this pull request as draft February 23, 2026 12:13

MohitMaliFtechiz mentioned this pull request Feb 26, 2026

Keyboard remains visible after back navigation from search screen in Reader's. #4722

Open

atharvyadav22 force-pushed the migrate/search branch from 235968a to 08177e5 Compare February 26, 2026 16:06

atharvyadav22 requested a review from MohitMaliFtechiz February 26, 2026 16:14

atharvyadav22 marked this pull request as ready for review February 26, 2026 16:29

atharvyadav22 mentioned this pull request Mar 1, 2026

move search in page to page options button #4731

Open

atharvyadav22 added 4 commits March 3, 2026 13:14

Migrated SearchFragment -> Search Composable

11e664c

Fixed Known issues and fixed searchViewModel test

f5848ea

Renaming the test cases

140feff

Handled Launched effect and Dispsable effect keys properly aligned to…

9c8bb31

… lifecycle

atharvyadav22 force-pushed the migrate/search branch from f4c42ee to 9c8bb31 Compare March 3, 2026 07:45

atharvyadav22 mentioned this pull request Mar 3, 2026

CI Failure For ZimManageViewModelTest #4737

Open

MohitMaliFtechiz mentioned this pull request Mar 5, 2026

Notification activation should not block downloads #4743

Open

MohitMaliFtechiz requested changes Mar 6, 2026

View reviewed changes

Uh oh!

Conversation

atharvyadav22 commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

atharvyadav22 commented Feb 18, 2026

Uh oh!

MohitMaliFtechiz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MohitMaliFtechiz commented Feb 23, 2026

Uh oh!

atharvyadav22 commented Feb 23, 2026

Uh oh!

MohitMaliFtechiz commented Feb 23, 2026

Uh oh!

MohitMaliFtechiz commented Feb 23, 2026

Uh oh!

atharvyadav22 commented Feb 26, 2026

Uh oh!

atharvyadav22 commented Feb 26, 2026

Uh oh!

codecov bot commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

MohitMaliFtechiz commented Feb 27, 2026

Uh oh!

MohitMaliFtechiz commented Feb 27, 2026

Uh oh!

atharvyadav22 commented Feb 27, 2026

Uh oh!

atharvyadav22 commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MohitMaliFtechiz commented Feb 27, 2026

Uh oh!

atharvyadav22 commented Feb 28, 2026

Uh oh!

MohitMaliFtechiz commented Mar 2, 2026

Uh oh!

MohitMaliFtechiz commented Mar 5, 2026

Uh oh!

MohitMaliFtechiz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

atharvyadav22 commented Feb 18, 2026 •

edited

Loading

codecov bot commented Feb 26, 2026 •

edited

Loading

atharvyadav22 commented Feb 27, 2026 •

edited

Loading