From c42a3ca97c29f1700ebdd363dd9e3c8b62384aa4 Mon Sep 17 00:00:00 2001 From: Alexander Capehart Date: Thu, 3 Aug 2023 13:14:12 -0600 Subject: [PATCH] ui: refine navigation listeners Make sure that we don't drop selections or playlist edits when we navigate to dialogs, this time achieved through a more general navigation listener implementation than prior. --- .../java/org/oxycblt/auxio/MainFragment.kt | 38 +++------ .../auxio/detail/PlaylistDetailFragment.kt | 52 ++++++------ .../org/oxycblt/auxio/list/ListViewModel.kt | 28 ++++--- .../oxycblt/auxio/list/SelectionFragment.kt | 2 +- .../auxio/list/menu/MenuDialogFragmentImpl.kt | 5 +- .../auxio/ui/DialogAwareNavigationListener.kt | 79 +++++++++++++++++++ app/src/main/res/navigation/inner.xml | 28 +++---- app/src/main/res/navigation/outer.xml | 12 +-- 8 files changed, 154 insertions(+), 90 deletions(-) create mode 100644 app/src/main/java/org/oxycblt/auxio/ui/DialogAwareNavigationListener.kt diff --git a/app/src/main/java/org/oxycblt/auxio/MainFragment.kt b/app/src/main/java/org/oxycblt/auxio/MainFragment.kt index 549ac2aa4..c86037eb1 100644 --- a/app/src/main/java/org/oxycblt/auxio/MainFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/MainFragment.kt @@ -27,8 +27,6 @@ import androidx.core.view.ViewCompat import androidx.core.view.isInvisible import androidx.core.view.updatePadding import androidx.fragment.app.activityViewModels -import androidx.navigation.NavController -import androidx.navigation.NavDestination import androidx.navigation.findNavController import androidx.navigation.fragment.findNavController import com.google.android.material.R as MR @@ -49,6 +47,7 @@ import org.oxycblt.auxio.playback.OpenPanel import org.oxycblt.auxio.playback.PlaybackBottomSheetBehavior import org.oxycblt.auxio.playback.PlaybackViewModel import org.oxycblt.auxio.playback.queue.QueueBottomSheetBehavior +import org.oxycblt.auxio.ui.DialogAwareNavigationListener import org.oxycblt.auxio.ui.ViewBindingFragment import org.oxycblt.auxio.util.collectImmediately import org.oxycblt.auxio.util.context @@ -67,9 +66,7 @@ import org.oxycblt.auxio.util.unlikelyToBeNull */ @AndroidEntryPoint class MainFragment : - ViewBindingFragment(), - ViewTreeObserver.OnPreDrawListener, - NavController.OnDestinationChangedListener { + ViewBindingFragment(), ViewTreeObserver.OnPreDrawListener { private val detailModel: DetailViewModel by activityViewModels() private val homeModel: HomeViewModel by activityViewModels() private val listModel: ListViewModel by activityViewModels() @@ -77,9 +74,9 @@ class MainFragment : private var sheetBackCallback: SheetBackPressedCallback? = null private var detailBackCallback: DetailBackPressedCallback? = null private var selectionBackCallback: SelectionBackPressedCallback? = null + private var selectionNavigationListener: DialogAwareNavigationListener? = null private var lastInsets: WindowInsets? = null private var elevationNormal = 0f - private var initialNavDestinationChange = true override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -111,6 +108,8 @@ class MainFragment : val selectionBackCallback = SelectionBackPressedCallback(listModel).also { selectionBackCallback = it } + selectionNavigationListener = DialogAwareNavigationListener(listModel::dropSelection) + // --- UI SETUP --- val context = requireActivity() @@ -162,8 +161,8 @@ class MainFragment : val binding = requireBinding() // Once we add the destination change callback, we will receive another initialization call, // so handle that by resetting the flag. - initialNavDestinationChange = false - binding.exploreNavHost.findNavController().addOnDestinationChangedListener(this) + requireNotNull(selectionNavigationListener) { "NavigationListener was not available" } + .attach(binding.exploreNavHost.findNavController()) // Listener could still reasonably fire even if we clear the binding, attach/detach // our pre-draw listener our listener in onStart/onStop respectively. binding.playbackSheet.viewTreeObserver.addOnPreDrawListener(this@MainFragment) @@ -184,7 +183,8 @@ class MainFragment : override fun onStop() { super.onStop() val binding = requireBinding() - binding.exploreNavHost.findNavController().removeOnDestinationChangedListener(this) + requireNotNull(selectionNavigationListener) { "NavigationListener was not available" } + .release(binding.exploreNavHost.findNavController()) binding.playbackSheet.viewTreeObserver.removeOnPreDrawListener(this) } @@ -193,6 +193,7 @@ class MainFragment : sheetBackCallback = null detailBackCallback = null selectionBackCallback = null + selectionNavigationListener = null } override fun onPreDraw(): Boolean { @@ -286,25 +287,6 @@ class MainFragment : return true } - override fun onDestinationChanged( - controller: NavController, - destination: NavDestination, - arguments: Bundle? - ) { - // Drop the initial call by NavController that simply provides us with the current - // destination. This would cause the selection state to be lost every time the device - // rotates. - if (!initialNavDestinationChange) { - initialNavDestinationChange = true - return - } - if (destination.id != R.id.selection_menu_dialog) { - // Drop any pending playlist edits when navigating away. This could actually happen - // if the user is quick enough. - listModel.dropSelection() - } - } - private fun handleShowOuter(outer: Outer?) { val directions = when (outer) { diff --git a/app/src/main/java/org/oxycblt/auxio/detail/PlaylistDetailFragment.kt b/app/src/main/java/org/oxycblt/auxio/detail/PlaylistDetailFragment.kt index 103d1034f..ed460bc33 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/PlaylistDetailFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/PlaylistDetailFragment.kt @@ -20,9 +20,8 @@ package org.oxycblt.auxio.detail import android.os.Bundle import android.view.LayoutInflater +import android.view.MenuItem import androidx.fragment.app.activityViewModels -import androidx.navigation.NavController -import androidx.navigation.NavDestination import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.navArgs import androidx.recyclerview.widget.ConcatAdapter @@ -51,6 +50,7 @@ import org.oxycblt.auxio.music.PlaylistDecision import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.playback.PlaybackDecision import org.oxycblt.auxio.playback.PlaybackViewModel +import org.oxycblt.auxio.ui.DialogAwareNavigationListener import org.oxycblt.auxio.util.collect import org.oxycblt.auxio.util.collectImmediately import org.oxycblt.auxio.util.logD @@ -68,8 +68,7 @@ import org.oxycblt.auxio.util.unlikelyToBeNull class PlaylistDetailFragment : ListFragment(), DetailHeaderAdapter.Listener, - PlaylistDetailListAdapter.Listener, - NavController.OnDestinationChangedListener { + PlaylistDetailListAdapter.Listener { private val detailModel: DetailViewModel by activityViewModels() override val listModel: ListViewModel by activityViewModels() override val musicModel: MusicViewModel by activityViewModels() @@ -80,7 +79,7 @@ class PlaylistDetailFragment : private val playlistHeaderAdapter = PlaylistDetailHeaderAdapter(this) private val playlistListAdapter = PlaylistDetailListAdapter(this) private var touchHelper: ItemTouchHelper? = null - private var initialNavDestinationChange = false + private var editNavigationListener: DialogAwareNavigationListener? = null override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -98,6 +97,8 @@ class PlaylistDetailFragment : override fun onBindingCreated(binding: FragmentDetailBinding, savedInstanceState: Bundle?) { super.onBindingCreated(binding, savedInstanceState) + editNavigationListener = DialogAwareNavigationListener(detailModel::dropPlaylistEdit) + // --- UI SETUP --- binding.detailNormalToolbar.apply { setNavigationOnClickListener { findNavController().navigateUp() } @@ -147,17 +148,31 @@ class PlaylistDetailFragment : collect(playbackModel.playbackDecision.flow, ::handlePlaybackDecision) } + override fun onMenuItemClick(item: MenuItem): Boolean { + if (super.onMenuItemClick(item)) { + return true + } + + if (item.itemId == R.id.action_save) { + detailModel.savePlaylistEdit() + return true + } + + return false + } + override fun onStart() { super.onStart() // Once we add the destination change callback, we will receive another initialization call, // so handle that by resetting the flag. - initialNavDestinationChange = false - findNavController().addOnDestinationChangedListener(this) + requireNotNull(editNavigationListener) { "NavigationListener was not available" } + .attach(findNavController()) } override fun onStop() { super.onStop() - findNavController().removeOnDestinationChangedListener(this) + requireNotNull(editNavigationListener) { "NavigationListener was not available" } + .release(findNavController()) } override fun onDestroyBinding(binding: FragmentDetailBinding) { @@ -168,26 +183,7 @@ class PlaylistDetailFragment : // Avoid possible race conditions that could cause a bad replace instruction to be consumed // during list initialization and crash the app. Could happen if the user is fast enough. detailModel.playlistSongInstructions.consume() - } - - override fun onDestinationChanged( - controller: NavController, - destination: NavDestination, - arguments: Bundle? - ) { - // Drop the initial call by NavController that simply provides us with the current - // destination. This would cause the selection state to be lost every time the device - // rotates. - if (!initialNavDestinationChange) { - initialNavDestinationChange = true - return - } - if (destination.id != R.id.playlist_detail_fragment && - destination.id != R.id.playlist_song_sort_dialog) { - // Drop any pending playlist edits when navigating away. This could actually happen - // if the user is quick enough. - detailModel.dropPlaylistEdit() - } + editNavigationListener = null } override fun onRealClick(item: Song) { diff --git a/app/src/main/java/org/oxycblt/auxio/list/ListViewModel.kt b/app/src/main/java/org/oxycblt/auxio/list/ListViewModel.kt index e1f4380a5..e223f439b 100644 --- a/app/src/main/java/org/oxycblt/auxio/list/ListViewModel.kt +++ b/app/src/main/java/org/oxycblt/auxio/list/ListViewModel.kt @@ -109,6 +109,22 @@ constructor(private val listSettings: ListSettings, private val musicRepository: _selected.value = selected } + /** + * Clear the current selection and return it. + * + * @return A list of [Song]s collated from each item selected. + */ + fun peekSelection() = + _selected.value.flatMap { + when (it) { + is Song -> listOf(it) + is Album -> listSettings.albumSongSort.songs(it.songs) + is Artist -> listSettings.artistSongSort.songs(it.songs) + is Genre -> listSettings.genreSongSort.songs(it.songs) + is Playlist -> it.songs + } + } + /** * Clear the current selection and return it. * @@ -116,17 +132,7 @@ constructor(private val listSettings: ListSettings, private val musicRepository: */ fun takeSelection(): List { logD("Taking selection") - return _selected.value - .flatMap { - when (it) { - is Song -> listOf(it) - is Album -> listSettings.albumSongSort.songs(it.songs) - is Artist -> listSettings.artistSongSort.songs(it.songs) - is Genre -> listSettings.genreSongSort.songs(it.songs) - is Playlist -> it.songs - } - } - .also { _selected.value = listOf() } + return peekSelection().also { _selected.value = listOf() } } /** diff --git a/app/src/main/java/org/oxycblt/auxio/list/SelectionFragment.kt b/app/src/main/java/org/oxycblt/auxio/list/SelectionFragment.kt index a0a73793f..69b58ac5f 100644 --- a/app/src/main/java/org/oxycblt/auxio/list/SelectionFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/list/SelectionFragment.kt @@ -49,7 +49,7 @@ abstract class SelectionFragment : setNavigationOnClickListener { listModel.dropSelection() } setOnMenuItemClickListener(this@SelectionFragment) overrideOnOverflowMenuClick { - listModel.openMenu(R.menu.selection, listModel.takeSelection()) + listModel.openMenu(R.menu.selection, listModel.peekSelection()) } } } diff --git a/app/src/main/java/org/oxycblt/auxio/list/menu/MenuDialogFragmentImpl.kt b/app/src/main/java/org/oxycblt/auxio/list/menu/MenuDialogFragmentImpl.kt index bb971728a..a7eef2392 100644 --- a/app/src/main/java/org/oxycblt/auxio/list/menu/MenuDialogFragmentImpl.kt +++ b/app/src/main/java/org/oxycblt/auxio/list/menu/MenuDialogFragmentImpl.kt @@ -79,10 +79,10 @@ class SongMenuDialogFragment : MenuDialogFragment() { playbackModel.addToQueue(menu.song) requireContext().showToast(R.string.lng_queue_added) } + R.id.action_playlist_add -> musicModel.addToPlaylist(menu.song) R.id.action_artist_details -> detailModel.showArtist(menu.song) R.id.action_album_details -> detailModel.showAlbum(menu.song.album) R.id.action_share -> requireContext().share(menu.song) - R.id.action_playlist_add -> musicModel.addToPlaylist(menu.song) R.id.action_detail -> detailModel.showSong(menu.song) else -> error("Unexpected menu item selected $item") } @@ -352,6 +352,7 @@ class SelectionMenuDialogFragment : MenuDialogFragment() { } override fun onClick(item: MenuItem, menu: Menu.ForSelection) { + listModel.dropSelection() when (item.itemId) { R.id.action_play -> playbackModel.play(menu.songs) R.id.action_shuffle -> playbackModel.shuffle(menu.songs) @@ -363,8 +364,8 @@ class SelectionMenuDialogFragment : MenuDialogFragment() { playbackModel.addToQueue(menu.songs) requireContext().showToast(R.string.lng_queue_added) } - R.id.action_share -> requireContext().share(menu.songs) R.id.action_playlist_add -> musicModel.addToPlaylist(menu.songs) + R.id.action_share -> requireContext().share(menu.songs) else -> error("Unexpected menu item selected $item") } } diff --git a/app/src/main/java/org/oxycblt/auxio/ui/DialogAwareNavigationListener.kt b/app/src/main/java/org/oxycblt/auxio/ui/DialogAwareNavigationListener.kt new file mode 100644 index 000000000..1c3e9f340 --- /dev/null +++ b/app/src/main/java/org/oxycblt/auxio/ui/DialogAwareNavigationListener.kt @@ -0,0 +1,79 @@ +/* + * Copyright (c) 2023 Auxio Project + * DialogAwareNavigationListener.kt is part of Auxio. + * + * 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.oxycblt.auxio.ui + +import android.os.Bundle +import androidx.navigation.NavController +import androidx.navigation.NavDestination + +/** + * A [NavController.OnDestinationChangedListener] that will call [callback] when moving between + * fragments only (not between dialogs or anything similar) + * + * @author Alexander Capehart (OxygenCobalt) + */ +class DialogAwareNavigationListener(private val callback: () -> Unit) : + NavController.OnDestinationChangedListener { + private var currentDestination: NavDestination? = null + + /** + * Attach this instance to a [NavController]. This should be done in the onStart method of a + * Fragment. + * + * @param navController The [NavController] to add to. + */ + fun attach(navController: NavController) { + currentDestination = null + navController.addOnDestinationChangedListener(this) + } + + /** + * Remove this listener from it's [NavController]. This should be done in the onStop method of a + * Fragment. + * + * @param navController The [NavController] to remove from. Should be the same on used in + * [attach]. + */ + fun release(navController: NavController) { + currentDestination = null + navController.removeOnDestinationChangedListener(this) + } + + override fun onDestinationChanged( + controller: NavController, + destination: NavDestination, + arguments: Bundle? + ) { + // Drop the initial call by NavController that simply provides us with the current + // destination. This would cause the selection state to be lost every time the device + // rotates. + val lastDestination = currentDestination + currentDestination = destination + if (lastDestination == null) { + return + } + + if (!lastDestination.isDialog() && !destination.isDialog()) { + callback() + } + } + + /** This relies on special label naming used in-app. */ + private fun NavDestination.isDialog() = label?.endsWith("dialog") == true +} diff --git a/app/src/main/res/navigation/inner.xml b/app/src/main/res/navigation/inner.xml index ef19e5474..d665a90d3 100644 --- a/app/src/main/res/navigation/inner.xml +++ b/app/src/main/res/navigation/inner.xml @@ -7,7 +7,7 @@ + android:label="settings_fragment"> @@ -41,7 +41,7 @@ + android:label="ui_preferences_fragment"> @@ -50,7 +50,7 @@ + android:label="personalize_preferences_fragment"> @@ -59,7 +59,7 @@ + android:label="personalize_preferences_fragment"> @@ -68,7 +68,7 @@ + android:label="personalize_preferences_fragment">