From 7ad672957879c5704e12601c0bb2cb2116781bc6 Mon Sep 17 00:00:00 2001 From: William Xiao Date: Wed, 17 Jul 2024 16:19:55 -0700 Subject: Consume but ignore glanceable hub touches when entering and exiting edit mode When entering edit mode, the communal scene is kept open until the edit mode activity is ready. Similarly when exiting edit mode, the communal scene is started before the device is locked. This allows for smooth transition animations to and from edit mode, but also means that the presence of the communal scene is no longer correlated to the GLANCEABLE_HUB keyguard state and other important logic. User interactions during these transition periods can cause a number of issues, like missing notifications, a missing shade background, landing on a blank edit mode activity, etc. To prevent users from seeing these issues, we consume but do nothing with touches in the hub and pause TouchMonitor during these times. This is a relatively short period of time but solves many funky issues. This change also adds noHistory to the edit mode activity so that it's finished in case the user exits with a back gesture, home button press, or gesture nav swipe, so that the editModeOpen state updates properly. Bug: 350815853 Bug: 352171134 Bug: 349385145 Bug: 349386602 Test: atest GlanceableHubContainerControllerTest also manually tested on device Flag: com.android.systemui.communal_hub Change-Id: I23475cfb3eac15ec2499cfd60424611a2e9a02a5 --- .../domain/interactor/CommunalInteractor.kt | 22 ++- .../ui/viewmodel/CommunalEditModeViewModel.kt | 8 ++ .../communal/widgets/EditWidgetsActivity.kt | 6 +- .../shade/GlanceableHubContainerController.kt | 53 ++++++- .../shade/GlanceableHubContainerControllerTest.kt | 156 ++++++++++++++++++--- 5 files changed, 217 insertions(+), 28 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/communal/domain/interactor/CommunalInteractor.kt b/packages/SystemUI/src/com/android/systemui/communal/domain/interactor/CommunalInteractor.kt index e13161f91f16..dbddc23d6146 100644 --- a/packages/SystemUI/src/com/android/systemui/communal/domain/interactor/CommunalInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/communal/domain/interactor/CommunalInteractor.kt @@ -121,9 +121,25 @@ constructor( private val _editModeOpen = MutableStateFlow(false) - /** Whether edit mode is currently open. */ + /** + * Whether edit mode is currently open. This will be true from onCreate to onDestroy in + * [EditWidgetsActivity] and thus does not correspond to whether or not the activity is visible. + * + * Note that since this is called in onDestroy, it's not guaranteed to ever be set to false when + * edit mode is closed, such as in the case that a user exits edit mode manually with a back + * gesture or navigation gesture. + */ val editModeOpen: StateFlow = _editModeOpen.asStateFlow() + private val _editActivityShowing = MutableStateFlow(false) + + /** + * Whether the edit mode activity is currently showing. This is true from onStart to onStop in + * [EditWidgetsActivity] so may be false even when the user is in edit mode, such as when a + * widget's individual configuration activity has launched. + */ + val editActivityShowing: StateFlow = _editActivityShowing.asStateFlow() + /** Whether communal features are enabled. */ val isCommunalEnabled: StateFlow = communalSettingsInteractor.isCommunalEnabled @@ -316,6 +332,10 @@ constructor( _editModeOpen.value = isOpen } + fun setEditActivityShowing(isOpen: Boolean) { + _editActivityShowing.value = isOpen + } + /** Show the widget editor Activity. */ fun showWidgetEditor( preselectedKey: String? = null, diff --git a/packages/SystemUI/src/com/android/systemui/communal/ui/viewmodel/CommunalEditModeViewModel.kt b/packages/SystemUI/src/com/android/systemui/communal/ui/viewmodel/CommunalEditModeViewModel.kt index 0353d2c043e8..b1e5135ce658 100644 --- a/packages/SystemUI/src/com/android/systemui/communal/ui/viewmodel/CommunalEditModeViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/communal/ui/viewmodel/CommunalEditModeViewModel.kt @@ -217,6 +217,14 @@ constructor( /** Sets whether edit mode is currently open */ fun setEditModeOpen(isOpen: Boolean) = communalInteractor.setEditModeOpen(isOpen) + /** + * Sets whether the edit mode activity is currently showing. + * + * See [CommunalInteractor.editActivityShowing] for more info. + */ + fun setEditActivityShowing(showing: Boolean) = + communalInteractor.setEditActivityShowing(showing) + /** Called when exiting the edit mode, before transitioning back to the communal scene. */ fun cleanupEditModeState() { communalSceneInteractor.setEditModeState(null) diff --git a/packages/SystemUI/src/com/android/systemui/communal/widgets/EditWidgetsActivity.kt b/packages/SystemUI/src/com/android/systemui/communal/widgets/EditWidgetsActivity.kt index 46f802fd2bce..08fe42ede5d3 100644 --- a/packages/SystemUI/src/com/android/systemui/communal/widgets/EditWidgetsActivity.kt +++ b/packages/SystemUI/src/com/android/systemui/communal/widgets/EditWidgetsActivity.kt @@ -96,8 +96,7 @@ constructor( run { Log.w(TAG, "No AppWidgetProviderInfo found in result.") } } } - } - ?: run { Log.w(TAG, "No data in result.") } + } ?: run { Log.w(TAG, "No data in result.") } } else -> Log.w( @@ -195,6 +194,8 @@ constructor( override fun onStart() { super.onStart() + communalViewModel.setEditActivityShowing(true) + if (shouldOpenWidgetPickerOnStart) { onOpenWidgetPicker() shouldOpenWidgetPickerOnStart = false @@ -206,6 +207,7 @@ constructor( override fun onStop() { super.onStop() + communalViewModel.setEditActivityShowing(false) logger.i("Stopping the communal widget editor activity") uiEventLogger.log(CommunalUiEvent.COMMUNAL_HUB_EDIT_MODE_GONE) diff --git a/packages/SystemUI/src/com/android/systemui/shade/GlanceableHubContainerController.kt b/packages/SystemUI/src/com/android/systemui/shade/GlanceableHubContainerController.kt index d090aea4cee5..b468d0e75a7a 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/GlanceableHubContainerController.kt +++ b/packages/SystemUI/src/com/android/systemui/shade/GlanceableHubContainerController.kt @@ -50,6 +50,9 @@ import com.android.systemui.communal.ui.viewmodel.CommunalViewModel import com.android.systemui.communal.util.CommunalColors import com.android.systemui.dagger.SysUISingleton import com.android.systemui.keyguard.domain.interactor.KeyguardInteractor +import com.android.systemui.keyguard.domain.interactor.KeyguardTransitionInteractor +import com.android.systemui.keyguard.shared.model.Edge +import com.android.systemui.keyguard.shared.model.KeyguardState import com.android.systemui.lifecycle.repeatWhenAttached import com.android.systemui.res.R import com.android.systemui.scene.shared.flag.SceneContainerFlag @@ -76,6 +79,7 @@ constructor( private val communalInteractor: CommunalInteractor, private val communalViewModel: CommunalViewModel, private val keyguardInteractor: KeyguardInteractor, + private val keyguardTransitionInteractor: KeyguardTransitionInteractor, private val shadeInteractor: ShadeInteractor, private val powerManager: PowerManager, private val communalColors: CommunalColors, @@ -147,6 +151,19 @@ constructor( */ private var hubShowing = false + /** + * True if we're transitioning to or from edit mode + * + * We block all touches and gestures when edit mode is open to prevent funky transition issues + * when entering and exiting edit mode because we delay exiting the hub scene when entering edit + * mode and enter the hub scene early when exiting edit mode to make for a smoother transition. + * Gestures during these transitions can result in broken and unexpected UI states. + * + * Tracks [CommunalInteractor.editActivityShowing] and the [KeyguardState.GONE] to + * [KeyguardState.GLANCEABLE_HUB] transition. + */ + private var inEditModeTransition = false + /** * True if either the primary or alternate bouncer are open, meaning the hub should not receive * any touch input. @@ -321,6 +338,22 @@ constructor( updateTouchHandlingState() } ) + collectFlow( + containerView, + // When leaving edit mode, editActivityShowing is true until the edit mode activity + // finishes itself and the device locks, after which isInTransition will be true until + // we're fully on the hub. + anyOf( + communalInteractor.editActivityShowing, + keyguardTransitionInteractor.isInTransition( + Edge.create(KeyguardState.GONE, KeyguardState.GLANCEABLE_HUB) + ) + ), + { + inEditModeTransition = it + updateTouchHandlingState() + } + ) collectFlow( containerView, combine( @@ -359,8 +392,11 @@ constructor( * Also clears gesture exclusion zones when the hub is occluded or gone. */ private fun updateTouchHandlingState() { + // Only listen to gestures when we're settled in the hub keyguard state and the shade + // bouncer are not showing on top. val shouldInterceptGestures = - hubShowing && !(shadeShowingAndConsumingTouches || anyBouncerShowing) + hubShowing && + !(shadeShowingAndConsumingTouches || anyBouncerShowing || inEditModeTransition) if (shouldInterceptGestures) { lifecycleRegistry.currentState = Lifecycle.State.RESUMED } else { @@ -412,10 +448,10 @@ constructor( return false } - return communalContainerView?.let { handleTouchEventOnCommunalView(it, ev) } ?: false + return communalContainerView?.let { handleTouchEventOnCommunalView(ev) } ?: false } - private fun handleTouchEventOnCommunalView(view: View, ev: MotionEvent): Boolean { + private fun handleTouchEventOnCommunalView(ev: MotionEvent): Boolean { val isDown = ev.actionMasked == MotionEvent.ACTION_DOWN val isUp = ev.actionMasked == MotionEvent.ACTION_UP val isMove = ev.actionMasked == MotionEvent.ACTION_MOVE @@ -431,7 +467,7 @@ constructor( if (isUp || isCancel) { isTrackingHubTouch = false } - return dispatchTouchEvent(view, ev) + return dispatchTouchEvent(ev) } return false @@ -441,7 +477,14 @@ constructor( * Dispatches the touch event to the communal container and sends a user activity event to reset * the screen timeout. */ - private fun dispatchTouchEvent(view: View, ev: MotionEvent): Boolean { + private fun dispatchTouchEvent(ev: MotionEvent): Boolean { + if (inEditModeTransition) { + // Consume but ignore touches while we're transitioning to or from edit mode so that the + // user can't trigger another transition, such as by swiping the hub away, tapping a + // widget, or opening the shade/bouncer. Doing any of these while transitioning can + // result in broken states. + return true + } try { var handled = false communalContainerWrapper?.dispatchTouchEvent(ev) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/shade/GlanceableHubContainerControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/shade/GlanceableHubContainerControllerTest.kt index 86c9ab789429..967df39c9269 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/shade/GlanceableHubContainerControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/shade/GlanceableHubContainerControllerTest.kt @@ -48,7 +48,12 @@ import com.android.systemui.communal.ui.compose.CommunalContent import com.android.systemui.communal.ui.viewmodel.CommunalViewModel import com.android.systemui.communal.util.CommunalColors import com.android.systemui.coroutines.collectLastValue +import com.android.systemui.keyguard.data.repository.fakeKeyguardTransitionRepository import com.android.systemui.keyguard.domain.interactor.keyguardInteractor +import com.android.systemui.keyguard.domain.interactor.keyguardTransitionInteractor +import com.android.systemui.keyguard.shared.model.KeyguardState +import com.android.systemui.keyguard.shared.model.TransitionState +import com.android.systemui.keyguard.shared.model.TransitionStep import com.android.systemui.kosmos.Kosmos import com.android.systemui.kosmos.testDispatcher import com.android.systemui.kosmos.testScope @@ -67,13 +72,13 @@ import org.junit.Assert.assertThrows import org.junit.Before import org.junit.Test import org.junit.runner.RunWith -import org.mockito.ArgumentMatchers.anyFloat -import org.mockito.Mock -import org.mockito.Mockito.`when` -import org.mockito.MockitoAnnotations +import org.mockito.kotlin.any import org.mockito.kotlin.doReturn import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.spy +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever @ExperimentalCoroutinesApi @RunWith(AndroidTestingRunner::class) @@ -87,11 +92,11 @@ class GlanceableHubContainerControllerTest : SysuiTestCase() { testDispatcher = UnconfinedTestDispatcher() } - @Mock private lateinit var communalViewModel: CommunalViewModel - @Mock private lateinit var powerManager: PowerManager - @Mock private lateinit var touchMonitor: TouchMonitor - @Mock private lateinit var communalColors: CommunalColors - @Mock private lateinit var communalContent: CommunalContent + private var communalViewModel = mock() + private var powerManager = mock() + private var touchMonitor = mock() + private var communalColors = mock() + private var communalContent = mock() private lateinit var ambientTouchComponentFactory: AmbientTouchComponent.Factory private lateinit var parentView: FrameLayout @@ -103,8 +108,6 @@ class GlanceableHubContainerControllerTest : SysuiTestCase() { @Before fun setUp() { - MockitoAnnotations.initMocks(this) - communalRepository = kosmos.fakeCommunalSceneRepository ambientTouchComponentFactory = @@ -124,6 +127,7 @@ class GlanceableHubContainerControllerTest : SysuiTestCase() { communalInteractor, communalViewModel, keyguardInteractor, + kosmos.keyguardTransitionInteractor, shadeInteractor, powerManager, communalColors, @@ -167,6 +171,7 @@ class GlanceableHubContainerControllerTest : SysuiTestCase() { communalInteractor, communalViewModel, keyguardInteractor, + kosmos.keyguardTransitionInteractor, shadeInteractor, powerManager, communalColors, @@ -192,6 +197,7 @@ class GlanceableHubContainerControllerTest : SysuiTestCase() { communalInteractor, communalViewModel, keyguardInteractor, + kosmos.keyguardTransitionInteractor, shadeInteractor, powerManager, communalColors, @@ -212,6 +218,7 @@ class GlanceableHubContainerControllerTest : SysuiTestCase() { communalInteractor, communalViewModel, keyguardInteractor, + kosmos.keyguardTransitionInteractor, shadeInteractor, powerManager, communalColors, @@ -235,12 +242,15 @@ class GlanceableHubContainerControllerTest : SysuiTestCase() { } @Test - fun lifecycle_resumedAfterCommunalShows() { - // Communal is open. - goToScene(CommunalScenes.Communal) + fun lifecycle_resumedAfterCommunalShows() = + with(kosmos) { + testScope.runTest { + // Communal is open. + goToScene(CommunalScenes.Communal) - assertThat(underTest.lifecycle.currentState).isEqualTo(Lifecycle.State.RESUMED) - } + assertThat(underTest.lifecycle.currentState).isEqualTo(Lifecycle.State.RESUMED) + } + } @Test fun lifecycle_startedAfterCommunalCloses() = @@ -288,6 +298,43 @@ class GlanceableHubContainerControllerTest : SysuiTestCase() { } } + @Test + fun lifecycle_startedWhenEditActivityShowing() = + with(kosmos) { + testScope.runTest { + // Communal is open. + goToScene(CommunalScenes.Communal) + + // Edit activity is showing. + communalInteractor.setEditActivityShowing(true) + testableLooper.processAllMessages() + + assertThat(underTest.lifecycle.currentState).isEqualTo(Lifecycle.State.STARTED) + } + } + + @Test + fun lifecycle_startedWhenEditModeTransitionStarted() = + with(kosmos) { + testScope.runTest { + // Communal is open. + goToScene(CommunalScenes.Communal) + + // Leaving edit mode to return to the hub. + fakeKeyguardTransitionRepository.sendTransitionStep( + TransitionStep( + from = KeyguardState.GONE, + to = KeyguardState.GLANCEABLE_HUB, + value = 1.0f, + transitionState = TransitionState.RUNNING + ) + ) + testableLooper.processAllMessages() + + assertThat(underTest.lifecycle.currentState).isEqualTo(Lifecycle.State.STARTED) + } + } + @Test fun lifecycle_createdAfterDisposeView() { // Container view disposed. @@ -486,10 +533,10 @@ class GlanceableHubContainerControllerTest : SysuiTestCase() { testScope.runTest { // Communal is closed. goToScene(CommunalScenes.Blank) - `when`( + whenever( notificationStackScrollLayoutController.isBelowLastNotification( - anyFloat(), - anyFloat() + any(), + any() ) ) .thenReturn(false) @@ -497,6 +544,62 @@ class GlanceableHubContainerControllerTest : SysuiTestCase() { } } + @Test + fun onTouchEvent_hubOpen_touchesDispatched() = + with(kosmos) { + testScope.runTest { + // Communal is open. + goToScene(CommunalScenes.Communal) + + // Touch event is sent to the container view. + assertThat(underTest.onTouchEvent(DOWN_EVENT)).isTrue() + verify(containerView).onTouchEvent(any()) + } + } + + @Test + fun onTouchEvent_editActivityShowing_touchesConsumedButNotDispatched() = + with(kosmos) { + testScope.runTest { + // Communal is open. + goToScene(CommunalScenes.Communal) + + // Transitioning to or from edit mode. + communalInteractor.setEditActivityShowing(true) + testableLooper.processAllMessages() + + // onTouchEvent returns true to consume the touch, but it is not sent to the + // container view. + assertThat(underTest.onTouchEvent(DOWN_EVENT)).isTrue() + verify(containerView, never()).onTouchEvent(any()) + } + } + + @Test + fun onTouchEvent_editModeTransitionStarted_touchesConsumedButNotDispatched() = + with(kosmos) { + testScope.runTest { + // Communal is open. + goToScene(CommunalScenes.Communal) + + // Leaving edit mode to return to the hub. + fakeKeyguardTransitionRepository.sendTransitionStep( + TransitionStep( + from = KeyguardState.GONE, + to = KeyguardState.GLANCEABLE_HUB, + value = 1.0f, + transitionState = TransitionState.RUNNING + ) + ) + testableLooper.processAllMessages() + + // onTouchEvent returns true to consume the touch, but it is not sent to the + // container view. + assertThat(underTest.onTouchEvent(DOWN_EVENT)).isTrue() + verify(containerView, never()).onTouchEvent(any()) + } + } + private fun initAndAttachContainerView() { val mockInsets = mock { @@ -515,8 +618,21 @@ class GlanceableHubContainerControllerTest : SysuiTestCase() { testableLooper.processAllMessages() } - private fun goToScene(scene: SceneKey) { + private suspend fun goToScene(scene: SceneKey) { communalRepository.changeScene(scene) + if (scene == CommunalScenes.Communal) { + kosmos.fakeKeyguardTransitionRepository.sendTransitionSteps( + from = KeyguardState.LOCKSCREEN, + to = KeyguardState.GLANCEABLE_HUB, + kosmos.testScope + ) + } else { + kosmos.fakeKeyguardTransitionRepository.sendTransitionSteps( + from = KeyguardState.GLANCEABLE_HUB, + to = KeyguardState.LOCKSCREEN, + kosmos.testScope + ) + } testableLooper.processAllMessages() } -- cgit v1.2.3-59-g8ed1b