Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix focus when navigating across compose fields #5195

Merged
merged 8 commits into from
Jun 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion payments-ui-core/api/payments-ui-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ public final class com/stripe/android/ui/core/elements/TextFieldController$Defau

public final class com/stripe/android/ui/core/elements/TextFieldUIKt {
public static final fun AnimatedIcons (Ljava/util/List;ZLandroidx/compose/runtime/Composer;I)V
public static final fun TextField-PwfN4xk (Lcom/stripe/android/ui/core/elements/TextFieldController;Landroidx/compose/ui/Modifier;IZLkotlin/jvm/functions/Function1;Landroidx/compose/runtime/Composer;II)V
public static final fun TextField-ndPIYpw (Lcom/stripe/android/ui/core/elements/TextFieldController;ZILandroidx/compose/ui/Modifier;Lkotlin/jvm/functions/Function1;IILandroidx/compose/runtime/Composer;II)V
public static final fun TextFieldSection-VyDzSTg (Lcom/stripe/android/ui/core/elements/TextFieldController;Landroidx/compose/ui/Modifier;Ljava/lang/Integer;IZLkotlin/jvm/functions/Function1;Landroidx/compose/runtime/Composer;II)V
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import androidx.compose.runtime.Composable
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier
import androidx.compose.ui.focus.FocusDirection
import androidx.compose.ui.layout.onSizeChanged
import androidx.compose.ui.unit.dp
import com.stripe.android.ui.core.paymentsColors
Expand All @@ -23,28 +24,41 @@ internal fun RowElementUI(
hiddenIdentifiers: List<IdentifierSpec>,
lastTextFieldIdentifier: IdentifierSpec?
) {
val fields = controller.fields
val numVisibleFields = fields.filter { !hiddenIdentifiers.contains(it.identifier) }.size
val visibleFields = controller.fields.filter { !hiddenIdentifiers.contains(it.identifier) }
val dividerHeight = remember { mutableStateOf(0.dp) }
// Only draw the row if the items in the row are not hidden, otherwise the entire
// Only draw the row if there are items in the row that are not hidden, otherwise the entire
// section will fail to draw
if (fields.map { it.identifier }.any { !hiddenIdentifiers.contains(it) }) {
if (visibleFields.isNotEmpty()) {
Row(modifier = Modifier.fillMaxWidth()) {
fields.forEachIndexed { index, field ->
visibleFields.forEachIndexed { index, field ->
val nextFocusDirection = if (index == visibleFields.lastIndex) {
FocusDirection.Down
} else {
FocusDirection.Right
}

val previousFocusDirection = if (index == 0) {
FocusDirection.Up
} else {
FocusDirection.Left
}

SectionFieldElementUI(
enabled,
field,
hiddenIdentifiers = hiddenIdentifiers,
lastTextFieldIdentifier = lastTextFieldIdentifier,
modifier = Modifier
.weight(1.0f / numVisibleFields.toFloat())
.weight(1.0f / visibleFields.size.toFloat())
.onSizeChanged {
dividerHeight.value =
(it.height / Resources.getSystem().displayMetrics.density).dp
}
},
nextFocusDirection = nextFocusDirection,
previousFocusDirection = previousFocusDirection
)

if (index != fields.lastIndex) {
if (index != visibleFields.lastIndex) {
Divider(
modifier = Modifier
.height(dividerHeight.value)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.stripe.android.ui.core.elements

import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.focus.FocusDirection
import androidx.compose.ui.text.input.ImeAction

@Composable
Expand All @@ -10,20 +11,24 @@ internal fun SectionFieldElementUI(
field: SectionFieldElement,
modifier: Modifier = Modifier,
hiddenIdentifiers: List<IdentifierSpec>? = null,
lastTextFieldIdentifier: IdentifierSpec?
lastTextFieldIdentifier: IdentifierSpec?,
nextFocusDirection: FocusDirection = FocusDirection.Down,
previousFocusDirection: FocusDirection = FocusDirection.Up
) {
if (hiddenIdentifiers?.contains(field.identifier) == false) {
when (val controller = field.sectionFieldErrorController()) {
is TextFieldController -> {
TextField(
textFieldController = controller,
enabled = enabled,
modifier = modifier,
imeAction = if (lastTextFieldIdentifier == field.identifier) {
ImeAction.Done
} else {
ImeAction.Next
}
},
modifier = modifier,
nextFocusDirection = nextFocusDirection,
previousFocusDirection = previousFocusDirection
)
}
is DropdownFieldController -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,12 @@ fun TextFieldSection(
@Composable
fun TextField(
textFieldController: TextFieldController,
modifier: Modifier = Modifier,
imeAction: ImeAction,
enabled: Boolean,
onTextStateChanged: (TextFieldState?) -> Unit = {}
imeAction: ImeAction,
modifier: Modifier = Modifier,
onTextStateChanged: (TextFieldState?) -> Unit = {},
nextFocusDirection: FocusDirection = FocusDirection.Next,
previousFocusDirection: FocusDirection = FocusDirection.Previous
) {
val focusManager = LocalFocusManager.current
val value by textFieldController.fieldValue.collectAsState("")
Expand All @@ -120,7 +122,7 @@ fun TextField(
@Suppress("UNUSED_VALUE")
processedIsFull = if (fieldState == TextFieldStateConstants.Valid.Full) {
if (!processedIsFull) {
focusManager.moveFocus(FocusDirection.Next)
focusManager.moveFocus(nextFocusDirection)
}
true
} else {
Expand All @@ -143,7 +145,7 @@ fun TextField(
event.nativeKeyEvent.keyCode == KeyEvent.KEYCODE_DEL &&
value.isEmpty()
) {
focusManager.moveFocus(FocusDirection.Previous)
focusManager.moveFocus(previousFocusDirection)
true
} else {
false
Expand Down Expand Up @@ -200,7 +202,7 @@ fun TextField(
),
keyboardActions = KeyboardActions(
onNext = {
focusManager.moveFocus(FocusDirection.Next)
focusManager.moveFocus(nextFocusDirection)
},
onDone = {
focusManager.clearFocus(true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,11 @@ internal abstract class BaseAddPaymentMethodFragment : Fragment() {
setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed)
setContent {
val processing by sheetViewModel.processing
.asFlow()
.collectAsState(initial = false)
.asFlow()
.collectAsState(initial = false)
val selectedItem by sheetViewModel.getAddFragmentSelectedLpm()
.asFlow()
.collectAsState(initial = initialSelectedItem)
.asFlow()
.collectAsState(initial = initialSelectedItem)
PaymentMethodsUI(
selectedIndex = paymentMethods.indexOf(selectedItem),
isEnabled = !processing,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package com.stripe.android.paymentsheet

import android.app.Application
import android.content.Context
import androidx.core.os.bundleOf
import androidx.fragment.app.testing.FragmentScenario
import androidx.fragment.app.testing.launchFragmentInContainer
import androidx.test.core.app.ApplicationProvider
import com.google.common.truth.Truth.assertThat
Expand All @@ -13,9 +14,11 @@ import com.stripe.android.model.PaymentIntentFixtures
import com.stripe.android.paymentsheet.databinding.FragmentPaymentsheetAddPaymentMethodBinding
import com.stripe.android.paymentsheet.model.FragmentConfig
import com.stripe.android.paymentsheet.model.FragmentConfigFixtures
import com.stripe.android.ui.core.forms.resources.LpmRepository
import com.stripe.android.utils.TestUtils
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runBlockingTest
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.runTest
import org.junit.After
import org.junit.Before
import org.junit.Test
Expand All @@ -26,6 +29,8 @@ import org.robolectric.RobolectricTestRunner
@ExperimentalCoroutinesApi
@RunWith(RobolectricTestRunner::class)
internal class PaymentOptionsAddPaymentMethodFragmentTest : PaymentOptionsViewModelTestInjection() {
private val context: Context = ApplicationProvider.getApplicationContext()
private val lpmRepository = LpmRepository(context.resources)

@Before
fun setup() {
Expand All @@ -43,52 +48,13 @@ internal class PaymentOptionsAddPaymentMethodFragmentTest : PaymentOptionsViewMo
@Test
fun `Factory gets initialized by Injector when Injector is available`() {
createFragment { fragment, _, viewModel ->
val factory = PaymentOptionsViewModel.Factory(
{ ApplicationProvider.getApplicationContext() },
{
PaymentOptionContract.Args(
PaymentIntentFixtures.PI_REQUIRES_PAYMENT_METHOD,
emptyList(),
null,
false,
null,
null,
DUMMY_INJECTOR_KEY,
false,
mock()
)
},
fragment
)
assertThat(fragment.sheetViewModel).isEqualTo(viewModel)

WeakMapInjectorRegistry.clear()
}
}

@Test
fun `Factory gets initialized with fallback when no Injector is available`() = runBlockingTest {
fun `Factory gets initialized with fallback when no Injector is available`() = runTest(UnconfinedTestDispatcher()) {
createFragment(registerInjector = false) { fragment, _, viewModel ->
val context = ApplicationProvider.getApplicationContext<Application>()
val productUsage = setOf("TestProductUsage")
PaymentConfiguration.init(context, "testKey")
val factory = PaymentOptionsViewModel.Factory(
{ context },
{
PaymentOptionContract.Args(
PaymentIntentFixtures.PI_REQUIRES_PAYMENT_METHOD,
emptyList(),
null,
false,
null,
null,
DUMMY_INJECTOR_KEY,
false,
productUsage
)
},
fragment
)
assertThat(fragment.sheetViewModel).isNotEqualTo(viewModel)
}
}
Expand All @@ -108,7 +74,7 @@ internal class PaymentOptionsAddPaymentMethodFragmentTest : PaymentOptionsViewMo
fragmentConfig: FragmentConfig? = FragmentConfigFixtures.DEFAULT,
registerInjector: Boolean = true,
onReady: (PaymentOptionsAddPaymentMethodFragment, FragmentPaymentsheetAddPaymentMethodBinding, PaymentOptionsViewModel) -> Unit
) {
): FragmentScenario<PaymentOptionsAddPaymentMethodFragment> {
assertThat(WeakMapInjectorRegistry.staticCacheMap.size).isEqualTo(0)
val viewModel = createViewModel(
paymentMethods = args.paymentMethods,
Expand All @@ -117,10 +83,11 @@ internal class PaymentOptionsAddPaymentMethodFragmentTest : PaymentOptionsViewMo
)
viewModel.setStripeIntent(args.stripeIntent)
TestUtils.idleLooper()

if (registerInjector) {
registerViewModel(args.injectorKey, viewModel, createFormViewModel())
registerViewModel(args.injectorKey, viewModel, lpmRepository)
}
launchFragmentInContainer<PaymentOptionsAddPaymentMethodFragment>(
return launchFragmentInContainer<PaymentOptionsAddPaymentMethodFragment>(
bundleOf(
PaymentOptionsActivity.EXTRA_FRAGMENT_CONFIG to fragmentConfig,
PaymentOptionsActivity.EXTRA_STARTER_ARGS to args
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.stripe.android.paymentsheet

import android.app.Application
import androidx.arch.core.executor.testing.InstantTaskExecutorRule
import androidx.lifecycle.SavedStateHandle
import androidx.test.core.app.ApplicationProvider
Expand All @@ -17,6 +18,7 @@ import com.stripe.android.paymentsheet.viewmodels.BaseSheetViewModel
import com.stripe.android.ui.core.forms.resources.LpmRepository
import com.stripe.android.ui.core.forms.resources.StaticResourceRepository
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.FlowPreview
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import org.junit.After
Expand Down Expand Up @@ -48,9 +50,6 @@ internal open class PaymentOptionsViewModelTestInjection {
@InjectorKey injectorKey: String,
args: PaymentOptionContract.Args = PaymentSheetFixtures.PAYMENT_OPTIONS_CONTRACT_ARGS
): PaymentOptionsViewModel = runBlocking {
val lpmRepository = mock<LpmRepository>()
whenever(lpmRepository.fromCode("card")).thenReturn(LpmRepository.HardcodedCard)

PaymentOptionsViewModel(
args,
prefsRepositoryFactory = {
Expand All @@ -62,28 +61,28 @@ internal open class PaymentOptionsViewModelTestInjection {
application = ApplicationProvider.getApplicationContext(),
logger = Logger.noop(),
injectorKey = injectorKey,
resourceRepository = StaticResourceRepository(mock(), lpmRepository),
resourceRepository = StaticResourceRepository(
mock(),
LpmRepository(ApplicationProvider.getApplicationContext<Application>().resources)
),
savedStateHandle = SavedStateHandle().apply {
set(BaseSheetViewModel.SAVE_RESOURCE_REPOSITORY_READY, true)
},
linkPaymentLauncherFactory = mock()
)
}

@ExperimentalCoroutinesApi
fun createFormViewModel(): FormViewModel = runBlocking {
FormViewModel(
paymentMethodCode = "",
config = mock(),
resourceRepository = mock(),
transformSpecToElement = mock()
)
}

@FlowPreview
fun registerViewModel(
@InjectorKey injectorKey: String,
viewModel: PaymentOptionsViewModel,
formViewModel: FormViewModel
lpmRepository: LpmRepository = mock(),
formViewModel: FormViewModel = FormViewModel(
paymentMethodCode = PaymentMethod.Type.Card.code,
config = mock(),
resourceRepository = StaticResourceRepository(mock(), lpmRepository),
transformSpecToElement = mock()
)
) {
val mockBuilder = mock<PaymentOptionsViewModelSubcomponent.Builder>()
val mockSubcomponent = mock<PaymentOptionsViewModelSubcomponent>()
Expand Down