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) + } + }