[Media] Ensure that we update the seekbar visibility whenever its
enabled status changes.
See b/228414210#comment4 for an in-depth explanation.
Fixes: 228414210
Test: manual: Start a media session then update the display size
or font size; verify that the seekbar still shows up.
Test: MediaControlPanelTest
Change-Id: I37ec1149ce24b81ca7f545ee524df68173b3c943
diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java
index 48a63ed..12369e5 100644
--- a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java
@@ -176,9 +176,12 @@
private String mPackageName;
private boolean mIsScrubbing = false;
+ private boolean mIsSeekBarEnabled = false;
private final SeekBarViewModel.ScrubbingChangeListener mScrubbingChangeListener =
this::setIsScrubbing;
+ private final SeekBarViewModel.EnabledChangeListener mEnabledChangeListener =
+ this::setIsSeekBarEnabled;
/**
* Initialize a new control panel
@@ -235,8 +238,9 @@
public void onDestroy() {
if (mSeekBarObserver != null) {
mSeekBarViewModel.getProgress().removeObserver(mSeekBarObserver);
- mSeekBarViewModel.removeScrubbingChangeListener(mScrubbingChangeListener);
}
+ mSeekBarViewModel.removeScrubbingChangeListener(mScrubbingChangeListener);
+ mSeekBarViewModel.removeEnabledChangeListener(mEnabledChangeListener);
mSeekBarViewModel.onDestroy();
mMediaViewController.onDestroy();
}
@@ -283,7 +287,7 @@
}
/** Sets whether the user is touching the seek bar to change the track position. */
- public void setIsScrubbing(boolean isScrubbing) {
+ private void setIsScrubbing(boolean isScrubbing) {
if (mMediaData == null || mMediaData.getSemanticActions() == null) {
return;
}
@@ -295,6 +299,14 @@
updateDisplayForScrubbingChange(mMediaData.getSemanticActions()));
}
+ private void setIsSeekBarEnabled(boolean isSeekBarEnabled) {
+ if (isSeekBarEnabled == this.mIsSeekBarEnabled) {
+ return;
+ }
+ this.mIsSeekBarEnabled = isSeekBarEnabled;
+ updateSeekBarVisibility();
+ }
+
/**
* Get the context
*
@@ -313,6 +325,7 @@
mSeekBarViewModel.getProgress().observeForever(mSeekBarObserver);
mSeekBarViewModel.attachTouchHandlers(vh.getSeekBar());
mSeekBarViewModel.setScrubbingChangeListener(mScrubbingChangeListener);
+ mSeekBarViewModel.setEnabledChangeListener(mEnabledChangeListener);
mMediaViewController.attach(player, MediaViewController.TYPE.PLAYER);
vh.getPlayer().setOnLongClickListener(v -> {
@@ -450,8 +463,8 @@
bindOutputSwitcherChip(data);
bindLongPressMenu(data);
- bindActionButtons(data);
bindScrubbingTime(data);
+ bindActionButtons(data);
boolean isSongUpdated = bindSongMetadata(data);
bindArtworkAndColors(data, isSongUpdated);
@@ -735,13 +748,18 @@
/* showInCompact= */ false);
}
}
+
+ updateSeekBarVisibility();
+ }
+
+ private void updateSeekBarVisibility() {
+ ConstraintSet expandedSet = mMediaViewController.getExpandedLayout();
expandedSet.setVisibility(R.id.media_progress_bar, getSeekBarVisibility());
- expandedSet.setAlpha(R.id.media_progress_bar, mSeekBarViewModel.getEnabled() ? 1.0f : 0.0f);
+ expandedSet.setAlpha(R.id.media_progress_bar, mIsSeekBarEnabled ? 1.0f : 0.0f);
}
private int getSeekBarVisibility() {
- boolean seekbarEnabled = mSeekBarViewModel.getEnabled();
- if (seekbarEnabled) {
+ if (mIsSeekBarEnabled) {
return ConstraintSet.VISIBLE;
}
// If disabled and "neighbours" are visible, set progress bar to INVISIBLE instead of GONE
@@ -751,8 +769,7 @@
private boolean areAnyExpandedBottomActionsVisible() {
ConstraintSet expandedSet = mMediaViewController.getExpandedLayout();
- int[] referencedIds = mMediaViewHolder.getActionsTopBarrier().getReferencedIds();
- for (int id : referencedIds) {
+ for (int id : MediaViewHolder.Companion.getExpandedBottomActionIds()) {
if (expandedSet.getVisibility(id) == ConstraintSet.VISIBLE) {
return true;
}
@@ -872,7 +889,6 @@
}
/** Updates all the views that might change due to a scrubbing state change. */
- // TODO(b/209656742): Handle scenarios where actionPrev and/or actionNext aren't active.
private void updateDisplayForScrubbingChange(@NonNull MediaButton semanticActions) {
// Update visibilities of the scrubbing time views and the scrubbing-dependent buttons.
bindScrubbingTime(mMediaData);
diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaViewHolder.kt b/packages/SystemUI/src/com/android/systemui/media/MediaViewHolder.kt
index 8964d71..b8b7318 100644
--- a/packages/SystemUI/src/com/android/systemui/media/MediaViewHolder.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaViewHolder.kt
@@ -187,5 +187,17 @@
R.id.action3,
R.id.action4
)
+
+ val expandedBottomActionIds = setOf(
+ R.id.actionPrev,
+ R.id.actionNext,
+ R.id.action0,
+ R.id.action1,
+ R.id.action2,
+ R.id.action3,
+ R.id.action4,
+ R.id.media_scrubbing_elapsed_time,
+ R.id.media_scrubbing_total_time
+ )
}
-}
\ No newline at end of file
+}
diff --git a/packages/SystemUI/src/com/android/systemui/media/SeekBarViewModel.kt b/packages/SystemUI/src/com/android/systemui/media/SeekBarViewModel.kt
index 5218492..193166b 100644
--- a/packages/SystemUI/src/com/android/systemui/media/SeekBarViewModel.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/SeekBarViewModel.kt
@@ -76,7 +76,11 @@
) {
private var _data = Progress(false, false, false, false, null, 0)
set(value) {
+ val enabledChanged = value.enabled != field.enabled
field = value
+ if (enabledChanged) {
+ enabledChangeListener?.onEnabledChanged(value.enabled)
+ }
_progress.postValue(value)
}
private val _progress = MutableLiveData<Progress>().apply {
@@ -122,6 +126,7 @@
}
private var scrubbingChangeListener: ScrubbingChangeListener? = null
+ private var enabledChangeListener: EnabledChangeListener? = null
/** Set to true when the user is touching the seek bar to change the position. */
private var scrubbing = false
@@ -136,8 +141,6 @@
lateinit var logSeek: () -> Unit
- fun getEnabled() = _data.enabled
-
/**
* Event indicating that the user has started interacting with the seek bar.
*/
@@ -189,6 +192,9 @@
/**
* Updates media information.
+ *
+ * This function makes a binder call, so it must happen on a worker thread.
+ *
* @param mediaController controller for media session
*/
@WorkerThread
@@ -232,6 +238,7 @@
cancel?.run()
cancel = null
scrubbingChangeListener = null
+ enabledChangeListener = null
}
@WorkerThread
@@ -279,11 +286,26 @@
}
}
+ fun setEnabledChangeListener(listener: EnabledChangeListener) {
+ enabledChangeListener = listener
+ }
+
+ fun removeEnabledChangeListener(listener: EnabledChangeListener) {
+ if (listener == enabledChangeListener) {
+ enabledChangeListener = null
+ }
+ }
+
/** Listener interface to be notified when the user starts or stops scrubbing. */
interface ScrubbingChangeListener {
fun onScrubbingChanged(scrubbing: Boolean)
}
+ /** Listener interface to be notified when the seekbar's enabled status changes. */
+ interface EnabledChangeListener {
+ fun onEnabledChanged(enabled: Boolean)
+ }
+
private class SeekBarChangeListener(
val viewModel: SeekBarViewModel
) : SeekBar.OnSeekBarChangeListener {
diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt
index 33db993..4e130d1 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt
@@ -291,7 +291,7 @@
seamlessButton = View(context)
seamlessIcon = ImageView(context)
seamlessText = TextView(context)
- seekBar = SeekBar(context)
+ seekBar = SeekBar(context).also { it.id = R.id.media_progress_bar }
settings = ImageButton(context)
cancel = View(context)
cancelText = TextView(context)
@@ -539,8 +539,8 @@
}
@Test
- fun bind_seekBarDisabled_seekBarVisibilityIsSetToInvisible() {
- whenever(seekBarViewModel.getEnabled()).thenReturn(false)
+ fun bind_seekBarDisabled_hasActions_seekBarVisibilityIsSetToInvisible() {
+ useRealConstraintSets()
val icon = context.getDrawable(android.R.drawable.ic_media_play)
val semanticActions = MediaButton(
@@ -550,21 +550,84 @@
val state = mediaData.copy(semanticActions = semanticActions)
player.attachPlayer(viewHolder)
+ getEnabledChangeListener().onEnabledChanged(enabled = false)
+
player.bindPlayer(state, PACKAGE)
- verify(expandedSet).setVisibility(R.id.media_progress_bar, ConstraintSet.INVISIBLE)
+ assertThat(expandedSet.getVisibility(seekBar.id)).isEqualTo(ConstraintSet.INVISIBLE)
}
@Test
fun bind_seekBarDisabled_noActions_seekBarVisibilityIsSetToGone() {
- whenever(seekBarViewModel.getEnabled()).thenReturn(false)
+ useRealConstraintSets()
+
+ val state = mediaData.copy(semanticActions = MediaButton())
+ player.attachPlayer(viewHolder)
+ getEnabledChangeListener().onEnabledChanged(enabled = false)
+
+ player.bindPlayer(state, PACKAGE)
+
+ assertThat(expandedSet.getVisibility(seekBar.id)).isEqualTo(ConstraintSet.GONE)
+ }
+
+ @Test
+ fun bind_seekBarEnabled_seekBarVisible() {
+ useRealConstraintSets()
+
+ val state = mediaData.copy(semanticActions = MediaButton())
+ player.attachPlayer(viewHolder)
+ getEnabledChangeListener().onEnabledChanged(enabled = true)
+
+ player.bindPlayer(state, PACKAGE)
+
+ assertThat(expandedSet.getVisibility(seekBar.id)).isEqualTo(ConstraintSet.VISIBLE)
+ }
+
+ @Test
+ fun seekBarChangesToEnabledAfterBind_seekBarChangesToVisible() {
+ useRealConstraintSets()
+
+ val state = mediaData.copy(semanticActions = MediaButton())
+ player.attachPlayer(viewHolder)
+ player.bindPlayer(state, PACKAGE)
+
+ getEnabledChangeListener().onEnabledChanged(enabled = true)
+
+ assertThat(expandedSet.getVisibility(seekBar.id)).isEqualTo(ConstraintSet.VISIBLE)
+ }
+
+ @Test
+ fun seekBarChangesToDisabledAfterBind_noActions_seekBarChangesToGone() {
+ useRealConstraintSets()
val state = mediaData.copy(semanticActions = MediaButton())
player.attachPlayer(viewHolder)
+ getEnabledChangeListener().onEnabledChanged(enabled = true)
player.bindPlayer(state, PACKAGE)
- verify(expandedSet).setVisibility(R.id.media_progress_bar, ConstraintSet.INVISIBLE)
+ getEnabledChangeListener().onEnabledChanged(enabled = false)
+
+ assertThat(expandedSet.getVisibility(seekBar.id)).isEqualTo(ConstraintSet.GONE)
+ }
+
+ @Test
+ fun seekBarChangesToDisabledAfterBind_hasActions_seekBarChangesToInvisible() {
+ useRealConstraintSets()
+
+ val icon = context.getDrawable(android.R.drawable.ic_media_play)
+ val semanticActions = MediaButton(
+ nextOrCustom = MediaAction(icon, Runnable {}, "next", null)
+ )
+ val state = mediaData.copy(semanticActions = semanticActions)
+
+ player.attachPlayer(viewHolder)
+ getEnabledChangeListener().onEnabledChanged(enabled = true)
+ player.bindPlayer(state, PACKAGE)
+
+ getEnabledChangeListener().onEnabledChanged(enabled = false)
+
+ assertThat(expandedSet.getVisibility(seekBar.id)).isEqualTo(ConstraintSet.INVISIBLE)
}
@Test
@@ -1317,4 +1380,24 @@
private fun getScrubbingChangeListener(): SeekBarViewModel.ScrubbingChangeListener =
withArgCaptor { verify(seekBarViewModel).setScrubbingChangeListener(capture()) }
+
+ private fun getEnabledChangeListener(): SeekBarViewModel.EnabledChangeListener =
+ withArgCaptor { verify(seekBarViewModel).setEnabledChangeListener(capture()) }
+
+ /**
+ * Update our test to use real ConstraintSets instead of mocks.
+ *
+ * Some item visibilities, such as the seekbar visibility, are dependent on other action's
+ * visibilities. If we use mocks for the ConstraintSets, then action visibility changes are
+ * just thrown away instead of being saved for reference later. This method sets us up to use
+ * ConstraintSets so that we do save visibility changes.
+ *
+ * TODO(b/229740380): Can/should we use real expanded and collapsed sets for all tests?
+ */
+ private fun useRealConstraintSets() {
+ expandedSet = ConstraintSet()
+ collapsedSet = ConstraintSet()
+ whenever(mediaViewController.expandedLayout).thenReturn(expandedSet)
+ whenever(mediaViewController.collapsedLayout).thenReturn(collapsedSet)
+ }
}
