From 949d71dbd1fccefd58e31a82b1b64b49dae8efae Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Fri, 26 Nov 2021 10:17:46 -0700 Subject: [PATCH] playback: actually fix controls not working Turns out playback controls wouldn't actually work because the view would detach but not the actual fragment, resulting in onCreateView never being called and the entire system falling apart. This fixes it by just giving PlaybackLayout the viewmodel instance it needs. I'll need to release a hotfix for this issue since this is really easy the trigger and really hard to fix unless you know why it occurs. Android lifecycles suck so much. --- .../java/org/oxycblt/auxio/MainFragment.kt | 44 +------------ .../oxycblt/auxio/playback/PlaybackBarView.kt | 65 +++++++++++-------- .../oxycblt/auxio/playback/PlaybackLayout.kt | 55 +++++----------- .../res/layout-w600dp/view_playback_bar.xml | 7 -- app/src/main/res/layout/view_playback_bar.xml | 5 -- info/ARCHITECTURE.md | 3 +- 6 files changed, 59 insertions(+), 120 deletions(-) diff --git a/app/src/main/java/org/oxycblt/auxio/MainFragment.kt b/app/src/main/java/org/oxycblt/auxio/MainFragment.kt index 2007e0828..23f5d46c9 100644 --- a/app/src/main/java/org/oxycblt/auxio/MainFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/MainFragment.kt @@ -34,7 +34,6 @@ import org.oxycblt.auxio.databinding.FragmentMainBinding import org.oxycblt.auxio.detail.DetailViewModel import org.oxycblt.auxio.music.MusicStore import org.oxycblt.auxio.music.MusicViewModel -import org.oxycblt.auxio.playback.PlaybackLayout import org.oxycblt.auxio.playback.PlaybackViewModel import org.oxycblt.auxio.util.logD @@ -43,7 +42,7 @@ import org.oxycblt.auxio.util.logD * the more high-level navigation features. * @author OxygenCobalt */ -class MainFragment : Fragment(), PlaybackLayout.ActionCallback { +class MainFragment : Fragment() { private val playbackModel: PlaybackViewModel by activityViewModels() private val detailModel: DetailViewModel by activityViewModels() private val musicModel: MusicViewModel by activityViewModels() @@ -78,23 +77,7 @@ class MainFragment : Fragment(), PlaybackLayout.ActionCallback { // We have to control the bar view from here since using a Fragment in PlaybackLayout // would result in annoying UI issues. - binding.playbackLayout.setActionCallback(this) - - binding.playbackLayout.setSong(playbackModel.song.value) - binding.playbackLayout.setPlaying(playbackModel.isPlaying.value!!) - binding.playbackLayout.setPosition(playbackModel.position.value!!) - - playbackModel.song.observe(viewLifecycleOwner) { song -> - binding.playbackLayout.setSong(song) - } - - playbackModel.isPlaying.observe(viewLifecycleOwner) { isPlaying -> - binding.playbackLayout.setPlaying(isPlaying) - } - - playbackModel.position.observe(viewLifecycleOwner) { pos -> - binding.playbackLayout.setPosition(pos) - } + binding.playbackLayout.setup(playbackModel, detailModel, viewLifecycleOwner) // Initialize music loading. Do it here so that it shows on every fragment that this // one contains. @@ -156,29 +139,6 @@ class MainFragment : Fragment(), PlaybackLayout.ActionCallback { callback?.isEnabled = false } - override fun onDestroyView() { - super.onDestroyView() - - // This callback has access to the binding, so make sure we clear it when we're done. - callback = null - } - - override fun onNavToItem() { - detailModel.navToItem(playbackModel.song.value ?: return) - } - - override fun onPrev() { - playbackModel.skipPrev() - } - - override fun onPlayPauseClick() { - playbackModel.invertPlayingStatus() - } - - override fun onNext() { - playbackModel.skipNext() - } - /** * A back press callback that handles how to respond to backwards navigation in the detail * fragments and the playback panel. diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackBarView.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackBarView.kt index a3e041421..350cb61ef 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackBarView.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackBarView.kt @@ -23,9 +23,11 @@ import android.util.AttributeSet import android.view.WindowInsets import androidx.constraintlayout.widget.ConstraintLayout import androidx.core.view.updatePadding +import androidx.lifecycle.LifecycleOwner import com.google.android.material.color.MaterialColors import org.oxycblt.auxio.R import org.oxycblt.auxio.databinding.ViewPlaybackBarBinding +import org.oxycblt.auxio.detail.DetailViewModel import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.util.inflater import org.oxycblt.auxio.util.resolveAttr @@ -41,16 +43,10 @@ class PlaybackBarView @JvmOverloads constructor( defStyleAttr: Int = -1 ) : ConstraintLayout(context, attrs, defStyleAttr) { private val binding = ViewPlaybackBarBinding.inflate(context.inflater, this, true) - private var mCallback: PlaybackLayout.ActionCallback? = null init { id = R.id.playback_bar - setOnLongClickListener { - mCallback?.onNavToItem() - true - } - // Deliberately override the progress bar color [in a Lollipop-friendly way] so that // we use colorSecondary instead of colorSurfaceVariant. This is for two reasons: // 1. colorSurfaceVariant is used with the assumption that the view that is using it @@ -66,28 +62,45 @@ class PlaybackBarView @JvmOverloads constructor( return insets } + fun setup( + playbackModel: PlaybackViewModel, + detailModel: DetailViewModel, + viewLifecycleOwner: LifecycleOwner + ) { + setOnLongClickListener { + playbackModel.song.value?.let { song -> + detailModel.navToItem(song) + } + true + } + + binding.playbackSkipPrev?.setOnClickListener { + playbackModel.skipPrev() + } + + binding.playbackPlayPause.setOnClickListener { + playbackModel.invertPlayingStatus() + } + + binding.playbackSkipNext?.setOnClickListener { + playbackModel.skipNext() + } + + binding.playbackPlayPause.isActivated = playbackModel.isPlaying.value!! + + playbackModel.isPlaying.observe(viewLifecycleOwner) { isPlaying -> + binding.playbackPlayPause.isActivated = isPlaying + } + + binding.playbackProgressBar.progress = playbackModel.position.value!!.toInt() + + playbackModel.position.observe(viewLifecycleOwner) { position -> + binding.playbackProgressBar.progress = position.toInt() + } + } + fun setSong(song: Song) { binding.song = song binding.executePendingBindings() } - - fun setPlaying(isPlaying: Boolean) { - binding.playbackPlayPause.isActivated = isPlaying - } - - fun setPosition(position: Long) { - binding.playbackProgressBar.progress = position.toInt() - } - - fun setCallback(callback: PlaybackLayout.ActionCallback) { - mCallback = callback - binding.callback = callback - binding.executePendingBindings() - } - - fun clearCallback() { - mCallback = null - binding.callback = null - binding.executePendingBindings() - } } diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackLayout.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackLayout.kt index 081f5c965..5afb8eca1 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackLayout.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackLayout.kt @@ -20,11 +20,12 @@ import android.widget.FrameLayout import androidx.appcompat.app.AppCompatActivity import androidx.core.view.isInvisible import androidx.customview.widget.ViewDragHelper +import androidx.lifecycle.LifecycleOwner import com.google.android.material.shape.MaterialShapeDrawable import org.oxycblt.auxio.BuildConfig import org.oxycblt.auxio.R +import org.oxycblt.auxio.detail.DetailViewModel import org.oxycblt.auxio.music.Song -import org.oxycblt.auxio.util.logD import org.oxycblt.auxio.util.resolveAttr import org.oxycblt.auxio.util.resolveDrawable import org.oxycblt.auxio.util.systemBarsCompat @@ -56,13 +57,6 @@ class PlaybackLayout @JvmOverloads constructor( EXPANDED, COLLAPSED, HIDDEN, DRAGGING } - interface ActionCallback { - fun onNavToItem() - fun onPrev() - fun onPlayPauseClick() - fun onNext() - } - private lateinit var contentView: View private val playbackContainerView: FrameLayout private val playbackBarView: PlaybackBarView @@ -182,7 +176,21 @@ class PlaybackLayout @JvmOverloads constructor( * Update the song that this layout is showing. This will be reflected in the compact view * at the bottom of the screen. */ - fun setSong(song: Song?) { + fun setup( + playbackModel: PlaybackViewModel, + detailModel: DetailViewModel, + viewLifecycleOwner: LifecycleOwner + ) { + setSong(playbackModel.song.value) + + playbackModel.song.observe(viewLifecycleOwner) { song -> + setSong(song) + } + + playbackBarView.setup(playbackModel, detailModel, viewLifecycleOwner) + } + + private fun setSong(song: Song?) { if (song != null) { playbackBarView.setSong(song) @@ -195,35 +203,11 @@ class PlaybackLayout @JvmOverloads constructor( } } - /** - * Update the playing status on this layout. This will be reflected in the compact view - * at the bottom of the screen. - */ - fun setPlaying(isPlaying: Boolean) { - playbackBarView.setPlaying(isPlaying) - } - - /** - * Update the playback position on this layout. This will be reflected in the compact view - * at the bottom of the screen. - */ - fun setPosition(position: Long) { - playbackBarView.setPosition(position) - } - - /** - * Add a callback for actions from the compact playback view in this layout. - */ - fun setActionCallback(callback: ActionCallback) { - playbackBarView.setCallback(callback) - } - /** * Collapse the panel if it is currently expanded. * @return If the panel was collapsed or not. */ fun collapse(): Boolean { - logD(panelState) if (panelState == PanelState.EXPANDED) { applyState(PanelState.COLLAPSED) return true @@ -416,11 +400,6 @@ class PlaybackLayout @JvmOverloads constructor( } } - override fun onDetachedFromWindow() { - super.onDetachedFromWindow() - playbackBarView.clearCallback() - } - override fun onSaveInstanceState(): Parcelable = Bundle().apply { putParcelable("superState", super.onSaveInstanceState()) putSerializable( diff --git a/app/src/main/res/layout-w600dp/view_playback_bar.xml b/app/src/main/res/layout-w600dp/view_playback_bar.xml index c6f0dd9d9..c22ad6151 100644 --- a/app/src/main/res/layout-w600dp/view_playback_bar.xml +++ b/app/src/main/res/layout-w600dp/view_playback_bar.xml @@ -10,10 +10,6 @@ name="song" type="org.oxycblt.auxio.music.Song" /> - - - - diff --git a/info/ARCHITECTURE.md b/info/ARCHITECTURE.md index e5df7f348..44fb18174 100644 --- a/info/ARCHITECTURE.md +++ b/info/ARCHITECTURE.md @@ -275,8 +275,7 @@ This module not only contains the playback system described above, but also mult - `system` contains the system-facing playback system The most important part of this module is `PlaybackLayout`, which is a custom `ViewGroup` that implements the playback bar and it's ability to -slide up into the full playback view. `MainFragment` controls this `ViewGroup`, more specifically the bar view, as it can't be an independent -fragment due to a couple of annoying reasons. +slide up into the full playback view. `MainFragment` controls this `ViewGroup`. #### `.search`