From 8370efcb252bbf056cd84c9ea116ae5d09d47499 Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Thu, 17 Sep 2026 12:38:24 -0300 Subject: [PATCH] Fix the reaction overlay dismissing on lift for non-reactable messages. --- .../conversation/ReactionScrubber.kt | 10 +++++--- .../conversation/v2/ChatReactionOverlay.kt | 3 --- .../v2/ChatReactionOverlayController.kt | 5 +++- .../conversation/ReactionScrubberTest.kt | 25 ++++++++++++++++--- 4 files changed, 32 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ReactionScrubber.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/ReactionScrubber.kt index 0a7ff3930f..4fd9f92c85 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ReactionScrubber.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ReactionScrubber.kt @@ -12,8 +12,12 @@ import kotlin.math.abs * The scrub gesture behind the reaction overlay * * @param emojiCount Slots in the strip. The last opens the full picker. + * @param deadZoneSize How far a pointer may drift before a tap becomes a scrub. Deliberately not + * part of [Geometry]: it owes nothing to where the strip landed, and a message that takes no + * reactions never lays one out, so a geometry-carried value would stay zero and turn every lift + * into a dismiss. */ -class ReactionScrubber(private val emojiCount: Int) { +class ReactionScrubber(private val emojiCount: Int, private val deadZoneSize: Float) { /** * Where the strip ended up, in the coordinate space the gesture arrives in. @@ -23,7 +27,6 @@ class ReactionScrubber(private val emojiCount: Int) { * decides which emoji an x belongs to. * * @param scrubTop Top of the taller band a scrub may wander through, down to [scrubBottom]. - * @param deadZoneSize How far a pointer may drift before a tap becomes a scrub. * @param isStripVisible False for a message that takes no reactions, making every point a miss. */ data class Geometry( @@ -33,7 +36,6 @@ class ReactionScrubber(private val emojiCount: Int) { val stripBottom: Float = 0f, val scrubTop: Float = 0f, val scrubBottom: Float = 0f, - val deadZoneSize: Float = 0f, val isStripVisible: Boolean = false ) @@ -101,7 +103,7 @@ class ReactionScrubber(private val emojiCount: Int) { } if (phase == Phase.DEADZONE) { - val escapedDeadZone = abs(deadZoneX - x) > geometry.deadZoneSize || abs(deadZoneY - y) > geometry.deadZoneSize + val escapedDeadZone = abs(deadZoneX - x) > deadZoneSize || abs(deadZoneY - y) > deadZoneSize if (escapedDeadZone) { phase = Phase.SCRUB diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ChatReactionOverlay.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ChatReactionOverlay.kt index f085851d38..70a20b54cc 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ChatReactionOverlay.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ChatReactionOverlay.kt @@ -134,7 +134,6 @@ fun ChatReactionOverlay( val scrubberWidth = dimensionResource(R.dimen.reaction_scrubber_width) val scrubberHeight = dimensionResource(R.dimen.conversation_reaction_scrubber_height) val horizontalMargin = dimensionResource(R.dimen.conversation_reaction_scrub_horizontal_margin) - val deadZoneSize = dimensionResource(R.dimen.conversation_reaction_touch_deadzone_size) val scrubDistanceBelowTouch = dimensionResource(R.dimen.conversation_reaction_scrub_deadzone_distance_from_touch_bottom) val barHeightPx = remember(context) { @@ -204,7 +203,6 @@ fun ChatReactionOverlay( } val bounds = remember(selection) { StripBounds() } - val deadZonePx = with(density) { deadZoneSize.toPx() } val pushGeometry = { scrubber.geometry = ReactionScrubber.Geometry( @@ -214,7 +212,6 @@ fun ChatReactionOverlay( stripBottom = bounds.barBottom, scrubTop = bounds.barTop, scrubBottom = selection.lastSeenDownY + scrubBelowTouchPx, - deadZoneSize = deadZonePx, isStripVisible = selection.canReact ) } diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ChatReactionOverlayController.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ChatReactionOverlayController.kt index 2e1973b4f4..a8634862e9 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ChatReactionOverlayController.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ChatReactionOverlayController.kt @@ -69,7 +69,10 @@ class ChatReactionOverlayController( var originInWindow: Offset by mutableStateOf(Offset.Zero) private set - val scrubber = ReactionScrubber(REACTION_EMOJI_COUNT) + val scrubber = ReactionScrubber( + emojiCount = REACTION_EMOJI_COUNT, + deadZoneSize = context.resources.getDimensionPixelSize(R.dimen.conversation_reaction_touch_deadzone_size).toFloat() + ) val isShowing: Boolean get() = selection != null diff --git a/app/src/test/java/org/thoughtcrime/securesms/conversation/ReactionScrubberTest.kt b/app/src/test/java/org/thoughtcrime/securesms/conversation/ReactionScrubberTest.kt index 83a4856f87..d131722a06 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/conversation/ReactionScrubberTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/conversation/ReactionScrubberTest.kt @@ -22,6 +22,7 @@ class ReactionScrubberTest { companion object { private const val EMOJI_COUNT = 7 + private const val DEAD_ZONE_SIZE = 20f /** Seven 100 wide segments from 100 to 800, a short strip band and a tall scrub band. */ private val LTR = ReactionScrubber.Geometry( @@ -31,7 +32,6 @@ class ReactionScrubberTest { stripBottom = 300f, scrubTop = 200f, scrubBottom = 900f, - deadZoneSize = 20f, isStripVisible = true ) @@ -40,7 +40,7 @@ class ReactionScrubberTest { } private fun scrubber(geometry: ReactionScrubber.Geometry = LTR): ReactionScrubber { - val scrubber = ReactionScrubber(EMOJI_COUNT) + val scrubber = ReactionScrubber(EMOJI_COUNT, DEAD_ZONE_SIZE) scrubber.geometry = geometry scrubber.open() @@ -214,8 +214,27 @@ class ReactionScrubberTest { assertThat(outcome).isInstanceOf(ReactionScrubber.Outcome.Dismiss::class) } + /** + * A message that takes no reactions never lays a strip out, so the dead zone cannot come from the + * geometry. Lifting after a little drift has to leave the context menu up rather than dismiss it. + */ + @Test + fun `a jittery lift holds the dead zone even with no strip laid out`() { + val scrubber = ReactionScrubber(EMOJI_COUNT, DEAD_ZONE_SIZE) + scrubber.geometry = ReactionScrubber.Geometry() + scrubber.open() + + scrubber.apply(MotionEvent.ACTION_MOVE, 400f, 1000f) + val outcome = scrubber.apply(MotionEvent.ACTION_UP, 403f, 1002f) + + assertThat(scrubber.phase).isEqualTo(ReactionScrubber.Phase.TAP) + assertThat(outcome).isInstanceOf(ReactionScrubber.Outcome.Scrubbing::class) + assertThat(outcome.consumed).isFalse() + assertThat(scrubber.isShowing).isTrue() + } + @Test(expected = IllegalStateException::class) fun `events before open are a programming error`() { - ReactionScrubber(EMOJI_COUNT).apply(MotionEvent.ACTION_MOVE, 250f, 250f) + ReactionScrubber(EMOJI_COUNT, DEAD_ZONE_SIZE).apply(MotionEvent.ACTION_MOVE, 250f, 250f) } }