From 1d3944dab5a604102671ff47ed1ebddde365a90d Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Fri, 11 Apr 2025 18:13:55 +0530 Subject: [PATCH] Refactored the `KiwixSearchView` to use externally provided placeholder for searchView. * Created the `PageFragmentScreenState` to encapsulates the all UI-related state to reduce the complexity in fragment. --- .../kiwixmobile/core/page/PageFragment.kt | 104 +++++++++++------- .../core/page/PageFragmentScreenState.kt | 45 ++++++++ .../kiwix/kiwixmobile/core/page/PageScreen.kt | 103 +++++++++-------- .../core/ui/components/KiwixSearchView.kt | 5 +- 4 files changed, 165 insertions(+), 92 deletions(-) create mode 100644 core/src/main/java/org/kiwix/kiwixmobile/core/page/PageFragmentScreenState.kt diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/page/PageFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/page/PageFragment.kt index b912a6f30..06adfc99f 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/page/PageFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/page/PageFragment.kt @@ -28,31 +28,26 @@ import android.view.ViewGroup import androidx.appcompat.app.AppCompatActivity import androidx.appcompat.view.ActionMode import androidx.appcompat.widget.SearchView -import androidx.appcompat.widget.Toolbar import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowBack import androidx.compose.material.icons.filled.Delete import androidx.compose.runtime.MutableState -import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.referentialEqualityPolicy -import androidx.compose.runtime.remember -import androidx.compose.runtime.setValue import androidx.compose.ui.platform.ComposeView import androidx.core.view.MenuHost import androidx.core.view.MenuProvider import androidx.lifecycle.Lifecycle import androidx.lifecycle.Observer import androidx.lifecycle.ViewModelProvider -import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.RecyclerView import io.reactivex.disposables.CompositeDisposable import org.kiwix.kiwixmobile.core.R import org.kiwix.kiwixmobile.core.base.BaseFragment import org.kiwix.kiwixmobile.core.base.FragmentActivityExtensions import org.kiwix.kiwixmobile.core.databinding.FragmentPageBinding +import org.kiwix.kiwixmobile.core.downloader.downloadManager.ZERO import org.kiwix.kiwixmobile.core.extensions.closeKeyboard -import org.kiwix.kiwixmobile.core.extensions.setToolTipWithContentDescription import org.kiwix.kiwixmobile.core.extensions.setUpSearchView import org.kiwix.kiwixmobile.core.main.CoreMainActivity import org.kiwix.kiwixmobile.core.page.adapter.OnItemClickListener @@ -98,19 +93,28 @@ abstract class PageFragment : OnItemClickListener, BaseFragment(), FragmentActiv policy = referentialEqualityPolicy() ) - /** - * Controls the visibility of the "Switch", and its controls. - * - * A [Triple] containing: - * - [String]: The text displayed with switch. - * - [Boolean]: Whether the switch is checked or not. - * - [Boolean]: Whether the switch is enabled or disabled. - */ - private val pageSwitchItem = mutableStateOf(Triple("", true, true)) + private val pageScreenState = mutableStateOf( + // Initial values are empty because this is an abstract class. + // Before the view is created, the abstract variables have no values. + // We update this state in `onViewCreated`, once the view is created and the + // abstract variables are initialized. + PageFragmentScreenState( + pageState = pageState.value, + isSearchActive = false, + searchQueryHint = "", + searchText = "", + searchValueChangedListener = {}, + screenTitle = ZERO, + noItemsString = "", + switchString = "", + switchIsChecked = true, + switchIsEnabled = true, + onSwitchCheckedChanged = {}, + deleteIconTitle = ZERO, + clearSearchButtonClickListener = {} + ) + ) private var fragmentPageBinding: FragmentPageBinding? = null - override val fragmentToolbar: Toolbar? by lazy { - fragmentPageBinding?.root?.findViewById(R.id.toolbar) - } private val actionModeCallback: ActionMode.Callback = object : ActionMode.Callback { @@ -177,15 +181,28 @@ abstract class PageFragment : OnItemClickListener, BaseFragment(), FragmentActiv override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) + pageScreenState.value = pageScreenState.value.copy( + isSearchActive = pageScreenState.value.isSearchActive, + searchQueryHint = searchQueryHint, + searchText = "", + searchValueChangedListener = { onTextChanged(it) }, + clearSearchButtonClickListener = { onTextChanged("") }, + screenTitle = screenTitle, + noItemsString = noItemsString, + switchString = switchString, + switchIsChecked = pageScreenState.value.switchIsChecked, + onSwitchCheckedChanged = { onSwitchCheckedChanged(it) }, + deleteIconTitle = deleteIconTitle + ) // setupMenu() val activity = requireActivity() as CoreMainActivity - fragmentPageBinding?.recyclerView?.apply { - layoutManager = - LinearLayoutManager(activity, RecyclerView.VERTICAL, false) - adapter = pageAdapter - fragmentTitle?.let(::setToolTipWithContentDescription) - } - fragmentPageBinding?.noPage?.text = noItemsString + // fragmentPageBinding?.recyclerView?.apply { + // layoutManager = + // LinearLayoutManager(activity, RecyclerView.VERTICAL, false) + // adapter = pageAdapter + // fragmentTitle?.let(::setToolTipWithContentDescription) + // } + // fragmentPageBinding?.noPage?.text = noItemsString // fragmentPageBinding?.pageSwitch?.apply { // text = switchString @@ -216,24 +233,20 @@ abstract class PageFragment : OnItemClickListener, BaseFragment(), FragmentActiv ): View? { return ComposeView(requireContext()).apply { setContent { - val isSearchActive = remember { mutableStateOf(false) } PageScreen( - pageState = pageState.value, - pageSwitchItem = pageSwitchItem.value, - screenTitle = screenTitle, - noItemsString = noItemsString, - searchQueryHint = searchQueryHint, - onSwitchChanged = { onSwitchCheckedChanged(it) }, + state = pageScreenState.value, itemClickListener = this@PageFragment, navigationIcon = { NavigationIcon( - iconItem = navigationIconItem(isSearchActive.value), - onClick = navigationIconClick(isSearchActive) + iconItem = navigationIconItem(pageScreenState.value.isSearchActive), + onClick = navigationIconClick() ) }, actionMenuItems = actionMenuList( - isSearchActive = isSearchActive.value, - onSearchClick = { isSearchActive.value = true }, + isSearchActive = pageScreenState.value.isSearchActive, + onSearchClick = { + pageScreenState.value = pageScreenState.value.copy(isSearchActive = true) + }, onDeleteClick = { pageViewModel.actions.offer(Action.UserClickedDeleteButton) } ) ) @@ -241,8 +254,13 @@ abstract class PageFragment : OnItemClickListener, BaseFragment(), FragmentActiv } } + private fun onTextChanged(searchText: String) { + pageScreenState.value = pageScreenState.value.copy(searchText = searchText) + pageViewModel.actions.offer(Action.Filter(searchText)) + } + private fun onSwitchCheckedChanged(isChecked: Boolean): () -> Unit = { - pageSwitchItem.value = pageSwitchItem.value.copy(second = isChecked) + pageScreenState.value = pageScreenState.value.copy(switchIsChecked = isChecked) pageViewModel.actions.offer(Action.UserClickedShowAllToggle(isChecked)) } @@ -252,9 +270,9 @@ abstract class PageFragment : OnItemClickListener, BaseFragment(), FragmentActiv IconItem.Vector(Icons.AutoMirrored.Filled.ArrowBack) } - private fun navigationIconClick(isSearchActive: MutableState): () -> Unit = { - if (isSearchActive.value) { - isSearchActive.value = false + private fun navigationIconClick(): () -> Unit = { + if (pageScreenState.value.isSearchActive) { + pageScreenState.value = pageScreenState.value.copy(isSearchActive = false) pageViewModel.actions.offer(Action.Exit) } else { requireActivity().onBackPressedDispatcher.onBackPressed() @@ -293,8 +311,10 @@ abstract class PageFragment : OnItemClickListener, BaseFragment(), FragmentActiv } private fun render(state: PageState<*>) { - pageState.value = state - pageSwitchItem.value = Triple(switchString, switchIsChecked, !state.isInSelectionState) + pageScreenState.value = pageScreenState.value.copy( + switchIsEnabled = !state.isInSelectionState, + pageState = state, + ) if (state.isInSelectionState) { if (actionMode == null) { actionMode = diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/page/PageFragmentScreenState.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/page/PageFragmentScreenState.kt new file mode 100644 index 000000000..3e26fedae --- /dev/null +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/page/PageFragmentScreenState.kt @@ -0,0 +1,45 @@ +/* + * Kiwix Android + * Copyright (c) 2025 Kiwix + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package org.kiwix.kiwixmobile.core.page + +import androidx.annotation.StringRes +import org.kiwix.kiwixmobile.core.page.viewmodel.PageState + +/** + * Represents the UI state for the PageFragment Screen. + * A Base screen for Bookmarks, History, and Notes screens. + * + * This data class encapsulates all UI-related states in a single object, + * reducing complexity in the Fragment. + */ +data class PageFragmentScreenState( + val pageState: PageState<*>, + val isSearchActive: Boolean, + val searchQueryHint: String, + val searchText: String, + val searchValueChangedListener: (String) -> Unit, + val clearSearchButtonClickListener: () -> Unit, + @StringRes val screenTitle: Int, + val noItemsString: String, + val switchString: String, + val switchIsChecked: Boolean, + val switchIsEnabled: Boolean = true, + val onSwitchCheckedChanged: (Boolean) -> Unit, + @StringRes val deleteIconTitle: Int +) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/page/PageScreen.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/page/PageScreen.kt index b160783b5..e6904c588 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/page/PageScreen.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/page/PageScreen.kt @@ -19,6 +19,7 @@ package org.kiwix.kiwixmobile.core.page import androidx.activity.compose.LocalActivity +import androidx.compose.foundation.background import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row @@ -39,84 +40,50 @@ import org.kiwix.kiwixmobile.core.main.CoreMainActivity import org.kiwix.kiwixmobile.core.page.adapter.OnItemClickListener import org.kiwix.kiwixmobile.core.page.adapter.Page import org.kiwix.kiwixmobile.core.page.history.adapter.HistoryListItem.DateItem -import org.kiwix.kiwixmobile.core.page.viewmodel.PageState import org.kiwix.kiwixmobile.core.ui.components.KiwixAppBar +import org.kiwix.kiwixmobile.core.ui.components.KiwixSearchView import org.kiwix.kiwixmobile.core.ui.models.ActionMenuItem +import org.kiwix.kiwixmobile.core.ui.theme.Black import org.kiwix.kiwixmobile.core.ui.theme.KiwixTheme -import org.kiwix.kiwixmobile.core.utils.ComposeDimens.EIGHT_DP import org.kiwix.kiwixmobile.core.utils.ComposeDimens.SIXTEEN_DP -@Suppress( - "LongParameterList", - "IgnoredReturnValue", - "UnusedParameter", - "ComposableLambdaParameterNaming" -) +@Suppress("ComposableLambdaParameterNaming") @OptIn(ExperimentalMaterial3Api::class) @Composable fun PageScreen( - pageState: PageState, - pageSwitchItem: Triple, - screenTitle: Int, - noItemsString: String, - searchQueryHint: String, - onSwitchChanged: (Boolean) -> Unit, + state: PageFragmentScreenState, itemClickListener: OnItemClickListener, actionMenuItems: List, - navigationIcon: @Composable () -> Unit, + navigationIcon: @Composable () -> Unit ) { - val context = LocalActivity.current as CoreMainActivity - KiwixTheme { Scaffold( topBar = { Column { KiwixAppBar( - titleId = screenTitle, + titleId = state.screenTitle, navigationIcon = navigationIcon, - actionMenuItems = actionMenuItems + actionMenuItems = actionMenuItems, + searchBar = searchBarIfActive(state) ) - // hide switches for custom apps, see more info here https://github.com/kiwix/kiwix-android/issues/3523 - if (!context.isCustomApp()) { - Row( - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = SIXTEEN_DP, vertical = EIGHT_DP), - verticalAlignment = Alignment.CenterVertically - ) { - Text(pageSwitchItem.first, modifier = Modifier.weight(1f)) - Switch( - checked = pageSwitchItem.second, - onCheckedChange = onSwitchChanged, - enabled = pageSwitchItem.third - ) - } - } + PageSwitchRow(state) } } ) { padding -> - val items = pageState.pageItems + val items = state.pageState.pageItems Box(modifier = Modifier.padding(padding)) { if (items.isEmpty()) { Text( - text = noItemsString, + text = state.noItemsString, style = MaterialTheme.typography.headlineSmall, modifier = Modifier.align(Alignment.Center) ) } else { LazyColumn { - items(pageState.visiblePageItems) { item -> + items(state.pageState.visiblePageItems) { item -> when (item) { - is Page -> { - PageListItem( - page = item, - itemClickListener = itemClickListener - ) - } - - is DateItem -> { - DateItemText(item) - } + is Page -> PageListItem(page = item, itemClickListener = itemClickListener) + is DateItem -> DateItemText(item) } } } @@ -126,6 +93,46 @@ fun PageScreen( } } +@Composable +private fun searchBarIfActive( + state: PageFragmentScreenState +): (@Composable () -> Unit)? = { + if (state.isSearchActive) { + KiwixSearchView( + placeholder = state.searchQueryHint, + value = state.searchText, + testTag = "", + onValueChange = { state.searchValueChangedListener(it) }, + onClearClick = { state.clearSearchButtonClickListener.invoke() } + ) + } else { + null + } +} + +@Composable +fun PageSwitchRow( + state: PageFragmentScreenState +) { + val context = LocalActivity.current as CoreMainActivity + // hide switches for custom apps, see more info here https://github.com/kiwix/kiwix-android/issues/3523 + if (!context.isCustomApp()) { + Row( + modifier = Modifier + .fillMaxWidth() + .background(Black), + verticalAlignment = Alignment.CenterVertically + ) { + Text(state.switchString) + Switch( + checked = state.switchIsChecked, + onCheckedChange = { state.onSwitchCheckedChanged(it) }, + enabled = state.switchIsEnabled + ) + } + } +} + @Composable fun DateItemText(dateItem: DateItem) { Text( diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/ui/components/KiwixSearchView.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/ui/components/KiwixSearchView.kt index b5140ec26..956165bdc 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/ui/components/KiwixSearchView.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/ui/components/KiwixSearchView.kt @@ -39,8 +39,9 @@ import org.kiwix.kiwixmobile.core.utils.ComposeDimens @Composable fun KiwixSearchView( - modifier: Modifier, + modifier: Modifier = Modifier, value: String, + placeholder: String = stringResource(R.string.search_label), testTag: String = "", onValueChange: (String) -> Unit, onClearClick: () -> Unit @@ -65,7 +66,7 @@ fun KiwixSearchView( value = value, placeholder = { Text( - text = stringResource(R.string.search_label), + text = placeholder, color = Color.LightGray, fontSize = ComposeDimens.EIGHTEEN_SP )