Fix unlocked split shade jumping back into place after over expanding

The problem was that there were separate code paths for over-scroll/ over-expansion for split shade and non split shade. In this case the non split shade path was altering translation at the end of the drag, even when on split shade. To avoid adding even more "if split shade" code, removed the split shade over-scrolling code path/classes. There is only one overscroll code path now, with just 1 "if split" shade condition to decide which components to over expand. Test: Manually on device. See videos. Fixes: 270005948 Change-Id: Ia47aa30957260a9c2d1efd7e5f18536f76a2dcc9
2023-02-20 15:44:37 +01:00
parent a100480ad7
commit cafd48c62c
6 changed files with 14 additions and 435 deletions
--- a/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java
+++ b/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java
@@ -1914,7 +1914,6 @@ public final class NotificationPanelViewController implements Dumpable {
        // we want to perform an overshoot animation when flinging open
        final boolean addOverscroll =
                expand
-                        && !mSplitShadeEnabled // Split shade has its own overscroll logic
                        && mStatusBarStateController.getState() != KEYGUARD
                        && mOverExpansion == 0.0f
                        && vel >= 0;
@@ -2590,9 +2589,14 @@ public final class NotificationPanelViewController implements Dumpable {
            return;
        }
        mOverExpansion = overExpansion;
-        // Translating the quick settings by half the overexpansion to center it in the background
-        // frame
-        mQsController.updateQsFrameTranslation();
+        if (mSplitShadeEnabled) {
+            mQsController.setOverScrollAmount((int) overExpansion);
+            mScrimController.setNotificationsOverScrollAmount((int) overExpansion);
+        } else {
+            // Translating the quick settings by half the overexpansion to center it in the
+            // background frame
+            mQsController.updateQsFrameTranslation();
+        }
        mNotificationStackScrollLayoutController.setOverExpansion(overExpansion);
    }

@@ -3594,7 +3598,7 @@ public final class NotificationPanelViewController implements Dumpable {

    private void fling(float vel, boolean expand, float collapseSpeedUpFactor,
            boolean expandBecauseOfFalsing) {
-        float target = expand ? getMaxPanelHeight() : 0;
+        float target = expand ? getMaxPanelTransitionDistance() : 0;
        if (!expand) {
            setClosing(true);
        }
@@ -3679,7 +3683,7 @@ public final class NotificationPanelViewController implements Dumpable {
            float maxPanelHeight = getMaxPanelTransitionDistance();
            if (mHeightAnimator == null) {
                // Split shade has its own overscroll logic
-                if (mTracking && !mSplitShadeEnabled) {
+                if (mTracking) {
                    float overExpansionPixels = Math.max(0, h - maxPanelHeight);
                    setOverExpansionInternal(overExpansionPixels, true /* isFromGesture */);
                }
--- a/packages/SystemUI/src/com/android/systemui/shade/QuickSettingsController.java
+++ b/packages/SystemUI/src/com/android/systemui/shade/QuickSettingsController.java
@@ -803,6 +803,10 @@ public class QuickSettingsController {
        }
    }

+    void setOverScrollAmount(int overExpansion) {
+        mQs.setOverScrollAmount(overExpansion);
+    }
+
    private void setOverScrolling(boolean overscrolling) {
        mStackScrollerOverscrolling = overscrolling;
        if (mQs != null) {
--- a/packages/SystemUI/src/com/android/systemui/shade/transition/ShadeTransitionController.kt
+++ b/packages/SystemUI/src/com/android/systemui/shade/transition/ShadeTransitionController.kt
@@ -43,8 +43,6 @@ constructor(
    shadeExpansionStateManager: ShadeExpansionStateManager,
    dumpManager: DumpManager,
    private val context: Context,
-    private val splitShadeOverScrollerFactory: SplitShadeOverScroller.Factory,
-    private val noOpOverScroller: NoOpOverScroller,
    private val scrimShadeTransitionController: ScrimShadeTransitionController,
    private val statusBarStateController: SysuiStatusBarStateController,
 ) {
@@ -57,17 +55,6 @@ constructor(
    private var currentPanelState: Int? = null
    private var lastShadeExpansionChangeEvent: ShadeExpansionChangeEvent? = null

-    private val splitShadeOverScroller by lazy {
-        splitShadeOverScrollerFactory.create({ qs }, { notificationStackScrollLayoutController })
-    }
-    private val shadeOverScroller: ShadeOverScroller
-        get() =
-            if (inSplitShade && isScreenUnlocked() && propertiesInitialized()) {
-                splitShadeOverScroller
-            } else {
-                noOpOverScroller
-            }
-
    init {
        updateResources()
        configurationController.addCallback(
@@ -89,21 +76,14 @@ constructor(

    private fun onPanelStateChanged(@PanelState state: Int) {
        currentPanelState = state
-        shadeOverScroller.onPanelStateChanged(state)
        scrimShadeTransitionController.onPanelStateChanged(state)
    }

    private fun onPanelExpansionChanged(event: ShadeExpansionChangeEvent) {
        lastShadeExpansionChangeEvent = event
-        shadeOverScroller.onDragDownAmountChanged(event.dragDownPxAmount)
        scrimShadeTransitionController.onPanelExpansionChanged(event)
    }

-    private fun propertiesInitialized() =
-        this::qs.isInitialized &&
-            this::notificationPanelViewController.isInitialized &&
-            this::notificationStackScrollLayoutController.isInitialized
-
    private fun dump(pw: PrintWriter) {
        pw.println(
            """
--- a/packages/SystemUI/src/com/android/systemui/shade/transition/SplitShadeOverScroller.kt
+++ b/packages/SystemUI/src/com/android/systemui/shade/transition/SplitShadeOverScroller.kt
@@ -1,167 +0,0 @@
-/*
- * Copyright (C) 2022 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.android.systemui.shade.transition
-
-import android.animation.Animator
-import android.animation.ValueAnimator
-import android.content.Context
-import android.content.res.Configuration
-import android.util.MathUtils
-import com.android.internal.annotations.VisibleForTesting
-import com.android.systemui.R
-import com.android.systemui.animation.Interpolators
-import com.android.systemui.dump.DumpManager
-import com.android.systemui.plugins.qs.QS
-import com.android.systemui.shade.PanelState
-import com.android.systemui.shade.STATE_CLOSED
-import com.android.systemui.shade.STATE_OPENING
-import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayoutController
-import com.android.systemui.statusbar.phone.ScrimController
-import com.android.systemui.statusbar.policy.ConfigurationController
-import dagger.assisted.Assisted
-import dagger.assisted.AssistedFactory
-import dagger.assisted.AssistedInject
-import java.io.PrintWriter
-
-class SplitShadeOverScroller
-@AssistedInject
-constructor(
-    configurationController: ConfigurationController,
-    dumpManager: DumpManager,
-    private val context: Context,
-    private val scrimController: ScrimController,
-    @Assisted private val qSProvider: () -> QS,
-    @Assisted private val nsslControllerProvider: () -> NotificationStackScrollLayoutController
-) : ShadeOverScroller {
-
-    private var releaseOverScrollDuration = 0L
-    private var maxOverScrollAmount = 0
-    private var previousOverscrollAmount = 0
-    private var dragDownAmount: Float = 0f
-    @PanelState private var panelState: Int = STATE_CLOSED
-
-    private var releaseOverScrollAnimator: Animator? = null
-
-    private val qS: QS
-        get() = qSProvider()
-
-    private val nsslController: NotificationStackScrollLayoutController
-        get() = nsslControllerProvider()
-
-    init {
-        updateResources()
-        configurationController.addCallback(
-            object : ConfigurationController.ConfigurationListener {
-                override fun onConfigChanged(newConfig: Configuration?) {
-                    updateResources()
-                }
-            })
-        dumpManager.registerCriticalDumpable("SplitShadeOverScroller") { printWriter, _ ->
-            dump(printWriter)
-        }
-    }
-
-    private fun updateResources() {
-        val resources = context.resources
-        maxOverScrollAmount = resources.getDimensionPixelSize(R.dimen.shade_max_over_scroll_amount)
-        releaseOverScrollDuration =
-            resources.getInteger(R.integer.lockscreen_shade_over_scroll_release_duration).toLong()
-    }
-
-    override fun onPanelStateChanged(@PanelState newPanelState: Int) {
-        if (shouldReleaseOverscroll(previousState = panelState, newState = newPanelState)) {
-            releaseOverScroll()
-        }
-        panelState = newPanelState
-    }
-
-    override fun onDragDownAmountChanged(newDragDownAmount: Float) {
-        if (dragDownAmount == newDragDownAmount) {
-            return
-        }
-        dragDownAmount = newDragDownAmount
-        if (shouldOverscroll()) {
-            overScroll(newDragDownAmount)
-        }
-    }
-
-    private fun shouldOverscroll() = panelState == STATE_OPENING
-
-    private fun shouldReleaseOverscroll(@PanelState previousState: Int, @PanelState newState: Int) =
-        previousState == STATE_OPENING && newState != STATE_OPENING
-
-    private fun overScroll(dragDownAmount: Float) {
-        val overscrollAmount: Int = calculateOverscrollAmount(dragDownAmount)
-        applyOverscroll(overscrollAmount)
-        previousOverscrollAmount = overscrollAmount
-    }
-
-    private fun calculateOverscrollAmount(dragDownAmount: Float): Int {
-        val fullHeight: Int = nsslController.height
-        val fullHeightProgress: Float = MathUtils.saturate(dragDownAmount / fullHeight)
-        return (fullHeightProgress * maxOverScrollAmount).toInt()
-    }
-
-    private fun applyOverscroll(overscrollAmount: Int) {
-        qS.setOverScrollAmount(overscrollAmount)
-        scrimController.setNotificationsOverScrollAmount(overscrollAmount)
-        nsslController.setOverScrollAmount(overscrollAmount)
-    }
-
-    private fun releaseOverScroll() {
-        val animator = ValueAnimator.ofInt(previousOverscrollAmount, 0)
-        animator.addUpdateListener {
-            val overScrollAmount = it.animatedValue as Int
-            qS.setOverScrollAmount(overScrollAmount)
-            scrimController.setNotificationsOverScrollAmount(overScrollAmount)
-            nsslController.setOverScrollAmount(overScrollAmount)
-        }
-        animator.interpolator = Interpolators.STANDARD
-        animator.duration = releaseOverScrollDuration
-        animator.start()
-        releaseOverScrollAnimator = animator
-        previousOverscrollAmount = 0
-    }
-
-    @VisibleForTesting
-    internal fun finishAnimations() {
-        releaseOverScrollAnimator?.end()
-        releaseOverScrollAnimator = null
-    }
-
-    private fun dump(pw: PrintWriter) {
-        pw.println(
-            """
-            SplitShadeOverScroller:
-                Resources:
-                    releaseOverScrollDuration: $releaseOverScrollDuration
-                    maxOverScrollAmount: $maxOverScrollAmount
-                State:
-                    previousOverscrollAmount: $previousOverscrollAmount
-                    dragDownAmount: $dragDownAmount
-                    panelState: $panelState
-            """.trimIndent())
-    }
-
-    @AssistedFactory
-    fun interface Factory {
-        fun create(
-            qSProvider: () -> QS,
-            nsslControllerProvider: () -> NotificationStackScrollLayoutController
-        ): SplitShadeOverScroller
-    }
-}
--- a/packages/SystemUI/tests/src/com/android/systemui/shade/transition/ShadeTransitionControllerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/shade/transition/ShadeTransitionControllerTest.kt
@@ -2,37 +2,24 @@ package com.android.systemui.shade.transition

 import android.testing.AndroidTestingRunner
 import androidx.test.filters.SmallTest
-import com.android.systemui.R
 import com.android.systemui.SysuiTestCase
 import com.android.systemui.dump.DumpManager
-import com.android.systemui.plugins.qs.QS
-import com.android.systemui.shade.NotificationPanelViewController
 import com.android.systemui.shade.STATE_OPENING
 import com.android.systemui.shade.ShadeExpansionChangeEvent
 import com.android.systemui.shade.ShadeExpansionStateManager
-import com.android.systemui.statusbar.StatusBarState
 import com.android.systemui.statusbar.SysuiStatusBarStateController
-import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayoutController
 import com.android.systemui.statusbar.policy.FakeConfigurationController
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mock
-import org.mockito.Mockito.reset
 import org.mockito.Mockito.verify
-import org.mockito.Mockito.verifyZeroInteractions
-import org.mockito.Mockito.`when` as whenever
 import org.mockito.MockitoAnnotations

@RunWith(AndroidTestingRunner::class)
@SmallTest
 class ShadeTransitionControllerTest : SysuiTestCase() {

-    @Mock private lateinit var npvc: NotificationPanelViewController
-    @Mock private lateinit var nsslController: NotificationStackScrollLayoutController
-    @Mock private lateinit var qs: QS
-    @Mock private lateinit var noOpOverScroller: NoOpOverScroller
-    @Mock private lateinit var splitShadeOverScroller: SplitShadeOverScroller
    @Mock private lateinit var scrimShadeTransitionController: ScrimShadeTransitionController
    @Mock private lateinit var dumpManager: DumpManager
    @Mock private lateinit var statusBarStateController: SysuiStatusBarStateController
@@ -52,119 +39,19 @@ class ShadeTransitionControllerTest : SysuiTestCase() {
                shadeExpansionStateManager,
                dumpManager,
                context,
-                splitShadeOverScrollerFactory = { _, _ -> splitShadeOverScroller },
-                noOpOverScroller,
                scrimShadeTransitionController,
                statusBarStateController,
            )
-
-        // Resetting as they are notified upon initialization.
-        reset(noOpOverScroller, splitShadeOverScroller)
-    }
-
-    @Test
-    fun onPanelExpansionChanged_inSplitShade_forwardsToSplitShadeOverScroller() {
-        initLateProperties()
-        enableSplitShade()
-
-        startPanelExpansion()
-
-        verify(splitShadeOverScroller).onPanelStateChanged(STATE_OPENING)
-        verify(splitShadeOverScroller).onDragDownAmountChanged(DEFAULT_DRAG_DOWN_AMOUNT)
-        verifyZeroInteractions(noOpOverScroller)
-    }
-
-    @Test
-    fun onPanelStateChanged_inSplitShade_propertiesNotInitialized_forwardsToNoOpOverScroller() {
-        enableSplitShade()
-
-        startPanelExpansion()
-
-        verify(noOpOverScroller).onPanelStateChanged(STATE_OPENING)
-        verify(noOpOverScroller).onDragDownAmountChanged(DEFAULT_DRAG_DOWN_AMOUNT)
-        verifyZeroInteractions(splitShadeOverScroller)
-    }
-
-    @Test
-    fun onPanelStateChanged_inSplitShade_onKeyguard_forwardsToNoOpOverScroller() {
-        initLateProperties()
-        enableSplitShade()
-        setOnKeyguard()
-
-        startPanelExpansion()
-
-        verify(noOpOverScroller).onPanelStateChanged(STATE_OPENING)
-        verify(noOpOverScroller).onDragDownAmountChanged(DEFAULT_DRAG_DOWN_AMOUNT)
-        verifyZeroInteractions(splitShadeOverScroller)
-    }
-
-    @Test
-    fun onPanelStateChanged_inSplitShade_onLockedShade_forwardsToNoOpOverScroller() {
-        initLateProperties()
-        enableSplitShade()
-        setOnLockedShade()
-
-        startPanelExpansion()
-
-        verify(noOpOverScroller).onPanelStateChanged(STATE_OPENING)
-        verify(noOpOverScroller).onDragDownAmountChanged(DEFAULT_DRAG_DOWN_AMOUNT)
-        verifyZeroInteractions(splitShadeOverScroller)
-    }
-
-    @Test
-    fun onPanelExpansionChanged_inSplitShade_onUnlockedShade_forwardsToSplitShadeOverScroller() {
-        initLateProperties()
-        enableSplitShade()
-        setOnUnlockedShade()
-
-        startPanelExpansion()
-
-        verify(splitShadeOverScroller).onPanelStateChanged(STATE_OPENING)
-        verify(splitShadeOverScroller).onDragDownAmountChanged(DEFAULT_DRAG_DOWN_AMOUNT)
-        verifyZeroInteractions(noOpOverScroller)
-    }
-
-    @Test
-    fun onPanelStateChanged_notInSplitShade_forwardsToNoOpOverScroller() {
-        initLateProperties()
-        disableSplitShade()
-
-        startPanelExpansion()
-
-        verify(noOpOverScroller).onPanelStateChanged(STATE_OPENING)
-        verify(noOpOverScroller).onDragDownAmountChanged(DEFAULT_DRAG_DOWN_AMOUNT)
-        verifyZeroInteractions(splitShadeOverScroller)
    }

    @Test
    fun onPanelStateChanged_forwardsToScrimTransitionController() {
-        initLateProperties()
-
        startPanelExpansion()

        verify(scrimShadeTransitionController).onPanelStateChanged(STATE_OPENING)
        verify(scrimShadeTransitionController).onPanelExpansionChanged(DEFAULT_EXPANSION_EVENT)
    }

-    private fun initLateProperties() {
-        controller.qs = qs
-        controller.notificationStackScrollLayoutController = nsslController
-        controller.notificationPanelViewController = npvc
-    }
-
-    private fun disableSplitShade() {
-        setSplitShadeEnabled(false)
-    }
-
-    private fun enableSplitShade() {
-        setSplitShadeEnabled(true)
-    }
-
-    private fun setSplitShadeEnabled(enabled: Boolean) {
-        overrideResource(R.bool.config_use_split_notification_shade, enabled)
-        configurationController.notifyConfigurationChanged()
-    }
-
    private fun startPanelExpansion() {
        shadeExpansionStateManager.onPanelExpansionChanged(
            DEFAULT_EXPANSION_EVENT.fraction,
@@ -174,23 +61,6 @@ class ShadeTransitionControllerTest : SysuiTestCase() {
        )
    }

-    private fun setOnKeyguard() {
-        setShadeState(StatusBarState.KEYGUARD)
-    }
-
-    private fun setOnLockedShade() {
-        setShadeState(StatusBarState.SHADE_LOCKED)
-    }
-
-    private fun setOnUnlockedShade() {
-        setShadeState(StatusBarState.SHADE)
-    }
-
-    private fun setShadeState(state: Int) {
-        whenever(statusBarStateController.state).thenReturn(state)
-        whenever(statusBarStateController.currentOrUpcomingState).thenReturn(state)
-    }
-
    companion object {
        private const val DEFAULT_DRAG_DOWN_AMOUNT = 123f
        private val DEFAULT_EXPANSION_EVENT =
--- a/packages/SystemUI/tests/src/com/android/systemui/shade/transition/SplitShadeOverScrollerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/shade/transition/SplitShadeOverScrollerTest.kt
@@ -1,112 +0,0 @@
-package com.android.systemui.shade.transition
-
-import android.testing.AndroidTestingRunner
-import android.testing.TestableLooper
-import androidx.test.filters.SmallTest
-import com.android.systemui.R
-import com.android.systemui.SysuiTestCase
-import com.android.systemui.dump.DumpManager
-import com.android.systemui.plugins.qs.QS
-import com.android.systemui.shade.STATE_CLOSED
-import com.android.systemui.shade.STATE_OPEN
-import com.android.systemui.shade.STATE_OPENING
-import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayoutController
-import com.android.systemui.statusbar.phone.ScrimController
-import com.android.systemui.statusbar.policy.FakeConfigurationController
-import org.junit.Before
-import org.junit.Test
-import org.junit.runner.RunWith
-import org.mockito.Mock
-import org.mockito.Mockito.atLeastOnce
-import org.mockito.Mockito.verify
-import org.mockito.Mockito.verifyZeroInteractions
-import org.mockito.Mockito.`when` as whenever
-import org.mockito.MockitoAnnotations
-
-@RunWith(AndroidTestingRunner::class)
-@TestableLooper.RunWithLooper(setAsMainLooper = true)
-@SmallTest
-class SplitShadeOverScrollerTest : SysuiTestCase() {
-
-    @Mock private lateinit var dumpManager: DumpManager
-    @Mock private lateinit var scrimController: ScrimController
-    @Mock private lateinit var qs: QS
-    @Mock private lateinit var nsslController: NotificationStackScrollLayoutController
-
-    private val configurationController = FakeConfigurationController()
-    private lateinit var overScroller: SplitShadeOverScroller
-
-    @Before
-    fun setUp() {
-        MockitoAnnotations.initMocks(this)
-
-        whenever(nsslController.height).thenReturn(1000)
-        overScroller =
-            SplitShadeOverScroller(
-                configurationController,
-                dumpManager,
-                context,
-                scrimController,
-                { qs },
-                { nsslController })
-    }
-
-    @Test
-    fun onDragDownAmountChanged_panelOpening_overScrolls_basedOnHeightAndMaxAmount() {
-        val maxOverScrollAmount = 50
-        val dragDownAmount = 100f
-        overrideResource(R.dimen.shade_max_over_scroll_amount, maxOverScrollAmount)
-        configurationController.notifyConfigurationChanged()
-
-        overScroller.onPanelStateChanged(STATE_OPENING)
-        overScroller.onDragDownAmountChanged(dragDownAmount)
-
-        val expectedOverScrollAmount =
-            (dragDownAmount / nsslController.height * maxOverScrollAmount).toInt()
-        verify(qs).setOverScrollAmount(expectedOverScrollAmount)
-        verify(nsslController).setOverScrollAmount(expectedOverScrollAmount)
-        verify(scrimController).setNotificationsOverScrollAmount(expectedOverScrollAmount)
-    }
-
-    @Test
-    fun onDragDownAmountChanged_panelClosed_doesNotOverScroll() {
-        overScroller.onPanelStateChanged(STATE_CLOSED)
-        overScroller.onDragDownAmountChanged(100f)
-
-        verifyZeroInteractions(qs, scrimController, nsslController)
-    }
-
-    @Test
-    fun onDragDownAmountChanged_panelOpen_doesNotOverScroll() {
-        overScroller.onPanelStateChanged(STATE_OPEN)
-        overScroller.onDragDownAmountChanged(100f)
-
-        verifyZeroInteractions(qs, scrimController, nsslController)
-    }
-
-    @Test
-    fun onPanelStateChanged_opening_thenOpen_releasesOverScroll() {
-        overScroller.onPanelStateChanged(STATE_OPENING)
-        overScroller.onDragDownAmountChanged(100f)
-
-        overScroller.onPanelStateChanged(STATE_OPEN)
-        overScroller.finishAnimations()
-
-        verify(qs, atLeastOnce()).setOverScrollAmount(0)
-        verify(scrimController, atLeastOnce()).setNotificationsOverScrollAmount(0)
-        verify(nsslController, atLeastOnce()).setOverScrollAmount(0)
-    }
-
-    @Test
-    fun onPanelStateChanged_opening_thenClosed_releasesOverScroll() {
-        overScroller.onPanelStateChanged(STATE_OPENING)
-        overScroller.onDragDownAmountChanged(100f)
-
-        overScroller.onPanelStateChanged(STATE_CLOSED)
-        overScroller.finishAnimations()
-
-        verify(qs, atLeastOnce()).setOverScrollAmount(0)
-        verify(scrimController, atLeastOnce()).setNotificationsOverScrollAmount(0)
-        verify(nsslController, atLeastOnce()).setOverScrollAmount(0)
-    }
-}