mirror of
https://github.com/signalapp/Signal-Android.git
synced 2026-09-19 16:24:41 +01:00
Fix the reaction overlay dismissing on lift for non-reactable messages.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
)
|
||||
}
|
||||
|
||||
+4
-1
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user