mirror of
https://github.com/block/buzz.git
synced 2026-11-11 11:51:13 +00:00
fix(mobile): keep a sent thread reply above the composer (#9026)
Fixes two thread symptoms that come from the same state: after sending a reply, the composer clears but the new reply sits hidden under the composer (or out of view entirely) until you leave and return, and the **Latest** button stays visible even though the list looks like it is at the bottom. ### Cause 1. A sent reply grows into place over its 500 ms entrance transition (`LocalSendRow`). The thread's one-shot tail correction can finish while the row is still growing. Once it has finished, nothing moves the list as the reply keeps growing, so the list stops short of the real end (about 40 pt on an iPhone 17 Pro simulator) and the reply ends up behind the composer. Because the list isn't actually at its end, **Latest** shows. It is visible even with the composer focused when an earlier drag detached tail-follow. 2. A thread opened at a specific reply (from Inbox, a push notification or a message link) skipped the new-reply follow logic for its whole lifetime, not just for the replies it opened with. If the reader scrolled at all before replying, their own sent reply was never brought into view: it landed below the viewport, and a quick swipe couldn't reach it before the next layout. ### Fix - While the thread follows its tail, a `ScrollMetricsNotification` listener keeps a position that was at its previous end on the new end whenever the extent grows. It only acts when the last reply is laid out, the reader isn't dragging or opted out, and no navigation to the tail is in progress, so programmatic jumps away from the tail are left alone. This also covers other growth at the tail, such as media or typing rows. - A targeted entry now absorbs the replies it hydrates with until its target jump has revealed the list. Hydration can come in two steps (the route's snapshot of the target, then the relay's full thread). Absorbing only the first let the second, when it held the reader's own earlier replies, schedule a tail correction that cancelled the one-shot target jump and left the list hidden behind its loading placeholder. Later arrivals use the ordinary rules: the reader's own reply comes into view, and other people's replies still don't move a reader who is away from the tail. ### Before and after Same flow on an iPhone 17 Pro simulator, in a synthetic QA channel thread: scroll the list away from the bottom, focus the composer, send a long reply that wraps onto many lines. Before (main) the sent reply ends with its bottom edge under the composer; after (this branch) it ends clear above it. Before (main): https://github.com/user-attachments/assets/5101b63d-e2df-46d1-bfd1-49038330ddf5 After (this branch): https://github.com/user-attachments/assets/1df81ad5-bb6b-4df6-b4e3-5c37fd8aab67 ### Validation - Test `a sent reply stays above the composer as it grows in` now runs for a plain entry, a targeted entry (`initialMessageId`), and a targeted entry whose relay hydration is delayed and includes the reader's own replies. The targeted row opens at an older reply, checks it is visible, drags a little, focuses the composer with the keyboard up, sends a multi-line reply, and checks that the reply ends above the composer and **Latest** is hidden. Each row fails without its fix (growth: reply bottom at 504, composer top at 402; targeted: the reply is never followed; delayed hydration: the target never becomes hit-testable because the list stays hidden). - Full `flutter analyze` and mobile test suite pass. - Before/after capture on an iPhone 17 Pro simulator (iOS 27, debug builds of this branch head and of current main), in a synthetic QA channel thread with enough replies to scroll. Flow: scroll away from the bottom, focus the composer, send a long wrapping reply. Over 7 runs on main, the reply's bottom ended 3 to 25 pt under the composer in 5; over 6 runs on this branch, it ended 17 pt above the composer every time. Limitation: debug simulator builds only, and the shortfall on main varies run to run. - Automated full-app run on an iPhone 17 Pro simulator (iOS 27) against the Remote Agents Pilot community. In a test DM thread, I sent short and wrapping replies with no prior scroll, after dragging up and returning, and after the follow-up narrowing. Each reply ended fully above the composer, and **Latest** stayed hidden at the bottom. I also dragged up with the keyboard dismissed and confirmed the list stays at the tail with no stray **Latest**. For the targeted entry, I opened a thread from a message link, scrolled, and sent replies: without the targeted-entry fix they stayed out of view under the keyboard; with it hot-reloaded they were followed (the combined build is covered by the widget test rows above). I then reopened that link right after an app restart, which hydrates in two steps: before the hydration fix the thread showed only its root with the replies hidden, and with it the list revealed at the target and a sent reply came into view. Human testing is waived per the owner's standing exception. Limitation: this run did not cover physical devices or media replies. The simulator recording was taken in an internal community, so it is shared internally rather than embedded here. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Tom Brow <tomb@squareup.com> Co-authored-by: Tom Brow <tomb@squareup.com> Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@localhost> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz>
This commit is contained in:
Tom Brow
@@ -16,6 +16,35 @@ Widget _trackActiveThreadScrollPosition(
|
||||
},
|
||||
);
|
||||
|
||||
/// Keeps a tail-following thread at its end while content there grows.
|
||||
///
|
||||
/// A sent reply's entrance transition grows its row over several frames, and
|
||||
/// media or typing rows can grow too. The one-shot tail correction can finish
|
||||
/// before that growth does, leaving the new content under the composer. Every
|
||||
/// growth reports new scroll metrics, so move a position that was at its old
|
||||
/// end to the new one while the reader follows the tail.
|
||||
void _followThreadTailOnMetricsChange(
|
||||
ScrollMetricsNotification notification, {
|
||||
required ScrollPosition? activePosition,
|
||||
required Expando<double> previousMaxExtents,
|
||||
required bool Function() followsTail,
|
||||
}) {
|
||||
final position = Scrollable.maybeOf(notification.context)?.position;
|
||||
if (position == null ||
|
||||
!identical(position, activePosition) ||
|
||||
!position.hasContentDimensions) {
|
||||
return;
|
||||
}
|
||||
final maxExtent = position.maxScrollExtent;
|
||||
final previousMaxExtent = previousMaxExtents[position];
|
||||
previousMaxExtents[position] = maxExtent;
|
||||
if (previousMaxExtent == null || maxExtent <= previousMaxExtent) return;
|
||||
final wasAtEnd =
|
||||
position.pixels >= previousMaxExtent - _threadTailScrollTolerance;
|
||||
if (!wasAtEnd || !followsTail()) return;
|
||||
position.jumpTo(maxExtent);
|
||||
}
|
||||
|
||||
bool _jumpActiveThreadScrollToTail(
|
||||
ObjectRef<ScrollPosition?> activePosition,
|
||||
bool Function()? testOverride,
|
||||
|
||||
@@ -291,6 +291,7 @@ class ThreadDetailPage extends HookConsumerWidget {
|
||||
final tailCorrectionInProgress = useRef(false);
|
||||
final tailCorrectionGeneration = useRef(0);
|
||||
final activeThreadScrollPosition = useRef<ScrollPosition?>(null);
|
||||
final threadMaxScrollExtents = useMemoized(Expando<double>.new);
|
||||
final composerHasFocus = useListenable(composerFocusNode).hasFocus;
|
||||
final viewportHeight = useListenable(listViewport.height).value;
|
||||
final previousViewportHeight = useRef(viewportHeight);
|
||||
@@ -645,7 +646,12 @@ class ThreadDetailPage extends HookConsumerWidget {
|
||||
headHeight.value == null) {
|
||||
return null;
|
||||
}
|
||||
if (initialMessageId != null) {
|
||||
// A targeted entry starts at its target, not the tail. Absorb the
|
||||
// replies it hydrates with (a provisional list, then the relay's) until
|
||||
// the target jump reveals the list; a tail correction scheduled earlier
|
||||
// would cancel that one-shot jump and leave the list hidden. Later
|
||||
// arrivals use the ordinary rules below, so a sent reply comes into view.
|
||||
if (initialMessageId != null && !initialViewportReady.value) {
|
||||
initialTailSettle.abandon();
|
||||
previousReplyCount.value = replies.length;
|
||||
return null;
|
||||
@@ -953,76 +959,102 @@ class ThreadDetailPage extends HookConsumerWidget {
|
||||
Column(
|
||||
children: [
|
||||
Expanded(
|
||||
child: _ThreadMessageList(
|
||||
dateScrollActivity: dateScrollActivity,
|
||||
headHeight: headHeight.value ?? 0,
|
||||
onHeadHeightChanged: (height) {
|
||||
if (context.mounted) headHeight.value = height;
|
||||
child: NotificationListener<ScrollMetricsNotification>(
|
||||
onNotification: (notification) {
|
||||
if (notification.depth == 0) {
|
||||
_followThreadTailOnMetricsChange(
|
||||
notification,
|
||||
activePosition: activeThreadScrollPosition.value,
|
||||
previousMaxExtents: threadMaxScrollExtents,
|
||||
followsTail: () =>
|
||||
followsThreadTail.value &&
|
||||
initialTailSettle.isComplete &&
|
||||
!tailIntent.isDragging &&
|
||||
!userOptedOutOfTailFollow.value &&
|
||||
!isNavigatingToThreadTail.value &&
|
||||
// A jump away from the tail can briefly leave the
|
||||
// position at an estimated end; only content
|
||||
// growing at the real tail should be followed.
|
||||
itemPositionsListener.itemPositions.value.any(
|
||||
(item) =>
|
||||
item.index ==
|
||||
_threadTailIndex(replies.length),
|
||||
),
|
||||
);
|
||||
}
|
||||
return false;
|
||||
},
|
||||
viewport: listViewport,
|
||||
onUserScrollStart: () {
|
||||
hidesLatestForInitialTailSettle.value = false;
|
||||
hidesLatestForComposerTailCorrection.value = false;
|
||||
initialTailSettle.abandon();
|
||||
initialViewportReady.value = true;
|
||||
tailCorrectionInProgress.value = false;
|
||||
isNavigatingToThreadTail.value = false;
|
||||
tailIntent.beginDrag();
|
||||
userOptedOutOfTailFollow.value = true;
|
||||
userDragDetachedTailFollow.value = true;
|
||||
followsThreadTail.value = false;
|
||||
},
|
||||
onUserScrollEnd: () {
|
||||
tailIntent.endDrag();
|
||||
tailIntent.schedule(
|
||||
allowed: userOptedOutOfTailFollow.value,
|
||||
revalidate: () =>
|
||||
context.mounted &&
|
||||
itemScrollController.isAttached &&
|
||||
!tailIntent.isDragging &&
|
||||
userOptedOutOfTailFollow.value,
|
||||
action: () {
|
||||
_resumeThreadTailFollow(
|
||||
isVisible: threadTailIsVisible,
|
||||
userOptedOut: userOptedOutOfTailFollow,
|
||||
followsTail: followsThreadTail,
|
||||
);
|
||||
if (!userOptedOutOfTailFollow.value) {
|
||||
userDragDetachedTailFollow.value = false;
|
||||
}
|
||||
},
|
||||
);
|
||||
},
|
||||
visible: threadViewportVisible,
|
||||
itemScrollController: itemScrollController,
|
||||
itemPositionsListener: itemPositionsListener,
|
||||
bottomInset: timelineBottomInset,
|
||||
replies: replies,
|
||||
knownAgents: threadTyping.isEmpty
|
||||
? const {}
|
||||
: ref.watch(meAgentInventoryProvider).value ?? const {},
|
||||
inlineTyping: isMember && !isArchived
|
||||
? threadTyping
|
||||
: const [],
|
||||
relayReplyState: relayReplyState,
|
||||
onRetryReplies: () =>
|
||||
ref.invalidate(threadRepliesProvider(repliesArgs)),
|
||||
localSendAnimations: localSendAnimations,
|
||||
trackActiveScrollPosition: trackActiveScrollPosition,
|
||||
headIsDeleted: liveDeletionHidesHead,
|
||||
head: liveHead,
|
||||
stickyDayTimestamp: stickyDayTimestamp,
|
||||
channelNames: channelNamesMap,
|
||||
channelId: channelId,
|
||||
currentPubkey: currentPubkey,
|
||||
highlightedMessageId: highlightedMessageId.value,
|
||||
allMessages: allMsgs,
|
||||
isMember: isMember,
|
||||
isArchived: isArchived,
|
||||
composerFocusNode: composerFocusNode,
|
||||
restoreComposerFocus: () =>
|
||||
restoreComposerFocus.value?.call(),
|
||||
childrenByParent: childrenByParent,
|
||||
child: _ThreadMessageList(
|
||||
dateScrollActivity: dateScrollActivity,
|
||||
headHeight: headHeight.value ?? 0,
|
||||
onHeadHeightChanged: (height) {
|
||||
if (context.mounted) headHeight.value = height;
|
||||
},
|
||||
viewport: listViewport,
|
||||
onUserScrollStart: () {
|
||||
hidesLatestForInitialTailSettle.value = false;
|
||||
hidesLatestForComposerTailCorrection.value = false;
|
||||
initialTailSettle.abandon();
|
||||
initialViewportReady.value = true;
|
||||
tailCorrectionInProgress.value = false;
|
||||
isNavigatingToThreadTail.value = false;
|
||||
tailIntent.beginDrag();
|
||||
userOptedOutOfTailFollow.value = true;
|
||||
userDragDetachedTailFollow.value = true;
|
||||
followsThreadTail.value = false;
|
||||
},
|
||||
onUserScrollEnd: () {
|
||||
tailIntent.endDrag();
|
||||
tailIntent.schedule(
|
||||
allowed: userOptedOutOfTailFollow.value,
|
||||
revalidate: () =>
|
||||
context.mounted &&
|
||||
itemScrollController.isAttached &&
|
||||
!tailIntent.isDragging &&
|
||||
userOptedOutOfTailFollow.value,
|
||||
action: () {
|
||||
_resumeThreadTailFollow(
|
||||
isVisible: threadTailIsVisible,
|
||||
userOptedOut: userOptedOutOfTailFollow,
|
||||
followsTail: followsThreadTail,
|
||||
);
|
||||
if (!userOptedOutOfTailFollow.value) {
|
||||
userDragDetachedTailFollow.value = false;
|
||||
}
|
||||
},
|
||||
);
|
||||
},
|
||||
visible: threadViewportVisible,
|
||||
itemScrollController: itemScrollController,
|
||||
itemPositionsListener: itemPositionsListener,
|
||||
bottomInset: timelineBottomInset,
|
||||
replies: replies,
|
||||
knownAgents: threadTyping.isEmpty
|
||||
? const {}
|
||||
: ref.watch(meAgentInventoryProvider).value ?? const {},
|
||||
inlineTyping: isMember && !isArchived
|
||||
? threadTyping
|
||||
: const [],
|
||||
relayReplyState: relayReplyState,
|
||||
onRetryReplies: () =>
|
||||
ref.invalidate(threadRepliesProvider(repliesArgs)),
|
||||
localSendAnimations: localSendAnimations,
|
||||
trackActiveScrollPosition: trackActiveScrollPosition,
|
||||
headIsDeleted: liveDeletionHidesHead,
|
||||
head: liveHead,
|
||||
stickyDayTimestamp: stickyDayTimestamp,
|
||||
channelNames: channelNamesMap,
|
||||
channelId: channelId,
|
||||
currentPubkey: currentPubkey,
|
||||
highlightedMessageId: highlightedMessageId.value,
|
||||
allMessages: allMsgs,
|
||||
isMember: isMember,
|
||||
isArchived: isArchived,
|
||||
composerFocusNode: composerFocusNode,
|
||||
restoreComposerFocus: () =>
|
||||
restoreComposerFocus.value?.call(),
|
||||
childrenByParent: childrenByParent,
|
||||
),
|
||||
),
|
||||
),
|
||||
],
|
||||
|
||||
@@ -17861,6 +17861,160 @@ void main() {
|
||||
},
|
||||
);
|
||||
|
||||
// A targeted entry can also hydrate in two steps: the route's snapshot
|
||||
// of the target, then the relay's full thread, which may include the
|
||||
// reader's own earlier replies.
|
||||
for (final (initialMessageId, delayedHydration) in [
|
||||
(null, false),
|
||||
('reply-5', false),
|
||||
('reply-5', true),
|
||||
]) {
|
||||
testWidgets('a sent reply stays above the composer as it grows in '
|
||||
'(target: $initialMessageId, delayed: $delayedHydration)', (
|
||||
tester,
|
||||
) async {
|
||||
final previousPlatform = debugDefaultTargetPlatformOverride;
|
||||
debugDefaultTargetPlatformOverride = TargetPlatform.iOS;
|
||||
addTearDown(
|
||||
() => debugDefaultTargetPlatformOverride = previousPlatform,
|
||||
);
|
||||
tester.view.physicalSize = const Size(400, 800);
|
||||
tester.view.devicePixelRatio = 1;
|
||||
tester.view.viewPadding = const FakeViewPadding(bottom: 20);
|
||||
addTearDown(tester.view.reset);
|
||||
|
||||
final rootEvent = _textMsg(
|
||||
id: threadIdThreadDashRoot,
|
||||
pubkey: 'alice',
|
||||
content: 'Thread root',
|
||||
createdAt: 1000,
|
||||
);
|
||||
final replies = [
|
||||
for (var i = 0; i < 30; i++)
|
||||
_textMsg(
|
||||
id: 'reply-$i',
|
||||
pubkey: delayedHydration && i.isEven ? 'self' : 'bob',
|
||||
content: 'Reply $i',
|
||||
createdAt: 1100 + i,
|
||||
extraTags: const [
|
||||
['e', threadIdThreadDashRoot, '', 'reply'],
|
||||
],
|
||||
),
|
||||
];
|
||||
|
||||
final hydration = Completer<List<NostrEvent>>();
|
||||
await tester.pumpWidget(
|
||||
_buildTestable(
|
||||
messages: [rootEvent],
|
||||
threadReplies: delayedHydration
|
||||
? const {}
|
||||
: {threadIdThreadDashRoot: replies},
|
||||
threadReplyLoaders: delayedHydration
|
||||
? {threadIdThreadDashRoot: () => hydration.future}
|
||||
: const {},
|
||||
users: const {
|
||||
'alice': UserProfile(pubkey: 'alice', displayName: 'Alice'),
|
||||
'bob': UserProfile(pubkey: 'bob', displayName: 'Bob'),
|
||||
},
|
||||
),
|
||||
);
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
final threadHead = formatTimeline([rootEvent]).single;
|
||||
final target = formatTimeline([
|
||||
rootEvent,
|
||||
replies[5],
|
||||
]).where((message) => message.id == 'reply-5');
|
||||
Navigator.of(tester.element(find.byType(ChannelDetailPage))).push(
|
||||
MaterialPageRoute<void>(
|
||||
builder: (_) => ThreadDetailPage(
|
||||
threadHead: threadHead,
|
||||
allMessages: [threadHead, if (delayedHydration) ...target],
|
||||
channelId: _channelId,
|
||||
currentPubkey: 'self',
|
||||
isMember: true,
|
||||
isArchived: false,
|
||||
// Inbox, push and link entries open at an older target and must
|
||||
// still bring the reader's own reply into view.
|
||||
initialMessageId: initialMessageId,
|
||||
),
|
||||
),
|
||||
);
|
||||
await tester.pumpAndSettle();
|
||||
if (delayedHydration) {
|
||||
hydration.complete(replies);
|
||||
await tester.pumpAndSettle();
|
||||
}
|
||||
if (initialMessageId != null) {
|
||||
expect(
|
||||
find.text('Reply 5').hitTestable(),
|
||||
findsOneWidget,
|
||||
reason: 'The target jump must reveal the hydrated list.',
|
||||
);
|
||||
}
|
||||
// Readers scroll a little before replying, which detaches tail follow
|
||||
// so focusing the composer no longer snaps to the tail by itself.
|
||||
await tester.drag(
|
||||
find.byType(ScrollablePositionedList),
|
||||
const Offset(0, 80),
|
||||
);
|
||||
await tester.pumpAndSettle();
|
||||
if (initialMessageId != null) {
|
||||
expect(find.text('Reply 5'), findsOneWidget);
|
||||
}
|
||||
await tester.tap(find.text('Reply in thread…').hitTestable());
|
||||
await tester.pumpAndSettle();
|
||||
tester.view.viewInsets = const FakeViewPadding(bottom: 300);
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
// Production adds a signed reply this way before the relay answers, and
|
||||
// its row then grows to full height over the entrance transition.
|
||||
final container = ProviderScope.containerOf(
|
||||
tester.element(find.byType(ThreadDetailPage)),
|
||||
);
|
||||
final sent = _textMsg(
|
||||
id: 'sent-reply',
|
||||
pubkey: 'self',
|
||||
content: 'A sent reply\nwith a second line\nand a third line',
|
||||
createdAt: 1200,
|
||||
extraTags: const [
|
||||
['e', threadIdThreadDashRoot, '', 'reply'],
|
||||
],
|
||||
);
|
||||
container
|
||||
.read(localMessageSendAnimationProvider(_channelId).notifier)
|
||||
.mark(sent.id);
|
||||
container
|
||||
.read(
|
||||
threadLocalRepliesProvider(
|
||||
const ThreadRepliesArgs(
|
||||
channelId: _channelId,
|
||||
rootId: threadIdThreadDashRoot,
|
||||
),
|
||||
).notifier,
|
||||
)
|
||||
.add(sent);
|
||||
await tester.pumpAndSettle();
|
||||
debugDefaultTargetPlatformOverride = previousPlatform;
|
||||
|
||||
final sentReply = find.byKey(
|
||||
const ValueKey('thread-message-group-sent-reply'),
|
||||
);
|
||||
final composerTop = tester
|
||||
.getTopLeft(find.byKey(const ValueKey('composer-surface')))
|
||||
.dy;
|
||||
expect(
|
||||
tester.getBottomLeft(sentReply).dy,
|
||||
lessThanOrEqualTo(composerTop),
|
||||
reason: 'The fully grown reply must not end under the composer.',
|
||||
);
|
||||
expect(
|
||||
find.byKey(const ValueKey('thread-jump-to-latest')),
|
||||
findsNothing,
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
testWidgets('thread hides initial tail placement until it is settled', (
|
||||
tester,
|
||||
) async {
|
||||
|
||||
Reference in New Issue
Block a user