diff --git a/mobile/lib/features/channels/thread_detail_helpers.dart b/mobile/lib/features/channels/thread_detail_helpers.dart index 9b535397659..4dbc6a563c6 100644 --- a/mobile/lib/features/channels/thread_detail_helpers.dart +++ b/mobile/lib/features/channels/thread_detail_helpers.dart @@ -9,9 +9,23 @@ part of 'thread_detail_page.dart'; bool threadTailCorrectionReachedEnd({ required bool tailIsVisible, required double? extentAfter, -}) => - tailIsVisible || - (extentAfter != null && extentAfter <= _threadTailScrollTolerance); +}) => tailIsVisible || threadScrollPositionIsAtTail(extentAfter); + +/// Whether the thread scroll position is already at its max extent. +/// +/// On iOS bouncing physics, `jumpTo(maxScrollExtent)` / `animateTo` at that +/// offset restarts rubber-banding even when pixels do not change. Short +/// threads on iPhone show that as a continuous header jank. +@visibleForTesting +bool threadScrollPositionIsAtTail(double? extentAfter) => + extentAfter != null && extentAfter <= _threadTailScrollTolerance; + +bool _jumpThreadScrollPositionToTail(ScrollPosition position) { + if (!position.hasContentDimensions) return false; + if (threadScrollPositionIsAtTail(position.extentAfter)) return true; + position.jumpTo(position.maxScrollExtent); + return true; +} int _threadTailIndex(int replyCount) => replyCount; diff --git a/mobile/lib/features/channels/thread_detail_page.dart b/mobile/lib/features/channels/thread_detail_page.dart index cbac3cff843..5e0163899aa 100644 --- a/mobile/lib/features/channels/thread_detail_page.dart +++ b/mobile/lib/features/channels/thread_detail_page.dart @@ -341,20 +341,18 @@ class ThreadDetailPage extends HookConsumerWidget { bool jumpActiveScrollPositionToTail() { final position = activeThreadScrollPosition.value; - if (position == null || !position.hasContentDimensions) return false; - // Move the one active viewport to its exact end. Unlike indexed - // jumpTo/scrollTo, this does not reset or cross-fade through a second - // list, so iOS never exposes the intermediate top-of-thread frame. - position.jumpTo(position.maxScrollExtent); - return true; + if (position == null) return false; + // Indexed jumpTo/scrollTo can cross-fade a second list and flash the + // thread head. Directly moving this position does not. + return _jumpThreadScrollPositionToTail(position); } Future animateActiveScrollPositionToTail() async { final position = activeThreadScrollPosition.value; if (position == null || !position.hasContentDimensions) return false; - if (MediaQuery.disableAnimationsOf(context)) { - position.jumpTo(position.maxScrollExtent); - return true; + if (threadScrollPositionIsAtTail(position.extentAfter) || + MediaQuery.disableAnimationsOf(context)) { + return _jumpThreadScrollPositionToTail(position); } // Match the channel's visible Latest glide while moving only the active // thread viewport. The indexed-list animation path can create a temporary @@ -384,7 +382,13 @@ class ThreadDetailPage extends HookConsumerWidget { isAtThreadTail.value = threadTailIsVisible(); return; } - final reachedTail = threadTailIsVisible(); + final position = activeThreadScrollPosition.value; + final reachedTail = threadTailCorrectionReachedEnd( + tailIsVisible: threadTailIsVisible(), + extentAfter: position != null && position.hasContentDimensions + ? position.extentAfter + : null, + ); // Lazy children can revise maxScrollExtent for several frames. Keep // moving the same active position until the measured tail is visible; // the cap only guards pathological layouts that never stabilize. @@ -403,17 +407,7 @@ class ThreadDetailPage extends HookConsumerWidget { tailCorrectionInProgress.value = false; isNavigatingToThreadTail.value = false; if (revealViewport) initialViewportReady.value = true; - // Item positions can trail the ScrollPosition by a frame after an - // animated jump. Once the bounded lazy-layout correction is exhausted, - // trust an exact end-of-scroll position too: there is nowhere further - // for Latest to navigate, so leaving the control visible is misleading. - final position = activeThreadScrollPosition.value; - isAtThreadTail.value = threadTailCorrectionReachedEnd( - tailIsVisible: reachedTail, - extentAfter: position != null && position.hasContentDimensions - ? position.extentAfter - : null, - ); + isAtThreadTail.value = reachedTail; } void correctThreadTailInstantly() { @@ -754,6 +748,12 @@ class ThreadDetailPage extends HookConsumerWidget { anchorPosition.itemLeadingEdge < targetAlignment))) { return; } + final scrollPosition = activeThreadScrollPosition.value; + if (scrollPosition != null && + scrollPosition.hasContentDimensions && + threadScrollPositionIsAtTail(scrollPosition.extentAfter)) { + return; + } // This runs once after Android's frame-by-frame IME metrics settle. // Keep the resulting layout correction instant. correctThreadTailInstantly(); diff --git a/mobile/test/features/channels/channel_detail_page_test.dart b/mobile/test/features/channels/channel_detail_page_test.dart index 195dcc25ad8..94008b0a3f4 100644 --- a/mobile/test/features/channels/channel_detail_page_test.dart +++ b/mobile/test/features/channels/channel_detail_page_test.dart @@ -9376,6 +9376,93 @@ void main() { ); }); + testWidgets( + 'short thread does not rubber-band after idle layout and viewport jitter', + (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: 'A short thread', + createdAt: 1000, + ); + final replyEvent = _textMsg( + id: 'reply-0', + pubkey: 'bob', + content: 'Short reply', + createdAt: 1100, + extraTags: const [ + ['e', 'thread-root', '', 'reply'], + ], + ); + + await tester.pumpWidget( + _buildTestable( + messages: [rootEvent, replyEvent], + threadReplies: { + 'thread-root': [replyEvent], + }, + 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: formatTimeline([rootEvent, replyEvent]), + channelId: _channelId, + currentPubkey: 'self', + isMember: true, + isArchived: false, + ), + ), + ); + await tester.pumpAndSettle(); + + final head = find.byKey( + const ValueKey('thread-message-group-thread-root'), + ); + final list = find.descendant( + of: find.byKey(const ValueKey('thread-message-list')), + matching: find.byType(Scrollable), + ); + final initialHeadY = tester.getTopLeft(head).dy; + final scrollable = tester.state(list.first); + final initialPixels = scrollable.position.pixels; + expect(scrollable.position.extentAfter, lessThanOrEqualTo(0.5)); + + tester.view.viewInsets = const FakeViewPadding(bottom: 1); + await tester.pump(); + tester.view.viewInsets = FakeViewPadding.zero; + await tester.pump(); + await tester.pump(const Duration(milliseconds: 250)); + await tester.pump(const Duration(milliseconds: 250)); + + expect( + tester.getTopLeft(head).dy, + closeTo(initialHeadY, 1), + reason: + 'Idle layout and inset jitter must not restart rubber-banding ' + 'on a short thread already at its tail.', + ); + expect( + scrollable.position.pixels, + closeTo(initialPixels, 0.5), + reason: 'Already-at-tail jumpTo must be a no-op.', + ); + expect(scrollable.position.isScrollingNotifier.value, isFalse); + }, + ); + for (final replyCount in [0, 1]) { testWidgets( 'cached writable $replyCount-reply thread defers dock correction until measured', @@ -11547,6 +11634,10 @@ void main() { threadTailCorrectionReachedEnd(tailIsVisible: false, extentAfter: 1), isFalse, ); + expect(threadScrollPositionIsAtTail(0), isTrue); + expect(threadScrollPositionIsAtTail(0.5), isTrue); + expect(threadScrollPositionIsAtTail(0.51), isFalse); + expect(threadScrollPositionIsAtTail(null), isFalse); }, );