Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Reject flings on seek bar

Browse Source 
This should help reduce accidentials seek. A gesture that includes a
fling will be rejected. The progress will be returned to the place where
it was located at the start of the gesture.

Fixes: 155673905
Test: atest tests/src/com/android/systemui/media/SeekBarViewModelTest.kt
Test: manual - fling seek bar. It should bounce back.
Change-Id: Id0846d671c17d7362d45afec3e77676e764d96b3
This commit is contained in:
Robert Snoeberger
2020-06-11 00:06:11 -04:00
parent 77d3dcab6f
commit 21d133ed1a
2 changed files with 259 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

125
packages/SystemUI/src/com/android/systemui/media/SeekBarViewModel.kt
View File

@@ -23,6 +23,7 @@ import android.os.SystemClock
import android.view.GestureDetector
import android.view.MotionEvent
import android.view.View
import android.view.ViewConfiguration
import android.widget.SeekBar
import androidx.annotation.AnyThread
import androidx.annotation.WorkerThread
@@ -31,10 +32,10 @@ import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.LiveData
import com.android.systemui.dagger.qualifiers.Background
import com.android.systemui.util.concurrency.RepeatableExecutor
import java.util.concurrent.Executor
import javax.inject.Inject
private const val POSITION_UPDATE_INTERVAL_MILLIS = 100L
private const val MIN_FLING_VELOCITY_SCALE_FACTOR = 10
private fun PlaybackState.isInMotion(): Boolean {
return this.state == PlaybackState.STATE_PLAYING ||
@@ -105,6 +106,9 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
}
private var cancel: Runnable? = null
/** Indicates if the seek interaction is considered a false guesture. */
private var isFalseSeek = false
/** Listening state (QS open or closed) is used to control polling of progress. */
var listening = true
set(value) = bgExecutor.execute {
@@ -114,16 +118,61 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
}
}
/** Set to true when the user is touching the seek bar to change the position. */
private var scrubbing = false
set(value) {
if (field != value) {
field = value
checkIfPollingNeeded()
}
}
/**
* Event indicating that the user has started interacting with the seek bar.
*/
@AnyThread
fun onSeekStarting() = bgExecutor.execute {
scrubbing = true
isFalseSeek = false
}
/**
* Event indicating that the user has moved the seek bar but hasn't yet finished the gesture.
* @param position Current location in the track.
*/
@AnyThread
fun onSeekProgress(position: Long) = bgExecutor.execute {
if (scrubbing) {
_data = _data.copy(elapsedTime = position.toInt())
}
}
/**
* Event indicating that the seek interaction is a false gesture and it should be ignored.
*/
@AnyThread
fun onSeekFalse() = bgExecutor.execute {
if (scrubbing) {
isFalseSeek = true
}
}
/**
* Handle request to change the current position in the media track.
* @param position Place to seek to in the track.
*/
@WorkerThread
fun onSeek(position: Long) {
controller?.transportControls?.seekTo(position)
// Invalidate the cached playbackState to avoid the thumb jumping back to the previous
// position.
playbackState = null
@AnyThread
fun onSeek(position: Long) = bgExecutor.execute {
if (isFalseSeek) {
scrubbing = false
checkPlaybackPosition()
} else {
controller?.transportControls?.seekTo(position)
// Invalidate the cached playbackState to avoid the thumb jumping back to the previous
// position.
playbackState = null
scrubbing = false
}
}
/**
@@ -181,7 +230,7 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
@WorkerThread
private fun checkIfPollingNeeded() {
val needed = listening && playbackState?.isInMotion() ?: false
val needed = listening && !scrubbing && playbackState?.isInMotion() ?: false
if (needed) {
if (cancel == null) {
cancel = bgExecutor.executeRepeatedly(this::checkPlaybackPosition, 0L,
@@ -196,33 +245,30 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
/** Gets a listener to attach to the seek bar to handle seeking. */
val seekBarListener: SeekBar.OnSeekBarChangeListener
get() {
return SeekBarChangeListener(this, bgExecutor)
return SeekBarChangeListener(this)
}
/** Attach touch handlers to the seek bar view. */
fun attachTouchHandlers(bar: SeekBar) {
bar.setOnSeekBarChangeListener(seekBarListener)
bar.setOnTouchListener(SeekBarTouchListener(bar))
bar.setOnTouchListener(SeekBarTouchListener(this, bar))
}
private class SeekBarChangeListener(
val viewModel: SeekBarViewModel,
val bgExecutor: Executor
val viewModel: SeekBarViewModel
) : SeekBar.OnSeekBarChangeListener {
override fun onProgressChanged(bar: SeekBar, progress: Int, fromUser: Boolean) {
if (fromUser) {
bgExecutor.execute {
viewModel.onSeek(progress.toLong())
}
viewModel.onSeekProgress(progress.toLong())
}
}
override fun onStartTrackingTouch(bar: SeekBar) {
viewModel.onSeekStarting()
}
override fun onStopTrackingTouch(bar: SeekBar) {
val pos = bar.progress.toLong()
bgExecutor.execute {
viewModel.onSeek(pos)
}
viewModel.onSeek(bar.progress.toLong())
}
}
@@ -233,14 +279,18 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
* they intend to scroll the carousel.
*/
private class SeekBarTouchListener(
private val viewModel: SeekBarViewModel,
private val bar: SeekBar
) : View.OnTouchListener, GestureDetector.OnGestureListener {
// Gesture detector helps decide which touch events to intercept.
private val detector = GestureDetectorCompat(bar.context, this)
// Defines a tap target around the thumb at the beginning of a gesture.
private var onDownTargetBoxMinX: Int = -1
private var onDownTargetBoxMaxX: Int = -1
// Velocity threshold used to decide when a fling is considered a false gesture.
private val flingVelocity: Int = ViewConfiguration.get(bar.context).run {
getScaledMinimumFlingVelocity() * MIN_FLING_VELOCITY_SCALE_FACTOR
}
// Indicates if the gesture should go to the seek bar or if it should be intercepted.
private var shouldGoToSeekBar = false
/**
* Decide which touch events to intercept before they reach the seek bar.
@@ -261,7 +311,7 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
if (view != bar) {
return false
}
val shouldGoToSeekBar = detector.onTouchEvent(event)
detector.onTouchEvent(event)
return !shouldGoToSeekBar
}
@@ -295,16 +345,16 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
// Set the min, max boundaries of the thumb box.
// I'm cheating by using the height of the seek bar as the width of the box.
val halfHeight: Int = bar.height / 2
onDownTargetBoxMinX = (Math.round(thumbX) - halfHeight).toInt()
onDownTargetBoxMaxX = (Math.round(thumbX) + halfHeight).toInt()
val targetBoxMinX = (Math.round(thumbX) - halfHeight).toInt()
val targetBoxMaxX = (Math.round(thumbX) + halfHeight).toInt()
// If the x position of the down event is within the box, then request that the parent
// not intercept the event.
val x = Math.round(event.x)
val accept = x >= onDownTargetBoxMinX && x <= onDownTargetBoxMaxX
if (accept) {
shouldGoToSeekBar = x >= targetBoxMinX && x <= targetBoxMaxX
if (shouldGoToSeekBar) {
bar.parent?.requestDisallowInterceptTouchEvent(true)
}
return accept
return shouldGoToSeekBar
}
/**
@@ -312,7 +362,10 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
*
* This enables the user to single tap anywhere on the seek bar to seek to that position.
*/
override fun onSingleTapUp(event: MotionEvent) = true
override fun onSingleTapUp(event: MotionEvent): Boolean {
shouldGoToSeekBar = true
return true
}
/**
* Handle scroll events when the down event is on the thumb.
@@ -325,17 +378,13 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
distanceX: Float,
distanceY: Float
): Boolean {
val x = Math.round(eventStart.x)
return x >= onDownTargetBoxMinX && x <= onDownTargetBoxMaxX
return shouldGoToSeekBar
}
/**
* Handle fling events when the down event is on the thumb.
*
* TODO: Ignore entire gesture when it includes a fling.
* If a user is flinging, then they are probably trying to page the carousel. It would be
* better to ignore the entire gesture when it includes a fling. This could be achieved by
* reseting the seek bar position to where it was when the gesture started.
* Gestures that include a fling are considered a false gesture on the seek bar.
*/
override fun onFling(
eventStart: MotionEvent,
@@ -343,8 +392,10 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
velocityX: Float,
velocityY: Float
): Boolean {
val x = Math.round(eventStart.x)
return x >= onDownTargetBoxMinX && x <= onDownTargetBoxMaxX
if (Math.abs(velocityX) > flingVelocity || Math.abs(velocityY) > flingVelocity) {
viewModel.onSeekFalse()
}
return shouldGoToSeekBar
}
override fun onShowPress(event: MotionEvent) {}

183
packages/SystemUI/tests/src/com/android/systemui/media/SeekBarViewModelTest.kt
View File

@@ -40,6 +40,7 @@ import org.junit.runner.RunWith
import org.mockito.ArgumentCaptor
import org.mockito.Mock
import org.mockito.Mockito.any
import org.mockito.Mockito.eq
import org.mockito.Mockito.mock
import org.mockito.Mockito.never
import org.mockito.Mockito.times
@@ -204,7 +205,7 @@ public class SeekBarViewModelTest : SysuiTestCase() {
@Test
fun updateElapsedTime() {
// GIVEN that the PlaybackState contins the current position
// GIVEN that the PlaybackState contains the current position
val position = 200L
val state = PlaybackState.Builder().run {
setState(PlaybackState.STATE_PLAYING, position, 1f)
@@ -246,7 +247,7 @@ public class SeekBarViewModelTest : SysuiTestCase() {
}
@Test
fun handleSeek() {
fun onSeek() {
whenever(mockController.getTransportControls()).thenReturn(mockTransport)
viewModel.updateController(mockController)
// WHEN user input is dispatched
@@ -258,19 +259,73 @@ public class SeekBarViewModelTest : SysuiTestCase() {
}
@Test
fun handleProgressChangedUser() {
fun onSeekWithFalse() {
whenever(mockController.getTransportControls()).thenReturn(mockTransport)
viewModel.updateController(mockController)
// WHEN user starts dragging the seek bar
val pos = 42
viewModel.seekBarListener.onProgressChanged(SeekBar(context), pos, true)
// WHEN a false is received during the seek gesture
val pos = 42L
with(viewModel) {
onSeekStarting()
onSeekFalse()
onSeek(pos)
}
fakeExecutor.runAllReady()
// THEN transport controls should be used
verify(mockTransport).seekTo(pos.toLong())
// THEN the seek is rejected and the transport never receives seekTo
verify(mockTransport, never()).seekTo(pos)
}
@Test
fun handleProgressChangedOther() {
fun onSeekProgress() {
val pos = 42L
with(viewModel) {
onSeekStarting()
onSeekProgress(pos)
}
fakeExecutor.runAllReady()
// THEN then elapsed time should be updated
assertThat(viewModel.progress.value!!.elapsedTime).isEqualTo(pos)
}
@Test
fun onSeekProgressWithSeekStarting() {
val pos = 42L
with(viewModel) {
onSeekProgress(pos)
}
fakeExecutor.runAllReady()
// THEN then elapsed time should not be updated
assertThat(viewModel.progress.value!!.elapsedTime).isNull()
}
@Test
fun onProgressChangedFromUser() {
// WHEN user starts dragging the seek bar
val pos = 42
val bar = SeekBar(context)
with(viewModel.seekBarListener) {
onStartTrackingTouch(bar)
onProgressChanged(bar, pos, true)
}
fakeExecutor.runAllReady()
// THEN then elapsed time should be updated
assertThat(viewModel.progress.value!!.elapsedTime).isEqualTo(pos)
}
@Test
fun onProgressChangedFromUserWithoutStartTrackingTouch() {
// WHEN user starts dragging the seek bar
val pos = 42
val bar = SeekBar(context)
with(viewModel.seekBarListener) {
onProgressChanged(bar, pos, true)
}
fakeExecutor.runAllReady()
// THEN then elapsed time should not be updated
assertThat(viewModel.progress.value!!.elapsedTime).isNull()
}
@Test
fun onProgressChangedNotFromUser() {
whenever(mockController.getTransportControls()).thenReturn(mockTransport)
viewModel.updateController(mockController)
// WHEN user starts dragging the seek bar
@@ -282,7 +337,7 @@ public class SeekBarViewModelTest : SysuiTestCase() {
}
@Test
fun handleStartTrackingTouch() {
fun onStartTrackingTouch() {
whenever(mockController.getTransportControls()).thenReturn(mockTransport)
viewModel.updateController(mockController)
// WHEN user starts dragging the seek bar
@@ -297,7 +352,7 @@ public class SeekBarViewModelTest : SysuiTestCase() {
}
@Test
fun handleStopTrackingTouch() {
fun onStopTrackingTouch() {
whenever(mockController.getTransportControls()).thenReturn(mockTransport)
viewModel.updateController(mockController)
// WHEN user ends drag
@@ -311,6 +366,26 @@ public class SeekBarViewModelTest : SysuiTestCase() {
verify(mockTransport).seekTo(pos.toLong())
}
@Test
fun onStopTrackingTouchAfterProgress() {
whenever(mockController.getTransportControls()).thenReturn(mockTransport)
viewModel.updateController(mockController)
// WHEN user starts dragging the seek bar
val pos = 42
val progPos = 84
val bar = SeekBar(context).apply {
progress = pos
}
with(viewModel.seekBarListener) {
onStartTrackingTouch(bar)
onProgressChanged(bar, progPos, true)
onStopTrackingTouch(bar)
}
fakeExecutor.runAllReady()
// THEN then elapsed time should be updated
verify(mockTransport).seekTo(eq(pos.toLong()))
}
@Test
fun queuePollTaskWhenPlaying() {
// GIVEN that the track is playing
@@ -369,7 +444,7 @@ public class SeekBarViewModelTest : SysuiTestCase() {
}
// AND the playback state is playing
val state = PlaybackState.Builder().run {
setState(PlaybackState.STATE_STOPPED, 200L, 1f)
setState(PlaybackState.STATE_PLAYING, 200L, 1f)
build()
}
whenever(mockController.getPlaybackState()).thenReturn(state)
@@ -397,6 +472,90 @@ public class SeekBarViewModelTest : SysuiTestCase() {
assertThat(fakeExecutor.numPending()).isEqualTo(1)
}
@Test
fun noQueuePollTaskWhenSeeking() {
// GIVEN listening
viewModel.listening = true
// AND the playback state is playing
val state = PlaybackState.Builder().run {
setState(PlaybackState.STATE_PLAYING, 200L, 1f)
build()
}
whenever(mockController.getPlaybackState()).thenReturn(state)
viewModel.updateController(mockController)
with(fakeExecutor) {
advanceClockToNext()
runAllReady()
}
// WHEN seek starts
viewModel.onSeekStarting()
with(fakeExecutor) {
advanceClockToNext()
runAllReady()
}
// THEN an update task is not queued because we don't want it fighting with the user when
// they are trying to move the thumb.
assertThat(fakeExecutor.numPending()).isEqualTo(0)
}
@Test
fun queuePollTaskWhenDoneSeekingWithFalse() {
// GIVEN listening
viewModel.listening = true
// AND the playback state is playing
val state = PlaybackState.Builder().run {
setState(PlaybackState.STATE_PLAYING, 200L, 1f)
build()
}
whenever(mockController.getPlaybackState()).thenReturn(state)
viewModel.updateController(mockController)
with(fakeExecutor) {
advanceClockToNext()
runAllReady()
}
// WHEN seek finishes after a false
with(viewModel) {
onSeekStarting()
onSeekFalse()
onSeek(42L)
}
with(fakeExecutor) {
advanceClockToNext()
runAllReady()
}
// THEN an update task is queued because the gesture was ignored and progress was restored.
assertThat(fakeExecutor.numPending()).isEqualTo(1)
}
@Test
fun noQueuePollTaskWhenDoneSeeking() {
// GIVEN listening
viewModel.listening = true
// AND the playback state is playing
val state = PlaybackState.Builder().run {
setState(PlaybackState.STATE_PLAYING, 200L, 1f)
build()
}
whenever(mockController.getPlaybackState()).thenReturn(state)
viewModel.updateController(mockController)
with(fakeExecutor) {
advanceClockToNext()
runAllReady()
}
// WHEN seek finishes after a false
with(viewModel) {
onSeekStarting()
onSeek(42L)
}
with(fakeExecutor) {
advanceClockToNext()
runAllReady()
}
// THEN no update task is queued because we are waiting for an updated playback state to be
// returned in response to the seek.
assertThat(fakeExecutor.numPending()).isEqualTo(0)
}
@Test
fun startListeningQueuesPollTask() {
// GIVEN not listening