Fixes bug where the indication text is not padded.
This is a fun one. When switching to a language where the "Unlock screen for access to all functions" indication area message (which only appears right after a reboot) is very long, the home controls quick affordance button can overlap with the message as per the linked bug. Normally, the padding of the indication area is controled based on the visibility of quick affordances. If at least one quick affordance button is visible, the KeyguardBottomAreaViewBinder pads the indication area to fit between the buttons. If none is visible, the padding is set to 0, so the indication area message can be as wide as possible. There are two separate issues here: 1. The home controls quick affordance config, which provides state to ultimately show the button does not emit an initial value. That makes the entire downstream flow never receive a value which means that the padding logic in the binder never runs. This CL addresses this issue by emitting an initially Hidden value for any quick affordance known to the quick affordance framework (as this bug can repeat itself if the next config does not emit an initial value). 2. The home controls quick affordance config is incorrectly being shown on the lock-screen before the first unlock after a device reboot. In the process of refactoring the code, the new implementation missed the need to include the ControlsComponent.getVisibility check that the old implementation was using. In a followup CL, we will address this too Fix: 244296596 Test: Unit tests. Manually made sure that, with a phone switched to Slovenian and rebooted, when the home controls button appears on the lock-screen before unlocking it (which is a bug as explained above), the long text is properly wrapped because the indication area is padded. Change-Id: I6fb612edb139113594040a5d45ee8e661f0a0e5c
This commit is contained in:
@@ -32,6 +32,7 @@ import javax.inject.Inject
|
||||
import kotlin.reflect.KClass
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.combine
|
||||
import kotlinx.coroutines.flow.onStart
|
||||
|
||||
@SysUISingleton
|
||||
class KeyguardQuickAffordanceInteractor
|
||||
@@ -88,7 +89,15 @@ constructor(
|
||||
position: KeyguardQuickAffordancePosition
|
||||
): Flow<KeyguardQuickAffordanceModel> {
|
||||
val configs = registry.getAll(position)
|
||||
return combine(configs.map { config -> config.state }) { states ->
|
||||
return combine(
|
||||
configs.map { config ->
|
||||
// We emit an initial "Hidden" value to make sure that there's always an initial
|
||||
// value and avoid subtle bugs where the downstream isn't receiving any values
|
||||
// because one config implementation is not emitting an initial value. For example,
|
||||
// see b/244296596.
|
||||
config.state.onStart { emit(KeyguardQuickAffordanceConfig.State.Hidden) }
|
||||
}
|
||||
) { states ->
|
||||
val index = states.indexOfFirst { it is KeyguardQuickAffordanceConfig.State.Visible }
|
||||
if (index != -1) {
|
||||
val visibleState = states[index] as KeyguardQuickAffordanceConfig.State.Visible
|
||||
|
||||
@@ -36,6 +36,7 @@ import com.google.common.truth.Truth.assertThat
|
||||
import kotlinx.coroutines.flow.launchIn
|
||||
import kotlinx.coroutines.flow.onEach
|
||||
import kotlinx.coroutines.test.runBlockingTest
|
||||
import kotlinx.coroutines.yield
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
@@ -110,6 +111,10 @@ class KeyguardQuickAffordanceInteractorTest : SysuiTestCase() {
|
||||
.quickAffordance(KeyguardQuickAffordancePosition.BOTTOM_START)
|
||||
.onEach { latest = it }
|
||||
.launchIn(this)
|
||||
// The interactor has an onStart { emit(Hidden) } to cover for upstream configs that don't
|
||||
// produce an initial value. We yield to give the coroutine time to emit the first real
|
||||
// value from our config.
|
||||
yield()
|
||||
|
||||
assertThat(latest).isInstanceOf(KeyguardQuickAffordanceModel.Visible::class.java)
|
||||
val visibleModel = latest as KeyguardQuickAffordanceModel.Visible
|
||||
@@ -136,6 +141,10 @@ class KeyguardQuickAffordanceInteractorTest : SysuiTestCase() {
|
||||
.quickAffordance(KeyguardQuickAffordancePosition.BOTTOM_END)
|
||||
.onEach { latest = it }
|
||||
.launchIn(this)
|
||||
// The interactor has an onStart { emit(Hidden) } to cover for upstream configs that don't
|
||||
// produce an initial value. We yield to give the coroutine time to emit the first real
|
||||
// value from our config.
|
||||
yield()
|
||||
|
||||
assertThat(latest).isInstanceOf(KeyguardQuickAffordanceModel.Visible::class.java)
|
||||
val visibleModel = latest as KeyguardQuickAffordanceModel.Visible
|
||||
|
||||
@@ -224,7 +224,10 @@ class KeyguardBottomAreaViewModelTest : SysuiTestCase() {
|
||||
repository.setAnimateDozingTransitions(false)
|
||||
yield()
|
||||
|
||||
assertThat(values).isEqualTo(listOf(false, true, false))
|
||||
// Note the extra false value in the beginning. This is to cover for the initial value
|
||||
// inserted by the quick affordance interactor which it does to cover for config
|
||||
// implementations that don't emit an initial value.
|
||||
assertThat(values).isEqualTo(listOf(false, false, true, false))
|
||||
job.cancel()
|
||||
}
|
||||
|
||||
@@ -372,6 +375,10 @@ class KeyguardBottomAreaViewModelTest : SysuiTestCase() {
|
||||
|
||||
var latest: KeyguardQuickAffordanceViewModel? = null
|
||||
val job = underTest.startButton.onEach { latest = it }.launchIn(this)
|
||||
// The interactor has an onStart { emit(Hidden) } to cover for upstream configs that don't
|
||||
// produce an initial value. We yield to give the coroutine time to emit the first real
|
||||
// value from our config.
|
||||
yield()
|
||||
|
||||
assertQuickAffordanceViewModel(
|
||||
viewModel = latest,
|
||||
|
||||
Reference in New Issue
Block a user