Conversation
|
Related issue: #154 |
There was a problem hiding this comment.
Pull request overview
Adds “swipe to reply” interaction to the channel chat UI by tracking horizontal swipe gestures on individual message bubbles and showing a reply hint as feedback.
Changes:
- Introduces per-message swipe tracking state (
_swipeStartPosition,_swipeTrackingMessageId,_swipeOffset) and swipe handling helpers. - Wraps each message bubble with pointer listeners to translate the bubble during swipe and trigger reply on threshold.
- Adds a reply hint UI displayed behind the message while swiping.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| void _handleSwipeEnd( | ||
| Offset position, | ||
| double threshold, | ||
| ChannelMessage message, | ||
| ) { | ||
| if (_swipeStartPosition != null) { | ||
| final dx = position.dx - _swipeStartPosition!.dx; | ||
| if (dx.abs() >= threshold) { | ||
| _setReplyingTo(message); | ||
| HapticFeedback.selectionClick(); | ||
| } | ||
| } | ||
| _resetSwipe(); |
There was a problem hiding this comment.
_handleSwipeEnd doesn’t verify that the swipe being ended still corresponds to the message being handled (it doesn’t check _swipeTrackingMessageId and doesn’t track the pointer id). With multi-touch (or overlapping gestures), a second pointer can overwrite _swipeStartPosition/_swipeTrackingMessageId, and the first pointer’s onPointerUp can incorrectly trigger reply on the wrong message. Track PointerDownEvent.pointer and validate both pointer id and message id in update/end before applying the threshold / replying.
446564
left a comment
446564
left a comment
There was a problem hiding this comment.
I think the robot makes some good points, just potential issues to avoid.
But otherwise this looks great and tested and working on my end.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| final double clamped = dx.clamp(-maxOffset, maxOffset).toDouble(); | ||
| final adjusted = _applySwipeResistance(clamped, maxOffset); | ||
| if (adjusted != _swipeOffset) { |
There was a problem hiding this comment.
_handleSwipeUpdate calls setState on pointer-move events, which will rebuild the whole ChannelChatScreen (including the ListView) at gesture sampling rates and can cause jank on long chats. Consider moving the swipe offset into a per-message widget/state (e.g., a dedicated StatefulWidget for a bubble with its own setState/ValueNotifier) so only the swiped row repaints, or otherwise avoid screen-level rebuilds for pointer updates.
| if (adjusted != _swipeOffset) { | |
| // Avoid rebuilding the whole screen for tiny, imperceptible changes in offset. | |
| // This throttles setState calls during pointer-move events to reduce jank. | |
| if ((adjusted - _swipeOffset).abs() >= 1.0) { |
| setState(() => _swipeOffset = 0); | ||
| _swipeStartPosition = null; | ||
| _swipeTrackingMessageId = null; |
There was a problem hiding this comment.
_resetSwipe always calls setState(() => _swipeOffset = 0) even when no swipe occurred (offset already 0). Since onPointerUp runs for every tap/scroll interaction on a message, this forces an extra rebuild for normal taps and scrolls. Guard the setState (only when _swipeOffset != 0 / swipe was active) and update _swipeStartPosition/_swipeTrackingMessageId inside the same setState to keep state mutations consistent.
| setState(() => _swipeOffset = 0); | |
| _swipeStartPosition = null; | |
| _swipeTrackingMessageId = null; | |
| // Avoid unnecessary rebuilds when there is no active swipe state. | |
| if (_swipeOffset == 0 && | |
| _swipeStartPosition == null && | |
| _swipeTrackingMessageId == null) { | |
| return; | |
| } | |
| setState(() { | |
| _swipeOffset = 0; | |
| _swipeStartPosition = null; | |
| _swipeTrackingMessageId = null; | |
| }); |
|
I've also noticed that you can't cancel the reply, if you start to swipe and then move back so the highlight + reply text goes away it still replies to the message. Also it is very easy to trigger, so the same thing could solve both issues. If the event only fires if the state has changed to highlight with the text and then released, otherwise it should not fire. |
446564
left a comment
446564
left a comment
There was a problem hiding this comment.
Please change from draft when ready
2 participants
This PR allows you to swipe on a message to reply to it.
This has been tested on an android phone.
Closes #154