From e9fa5ad47494a586bba74c1407351c731dfa452c Mon Sep 17 00:00:00 2001 From: Nacho Date: Tue, 25 Aug 2026 07:17:50 +0200 Subject: [PATCH] fix(mobile): skip thread tail jumps that retrigger iOS bounce On short threads, follow-tail kept calling jumpTo(maxScrollExtent) while item positions still reported the tail as not visible. On iOS bouncing physics that restarts rubber-banding even when extentAfter is already 0, which is the header jank on iPhone 16 Pro. Stop the correction loop once the scroll position is at the end, and do not jump or animate when already there. Related: #5911 (stale, unmerged), #6399 (Latest hide only). Co-authored-by: Nacho Signed-off-by: Nacho --- .../channels/thread_detail_helpers.dart | 20 +++- .../features/channels/thread_detail_page.dart | 42 ++++----- .../channels/channel_detail_page_test.dart | 91 +++++++++++++++++++ 3 files changed, 129 insertions(+), 24 deletions(-) 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); }, );