-
Notifications
You must be signed in to change notification settings - Fork 628
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
Networking Sign Up Pane (Native): Add back auto-focus #8557
base: master
Are you sure you want to change the base?
Changes from all commits
71b151f
3955f6b
e3c5969
95d3776
911e625
cae8c07
eb7d419
aee117e
6e48efb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -134,15 +134,24 @@ private fun NetworkingLinkSignupLoaded( | |
) { | ||
val scrollState = rememberScrollState() | ||
val phoneNumberFocusRequester = remember { FocusRequester() } | ||
val emailFocusRequester = remember { FocusRequester() } | ||
|
||
// When pane loads, focus on email field if it's empty | ||
LaunchedEffect(true) { | ||
if (payload.emailController.fieldValue.value.isEmpty()) { | ||
emailFocusRequester.requestFocus() | ||
} | ||
} | ||
// Everytime full form is shown, scroll to bottom and focus on phone number field (if not empty) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. |
||
LaunchedEffect(showFullForm) { | ||
if (showFullForm) { | ||
if (showFullForm && payload.phoneController.fieldValue.value.isEmpty()) { | ||
scrollState.animateScrollToBottom() | ||
phoneNumberFocusRequester.requestFocus() | ||
} | ||
} | ||
|
||
Layout( | ||
applyImePadding = false, | ||
scrollState = scrollState, | ||
body = { | ||
Title(payload.content.title) | ||
|
@@ -158,6 +167,7 @@ private fun NetworkingLinkSignupLoaded( | |
|
||
EmailSection( | ||
showFullForm = showFullForm, | ||
focusRequester = emailFocusRequester, | ||
loading = lookupAccountSync is Loading, | ||
emailController = payload.emailController, | ||
enabled = true, | ||
|
@@ -272,7 +282,8 @@ internal fun EmailSection( | |
enabled: Boolean, | ||
emailController: TextFieldController, | ||
showFullForm: Boolean, | ||
loading: Boolean | ||
loading: Boolean, | ||
focusRequester: FocusRequester = remember { FocusRequester() } | ||
) { | ||
var focused by remember { mutableStateOf(false) } | ||
StripeThemeForConnections { | ||
|
@@ -284,6 +295,7 @@ internal fun EmailSection( | |
) { | ||
TextFieldSection( | ||
modifier = Modifier.onFocusChanged { focused = it.isFocused }, | ||
focusRequester = focusRequester, | ||
isSelected = focused, | ||
textFieldController = emailController, | ||
imeAction = if (showFullForm) ImeAction.Next else ImeAction.Done, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package com.stripe.android.financialconnections.ui | ||
|
||
import androidx.compose.ui.Modifier | ||
|
||
/** | ||
* Conditionally applies the given [Modifier] functions based on the [condition]. | ||
*/ | ||
internal inline fun Modifier.conditional( | ||
condition: Boolean, | ||
ifTrue: Modifier.() -> Modifier, | ||
ifFalse: Modifier.() -> Modifier = { this }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: We don’t seem to use |
||
): Modifier = if (condition) { | ||
then(ifTrue(Modifier)) | ||
} else { | ||
then(ifFalse(Modifier)) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ import androidx.compose.foundation.layout.Spacer | |
import androidx.compose.foundation.layout.fillMaxSize | ||
import androidx.compose.foundation.layout.fillMaxWidth | ||
import androidx.compose.foundation.layout.height | ||
import androidx.compose.foundation.layout.imePadding | ||
import androidx.compose.foundation.layout.padding | ||
import androidx.compose.foundation.lazy.LazyColumn | ||
import androidx.compose.foundation.lazy.LazyListScope | ||
|
@@ -36,6 +37,7 @@ import com.stripe.android.financialconnections.ui.components.DragHandle | |
import com.stripe.android.financialconnections.ui.components.FinancialConnectionsButton | ||
import com.stripe.android.financialconnections.ui.components.FinancialConnectionsScaffold | ||
import com.stripe.android.financialconnections.ui.components.FinancialConnectionsTopAppBar | ||
import com.stripe.android.financialconnections.ui.conditional | ||
import com.stripe.android.financialconnections.ui.theme.FinancialConnectionsTheme.typography | ||
|
||
/** | ||
|
@@ -47,6 +49,7 @@ import com.stripe.android.financialconnections.ui.theme.FinancialConnectionsThem | |
* @param inModal whether the layout is being used in a modal or not. If true, the [body] won't expand to fill the | ||
* available content. | ||
* @param showFooterShadowWhenScrollable whether to show a shadow at the top of the footer when the body is scrollable. | ||
* @param applyImePadding whether to apply IME padding. When false, footer will be covered by the keyboard. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Should we rename this to |
||
* @param scrollState the [ScrollState] to use for the scrollable body. | ||
*/ | ||
@Composable | ||
|
@@ -56,6 +59,7 @@ internal fun Layout( | |
inModal: Boolean = false, | ||
loading: Boolean = false, | ||
showPillOnSlowLoad: Boolean = false, | ||
applyImePadding: Boolean = true, | ||
verticalArrangement: Arrangement.Vertical = Arrangement.Top, | ||
showFooterShadowWhenScrollable: Boolean = true, | ||
scrollState: ScrollState = rememberScrollState(), | ||
|
@@ -66,6 +70,7 @@ internal fun Layout( | |
canScrollForward = scrollState.canScrollForward, | ||
canScrollBackward = scrollState.canScrollBackward, | ||
inModal = inModal, | ||
applyImePadding = applyImePadding, | ||
loading = loading, | ||
showPillOnSlowLoad = showPillOnSlowLoad, | ||
showFooterShadowWhenScrollable = showFooterShadowWhenScrollable, | ||
|
@@ -101,6 +106,7 @@ internal fun Layout( | |
internal fun LazyLayout( | ||
modifier: Modifier = Modifier, | ||
bodyPadding: PaddingValues = PaddingValues(horizontal = 24.dp), | ||
applyImePadding: Boolean = true, | ||
inModal: Boolean = false, | ||
loading: Boolean = false, | ||
showPillOnSlowLoad: Boolean = false, | ||
|
@@ -115,6 +121,7 @@ internal fun LazyLayout( | |
canScrollBackward = lazyListState.canScrollBackward, | ||
inModal = inModal, | ||
loading = loading, | ||
applyImePadding = applyImePadding, | ||
showPillOnSlowLoad = showPillOnSlowLoad, | ||
showFooterShadowWhenScrollable = showFooterShadowWhenScrollable, | ||
modifier = modifier, | ||
|
@@ -137,6 +144,7 @@ private fun LayoutScaffold( | |
showPillOnSlowLoad: Boolean, | ||
inModal: Boolean, | ||
showFooterShadowWhenScrollable: Boolean, | ||
applyImePadding: Boolean, | ||
modifier: Modifier = Modifier, | ||
footer: (@Composable () -> Unit)?, | ||
body: @Composable () -> Unit, | ||
|
@@ -148,8 +156,15 @@ private fun LayoutScaffold( | |
} | ||
|
||
Column( | ||
modifier | ||
.also { if (inModal.not()) it.fillMaxSize() } | ||
modifier = modifier | ||
.conditional( | ||
condition = inModal.not(), | ||
ifTrue = { fillMaxSize() } | ||
) | ||
.conditional( | ||
condition = applyImePadding, | ||
ifTrue = { imePadding() } | ||
) | ||
) { | ||
if (inModal) { | ||
DragHandle( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ package com.stripe.android.common.ui | |
import android.os.Build | ||
import androidx.compose.animation.core.animateFloatAsState | ||
import androidx.compose.animation.core.tween | ||
import androidx.compose.foundation.layout.imePadding | ||
import androidx.compose.material.ExperimentalMaterialApi | ||
import androidx.compose.material.ModalBottomSheetValue.Expanded | ||
import androidx.compose.runtime.Composable | ||
|
@@ -56,7 +57,8 @@ internal fun ElementsBottomSheetLayout( | |
StripeBottomSheetLayout( | ||
state = state, | ||
layoutInfo = layoutInfo, | ||
modifier = modifier, | ||
modifier = modifier | ||
.imePadding(), | ||
Comment on lines
+60
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hoisted imePadding a bit up so that users can customize it, and adds it for the Elements bottom sheet to keep current behavior. |
||
onDismissed = onDismissed, | ||
sheetContent = content, | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I don’t think we need these comments, as the code already explains it :)