From 29c97b1cb71a88b4b43564822b9ce10ef25f23e0 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 23 Sep 2026 15:13:39 -0400 Subject: [PATCH] fix(chat): widen the back button's tap area in the chat top bar In a chat, back is a 40dp circle at 15-55dp and the avatar, which opens the profile, starts at 70dp. The strip between them and the corners around the circle did nothing, so a thumb that missed the arrow either did nothing or, reaching further right, opened the profile. Add Modifier.expandedTouchTarget(outset, onTap) to :ui:core. It grows an element's tap area by an outset without changing its layout: an inner layout grows by the outset so the tap handler covers it, and an outer layout reports the original size to the parent. The control's own click still takes taps inside it and consumes the up, so a tap there fires once. Compose 1.12's PointerInputModifierNode.touchBoundsExpansion looks like the tool for this, but NodeCoordinator.isInExpandedTouchBounds only honours it for stylus and eraser pointers, so a finger never reaches it. AppBarDefaults.leadingControlTouchOutset() sizes the outset for a leading control: to the bar's leading edge, its full height, and across the gap up to where the title starts. The chat bar applies it to back. The avatar keeps its own bounds, so a tap on the avatar itself still opens the profile. --- .../internal/screens/components/ChatTopBar.kt | 12 +- .../internal/ConversationTitleBarTest.kt | 47 ++++++- .../ui/components/CircularIconButton.kt | 7 +- .../com/getcode/ui/components/TitleBar.kt | 43 ++++++- .../ui/core/ExpandedTouchTargetTest.kt | 117 ++++++++++++++++++ .../getcode/ui/core/ExpandedTouchTarget.kt | 57 +++++++++ 6 files changed, 276 insertions(+), 7 deletions(-) create mode 100644 ui/components/src/test/kotlin/com/getcode/ui/core/ExpandedTouchTargetTest.kt create mode 100644 ui/core/src/main/kotlin/com/getcode/ui/core/ExpandedTouchTarget.kt diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt index ba7d530d91..e892c621e3 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt @@ -62,6 +62,7 @@ import com.getcode.theme.extraLarge import com.getcode.ui.components.AppBarDefaults import com.getcode.ui.components.AppBarWithTitle import com.getcode.ui.components.CircularIconButton +import com.getcode.ui.core.expandedTouchTarget import com.getcode.ui.core.measured import com.getcode.ui.core.noRippleClickable import com.getcode.ui.utils.KeyboardController @@ -161,7 +162,16 @@ private fun ConversationTitleBar( ) { AppBarWithTitle( leftIcon = { - AppBarDefaults.UpNavigation { navigator.pop() } + // The avatar sits just past back and opens the profile, so a thumb that missed the + // arrow landed in the dead strip before it or on the avatar. Everything before the + // avatar goes back instead. + AppBarDefaults.UpNavigation( + modifier = Modifier.expandedTouchTarget( + AppBarDefaults.leadingControlTouchOutset(), + onTap = { navigator.pop() }, + ), + onClick = { navigator.pop() }, + ) }, title = { Row( diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ConversationTitleBarTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ConversationTitleBarTest.kt index 9af27f5dac..a1d4de7119 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ConversationTitleBarTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ConversationTitleBarTest.kt @@ -5,10 +5,15 @@ import androidx.compose.ui.semantics.Role import androidx.compose.ui.semantics.SemanticsProperties import androidx.compose.ui.test.SemanticsMatcher import androidx.compose.ui.test.assert +import androidx.compose.ui.test.click import androidx.compose.ui.test.hasClickAction import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.geometry.Offset import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.onRoot import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performTouchInput +import androidx.compose.ui.unit.dp import com.flipcash.app.messenger.internal.screens.components.ChatTopBar import com.flipcash.app.theme.FlipcashPreview import com.flipcash.services.models.chat.ChatId @@ -16,6 +21,7 @@ import com.flipcash.shared.chat.models.ChatAction import com.flipcash.services.models.chat.ChatType import com.getcode.navigation.core.CodeNavigator import io.mockk.mockk +import io.mockk.verify import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -45,11 +51,13 @@ class ConversationTitleBarTest { isMember = true, ) + private val navigator = mockk(relaxed = true) + private fun show(onAction: (ChatAction) -> Unit = {}) { composeTestRule.setContent { FlipcashPreview { ChatTopBar( - navigator = mockk(relaxed = true), + navigator = navigator, state = ChatViewModel.State(chatType = ChatType.GROUP, subject = group), onBarHeightChange = {}, chatActionHandler = onAction, @@ -77,4 +85,41 @@ class ConversationTitleBarTest { assertEquals(listOf(ChatAction.ViewProfile), actions.toList()) } + + /** + * The back arrow is a 40dp circle and the avatar starts 15dp past it. A tap that lands in that + * gap is aimed at back — nothing else is there — so it goes back rather than doing nothing, and + * a thumb reaching for back has the whole strip left of the avatar to land in. + */ + @Test + fun `tapping between the back arrow and the avatar goes back`() { + val actions = mutableListOf() + show(onAction = { actions += it }) + + // The arrow spans 15..55dp and the avatar starts at 70dp; 13..53dp is the pair's height. + composeTestRule.onRoot().performTouchInput { click(Offset(62.dp.toPx(), 33.dp.toPx())) } + + verify(exactly = 1) { navigator.pop() } + assertEquals(emptyList(), actions.toList()) + } + + @Test + fun `tapping the corner outside the back circle goes back`() { + show() + + // Inside the arrow's square but outside its 40dp circle, which clips its own click. + composeTestRule.onRoot().performTouchInput { click(Offset(17.dp.toPx(), 15.dp.toPx())) } + + verify(exactly = 1) { navigator.pop() } + } + + /** The arrow's own click and the expanded target around it must not both fire. */ + @Test + fun `tapping the back arrow goes back once`() { + show() + + composeTestRule.onRoot().performTouchInput { click(Offset(35.dp.toPx(), 33.dp.toPx())) } + + verify(exactly = 1) { navigator.pop() } + } } diff --git a/ui/components/src/main/kotlin/com/getcode/ui/components/CircularIconButton.kt b/ui/components/src/main/kotlin/com/getcode/ui/components/CircularIconButton.kt index e407244c31..8de4171f76 100644 --- a/ui/components/src/main/kotlin/com/getcode/ui/components/CircularIconButton.kt +++ b/ui/components/src/main/kotlin/com/getcode/ui/components/CircularIconButton.kt @@ -23,8 +23,13 @@ import dev.chrisbanes.haze.blur.HazeBlurStyle import dev.chrisbanes.haze.blur.HazeColorEffect import dev.chrisbanes.haze.blur.hazeBlur +object CircularIconButtonDefaults { + val ButtonSize: Dp + @Composable get() = CodeTheme.dimens.staticGrid.x8 +} + private val ButtonSize: Dp - @Composable get() = CodeTheme.dimens.staticGrid.x8 + @Composable get() = CircularIconButtonDefaults.ButtonSize private val ButtonBackground = Color.White.copy(alpha = 0.1f) diff --git a/ui/components/src/main/kotlin/com/getcode/ui/components/TitleBar.kt b/ui/components/src/main/kotlin/com/getcode/ui/components/TitleBar.kt index 991ecb0a58..1d123d761f 100644 --- a/ui/components/src/main/kotlin/com/getcode/ui/components/TitleBar.kt +++ b/ui/components/src/main/kotlin/com/getcode/ui/components/TitleBar.kt @@ -7,6 +7,8 @@ import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.RowScope import androidx.compose.foundation.layout.WindowInsets +import androidx.compose.foundation.layout.calculateEndPadding +import androidx.compose.foundation.layout.calculateStartPadding import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.requiredSize @@ -27,10 +29,12 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.layout.SubcomposeLayout +import androidx.compose.ui.platform.LocalLayoutDirection import androidx.compose.ui.res.painterResource import androidx.compose.ui.text.TextStyle import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import com.getcode.navigation.core.LocalCodeNavigator import dev.chrisbanes.haze.HazeState @@ -52,6 +56,37 @@ object AppBarDefaults { internal val IconSize = 20.dp + // The bar's own height and the padding its leading and trailing slots put around a control. + // Named because [leadingControlTouchOutset] has to agree with [TopAppBarBase] on both. + internal val BarHeight = 56.dp + internal val SlotPadding = 5.dp + + /** + * How far a leading control's touch target can grow past it, for a start-aligned title: to the bar's + * leading edge, top and bottom, and across the gap up to where the title starts. For use with + * `Modifier.expandedTouchTarget` on a control whose title is tappable too, so a tap that misses + * the control lands on it rather than in dead space next to the title. + */ + @Composable + fun leadingControlTouchOutset( + contentPadding: PaddingValues = ContentPadding, + controlSize: Dp = CircularIconButtonDefaults.ButtonSize, + ): PaddingValues { + val direction = LocalLayoutDirection.current + val start = contentPadding.calculateStartPadding(direction) + val end = contentPadding.calculateEndPadding(direction) + // Mirrors TopAppBarBase: the slot is centred in the bar and then pushed down by the top + // padding, and the title starts one slot plus both horizontal paddings in. + val controlTop = (BarHeight - (controlSize + SlotPadding * 2)) / 2 + + contentPadding.calculateTopPadding() + SlotPadding + return PaddingValues( + start = start + SlotPadding, + top = controlTop, + end = SlotPadding + end, + bottom = BarHeight - controlTop - controlSize, + ) + } + @Composable fun UpNavigation(modifier: Modifier = Modifier, hazeState: HazeState? = null, onClick: () -> Unit) { CircularIconButton( @@ -303,18 +338,18 @@ private fun TopAppBarBase( val horizontal = contentPadding.calculateHorizontalPadding() val emptyLeftSlot = @Composable { - Box(modifier = Modifier.padding(5.dp)) { + Box(modifier = Modifier.padding(AppBarDefaults.SlotPadding)) { AppBarDefaults.UpNavigation { } } } val leftSlot = @Composable { - Box(modifier = Modifier.padding(5.dp)) { + Box(modifier = Modifier.padding(AppBarDefaults.SlotPadding)) { leftIcon() } } val rightSlot = @Composable { - Box(modifier = Modifier.padding(5.dp)) { + Box(modifier = Modifier.padding(AppBarDefaults.SlotPadding)) { EndActionSlotHolder { rightContents() } @@ -330,7 +365,7 @@ private fun TopAppBarBase( Modifier.windowInsetsPadding(WindowInsets.statusBars) } else Modifier ) - .height(56.dp), + .height(AppBarDefaults.BarHeight), ) { constraints -> // A centered title needs a phantom leading slot (a back button's worth of width) so it // stays optically centered against the end actions even when there's no real leading diff --git a/ui/components/src/test/kotlin/com/getcode/ui/core/ExpandedTouchTargetTest.kt b/ui/components/src/test/kotlin/com/getcode/ui/core/ExpandedTouchTargetTest.kt new file mode 100644 index 0000000000..b8c4d294ce --- /dev/null +++ b/ui/components/src/test/kotlin/com/getcode/ui/core/ExpandedTouchTargetTest.kt @@ -0,0 +1,117 @@ +package com.getcode.ui.core + +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.PaddingValues +import androidx.compose.foundation.layout.offset +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.shape.CircleShape +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip +import androidx.compose.ui.geometry.Offset +import androidx.compose.ui.test.click +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onRoot +import androidx.compose.ui.test.performTouchInput +import androidx.compose.ui.layout.onSizeChanged +import androidx.compose.ui.unit.IntSize +import androidx.compose.ui.unit.dp +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import kotlin.test.assertEquals + +@RunWith(RobolectricTestRunner::class) +class ExpandedTouchTargetTest { + + @get:Rule + val composeTestRule = createComposeRule() + + private var taps = 0 + private var clicks = 0 + + // A 40dp circle spanning 20..60dp, inside a 70dp parent at the root's corner, grown 15dp on + // every side to 5..75dp. Compose treats a clickable smaller than 48dp as 48dp for hit testing, so the + // circle's own click also takes taps up to 4dp outside it (16..64dp). + private fun show() { + composeTestRule.setContent { + Box(Modifier.size(100.dp)) { + Box(Modifier.padding(end = 30.dp, bottom = 30.dp)) { + Box( + Modifier + .offset(20.dp, 20.dp) + .expandedTouchTarget(PaddingValues(15.dp)) { taps++ } + .size(40.dp) + .clip(CircleShape) + .clickable { clicks++ } + ) + } + } + } + } + + private fun tapAt(x: Int, y: Int) { + composeTestRule.onRoot().performTouchInput { click(Offset(x.dp.toPx(), y.dp.toPx())) } + composeTestRule.waitForIdle() + } + + @Test + fun `a tap on the control fires its own click only`() { + show() + tapAt(40, 40) + assertEquals(0 to 1, taps to clicks) + } + + @Test + fun `a tap just outside the control goes to its own click, once`() { + show() + tapAt(18, 18) + assertEquals(0 to 1, taps to clicks) + } + + @Test + fun `a tap in the expansion past the control's own reach fires the expansion`() { + show() + tapAt(8, 8) + assertEquals(1 to 0, taps to clicks) + } + + @Test + fun `a tap beside the control fires the expansion`() { + show() + tapAt(67, 40) + assertEquals(1 to 0, taps to clicks) + } + + @Test + fun `the expansion reaches past the parent's bounds`() { + show() + tapAt(73, 40) + assertEquals(1 to 0, taps to clicks) + } + + @Test + fun `a tap past the expansion does nothing`() { + show() + tapAt(78, 40) + assertEquals(0 to 0, taps to clicks) + } + + /** The outer layout has to hand the parent the control's own size, or siblings would move. */ + @Test + fun `the expansion leaves the control's layout alone`() { + var size = IntSize.Zero + composeTestRule.setContent { + Box( + Modifier + .onSizeChanged { size = it } + .expandedTouchTarget(PaddingValues(15.dp)) { } + .size(40.dp) + ) + } + composeTestRule.waitForIdle() + with(composeTestRule.density) { assertEquals(IntSize(40.dp.roundToPx(), 40.dp.roundToPx()), size) } + } +} diff --git a/ui/core/src/main/kotlin/com/getcode/ui/core/ExpandedTouchTarget.kt b/ui/core/src/main/kotlin/com/getcode/ui/core/ExpandedTouchTarget.kt new file mode 100644 index 0000000000..ae1abe12b4 --- /dev/null +++ b/ui/core/src/main/kotlin/com/getcode/ui/core/ExpandedTouchTarget.kt @@ -0,0 +1,57 @@ +package com.getcode.ui.core + +import androidx.compose.foundation.gestures.detectTapGestures +import androidx.compose.foundation.layout.PaddingValues +import androidx.compose.foundation.layout.calculateEndPadding +import androidx.compose.foundation.layout.calculateStartPadding +import androidx.compose.ui.Modifier +import androidx.compose.ui.input.pointer.pointerInput +import androidx.compose.ui.layout.layout +import androidx.compose.ui.unit.constrainHeight +import androidx.compose.ui.unit.constrainWidth +import androidx.compose.ui.unit.offset + +/** + * Answers taps that land up to [outset] outside this element with [onTap], without changing its + * layout. + * + * For a control drawn smaller than the area a thumb aims at, such as a 40dp circle with dead space + * around it. Put it at the front of the control's modifier chain, ahead of any clip, because a clip + * also clips hit testing for everything after it. The control's own click still handles taps + * inside its bounds: it sees the up event first and consumes it, so a tap there fires once. It + * adds no semantics, because the control already announces the action. + * + * The outset only wins where nothing drawn above it takes the tap. Size it to stop where a + * neighbouring target starts: a neighbour that isn't clickable lets taps fall through to this. + * A clickable smaller than 48dp also takes taps up to that minimum itself, and those near hits + * win over the outset, which is harmless when both do the same thing. + */ +fun Modifier.expandedTouchTarget(outset: PaddingValues, onTap: () -> Unit): Modifier = + // Compose hit-tests a pointer handler against the size of the layout it wraps. The inner layout + // grows by the outset, so the tap handler covers it; the outer one reports the original size + // and pulls the grown box back by the outset, so the parent lays this out as before. + // Compose's own touchBoundsExpansion can't do this: it applies to stylus pointers only. + layout { measurable, constraints -> + val start = outset.calculateStartPadding(layoutDirection).roundToPx() + val top = outset.calculateTopPadding().roundToPx() + val horizontal = start + outset.calculateEndPadding(layoutDirection).roundToPx() + val vertical = top + outset.calculateBottomPadding().roundToPx() + val grown = measurable.measure(constraints.offset(horizontal, vertical)) + layout( + constraints.constrainWidth(grown.width - horizontal), + constraints.constrainHeight(grown.height - vertical), + ) { + grown.placeRelative(-start, -top) + } + } + .pointerInput(onTap) { detectTapGestures(onTap = { onTap() }) } + .layout { measurable, constraints -> + val start = outset.calculateStartPadding(layoutDirection).roundToPx() + val top = outset.calculateTopPadding().roundToPx() + val horizontal = start + outset.calculateEndPadding(layoutDirection).roundToPx() + val vertical = top + outset.calculateBottomPadding().roundToPx() + val control = measurable.measure(constraints.offset(-horizontal, -vertical)) + layout(control.width + horizontal, control.height + vertical) { + control.placeRelative(start, top) + } + }