From 630185ed36e0bb17fb0edc48d959d4a81f0599df Mon Sep 17 00:00:00 2001 From: Alejandro Nijamkin Date: Fri, 19 Aug 2022 15:26:29 -0700 Subject: [PATCH] Affordances no longer clickable when invisible. This CL fixes a bug where the user can click on the invisible (alpha = 0) quick affordances on the lock-screen. The reason they were clickable was because the affordance views are fully faded-out, not truly invisible or gone. The approach taken is simply to make the views not clickable as soon as they start being faded out. Fix: 241830987 Test: manually verified that the buttons are no longer clickable when the screen is locked and the notifications shade is pulled down (which is how the manual test failed) and added unit tests for the view-model. Change-Id: Ic8f6305cef2b544fc468290cd56fc381c48e0877 --- .../ui/binder/KeyguardBottomAreaViewBinder.kt | 16 ++- .../viewmodel/KeyguardBottomAreaViewModel.kt | 39 +++++- .../KeyguardQuickAffordanceViewModel.kt | 1 + .../KeyguardBottomAreaViewModelTest.kt | 132 ++++++++++++++++++ 4 files changed, 184 insertions(+), 4 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/ui/binder/KeyguardBottomAreaViewBinder.kt b/packages/SystemUI/src/com/android/systemui/keyguard/ui/binder/KeyguardBottomAreaViewBinder.kt index 19c6249a12c0f..c4e3d4e4c1b58 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/ui/binder/KeyguardBottomAreaViewBinder.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/ui/binder/KeyguardBottomAreaViewBinder.kt @@ -251,9 +251,21 @@ object KeyguardBottomAreaViewBinder { Utils.getColorAttr(view.context, com.android.internal.R.attr.colorSurface) view.contentDescription = view.context.getString(viewModel.contentDescriptionResourceId) - view.setOnClickListener { + view.isClickable = viewModel.isClickable + if (viewModel.isClickable) { + view.setOnClickListener(OnClickListener(viewModel, falsingManager)) + } else { + view.setOnClickListener(null) + } + } + + private class OnClickListener( + private val viewModel: KeyguardQuickAffordanceViewModel, + private val falsingManager: FalsingManager, + ) : View.OnClickListener { + override fun onClick(view: View) { if (falsingManager.isFalseTap(FalsingManager.LOW_PENALTY)) { - return@setOnClickListener + return } if (viewModel.configKey != null) { diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/ui/viewmodel/KeyguardBottomAreaViewModel.kt b/packages/SystemUI/src/com/android/systemui/keyguard/ui/viewmodel/KeyguardBottomAreaViewModel.kt index 01d5e5c493ce8..e3ebac60febbb 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/ui/viewmodel/KeyguardBottomAreaViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/ui/viewmodel/KeyguardBottomAreaViewModel.kt @@ -16,6 +16,7 @@ package com.android.systemui.keyguard.ui.viewmodel +import androidx.annotation.VisibleForTesting import com.android.systemui.doze.util.BurnInHelperWrapper import com.android.systemui.keyguard.domain.interactor.KeyguardBottomAreaInteractor import com.android.systemui.keyguard.domain.interactor.KeyguardInteractor @@ -37,6 +38,23 @@ constructor( private val bottomAreaInteractor: KeyguardBottomAreaInteractor, private val burnInHelperWrapper: BurnInHelperWrapper, ) { + /** + * Whether quick affordances are "opaque enough" to be considered visible to and interactive by + * the user. If they are not interactive, user input should not be allowed on them. + * + * Note that there is a margin of error, where we allow very, very slightly transparent views to + * be considered "fully opaque" for the purpose of being interactive. This is to accommodate the + * error margin of floating point arithmetic. + * + * A view that is visible but with an alpha of less than our threshold either means it's not + * fully done fading in or is fading/faded out. Either way, it should not be + * interactive/clickable unless "fully opaque" to avoid issues like in b/241830987. + */ + private val areQuickAffordancesFullyOpaque: Flow = + bottomAreaInteractor.alpha + .map { alpha -> alpha >= AFFORDANCE_FULLY_OPAQUE_ALPHA_THRESHOLD } + .distinctUntilChanged() + /** An observable for the view-model of the "start button" quick affordance. */ val startButton: Flow = button(KeyguardQuickAffordancePosition.BOTTOM_START) @@ -77,14 +95,19 @@ constructor( return combine( quickAffordanceInteractor.quickAffordance(position), bottomAreaInteractor.animateDozingTransitions.distinctUntilChanged(), - ) { model, animateReveal -> - model.toViewModel(animateReveal) + areQuickAffordancesFullyOpaque, + ) { model, animateReveal, isFullyOpaque -> + model.toViewModel( + animateReveal = animateReveal, + isClickable = isFullyOpaque, + ) } .distinctUntilChanged() } private fun KeyguardQuickAffordanceModel.toViewModel( animateReveal: Boolean, + isClickable: Boolean, ): KeyguardQuickAffordanceViewModel { return when (this) { is KeyguardQuickAffordanceModel.Visible -> @@ -100,8 +123,20 @@ constructor( animationController = parameters.animationController, ) }, + isClickable = isClickable, ) is KeyguardQuickAffordanceModel.Hidden -> KeyguardQuickAffordanceViewModel() } } + + companion object { + // We select a value that's less than 1.0 because we want floating point math precision to + // not be a factor in determining whether the affordance UI is fully opaque. The number we + // choose needs to be close enough 1.0 such that the user can't easily tell the difference + // between the UI with an alpha at the threshold and when the alpha is 1.0. At the same + // time, we don't want the number to be too close to 1.0 such that there is a chance that we + // never treat the affordance UI as "fully opaque" as that would risk making it forever not + // clickable. + @VisibleForTesting const val AFFORDANCE_FULLY_OPAQUE_ALPHA_THRESHOLD = 0.95f + } } diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/ui/viewmodel/KeyguardQuickAffordanceViewModel.kt b/packages/SystemUI/src/com/android/systemui/keyguard/ui/viewmodel/KeyguardQuickAffordanceViewModel.kt index 985ab623764a7..b1de27d262cfe 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/ui/viewmodel/KeyguardQuickAffordanceViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/ui/viewmodel/KeyguardQuickAffordanceViewModel.kt @@ -31,6 +31,7 @@ data class KeyguardQuickAffordanceViewModel( val icon: ContainedDrawable = ContainedDrawable.WithResource(0), @StringRes val contentDescriptionResourceId: Int = 0, val onClicked: (OnClickedParameters) -> Unit = {}, + val isClickable: Boolean = false, ) { data class OnClickedParameters( val configKey: KClass, diff --git a/packages/SystemUI/tests/src/com/android/systemui/keyguard/ui/viewmodel/KeyguardBottomAreaViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/keyguard/ui/viewmodel/KeyguardBottomAreaViewModelTest.kt index 19491f41a0c11..14b85b8b5e565 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/keyguard/ui/viewmodel/KeyguardBottomAreaViewModelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/keyguard/ui/viewmodel/KeyguardBottomAreaViewModelTest.kt @@ -37,6 +37,8 @@ import com.android.systemui.statusbar.policy.KeyguardStateController import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.mock import com.google.common.truth.Truth.assertThat +import kotlin.math.max +import kotlin.math.min import kotlin.reflect.KClass import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach @@ -127,6 +129,7 @@ class KeyguardBottomAreaViewModelTest : SysuiTestCase() { val testConfig = TestConfig( isVisible = true, + isClickable = true, icon = mock(), canShowWhileLocked = false, intent = Intent("action"), @@ -154,6 +157,7 @@ class KeyguardBottomAreaViewModelTest : SysuiTestCase() { val config = TestConfig( isVisible = true, + isClickable = true, icon = mock(), canShowWhileLocked = false, intent = null, // This will cause it to tell the system that the click was handled. @@ -201,6 +205,7 @@ class KeyguardBottomAreaViewModelTest : SysuiTestCase() { val testConfig = TestConfig( isVisible = true, + isClickable = true, icon = mock(), canShowWhileLocked = false, intent = Intent("action"), @@ -260,6 +265,7 @@ class KeyguardBottomAreaViewModelTest : SysuiTestCase() { testConfig = TestConfig( isVisible = true, + isClickable = true, icon = mock(), canShowWhileLocked = true, ) @@ -269,6 +275,7 @@ class KeyguardBottomAreaViewModelTest : SysuiTestCase() { testConfig = TestConfig( isVisible = true, + isClickable = true, icon = mock(), canShowWhileLocked = false, ) @@ -342,6 +349,129 @@ class KeyguardBottomAreaViewModelTest : SysuiTestCase() { job.cancel() } + @Test + fun `isClickable - true when alpha at threshold`() = runBlockingTest { + repository.setKeyguardShowing(true) + repository.setBottomAreaAlpha( + KeyguardBottomAreaViewModel.AFFORDANCE_FULLY_OPAQUE_ALPHA_THRESHOLD + ) + + val testConfig = + TestConfig( + isVisible = true, + isClickable = true, + icon = mock(), + canShowWhileLocked = false, + intent = Intent("action"), + ) + val configKey = + setUpQuickAffordanceModel( + position = KeyguardQuickAffordancePosition.BOTTOM_START, + testConfig = testConfig, + ) + + var latest: KeyguardQuickAffordanceViewModel? = null + val job = underTest.startButton.onEach { latest = it }.launchIn(this) + + assertQuickAffordanceViewModel( + viewModel = latest, + testConfig = testConfig, + configKey = configKey, + ) + job.cancel() + } + + @Test + fun `isClickable - true when alpha above threshold`() = runBlockingTest { + repository.setKeyguardShowing(true) + var latest: KeyguardQuickAffordanceViewModel? = null + val job = underTest.startButton.onEach { latest = it }.launchIn(this) + repository.setBottomAreaAlpha( + min(1f, KeyguardBottomAreaViewModel.AFFORDANCE_FULLY_OPAQUE_ALPHA_THRESHOLD + 0.1f), + ) + + val testConfig = + TestConfig( + isVisible = true, + isClickable = true, + icon = mock(), + canShowWhileLocked = false, + intent = Intent("action"), + ) + val configKey = + setUpQuickAffordanceModel( + position = KeyguardQuickAffordancePosition.BOTTOM_START, + testConfig = testConfig, + ) + + assertQuickAffordanceViewModel( + viewModel = latest, + testConfig = testConfig, + configKey = configKey, + ) + job.cancel() + } + + @Test + fun `isClickable - false when alpha below threshold`() = runBlockingTest { + repository.setKeyguardShowing(true) + var latest: KeyguardQuickAffordanceViewModel? = null + val job = underTest.startButton.onEach { latest = it }.launchIn(this) + repository.setBottomAreaAlpha( + max(0f, KeyguardBottomAreaViewModel.AFFORDANCE_FULLY_OPAQUE_ALPHA_THRESHOLD - 0.1f), + ) + + val testConfig = + TestConfig( + isVisible = true, + isClickable = false, + icon = mock(), + canShowWhileLocked = false, + intent = Intent("action"), + ) + val configKey = + setUpQuickAffordanceModel( + position = KeyguardQuickAffordancePosition.BOTTOM_START, + testConfig = testConfig, + ) + + assertQuickAffordanceViewModel( + viewModel = latest, + testConfig = testConfig, + configKey = configKey, + ) + job.cancel() + } + + @Test + fun `isClickable - false when alpha at zero`() = runBlockingTest { + repository.setKeyguardShowing(true) + var latest: KeyguardQuickAffordanceViewModel? = null + val job = underTest.startButton.onEach { latest = it }.launchIn(this) + repository.setBottomAreaAlpha(0f) + + val testConfig = + TestConfig( + isVisible = true, + isClickable = false, + icon = mock(), + canShowWhileLocked = false, + intent = Intent("action"), + ) + val configKey = + setUpQuickAffordanceModel( + position = KeyguardQuickAffordancePosition.BOTTOM_START, + testConfig = testConfig, + ) + + assertQuickAffordanceViewModel( + viewModel = latest, + testConfig = testConfig, + configKey = configKey, + ) + job.cancel() + } + private suspend fun setDozeAmountAndCalculateExpectedTranslationY(dozeAmount: Float): Float { repository.setDozeAmount(dozeAmount) return dozeAmount * (RETURNED_BURN_IN_OFFSET - DEFAULT_BURN_IN_OFFSET) @@ -384,6 +514,7 @@ class KeyguardBottomAreaViewModelTest : SysuiTestCase() { ) { checkNotNull(viewModel) assertThat(viewModel.isVisible).isEqualTo(testConfig.isVisible) + assertThat(viewModel.isClickable).isEqualTo(testConfig.isClickable) if (testConfig.isVisible) { assertThat(viewModel.icon).isEqualTo(testConfig.icon) viewModel.onClicked.invoke( @@ -404,6 +535,7 @@ class KeyguardBottomAreaViewModelTest : SysuiTestCase() { private data class TestConfig( val isVisible: Boolean, + val isClickable: Boolean = false, val icon: ContainedDrawable? = null, val canShowWhileLocked: Boolean = false, val intent: Intent? = null,