PC-I25: Prevent Mini Player From Animating When Changing Theme #513
Conversation
| binding.miniPlayer.setUpNext(upNext, theme) | ||
|
|
||
| // only show the mini player when an episode is loaded | ||
| if (upNext is UpNextQueue.State.Loaded) { | ||
| if ((isHidden() || !hasLoadedFirstTime)) { | ||
| show() | ||
| if (!shouldPlayerOpenOnAttach) { | ||
| if (!shouldPlayerOpenOnAttach && shouldAnimateOnAttach) { |
There was a problem hiding this comment.
This is preventing listener?.onMiniPlayerVisible() from getting called on line 148 after changing themes. As a result, the padding isn't being updated to handle the mini player being visible, and we're losing content under the miniplayer. I was able to see this by following these steps:
- Open the app and go to Profile tab -> Gear Icon
- Scroll to the bottom of the screen and observe that the last two settings are "Privacy" followed by "About"
- Tap on Appearance and change the theme
- Tap back and scroll to the bottom
- Observe that Privacy is now the last visible setting (because "About" is now hidden behind the miniplayer).
device-2022-10-31-134952.mp4
Similarly, it seems like we should also be updating the hasLoadedFirstTime field here, although I'm not sure that not updating it is actually causing any problems currently.
| @@ -219,7 +220,10 @@ class MainActivity : | |||
| activity = this | |||
| ) | |||
|
|
|||
| setupPlayerViews() | |||
| val showMiniPlayerImmediately = savedInstanceState?.getBoolean(SAVEDSTATE_MINIPLAYER_SHOWN, false) ?: false | |||
| binding.playerBottomSheet.isVisible = showMiniPlayerImmediately | |||
There was a problem hiding this comment.
Am I understanding correctly that we're updating the visibility here even though it will get updated later in order to avoid (or at least reduce) the miniplayer "flickering" when changing themes? If so, it might be worth adding a comment here briefly explaining that.
There was a problem hiding this comment.
Thanks for the contribution @owenlejeune ! 🙇 This is looking nice and it fixes the animation issue. 🎉
I left a comment below about an issue with the padding not being updated that I noticed. It would be good to address that before we merge this.
One other thing I noticed is that now, if a podcast is playing, the miniplayer's play button quickly switch back and forth between play and pause a few times when we switch themes. I suspect this was probably happening before, but the animation just hid it.
| Before | After |
|---|---|
device-2022-10-31-140719.mp4 |
device-2022-10-31-140137.mp4 |
We can log an issue for this (I wonder if it might have a similar cause as #52), and not address it in this PR if you'd like though because it's not directly related to the animation issue you're addressing here. Just let me know what you'd like to do.
|
@mchowning good catch! The padding issue should be addressed now. I don't seem to be able to reproduce the play button animation on my end though, may be best to open a new issue to address that separately. |
There was a problem hiding this comment.
Looks good. Thanks so much for taking care of this @owenlejeune!
I don't seem to be able to reproduce the play button animation on my end though, may be best to open a new issue to address that separately.
👍 Opened here: #523
Description
Store the state of the mini player when recreating the MainActivity so that the mini player animation can be disabled when changing the theme.
Fixes #25
Testing Instructions
Screenshots or Screencast
22-10-29-09-55-51.mp4