From 6e36493704a0d4d965654d65c2a25975bff2e149 Mon Sep 17 00:00:00 2001 From: Kenny Lopez Date: Mon, 17 Aug 2026 07:48:40 +0100 Subject: [PATCH] fix(mobile): highlight hydrated thread target on query failure Co-authored-by: Kenny Lopez Signed-off-by: Kenny Lopez --- .../features/channels/thread_detail_page.dart | 89 +++++++++++-------- .../channels/channel_detail_page_test.dart | 64 +++++++++++++ 2 files changed, 116 insertions(+), 37 deletions(-) diff --git a/mobile/lib/features/channels/thread_detail_page.dart b/mobile/lib/features/channels/thread_detail_page.dart index ef5dd02b2..101ddd866 100644 --- a/mobile/lib/features/channels/thread_detail_page.dart +++ b/mobile/lib/features/channels/thread_detail_page.dart @@ -114,6 +114,11 @@ class ThreadDetailPage extends HookConsumerWidget { }); final fetchedReplies = replyMessages.value; + // A terminal query error cannot produce a more authoritative list. Keep + // loading states provisional, but let the hydrated route snapshot drive + // the one-shot target jump when the relay query has definitively failed. + final canUseMessagesForInitialTarget = + fetchedReplies != null || replyMessages.hasError; final liveDeletionHidesHead = _isDeletedBy( liveChannelEvents, threadHead.id, @@ -342,43 +347,53 @@ class ThreadDetailPage extends HookConsumerWidget { ); } - useEffect(() { - final messageId = initialMessageId; - // Wait for the authoritative thread query before consuming the one-shot - // jump; the fallback main-timeline list can contain only the linked reply. - if (messageId == null || fetchedReplies == null) return null; - final chronologicalIndex = replies.indexWhere( - (reply) => reply.id == messageId, - ); - final targetIndex = messageId == threadHead.id - ? headIndex - : chronologicalIndex < 0 - ? null - : indexForReply(chronologicalIndex); - if (targetIndex == null || didJumpToInitialMessage.value) return null; - didJumpToInitialMessage.value = true; - initialTailSettle.abandon(); - userOptedOutOfTailFollow.value = true; - userDragDetachedTailFollow.value = false; - tailIntent.schedule( - allowed: true, - revalidate: () => - context.mounted && - itemScrollController.isAttached && - !tailIntent.isDragging, - action: () { - // 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. - tailIntent.detach(); - followsThreadTail.value = false; - isAtThreadTail.value = false; - itemScrollController.jumpTo(index: targetIndex, alignment: 0.35); - initialHighlightTargetIndex.value = targetIndex; - }, - ); - return null; - }, [initialMessageId, fetchedReplies, replies.length]); + useEffect( + () { + final messageId = initialMessageId; + // Wait for either the authoritative thread query or a terminal query + // error before consuming the one-shot jump. During loading, the fallback + // main-timeline list can contain only the linked reply; after an error, + // that hydrated snapshot is the best available target list. + if (messageId == null || !canUseMessagesForInitialTarget) return null; + final chronologicalIndex = replies.indexWhere( + (reply) => reply.id == messageId, + ); + final targetIndex = messageId == threadHead.id + ? headIndex + : chronologicalIndex < 0 + ? null + : indexForReply(chronologicalIndex); + if (targetIndex == null || didJumpToInitialMessage.value) return null; + didJumpToInitialMessage.value = true; + initialTailSettle.abandon(); + userOptedOutOfTailFollow.value = true; + userDragDetachedTailFollow.value = false; + tailIntent.schedule( + allowed: true, + revalidate: () => + context.mounted && + itemScrollController.isAttached && + !tailIntent.isDragging, + action: () { + // 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. + tailIntent.detach(); + followsThreadTail.value = false; + isAtThreadTail.value = false; + itemScrollController.jumpTo(index: targetIndex, alignment: 0.35); + initialHighlightTargetIndex.value = targetIndex; + }, + ); + return null; + }, + [ + initialMessageId, + canUseMessagesForInitialTarget, + fetchedReplies, + replies.length, + ], + ); useEffect( () { diff --git a/mobile/test/features/channels/channel_detail_page_test.dart b/mobile/test/features/channels/channel_detail_page_test.dart index 0f5a04057..dbba60234 100644 --- a/mobile/test/features/channels/channel_detail_page_test.dart +++ b/mobile/test/features/channels/channel_detail_page_test.dart @@ -193,6 +193,7 @@ Widget _buildTestable({ Map>> pendingThreadReplies = const {}, TextScaler textScaler = TextScaler.noScaling, bool disableAnimations = false, + bool disableRetries = false, RelaySessionNotifier? relaySessionNotifier, http.Client? mediaClient, Widget? home, @@ -203,6 +204,7 @@ Widget _buildTestable({ final fakeMessagesNotifier = messagesNotifier ?? _FakeMessagesNotifier(messages); return ProviderScope( + retry: disableRetries ? (_, _) => null : null, overrides: [ channelMessagesProvider( _channelId, @@ -4469,6 +4471,68 @@ void main() { expect(enteringDecoration.color!.a, lessThan(0.12 * 0.4)); }); + testWidgets('highlights a hydrated target after the thread query fails', ( + tester, + ) async { + final root = _textMsg( + id: 'root', + pubkey: 'alice', + content: 'Thread root', + createdAt: 1000, + ); + final target = _textMsg( + id: 'target', + pubkey: 'bob', + content: 'Hydrated target', + createdAt: 1100, + extraTags: const [ + ['e', 'root', '', 'reply'], + ], + ); + final timelineMessages = formatTimeline([root, target]); + final replyCompleter = Completer>(); + + await tester.pumpWidget( + _buildTestable( + messages: [root, target], + pendingThreadReplies: {'root': replyCompleter.future}, + disableRetries: true, + users: const { + 'alice': UserProfile(pubkey: 'alice', displayName: 'Alice'), + 'bob': UserProfile(pubkey: 'bob', displayName: 'Bob'), + }, + home: ThreadDetailPage( + threadHead: timelineMessages.first, + allMessages: timelineMessages, + channelId: _testChannel.id, + currentPubkey: null, + isMember: true, + isArchived: false, + initialMessageId: 'target', + ), + ), + ); + await tester.pumpAndSettle(); + + final targetFinder = find.byKey(const ValueKey('thread-message-target')); + expect(targetFinder, findsOneWidget); + final loadingDecoration = + tester.widget(targetFinder).decoration as BoxDecoration; + expect(loadingDecoration.color, Colors.transparent); + + replyCompleter.completeError(Exception('thread query failed')); + for (var i = 0; i < 8; i++) { + await tester.pump(); + } + await tester.pump(const Duration(milliseconds: 50)); + await tester.pump(const Duration(milliseconds: 150)); + + final highlightedDecoration = + tester.widget(targetFinder).decoration as BoxDecoration; + expect(highlightedDecoration.color!.a, greaterThan(0)); + expect(highlightedDecoration.color!.a, lessThan(0.12 * 0.4)); + }); + testWidgets('opens a nested reply in its direct-parent thread', ( tester, ) async {