From b45e6f81662ea5bf624479689ec453ab9e162ed2 Mon Sep 17 00:00:00 2001 From: Krunal Patel Date: Sun, 20 Nov 2022 16:45:14 +0530 Subject: [PATCH 1/5] Fix duplicate entries in backstack Set empty listener to `setOnItemReselectedListener` to prevent repeated click Check the backstack before navigating to a new destination using bottom navigation --- .../github/libretube/ui/activities/MainActivity.kt | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt b/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt index a3c6f7cf1..b1340d014 100644 --- a/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt +++ b/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt @@ -105,6 +105,10 @@ class MainActivity : BaseActivity() { binding.bottomNav.setOnApplyWindowInsetsListener(null) + // Prevent adding duplicate entries into backstack on multiple + // click on bottom navigation item + binding.bottomNav.setOnItemReselectedListener { } + binding.bottomNav.setOnItemSelectedListener { // clear backstack if it's the start fragment if (startFragmentId == it.itemId) navController.backQueue.clear() @@ -115,8 +119,11 @@ class MainActivity : BaseActivity() { removeSearchFocus() - // navigate to the selected fragment - navController.navigate(it.itemId) + // navigate to the selected fragment, if the fragment already + // exists in backstack then pop up to that entry + if (!navController.popBackStack(it.itemId, false)) { + navController.navigate(it.itemId) + } false } From d03169b11acaac97b3ef5f65f3eef99437c3389f Mon Sep 17 00:00:00 2001 From: Krunal Patel Date: Sun, 20 Nov 2022 23:29:12 +0530 Subject: [PATCH 2/5] Fix navigation and backstack problems Check if the search view is iconified or current destination is bottom nav, then avoid navigation to search fragment. Move the `searchFragment` navigation to `onMenuItemActionExpand` so every time search expand, try to get the search fragment. Clear the focus of `searchView` when a query is submitted. Helps in decreasing the number of back presses to minimize player and navigate back. No need to remove focus using `SearchResultFragment#onStop`, `BackPressedDispatche` can handle it. --- .../libretube/ui/activities/MainActivity.kt | 59 +++++++++++++++---- .../ui/fragments/SearchResultFragment.kt | 12 ---- 2 files changed, 49 insertions(+), 22 deletions(-) diff --git a/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt b/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt index b1340d014..477d06258 100644 --- a/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt +++ b/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt @@ -17,6 +17,7 @@ import android.view.WindowManager import androidx.activity.OnBackPressedCallback import androidx.appcompat.widget.SearchView import androidx.core.os.bundleOf +import androidx.core.view.children import androidx.lifecycle.ViewModelProvider import androidx.navigation.NavController import androidx.navigation.findNavController @@ -117,13 +118,17 @@ class MainActivity : BaseActivity() { binding.bottomNav.removeBadge(R.id.subscriptionsFragment) } - removeSearchFocus() - // navigate to the selected fragment, if the fragment already // exists in backstack then pop up to that entry if (!navController.popBackStack(it.itemId, false)) { navController.navigate(it.itemId) } + + // Remove focus from search view when navigating to bottom view. + // Call only after navigate to destination, so it can be used in + // onMenuItemActionCollapse for backstack management + removeSearchFocus() + false } @@ -154,7 +159,9 @@ class MainActivity : BaseActivity() { if (navController.currentDestination?.id == startFragmentId) { moveTaskToBack(true) } else { - navController.popBackStack() + navController.popBackStack(R.id.searchResultFragment, false) || + navController.popBackStack(R.id.searchFragment, true) || + navController.popBackStack() } } }) @@ -226,23 +233,26 @@ class MainActivity : BaseActivity() { val searchViewModel = ViewModelProvider(this)[SearchViewModel::class.java] - searchView.setOnSearchClickListener { - if (navController.currentDestination?.id != R.id.searchResultFragment) { - searchViewModel.setQuery(null) - navController.navigate(R.id.searchFragment) - } - } - searchView.setOnQueryTextListener(object : SearchView.OnQueryTextListener { override fun onQueryTextSubmit(query: String?): Boolean { val bundle = Bundle() bundle.putString("query", query) navController.navigate(R.id.searchResultFragment, bundle) searchViewModel.setQuery("") + searchView.clearFocus() return true } override fun onQueryTextChange(newText: String?): Boolean { + // Prevent navigation when search view is collapsed + if (searchView.isIconified || + binding.bottomNav.menu.children.any { + it.itemId == navController.currentDestination?.id + } + ) { + return true + } + // prevent malicious navigation when the search view is getting collapsed if (navController.currentDestination?.id in listOf( R.id.searchResultFragment, @@ -265,6 +275,35 @@ class MainActivity : BaseActivity() { return true } }) + + searchItem.setOnActionExpandListener(object : MenuItem.OnActionExpandListener { + override fun onMenuItemActionExpand(item: MenuItem): Boolean { + if (navController.currentDestination?.id != R.id.searchResultFragment) { + searchViewModel.setQuery(null) + navController.navigate(R.id.searchFragment) + } + return true + } + + override fun onMenuItemActionCollapse(item: MenuItem): Boolean { + if (binding.mainMotionLayout.progress == 0F) { + try { + minimizePlayer() + } catch (e: Exception) { + // current fragment isn't the player fragment + } + } + // Handover back press to `BackPressedDispatcher` + else if (binding.bottomNav.menu.children.none { + it.itemId == navController.currentDestination?.id + } + ) { + this@MainActivity.onBackPressedDispatcher.onBackPressed() + } + + return true + } + }) return super.onCreateOptionsMenu(menu) } diff --git a/app/src/main/java/com/github/libretube/ui/fragments/SearchResultFragment.kt b/app/src/main/java/com/github/libretube/ui/fragments/SearchResultFragment.kt index 780c29f1b..d85d39942 100644 --- a/app/src/main/java/com/github/libretube/ui/fragments/SearchResultFragment.kt +++ b/app/src/main/java/com/github/libretube/ui/fragments/SearchResultFragment.kt @@ -6,7 +6,6 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import androidx.lifecycle.lifecycleScope -import androidx.navigation.fragment.findNavController import androidx.recyclerview.widget.LinearLayoutManager import com.github.libretube.R import com.github.libretube.api.RetrofitInstance @@ -16,7 +15,6 @@ import com.github.libretube.db.DatabaseHelper import com.github.libretube.db.obj.SearchHistoryItem import com.github.libretube.extensions.TAG import com.github.libretube.extensions.hideKeyboard -import com.github.libretube.ui.activities.MainActivity import com.github.libretube.ui.adapters.SearchAdapter import com.github.libretube.ui.base.BaseFragment import com.github.libretube.util.PreferenceHelper @@ -143,14 +141,4 @@ class SearchResultFragment : BaseFragment() { ) } } - - override fun onStop() { - if (findNavController().currentDestination?.id != R.id.searchFragment) { - // remove the search focus - (activity as MainActivity) - .binding.toolbar.menu - .findItem(R.id.action_search).collapseActionView() - } - super.onStop() - } } From 1c03396a2bf383574517e334afffe36ce2cd2a74 Mon Sep 17 00:00:00 2001 From: Krunal Patel Date: Mon, 21 Nov 2022 09:31:12 +0530 Subject: [PATCH 3/5] Fix reselection of bottom nav item when on different destination Check if the reselected item and current visible fragment is same, if not navigate to bottom selected item. Move bottom navigation logic to seprate method `navigateToBottomSelectedItem(MenuItem)`, so it can be reused. --- .../libretube/ui/activities/MainActivity.kt | 49 +++++++++++-------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt b/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt index 477d06258..143875038 100644 --- a/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt +++ b/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt @@ -106,29 +106,16 @@ class MainActivity : BaseActivity() { binding.bottomNav.setOnApplyWindowInsetsListener(null) - // Prevent adding duplicate entries into backstack on multiple - // click on bottom navigation item - binding.bottomNav.setOnItemReselectedListener { } + // Prevent duplicate entries into backstack, if selected item and current + // visible fragment is different, then navigate to selected item. + binding.bottomNav.setOnItemReselectedListener { + if (it.itemId != navController.currentDestination?.id) { + navigateToBottomSelectedItem(it) + } + } binding.bottomNav.setOnItemSelectedListener { - // clear backstack if it's the start fragment - if (startFragmentId == it.itemId) navController.backQueue.clear() - - if (it.itemId == R.id.subscriptionsFragment) { - binding.bottomNav.removeBadge(R.id.subscriptionsFragment) - } - - // navigate to the selected fragment, if the fragment already - // exists in backstack then pop up to that entry - if (!navController.popBackStack(it.itemId, false)) { - navController.navigate(it.itemId) - } - - // Remove focus from search view when navigating to bottom view. - // Call only after navigate to destination, so it can be used in - // onMenuItemActionCollapse for backstack management - removeSearchFocus() - + navigateToBottomSelectedItem(it) false } @@ -496,6 +483,26 @@ class MainActivity : BaseActivity() { } } + private fun navigateToBottomSelectedItem(item: MenuItem) { + // clear backstack if it's the start fragment + if (startFragmentId == item.itemId) navController.backQueue.clear() + + if (item.itemId == R.id.subscriptionsFragment) { + binding.bottomNav.removeBadge(R.id.subscriptionsFragment) + } + + // navigate to the selected fragment, if the fragment already + // exists in backstack then pop up to that entry + if (!navController.popBackStack(item.itemId, false)) { + navController.navigate(item.itemId) + } + + // Remove focus from search view when navigating to bottom view. + // Call only after navigate to destination, so it can be used in + // onMenuItemActionCollapse for backstack management + removeSearchFocus() + } + override fun onUserLeaveHint() { super.onUserLeaveHint() supportFragmentManager.fragments.forEach { fragment -> From 3661660dd19d9cb7829e53e7f69f6f43db0c47c3 Mon Sep 17 00:00:00 2001 From: Krunal Patel Date: Mon, 21 Nov 2022 09:45:02 +0530 Subject: [PATCH 4/5] Fix search option overflow when searchView expanded --- .../main/java/com/github/libretube/ui/activities/MainActivity.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt b/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt index 143875038..591c9d6dd 100644 --- a/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt +++ b/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt @@ -269,6 +269,7 @@ class MainActivity : BaseActivity() { searchViewModel.setQuery(null) navController.navigate(R.id.searchFragment) } + item.setShowAsAction(MenuItem.SHOW_AS_ACTION_ALWAYS or MenuItem.SHOW_AS_ACTION_COLLAPSE_ACTION_VIEW) return true } From 5f5eee3be2d927392278fbfb03690b9305a1dbc8 Mon Sep 17 00:00:00 2001 From: Krunal Patel Date: Mon, 21 Nov 2022 10:23:05 +0530 Subject: [PATCH 5/5] Fix multiple backstack entries get removed on some fragments --- .../github/libretube/ui/activities/MainActivity.kt | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt b/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt index 591c9d6dd..08a00e62a 100644 --- a/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt +++ b/app/src/main/java/com/github/libretube/ui/activities/MainActivity.kt @@ -143,12 +143,17 @@ class MainActivity : BaseActivity() { } } - if (navController.currentDestination?.id == startFragmentId) { - moveTaskToBack(true) - } else { - navController.popBackStack(R.id.searchResultFragment, false) || + when (navController.currentDestination?.id) { + startFragmentId -> { + moveTaskToBack(true) + } + R.id.searchResultFragment -> { navController.popBackStack(R.id.searchFragment, true) || + navController.popBackStack() + } + else -> { navController.popBackStack() + } } } })