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
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 19c6249..c4e3d4e 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 @@ 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 01d5e5c..e3ebac6 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 @@ 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<Boolean> = + 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<KeyguardQuickAffordanceViewModel> = button(KeyguardQuickAffordancePosition.BOTTOM_START) @@ -77,14 +95,19 @@ 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 @@ 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 985ab62..b1de27d 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 @@ 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<out KeyguardQuickAffordanceConfig>,
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 19491f4..14b85b8 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.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 @@ val testConfig = TestConfig( isVisible = true, + isClickable = true, icon = mock(), canShowWhileLocked = false, intent = Intent("action"), @@ -154,6 +157,7 @@ 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 @@ val testConfig = TestConfig( isVisible = true, + isClickable = true, icon = mock(), canShowWhileLocked = false, intent = Intent("action"), @@ -260,6 +265,7 @@ testConfig = TestConfig( isVisible = true, + isClickable = true, icon = mock(), canShowWhileLocked = true, ) @@ -269,6 +275,7 @@ testConfig = TestConfig( isVisible = true, + isClickable = true, icon = mock(), canShowWhileLocked = false, ) @@ -342,6 +349,129 @@ 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 @@ ) { 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 @@ private data class TestConfig( val isVisible: Boolean, + val isClickable: Boolean = false, val icon: ContainedDrawable? = null, val canShowWhileLocked: Boolean = false, val intent: Intent? = null,