Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions mobile/lib/features/channels/thread_detail_helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
42 changes: 21 additions & 21 deletions mobile/lib/features/channels/thread_detail_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> 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
Expand Down Expand Up @@ -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.
Expand All @@ -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() {
Expand Down Expand Up @@ -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();
Expand Down
91 changes: 91 additions & 0 deletions mobile/test/features/channels/channel_detail_page_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>(
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<ScrollableState>(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',
Expand Down Expand Up @@ -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);
},
);

Expand Down