Letting View handle click and longclick detection

The touch listener for long-press effects in QS tiles is removed. An
investgation showed that this was the cause of the regression that this
CL aims to fix. With the listener removed, the tile view is left to
handle its own touch events and call the onTouchEvent method of the View
to allow the click and long-click detection logic to run. The
QSLongPressEffect class is now modified to act according to the
detection of clicks by the View, the triggering of long-clicks, and the
necessary tap timeout wait (as a new action in the Flow of actions).

Test: atest SystemUITests:QSTileViewImplTest
Test: atest SystemUiRobotTests:QSLongPressEffectTest
Bug: 341890266
Flag: com.android.systemui.quick_settings_visual_haptics_longpress

Change-Id: Iac48d6563be4e7442deb891c0be61551ac125670

5 files changed, 133 insertions, 69 deletions

diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/haptics/qs/QSLongPressEffectTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/haptics/qs/QSLongPressEffectTest.kt
index c51413a2cc78..4849e66d37d5 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/haptics/qs/QSLongPressEffectTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/haptics/qs/QSLongPressEffectTest.kt
@@ -26,6 +26,7 @@ import com.android.systemui.haptics.vibratorHelper
 import com.android.systemui.keyguard.data.repository.fakeKeyguardRepository
 import com.android.systemui.keyguard.domain.interactor.keyguardInteractor
 import com.android.systemui.kosmos.testScope
+import com.android.systemui.qs.qsTileFactory
 import com.android.systemui.testKosmos
 import com.google.common.truth.Truth.assertThat
 import kotlinx.coroutines.test.TestScope
@@ -41,6 +42,7 @@ class QSLongPressEffectTest : SysuiTestCase() {
 
     private val kosmos = testKosmos()
     private val vibratorHelper = kosmos.vibratorHelper
+    private val qsTile = kosmos.qsTileFactory.createTile("Test Tile")
 
     private val effectDuration = 400
     private val lowTickDuration = 12
@@ -61,6 +63,7 @@ class QSLongPressEffectTest : SysuiTestCase() {
                 vibratorHelper,
                 kosmos.keyguardInteractor,
             )
+        longPressEffect.qsTile = qsTile
     }
 
     @Test
@@ -91,8 +94,10 @@ class QSLongPressEffectTest : SysuiTestCase() {
         // GIVEN an action down event occurs
         longPressEffect.handleActionDown()
 
-        // THEN the effect moves to the TIMEOUT_WAIT state
+        // THEN the effect moves to the TIMEOUT_WAIT state and starts the wait
+        val action by collectLastValue(longPressEffect.actionType)
         assertThat(longPressEffect.state).isEqualTo(QSLongPressEffect.State.TIMEOUT_WAIT)
+        assertThat(action).isEqualTo(QSLongPressEffect.ActionType.WAIT_TAP_TIMEOUT)
     }
 
     @Test
@@ -107,20 +112,6 @@ class QSLongPressEffectTest : SysuiTestCase() {
         }
 
     @Test
-    fun onActionUp_whileWaiting_performsClick() =
-        testWhileInState(QSLongPressEffect.State.TIMEOUT_WAIT) {
-            // GIVEN an action is being collected
-            val action by collectLastValue(longPressEffect.actionType)
-
-            // GIVEN an action up occurs
-            longPressEffect.handleActionUp()
-
-            // THEN the action to invoke is the click action and the effect does not start
-            assertThat(action).isEqualTo(QSLongPressEffect.ActionType.CLICK)
-            assertEffectDidNotStart()
-        }
-
-    @Test
     fun onWaitComplete_whileWaiting_beginsEffect() =
         testWhileInState(QSLongPressEffect.State.TIMEOUT_WAIT) {
             // GIVEN the pressed timeout is complete
@@ -221,8 +212,10 @@ class QSLongPressEffectTest : SysuiTestCase() {
             // GIVEN that the animator was cancelled
             longPressEffect.handleAnimationCancel()
 
-            // THEN the state goes to the timeout wait
+            // THEN the state goes to the timeout wait and the wait is posted
+            val action by collectLastValue(longPressEffect.actionType)
             assertThat(longPressEffect.state).isEqualTo(QSLongPressEffect.State.TIMEOUT_WAIT)
+            assertThat(action).isEqualTo(QSLongPressEffect.ActionType.WAIT_TAP_TIMEOUT)
         }
 
     @Test
@@ -238,6 +231,29 @@ class QSLongPressEffectTest : SysuiTestCase() {
             assertThat(longPressEffect.state).isEqualTo(QSLongPressEffect.State.IDLE)
         }
 
+    @Test
+    fun onTileClick_whileWaiting_withQSTile_clicks() =
+        testWhileInState(QSLongPressEffect.State.TIMEOUT_WAIT) {
+            // GIVEN that a click was detected
+            val couldClick = longPressEffect.onTileClick()
+
+            // THEN the click is successful
+            assertThat(couldClick).isTrue()
+        }
+
+    @Test
+    fun onTileClick_whileWaiting_withoutQSTile_cannotClick() =
+        testWhileInState(QSLongPressEffect.State.TIMEOUT_WAIT) {
+            // GIVEN that no QSTile has been set
+            longPressEffect.qsTile = null
+
+            // GIVEN that a click was detected
+            val couldClick = longPressEffect.onTileClick()
+
+            // THEN the click is not successful
+            assertThat(couldClick).isFalse()
+        }
+
     private fun testWithScope(initialize: Boolean = true, test: suspend TestScope.() -> Unit) =
         with(kosmos) {
             testScope.runTest {
diff --git a/packages/SystemUI/src/com/android/systemui/haptics/qs/QSLongPressEffect.kt b/packages/SystemUI/src/com/android/systemui/haptics/qs/QSLongPressEffect.kt
index ea8d7d778851..30b958393b60 100644
--- a/packages/SystemUI/src/com/android/systemui/haptics/qs/QSLongPressEffect.kt
+++ b/packages/SystemUI/src/com/android/systemui/haptics/qs/QSLongPressEffect.kt
@@ -19,7 +19,9 @@ package com.android.systemui.haptics.qs
 import android.os.VibrationEffect
 import android.view.View
 import androidx.annotation.VisibleForTesting
+import com.android.systemui.animation.Expandable
 import com.android.systemui.keyguard.domain.interactor.KeyguardInteractor
+import com.android.systemui.plugins.qs.QSTile
 import com.android.systemui.statusbar.VibratorHelper
 import javax.inject.Inject
 import kotlinx.coroutines.flow.Flow
@@ -51,6 +53,10 @@ constructor(
     var state = State.IDLE
         private set
 
+    /** The QSTile and Expandable used to perform a long-click and click actions */
+    var qsTile: QSTile? = null
+    var expandable: Expandable? = null
+
     /** Flow for view control and action */
     private val _postedActionType = MutableStateFlow<ActionType?>(null)
     val actionType: Flow<ActionType?> =
@@ -105,6 +111,7 @@ constructor(
         when (state) {
             State.IDLE -> {
                 setState(State.TIMEOUT_WAIT)
+                _postedActionType.value = ActionType.WAIT_TAP_TIMEOUT
             }
             State.RUNNING_BACKWARDS -> _postedActionType.value = ActionType.CANCEL_ANIMATOR
             else -> {}
@@ -112,16 +119,9 @@ constructor(
     }
 
     fun handleActionUp() {
-        when (state) {
-            State.TIMEOUT_WAIT -> {
-                _postedActionType.value = ActionType.CLICK
-                setState(State.IDLE)
-            }
-            State.RUNNING_FORWARD -> {
-                _postedActionType.value = ActionType.REVERSE_ANIMATOR
-                setState(State.RUNNING_BACKWARDS)
-            }
-            else -> {}
+        if (state == State.RUNNING_FORWARD) {
+            _postedActionType.value = ActionType.REVERSE_ANIMATOR
+            setState(State.RUNNING_BACKWARDS)
         }
     }
 
@@ -129,6 +129,7 @@ constructor(
         when (state) {
             State.TIMEOUT_WAIT -> {
                 setState(State.IDLE)
+                clearActionType()
             }
             State.RUNNING_FORWARD -> {
                 _postedActionType.value = ActionType.REVERSE_ANIMATOR
@@ -145,18 +146,23 @@ constructor(
 
     /** This function is called both when an animator completes or gets cancelled */
     fun handleAnimationComplete() {
-        if (state == State.RUNNING_FORWARD) {
-            vibrate(snapEffect)
-            _postedActionType.value = ActionType.LONG_PRESS
-        }
-        if (state != State.TIMEOUT_WAIT) {
-            // This will happen if the animator did not finish by being cancelled
-            setState(State.IDLE)
+        when (state) {
+            State.RUNNING_FORWARD -> {
+                setState(State.IDLE)
+                vibrate(snapEffect)
+                _postedActionType.value = ActionType.LONG_PRESS
+            }
+            State.RUNNING_BACKWARDS -> {
+                setState(State.IDLE)
+                clearActionType()
+            }
+            else -> {}
         }
     }
 
     fun handleAnimationCancel() {
         setState(State.TIMEOUT_WAIT)
+        _postedActionType.value = ActionType.WAIT_TAP_TIMEOUT
     }
 
     fun handleTimeoutComplete() {
@@ -190,9 +196,22 @@ constructor(
                 effectDuration
             )
         setState(State.IDLE)
+        clearActionType()
         return true
     }
 
+    fun onTileClick(): Boolean {
+        if (state == State.TIMEOUT_WAIT) {
+            setState(State.IDLE)
+            clearActionType()
+            qsTile?.let {
+                it.click(expandable)
+                return true
+            }
+        }
+        return false
+    }
+
     enum class State {
         IDLE, /* The effect is idle waiting for touch input */
         TIMEOUT_WAIT, /* The effect is waiting for a [PRESSED_TIMEOUT] period */
@@ -202,7 +221,7 @@ constructor(
 
     /* A type of action to perform on the view depending on the effect's state and logic */
     enum class ActionType {
-        CLICK,
+        WAIT_TAP_TIMEOUT,
         LONG_PRESS,
         RESET_AND_LONG_PRESS,
         START_ANIMATOR,
diff --git a/packages/SystemUI/src/com/android/systemui/haptics/qs/QSLongPressEffectViewBinder.kt b/packages/SystemUI/src/com/android/systemui/haptics/qs/QSLongPressEffectViewBinder.kt
index 4875f481cce6..92a55ef0e74f 100644
--- a/packages/SystemUI/src/com/android/systemui/haptics/qs/QSLongPressEffectViewBinder.kt
+++ b/packages/SystemUI/src/com/android/systemui/haptics/qs/QSLongPressEffectViewBinder.kt
@@ -17,8 +17,6 @@
 package com.android.systemui.haptics.qs
 
 import android.animation.ValueAnimator
-import android.annotation.SuppressLint
-import android.view.MotionEvent
 import android.view.ViewConfiguration
 import android.view.animation.AccelerateDecelerateInterpolator
 import androidx.core.animation.doOnCancel
@@ -30,6 +28,7 @@ import com.android.app.tracing.coroutines.launch
 import com.android.systemui.lifecycle.repeatWhenAttached
 import com.android.systemui.qs.tileimpl.QSTileViewImpl
 import kotlinx.coroutines.DisposableHandle
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.filterNotNull
 
 object QSLongPressEffectViewBinder {
@@ -41,9 +40,6 @@ object QSLongPressEffectViewBinder {
     ): DisposableHandle? {
         if (qsLongPressEffect == null) return null
 
-        // Set the touch listener as the long-press effect
-        setTouchListener(tile, qsLongPressEffect)
-
         return tile.repeatWhenAttached {
             repeatOnLifecycle(Lifecycle.State.CREATED) {
                 // Action to perform
@@ -52,18 +48,18 @@ object QSLongPressEffectViewBinder {
 
                     qsLongPressEffect.actionType.filterNotNull().collect { action ->
                         when (action) {
-                            QSLongPressEffect.ActionType.CLICK -> {
-                                tile.performClick()
-                                qsLongPressEffect.clearActionType()
+                            QSLongPressEffect.ActionType.WAIT_TAP_TIMEOUT -> {
+                                delay(ViewConfiguration.getTapTimeout().toLong())
+                                qsLongPressEffect.handleTimeoutComplete()
                             }
                             QSLongPressEffect.ActionType.LONG_PRESS -> {
                                 tile.prepareForLaunch()
-                                tile.performLongClick()
+                                qsLongPressEffect.qsTile?.longClick(qsLongPressEffect.expandable)
                                 qsLongPressEffect.clearActionType()
                             }
                             QSLongPressEffect.ActionType.RESET_AND_LONG_PRESS -> {
                                 tile.resetLongPressEffectProperties()
-                                tile.performLongClick()
+                                qsLongPressEffect.qsTile?.longClick(qsLongPressEffect.expandable)
                                 qsLongPressEffect.clearActionType()
                             }
                             QSLongPressEffect.ActionType.START_ANIMATOR -> {
@@ -106,22 +102,4 @@ object QSLongPressEffectViewBinder {
             }
         }
     }
-
-    @SuppressLint("ClickableViewAccessibility")
-    private fun setTouchListener(tile: QSTileViewImpl, longPressEffect: QSLongPressEffect?) {
-        tile.setOnTouchListener { _, event ->
-            when (event.actionMasked) {
-                MotionEvent.ACTION_DOWN -> {
-                    tile.postDelayed(
-                        { longPressEffect?.handleTimeoutComplete() },
-                        ViewConfiguration.getTapTimeout().toLong(),
-                    )
-                    longPressEffect?.handleActionDown()
-                }
-                MotionEvent.ACTION_UP -> longPressEffect?.handleActionUp()
-                MotionEvent.ACTION_CANCEL -> longPressEffect?.handleActionCancel()
-            }
-            true
-        }
-    }
 }
diff --git a/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileViewImpl.kt b/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileViewImpl.kt
index c6dfdd5c137b..1c4404db1fb1 100644
--- a/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileViewImpl.kt
+++ b/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileViewImpl.kt
@@ -19,6 +19,7 @@ package com.android.systemui.qs.tileimpl
 import android.animation.ArgbEvaluator
 import android.animation.PropertyValuesHolder
 import android.animation.ValueAnimator
+import android.annotation.SuppressLint
 import android.content.Context
 import android.content.res.ColorStateList
 import android.content.res.Configuration
@@ -37,6 +38,7 @@ import android.util.Log
 import android.util.TypedValue
 import android.view.Gravity
 import android.view.LayoutInflater
+import android.view.MotionEvent
 import android.view.View
 import android.view.ViewGroup
 import android.view.accessibility.AccessibilityEvent
@@ -380,15 +382,22 @@ open class QSTileViewImpl @JvmOverloads constructor(
 
     override fun init(tile: QSTile) {
         val expandable = Expandable.fromView(this)
-        init(
+        if (quickSettingsVisualHapticsLongpress()) {
+            isHapticFeedbackEnabled = false
+            longPressEffect?.qsTile = tile
+            longPressEffect?.expandable = expandable
+            init(
+                { _: View? -> longPressEffect?.onTileClick() },
+                null, // Haptics and long-clicks will be handled by the [QSLongPressEffect]
+            )
+        } else {
+            init(
                 { _: View? -> tile.click(expandable) },
                 { _: View? ->
                     tile.longClick(expandable)
                     true
-                }
-        )
-        if (quickSettingsVisualHapticsLongpress()) {
-            isHapticFeedbackEnabled = false // Haptics will be handled by the [QSLongPressEffect]
+                },
+            )
         }
     }
 
@@ -526,6 +535,20 @@ open class QSTileViewImpl @JvmOverloads constructor(
         return sb.toString()
     }
 
+    @SuppressLint("ClickableViewAccessibility")
+    override fun onTouchEvent(event: MotionEvent?): Boolean {
+        // let the View run the onTouch logic for click and long-click detection
+        val result = super.onTouchEvent(event)
+        if (longPressEffect != null) {
+            when (event?.actionMasked) {
+                MotionEvent.ACTION_DOWN -> longPressEffect.handleActionDown()
+                MotionEvent.ACTION_UP -> longPressEffect.handleActionUp()
+                MotionEvent.ACTION_CANCEL -> longPressEffect.handleActionCancel()
+            }
+        }
+        return result
+    }
+
     // HANDLE STATE CHANGES RELATED METHODS
 
     protected open fun handleStateChanged(state: QSTile.State) {
@@ -660,7 +683,6 @@ open class QSTileViewImpl @JvmOverloads constructor(
             // Long-press effects might have been enabled before but the new state does not
             // handle a long-press. In this case, we go back to the behaviour of a regular tile
             // and clean-up the resources
-            setOnTouchListener(null)
             unbindLongPressEffect()
             showRippleEffect = isClickable
             initialLongPressProperties = null
diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/QSTileViewImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/QSTileViewImplTest.kt
index db11c3e89160..196f654a205a 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/QSTileViewImplTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/QSTileViewImplTest.kt
@@ -19,6 +19,8 @@ package com.android.systemui.qs.tileimpl
 import android.content.Context
 import android.graphics.Rect
 import android.graphics.drawable.Drawable
+import android.platform.test.annotations.DisableFlags
+import android.platform.test.annotations.EnableFlags
 import android.service.quicksettings.Tile
 import android.testing.AndroidTestingRunner
 import android.testing.TestableLooper
@@ -28,11 +30,13 @@ import android.view.View
 import android.view.accessibility.AccessibilityNodeInfo
 import android.widget.TextView
 import androidx.test.filters.SmallTest
+import com.android.systemui.Flags.FLAG_QUICK_SETTINGS_VISUAL_HAPTICS_LONGPRESS
 import com.android.systemui.res.R
 import com.android.systemui.SysuiTestCase
 import com.android.systemui.haptics.qs.QSLongPressEffect
 import com.android.systemui.haptics.qs.qsLongPressEffect
 import com.android.systemui.plugins.qs.QSTile
+import com.android.systemui.qs.qsTileFactory
 import com.android.systemui.testKosmos
 import com.google.common.truth.Truth.assertThat
 import org.junit.Before
@@ -536,10 +540,30 @@ class QSTileViewImplTest : SysuiTestCase() {
         assertThat(tileView.haveLongPressPropertiesBeenReset).isTrue()
     }
 
+    @Test
+    @EnableFlags(FLAG_QUICK_SETTINGS_VISUAL_HAPTICS_LONGPRESS)
+    fun onInit_withLongPressEffect_longPressEffectHasTileAndExpandable() {
+        val tile = kosmos.qsTileFactory.createTile("Test Tile")
+        tileView.init(tile)
+
+        assertThat(tileView.isTileAddedToLongPress).isTrue()
+        assertThat(tileView.isExpandableAddedToLongPress).isTrue()
+    }
+
+    @Test
+    @DisableFlags(FLAG_QUICK_SETTINGS_VISUAL_HAPTICS_LONGPRESS)
+    fun onInit_withoutLongPressEffect_longPressEffectDoesNotHaveTileAndExpandable() {
+        val tile = kosmos.qsTileFactory.createTile("Test Tile")
+        tileView.init(tile)
+
+        assertThat(tileView.isTileAddedToLongPress).isFalse()
+        assertThat(tileView.isExpandableAddedToLongPress).isFalse()
+    }
+
     class FakeTileView(
         context: Context,
         collapsed: Boolean,
-        longPressEffect: QSLongPressEffect?,
+        private val longPressEffect: QSLongPressEffect?,
     ) : QSTileViewImpl(
             ContextThemeWrapper(context, R.style.Theme_SystemUI_QuickSettings),
             collapsed,
@@ -547,6 +571,11 @@ class QSTileViewImplTest : SysuiTestCase() {
     ) {
         var constantLongPressEffectDuration = 500
 
+        val isTileAddedToLongPress: Boolean
+            get() = longPressEffect?.qsTile != null
+        val isExpandableAddedToLongPress: Boolean
+            get() = longPressEffect?.expandable != null
+
         override fun getLongPressEffectDuration(): Int = constantLongPressEffectDuration
         fun changeState(state: QSTile.State) {
             handleStateChanged(state)