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

[FC] Debounce clicks on buttons and clickable surfaces #6836

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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## XX.XX.XX - 2023-XX-XX

### Financial Connections
* [FIXED][6836](https://github.com/stripe/stripe-android/pull/6836) Prevents double navigation when tapping too quickly.
*
## 20.25.5 - 2023-06-05

### PaymentSheet
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.stripe.android.financialconnections.features.common

import androidx.compose.foundation.border
import androidx.compose.foundation.clickable
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
Expand All @@ -24,6 +23,7 @@ import androidx.compose.ui.text.style.TextOverflow
import androidx.compose.ui.unit.dp
import androidx.core.os.ConfigurationCompat.getLocales
import com.stripe.android.financialconnections.model.PartnerAccount
import com.stripe.android.financialconnections.ui.components.clickableSingle
import com.stripe.android.financialconnections.ui.theme.FinancialConnectionsTheme
import com.stripe.android.uicore.format.CurrencyFormatter
import com.stripe.android.uicore.text.MiddleEllipsisText
Expand Down Expand Up @@ -52,7 +52,7 @@ internal fun AccountItem(
},
shape = shape
)
.clickable(enabled = account.allowSelection) { onAccountClicked(account) }
.clickableSingle(enabled = account.allowSelection) { onAccountClicked(account) }
.padding(vertical = verticalPadding, horizontal = 16.dp)
) {
Row(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ import com.stripe.android.financialconnections.ui.LocalImageLoader
import com.stripe.android.financialconnections.ui.components.FinancialConnectionsOutlinedTextField
import com.stripe.android.financialconnections.ui.components.FinancialConnectionsScaffold
import com.stripe.android.financialconnections.ui.components.FinancialConnectionsTopAppBar
import com.stripe.android.financialconnections.ui.components.clickableSingle
import com.stripe.android.financialconnections.ui.theme.Brand100
import com.stripe.android.financialconnections.ui.theme.FinancialConnectionsTheme
import com.stripe.android.uicore.image.StripeImage
Expand Down Expand Up @@ -397,7 +398,7 @@ private fun InstitutionResultTile(
verticalAlignment = Alignment.CenterVertically,
modifier = Modifier
.fillMaxSize()
.clickable { onInstitutionSelected(institution) }
.clickableSingle { onInstitutionSelected(institution) }
.padding(
vertical = 8.dp,
horizontal = 24.dp
Expand Down Expand Up @@ -472,7 +473,7 @@ private fun FeaturedInstitutionsGrid(
color = FinancialConnectionsTheme.colors.borderDefault,
shape = RoundedCornerShape(6.dp)
)
.clickable(
.clickableSingle(
interactionSource = remember { MutableInteractionSource() },
indication = rememberRipple(
color = FinancialConnectionsTheme.colors.textSecondary
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import androidx.compose.foundation.Image
import androidx.compose.foundation.ScrollState
import androidx.compose.foundation.background
import androidx.compose.foundation.border
import androidx.compose.foundation.clickable
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Row
Expand Down Expand Up @@ -61,6 +60,7 @@ import com.stripe.android.financialconnections.ui.components.AnnotatedText
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.components.clickableSingle
import com.stripe.android.financialconnections.ui.components.elevation
import com.stripe.android.financialconnections.ui.theme.FinancialConnectionsTheme
import com.stripe.android.uicore.image.StripeImage
Expand Down Expand Up @@ -235,7 +235,7 @@ private fun SelectNewAccount(
color = FinancialConnectionsTheme.colors.borderDefault,
shape = shape
)
.clickable { onClick() }
.clickableSingle { onClick() }
.padding(16.dp)
) {
Row(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package com.stripe.android.financialconnections.features.networkinglinkloginwarm
import androidx.annotation.RestrictTo
import androidx.compose.foundation.ScrollState
import androidx.compose.foundation.border
import androidx.compose.foundation.clickable
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Row
Expand Down Expand Up @@ -50,6 +49,7 @@ import com.stripe.android.financialconnections.ui.components.AnnotatedText
import com.stripe.android.financialconnections.ui.components.FinancialConnectionsScaffold
import com.stripe.android.financialconnections.ui.components.FinancialConnectionsTopAppBar
import com.stripe.android.financialconnections.ui.components.StringAnnotation
import com.stripe.android.financialconnections.ui.components.clickableSingle
import com.stripe.android.financialconnections.ui.theme.FinancialConnectionsTheme

@Composable
Expand Down Expand Up @@ -210,7 +210,7 @@ internal fun ExistingEmailSection(
.fillMaxWidth()
.semantics { testTagsAsResourceId = true }
.testTag("existing_email-button")
.clickable { onContinueClick() }
.clickableSingle { onContinueClick() }
.clip(RoundedCornerShape(8.dp))
.border(
width = 1.dp,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,9 @@ internal class FinancialConnectionsSheetNativeActivity : AppCompatActivity(), Ma
private fun NavigationEffect(navController: NavHostController) {
LaunchedEffect(navigationManager.commands) {
navigationManager.commands.collect { command ->
if (command.destination.isNotEmpty()) {
val from = navController.currentDestination?.route
if (command.destination.isNotEmpty() && command.destination != from) {
logger.debug("Navigating from $from to ${command.destination}")
navController.navigate(command.destination) {
launchSingleTop = true
popUpIfNotBackwardsNavigable(navController)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import androidx.compose.material.ripple.RippleAlpha
import androidx.compose.material.ripple.RippleTheme
import androidx.compose.runtime.Composable
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.tooling.preview.Preview
Expand All @@ -49,9 +50,14 @@ internal fun FinancialConnectionsButton(
loading: Boolean = false,
content: @Composable (RowScope.() -> Unit)
) {
val multipleEventsCutter = remember { MultipleEventsCutter.get() }
CompositionLocalProvider(LocalRippleTheme provides type.rippleTheme()) {
Button(
onClick = { if (loading.not()) onClick() },
onClick = {
multipleEventsCutter.processEvent {
if (loading.not()) onClick()
}
},
modifier = modifier,
elevation = ButtonDefaults.elevation(
defaultElevation = 0.dp,
Expand Down Expand Up @@ -198,40 +204,40 @@ internal fun FinancialConnectionsButtonPreview() {
verticalArrangement = Arrangement.SpaceEvenly
) {
FinancialConnectionsButton(
onClick = { },
modifier = Modifier.fillMaxWidth(),
loading = false,
onClick = { }
loading = false
) {
Text(text = "Primary")
}
FinancialConnectionsButton(
onClick = { },
modifier = Modifier.fillMaxWidth(),
loading = true,
onClick = { }
loading = true
) {
Text(text = "Primary - loading")
}
FinancialConnectionsButton(
onClick = { },
modifier = Modifier.fillMaxWidth(),
enabled = false,
onClick = { }
enabled = false
) {
Text(text = "Primary - disabled")
}
FinancialConnectionsButton(
onClick = { },
modifier = Modifier.fillMaxWidth(),
type = Type.Secondary,
loading = false,
onClick = { }
loading = false
) {
Text(text = "Secondary")
}
FinancialConnectionsButton(
onClick = { },
modifier = Modifier.fillMaxWidth(),
type = Type.Secondary,
loading = false,
enabled = false,
onClick = { }
loading = false
) {
Text(text = "Secondary disabled")
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
package com.stripe.android.financialconnections.ui.components

import androidx.compose.foundation.Indication
import androidx.compose.foundation.LocalIndication
import androidx.compose.foundation.clickable
import androidx.compose.foundation.interaction.MutableInteractionSource
import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier
import androidx.compose.ui.composed
import androidx.compose.ui.platform.debugInspectorInfo
import androidx.compose.ui.semantics.Role

/**
* A wrapper around [clickable] that prevents multiple clicks from being registered within a
* short time frame.
*/
internal interface MultipleEventsCutter {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we call this MultipleEventsDebouncer? I haven't heard of the term Cutter before.

fun processEvent(event: () -> Unit)

companion object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very interesting syntax!

}

internal fun MultipleEventsCutter.Companion.get(): MultipleEventsCutter =
MultipleEventsCutterImpl()

private class MultipleEventsCutterImpl : MultipleEventsCutter {
private val now: Long
get() = System.currentTimeMillis()

private var lastEventTimeMs: Long = 0

override fun processEvent(event: () -> Unit) {
if (now - lastEventTimeMs >= DEBOUNCE_MS) {
event.invoke()
}
lastEventTimeMs = now
}

companion object {
private const val DEBOUNCE_MS = 500L
}
}

/**
* A wrapper around [clickable] that prevents multiple clicks from being registered within a
* short time frame.
*/
internal fun Modifier.clickableSingle(
enabled: Boolean = true,
onClickLabel: String? = null,
role: Role? = null,
onClick: () -> Unit

) = composed(
inspectorInfo = debugInspectorInfo {
name = "clickable"
properties["enabled"] = enabled
properties["onClickLabel"] = onClickLabel
properties["role"] = role
properties["onClick"] = onClick
},

factory = {
val multipleEventsCutter = remember { MultipleEventsCutter.get() }
Modifier.clickable(
interactionSource = remember { MutableInteractionSource() },
indication = LocalIndication.current,
enabled = enabled,
onClickLabel = onClickLabel,
onClick = { multipleEventsCutter.processEvent { onClick() } },
role = role,
)
}
)

/**
* A wrapper around [clickable] that prevents multiple clicks from being registered within a
* short time frame.
*/
internal fun Modifier.clickableSingle(
interactionSource: MutableInteractionSource,
indication: Indication?,
enabled: Boolean = true,
onClickLabel: String? = null,
role: Role? = null,
onClick: () -> Unit

) = composed(
inspectorInfo = debugInspectorInfo {
name = "clickable"
properties["enabled"] = enabled
properties["onClickLabel"] = onClickLabel
properties["role"] = role
properties["onClick"] = onClick
},

factory = {
val multipleEventsCutter = remember { MultipleEventsCutter.get() }
Modifier.clickable(
interactionSource = interactionSource,
indication = indication,
enabled = enabled,
onClickLabel = onClickLabel,
onClick = { multipleEventsCutter.processEvent { onClick() } },
role = role,
)
}
)