diff --git a/mobile/lib/features/channels/initial_thread_tail_settle.dart b/mobile/lib/features/channels/initial_thread_tail_settle.dart new file mode 100644 index 0000000000..aaac003b66 --- /dev/null +++ b/mobile/lib/features/channels/initial_thread_tail_settle.dart @@ -0,0 +1,65 @@ +import 'package:flutter/widgets.dart'; +import 'package:scrollable_positioned_list/scrollable_positioned_list.dart'; + +/// Settles an ordinary thread open on the latest hydrated reply after layout. +/// +/// Scheduling again before completion invalidates callbacks aimed at an older +/// tail, allowing a rebuild with newly arrived replies to choose the target. +class InitialThreadTailSettle { + var _generation = 0; + var _isComplete = false; + + bool get isComplete => _isComplete; + + void schedule({ + required BuildContext context, + required ItemScrollController controller, + required ItemPositionsListener positionsListener, + required int? targetIndex, + required double hiddenBottomFraction, + }) { + if (_isComplete) return; + + final generation = ++_generation; + if (targetIndex == null) { + _isComplete = true; + return; + } + + WidgetsBinding.instance.addPostFrameCallback((_) { + if (!context.mounted || generation != _generation) return; + + // Let events received during hydration rebuild the list before committing + // the target. That rebuild schedules a new generation at the current tail. + WidgetsBinding.instance.addPostFrameCallback((_) { + if (!context.mounted || + !controller.isAttached || + generation != _generation) { + return; + } + final targetIsFullyVisible = positionsListener.itemPositions.value.any( + (position) => + position.index == targetIndex && + position.itemLeadingEdge >= 0 && + position.itemTrailingEdge <= 1 - hiddenBottomFraction, + ); + // Short threads already expose their tail from the top anchor. Moving + // that fully visible target down would only add empty space above the + // head. A clipped tail still takes the measured correction path. + if (targetIsFullyVisible) { + _isComplete = true; + return; + } + controller + .scrollTo( + index: targetIndex, + alignment: 0.0, + duration: const Duration(milliseconds: 1), + ) + .whenComplete(() { + if (generation == _generation) _isComplete = true; + }); + }); + }); + } +} diff --git a/mobile/lib/features/channels/thread_detail_page.dart b/mobile/lib/features/channels/thread_detail_page.dart index fff7f5834b..ec4dc4897a 100644 --- a/mobile/lib/features/channels/thread_detail_page.dart +++ b/mobile/lib/features/channels/thread_detail_page.dart @@ -24,6 +24,7 @@ import 'composer_dock_size_reporter.dart'; import 'date_formatters.dart'; import 'day_divider.dart'; import '../profile/user_profile_sheet.dart'; +import 'initial_thread_tail_settle.dart'; import 'message_actions.dart'; import 'message_content.dart'; import 'reaction_row.dart'; @@ -117,10 +118,9 @@ class ThreadDetailPage extends HookConsumerWidget { final itemPositionsListener = useMemoized(ItemPositionsListener.create); final didJumpToInitialMessage = useRef(false); final followsThreadTail = useRef(false); + final userOptedOutOfTailFollow = useRef(false); final pendingTailAlignment = useRef(null); final tailRealignmentQueued = useRef(false); - - // Item 0 is the thread head; reply `i` lives at `i + 1`. const headIndex = 0; int indexForReply(int chronologicalIndex) => chronologicalIndex + 1; @@ -136,7 +136,9 @@ class ThreadDetailPage extends HookConsumerWidget { useEffect(() { void onPositionsChanged() { - if (threadTailIsVisible()) followsThreadTail.value = true; + if (!userOptedOutOfTailFollow.value && threadTailIsVisible()) { + followsThreadTail.value = true; + } } itemPositionsListener.itemPositions.addListener(onPositionsChanged); @@ -161,9 +163,6 @@ class ThreadDetailPage extends HookConsumerWidget { if (targetIndex == null || didJumpToInitialMessage.value) return null; WidgetsBinding.instance.addPostFrameCallback((_) { if (!context.mounted || !itemScrollController.isAttached) return; - // The provisional route snapshot can make the linked reply look like - // the tail. This authoritative deep-link jump intentionally leaves - // the user at an older item, so it must opt out of follow-tail first. followsThreadTail.value = false; pendingTailAlignment.value = null; itemScrollController.jumpTo(index: targetIndex, alignment: 0.35); @@ -177,15 +176,22 @@ class ThreadDetailPage extends HookConsumerWidget { // while the last item is on screen, scroll it into view. If the user has // scrolled up to read, leave them where they are. final hasFetchedReplies = fetchedReplies != null; - final didEstablishInitialReplies = useRef(hasFetchedReplies); + final initialTailSettle = useMemoized(InitialThreadTailSettle.new); final previousReplyCount = useRef(replies.length); useEffect(() { - // The first authoritative query result is hydration, not a live arrival. - // Establish the baseline without moving the user away from the head. if (!hasFetchedReplies) return null; - if (!didEstablishInitialReplies.value) { - didEstablishInitialReplies.value = true; + if (!initialTailSettle.isComplete) { previousReplyCount.value = replies.length; + initialTailSettle.schedule( + context: context, + controller: itemScrollController, + positionsListener: itemPositionsListener, + targetIndex: initialMessageId == null && replies.isNotEmpty + ? indexForReply(replies.length - 1) + : null, + hiddenBottomFraction: + composerDockHeight.value / MediaQuery.sizeOf(context).height, + ); return null; } @@ -194,15 +200,12 @@ class ThreadDetailPage extends HookConsumerWidget { if (replies.length <= previous) return null; final positions = itemPositionsListener.itemPositions.value; final lastIndex = indexForReply(replies.length - 1); - // Positions still describe the list as it was *before* these replies, so - // compare against the old tail. Measuring against the new one only reads - // as "at the tail" when exactly one reply arrived. final previousLastIndex = previous == 0 ? headIndex : indexForReply(previous - 1); - final wasAtTail = - positions.isEmpty || - positions.any((position) => position.index >= previousLastIndex); + final wasAtTail = positions.any( + (position) => position.index == previousLastIndex, + ); final localPubkey = currentPubkey?.toLowerCase(); final hasNewLocalReply = localPubkey != null && @@ -264,7 +267,9 @@ class ThreadDetailPage extends HookConsumerWidget { final heightDelta = height - previousHeight; if (heightDelta.abs() < 0.5) return; - final shouldFollowTail = followsThreadTail.value || threadTailIsVisible(); + final shouldFollowTail = + !userOptedOutOfTailFollow.value && + (followsThreadTail.value || threadTailIsVisible()); if (shouldFollowTail) followsThreadTail.value = true; composerDockHeight.value = height; if (heightDelta <= 0 || !shouldFollowTail) { @@ -297,7 +302,9 @@ class ThreadDetailPage extends HookConsumerWidget { // keyboard appears. Re-align after that latter layout pass too, but only // while the user was already following the thread tail. void realignThreadTailAfterMetricsChange() { - final shouldFollowTail = followsThreadTail.value || threadTailIsVisible(); + final shouldFollowTail = + !userOptedOutOfTailFollow.value && + (followsThreadTail.value || threadTailIsVisible()); if (!shouldFollowTail || tailRealignmentQueued.value) return; followsThreadTail.value = true; tailRealignmentQueued.value = true; @@ -349,6 +356,7 @@ class ThreadDetailPage extends HookConsumerWidget { Expanded( child: KeyboardDismissOnDrag( onUserScrollStart: () { + userOptedOutOfTailFollow.value = true; followsThreadTail.value = false; pendingTailAlignment.value = null; }, diff --git a/mobile/test/features/channels/channel_detail_page_test.dart b/mobile/test/features/channels/channel_detail_page_test.dart index 912233aba0..31a5c0f021 100644 --- a/mobile/test/features/channels/channel_detail_page_test.dart +++ b/mobile/test/features/channels/channel_detail_page_test.dart @@ -3447,8 +3447,82 @@ void main() { ); }); + testWidgets('short initial thread hydration remains top-anchored', ( + tester, + ) async { + final rootEvent = _textMsg( + id: 'thread-root', + pubkey: 'alice', + content: 'Thread root', + createdAt: 1000, + ); + final replies = [ + _textMsg( + id: 'reply-1', + pubkey: 'bob', + content: 'First reply', + createdAt: 1100, + extraTags: const [ + ['e', 'thread-root', '', 'reply'], + ], + ), + _textMsg( + id: 'reply-2', + pubkey: 'bob', + content: 'Second reply', + createdAt: 1101, + extraTags: const [ + ['e', 'thread-root', '', 'reply'], + ], + ), + ]; + final completer = Completer>(); + + await tester.pumpWidget( + _buildTestable( + messages: [rootEvent], + pendingThreadReplies: {'thread-root': completer.future}, + users: const { + 'alice': UserProfile(pubkey: 'alice', displayName: 'Alice'), + 'bob': UserProfile(pubkey: 'bob', displayName: 'Bob'), + }, + ), + ); + await tester.pumpAndSettle(); + + final threadHead = formatTimeline([rootEvent]).single; + Navigator.of(tester.element(find.byType(ChannelDetailPage))).push( + MaterialPageRoute( + builder: (_) => ThreadDetailPage( + threadHead: threadHead, + allMessages: [threadHead], + channelId: _channelId, + currentPubkey: 'self', + isMember: true, + isArchived: false, + ), + ), + ); + await tester.pumpAndSettle(); + + final headFinder = find.byKey( + const ValueKey('thread-message-group-thread-root'), + ); + final initialHeadY = tester.getTopLeft(headFinder).dy; + + completer.complete(replies); + await tester.pumpAndSettle(); + + expect(headFinder, findsOneWidget); + expect( + find.byKey(const ValueKey('thread-message-group-reply-2')), + findsOneWidget, + ); + expect(tester.getTopLeft(headFinder).dy, closeTo(initialHeadY, 0.5)); + }); + testWidgets( - 'initial thread hydration keeps the head visible instead of following the tail', + 'initial thread hydration settles on the latest reply after pagination', (tester) async { final rootEvent = _textMsg( id: 'thread-root', @@ -3469,10 +3543,12 @@ void main() { ), ]; final completer = Completer>(); + final messagesNotifier = _FakeMessagesNotifier([rootEvent]); await tester.pumpWidget( _buildTestable( messages: [rootEvent], + messagesNotifier: messagesNotifier, pendingThreadReplies: {'thread-root': completer.future}, users: const { 'alice': UserProfile(pubkey: 'alice', displayName: 'Alice'), @@ -3502,16 +3578,134 @@ void main() { ); completer.complete(replies); + await tester.pump(); + final latestLiveReply = _textMsg( + id: 'reply-live', + pubkey: 'bob', + content: List.filled(10, 'Tall live reply').join('\n'), + createdAt: 1200, + extraTags: const [ + ['e', 'thread-root', '', 'reply'], + ], + ); + messagesNotifier.setMessages([rootEvent, latestLiveReply]); await tester.pumpAndSettle(); expect( find.byKey(const ValueKey('thread-message-group-thread-root')), + findsNothing, + ); + expect( + find.byKey(const ValueKey('thread-message-group-reply-live')), findsOneWidget, ); + final listRect = tester.getRect( + find.byKey(const ValueKey('thread-message-list')), + ); + final latestReply = find.byKey( + const ValueKey('thread-message-group-reply-live'), + ); + final latestRect = tester.getRect(latestReply); + final composerTop = tester + .getTopLeft(find.byKey(const ValueKey('composer-surface'))) + .dy; + expect(latestRect.bottom, lessThanOrEqualTo(composerTop)); + expect(latestRect.bottom, greaterThan(listRect.center.dy)); + }, + ); + + testWidgets( + 'dragging away from the settled tail opts out of keyboard realignment', + (tester) async { + tester.view.physicalSize = const Size(400, 800); + tester.view.devicePixelRatio = 1; + addTearDown(tester.view.reset); + + final rootEvent = _textMsg( + id: 'thread-root', + pubkey: 'alice', + content: 'Thread root', + createdAt: 1000, + ); + final replies = [ + for (var i = 0; i < 30; i++) + _textMsg( + id: 'reply-$i', + pubkey: 'bob', + content: 'Reply $i', + createdAt: 1100 + i, + extraTags: const [ + ['e', 'thread-root', '', 'reply'], + ], + ), + ]; + + await tester.pumpWidget( + _buildTestable( + messages: [rootEvent], + threadReplies: {'thread-root': replies}, + users: const { + 'alice': UserProfile(pubkey: 'alice', displayName: 'Alice'), + 'bob': UserProfile(pubkey: 'bob', displayName: 'Bob'), + }, + ), + ); + await tester.pumpAndSettle(); + + final threadHead = formatTimeline([rootEvent]).single; + Navigator.of(tester.element(find.byType(ChannelDetailPage))).push( + MaterialPageRoute( + builder: (_) => ThreadDetailPage( + threadHead: threadHead, + allMessages: [threadHead], + channelId: _channelId, + currentPubkey: 'self', + isMember: true, + isArchived: false, + ), + ), + ); + await tester.pumpAndSettle(); + + final list = find.byKey(const ValueKey('thread-message-list')); + for (var i = 0; i < 4; i++) { + await tester.drag(list, const Offset(0, 100)); + await tester.pumpAndSettle(); + } expect( find.byKey(const ValueKey('thread-message-group-reply-29')), findsNothing, ); + final visibleBeforeResize = tester + .widgetList( + find.byWidgetPredicate( + (widget) => + widget.key is ValueKey && + (widget.key! as ValueKey).value.startsWith( + 'thread-message-group-', + ), + ), + ) + .map((widget) => widget.key) + .toSet(); + + tester.view.viewInsets = const FakeViewPadding(bottom: 300); + await tester.pumpAndSettle(); + + expect( + find.byKey(const ValueKey('thread-message-group-reply-29')), + findsNothing, + ); + expect( + tester + .widgetList( + find.byWidgetPredicate( + (widget) => visibleBeforeResize.contains(widget.key), + ), + ) + .map((widget) => widget.key), + isNotEmpty, + ); }, );