diff --git a/mobile/lib/features/channels/thread_detail_page.dart b/mobile/lib/features/channels/thread_detail_page.dart index 154193b3d2..0bb4cedb34 100644 --- a/mobile/lib/features/channels/thread_detail_page.dart +++ b/mobile/lib/features/channels/thread_detail_page.dart @@ -128,14 +128,15 @@ class ThreadDetailPage extends HookConsumerWidget { final liveChannelEvents = ref.watch(channelMessagesProvider(channelId)).value ?? const []; - final replyMessages = repliesState.whenData((events) { - return formatTimeline( - mergeThreadEvents(events, liveChannelEvents), - currentPubkey: currentPubkey, - ); - }); - - final fetchedReplies = replyMessages.value; + // Loading/error states can still carry replies from the last query. + // whenData drops that retained value during retries. + final replyEvents = repliesState.value; + final fetchedReplies = replyEvents == null + ? null + : formatTimeline( + mergeThreadEvents(replyEvents, liveChannelEvents), + currentPubkey: currentPubkey, + ); final hasFetchedReplies = fetchedReplies != null; // A terminal query error cannot produce a more authoritative list. Keep // loading states provisional, but let the hydrated route snapshot drive @@ -892,6 +893,7 @@ class ThreadDetailPage extends HookConsumerWidget { itemPositionsListener: itemPositionsListener, bottomInset: timelineBottomInset, replies: replies, + relayReplyState: relayReplyState, localSendAnimations: localSendAnimations, trackActiveScrollPosition: trackActiveScrollPosition, headIsDeleted: liveDeletionHidesHead, diff --git a/mobile/lib/features/channels/thread_detail_page/message_list.dart b/mobile/lib/features/channels/thread_detail_page/message_list.dart index 90e227ab7d..a33aa9a4ee 100644 --- a/mobile/lib/features/channels/thread_detail_page/message_list.dart +++ b/mobile/lib/features/channels/thread_detail_page/message_list.dart @@ -9,6 +9,7 @@ class _ThreadMessageList extends StatelessWidget { final ItemPositionsListener itemPositionsListener; final double bottomInset; final List replies; + final AsyncValue> relayReplyState; final Map localSendAnimations; final Widget Function(Widget child) trackActiveScrollPosition; final bool headIsDeleted; @@ -34,6 +35,7 @@ class _ThreadMessageList extends StatelessWidget { required this.itemPositionsListener, required this.bottomInset, required this.replies, + required this.relayReplyState, required this.localSendAnimations, required this.trackActiveScrollPosition, required this.headIsDeleted, @@ -51,6 +53,18 @@ class _ThreadMessageList extends StatelessWidget { required this.childrenByParent, }); + String get _replySummary { + if (!relayReplyState.hasValue && replies.isEmpty) { + return relayReplyState.isLoading + ? 'Loading replies…' + : 'Couldn’t load replies'; + } + final count = + '${replies.length} ${replies.length == 1 ? 'reply' : 'replies'}'; + if (!relayReplyState.hasError) return count; + return '$count · ${relayReplyState.isLoading ? 'Retrying…' : 'Couldn’t refresh'}'; + } + @override Widget build(BuildContext context) { const headIndex = 0; @@ -136,13 +150,19 @@ class _ThreadMessageList extends StatelessWidget { ), child: Row( children: [ - Text( - '${replies.length} ${replies.length == 1 ? 'reply' : 'replies'}', - style: context.textTheme.labelMedium - ?.copyWith( - color: context.colors.onSurfaceVariant, - fontWeight: FontWeight.w600, - ), + Flexible( + child: Semantics( + liveRegion: true, + child: Text( + _replySummary, + style: context.textTheme.labelMedium + ?.copyWith( + color: + context.colors.onSurfaceVariant, + fontWeight: FontWeight.w600, + ), + ), + ), ), const SizedBox(width: Grid.xxs), Expanded( diff --git a/mobile/lib/features/channels/thread_replies_provider.dart b/mobile/lib/features/channels/thread_replies_provider.dart index 1fb19432f1..c49461834e 100644 --- a/mobile/lib/features/channels/thread_replies_provider.dart +++ b/mobile/lib/features/channels/thread_replies_provider.dart @@ -130,11 +130,10 @@ final threadRepliesWithLocalProvider = Provider.autoDispose } } if (localReplies.isEmpty) return relayReplies; - return relayReplies.when( - data: (events) => AsyncData(_mergeReplies(events, localReplies)), - loading: () => AsyncData(localReplies), - error: (error, stackTrace) => AsyncData(localReplies), - ); + // This provider supplies display events; the original query remains + // the source of loading/error status. Preserve its retained value when + // merging optimistic replies during a failed refresh or retry. + return AsyncData(_mergeReplies(authoritative ?? const [], localReplies)); }); /// Union two event lists by id, newest-wins, in timeline order. diff --git a/mobile/test/features/channels/channel_detail_page_test.dart b/mobile/test/features/channels/channel_detail_page_test.dart index 7159fb2fd1..23a83ddab3 100644 --- a/mobile/test/features/channels/channel_detail_page_test.dart +++ b/mobile/test/features/channels/channel_detail_page_test.dart @@ -64,6 +64,8 @@ import 'package:buzz/shared/widgets/masked_avatar_badge.dart'; import 'package:buzz/shared/widgets/skeleton.dart'; import 'package:shared_preferences/shared_preferences.dart'; +part 'thread_reply_refresh_cases.dart'; + const _channelId = '11111111-2222-4333-8444-555555555555'; const _huddleChannelId = '8d764100-fd8f-44cf-9c98-6d8fbd739b8c'; const _otherChannelId = '22222222-3333-4444-8555-666666666666'; @@ -471,6 +473,7 @@ double? effectiveFontSizeForText( } void main() { + threadReplyRefreshTests(); setUp(() async { SharedPreferences.setMockInitialValues({}); _testPrefs = await SharedPreferences.getInstance(); diff --git a/mobile/test/features/channels/thread_replies_provider_test.dart b/mobile/test/features/channels/thread_replies_provider_test.dart index cdfc16fe81..419bd1e533 100644 --- a/mobile/test/features/channels/thread_replies_provider_test.dart +++ b/mobile/test/features/channels/thread_replies_provider_test.dart @@ -215,6 +215,53 @@ void main() { }, ); + test( + 'local replies preserve cached authoritative replies after refresh failure', + () async { + final cached = _reply('cached', 1000); + final local = _reply('local', 1001); + final session = _FakeRelaySession()..replies = [cached]; + final container = ProviderContainer( + retry: (_, _) => null, + overrides: [relaySessionProvider.overrideWith(() => session)], + ); + addTearDown(container.dispose); + final combined = container.listen( + threadRepliesWithLocalProvider(args), + (_, _) {}, + ); + await container.read(threadRepliesProvider(args).future); + container.read(threadLocalRepliesProvider(args).notifier).add(local); + await container.pump(); + final refresh = Completer>(); + session.nextQueryGate = refresh; + container.invalidate(threadRepliesProvider(args)); + await container.pump(); + expect(combined.read().value?.map((event) => event.id), [ + 'cached', + 'local', + ]); + refresh.completeError(Exception('Refresh failed')); + await container.pump(); + expect(container.read(threadRepliesProvider(args)).hasError, isTrue); + expect(combined.read().value?.map((event) => event.id), [ + 'cached', + 'local', + ]); + expect(container.read(threadLocalRepliesProvider(args)), [local]); + session.replies = [cached, local]; + container.invalidate(threadRepliesProvider(args)); + await container.read(threadRepliesProvider(args).future); + await container.pump(); + expect(combined.read().value?.map((event) => event.id), [ + 'cached', + 'local', + ]); + await Future.delayed(Duration.zero); + expect(container.read(threadLocalRepliesProvider(args)), isEmpty); + }, + ); + test('reopening a disposed thread performs a fresh load', () async { final (container, fakeSession, subscription) = makeHarness([ _reply('r1', 1000), diff --git a/mobile/test/features/channels/thread_reply_refresh_cases.dart b/mobile/test/features/channels/thread_reply_refresh_cases.dart new file mode 100644 index 0000000000..c788731fc5 --- /dev/null +++ b/mobile/test/features/channels/thread_reply_refresh_cases.dart @@ -0,0 +1,195 @@ +part of 'channel_detail_page_test.dart'; + +void threadReplyRefreshTests() { + const args = ThreadRepliesArgs(channelId: _channelId, rootId: 'refresh-root'); + final root = _textMsg( + id: args.rootId, + pubkey: 'alice', + content: 'Refresh root', + ); + final reply = _textMsg( + id: 'cached-reply', + pubkey: 'bob', + content: 'Cached reply', + createdAt: 1001, + extraTags: [ + ['e', args.rootId, '', 'reply'], + ], + ); + + Future<(NavigatorState, ProviderContainer)> mount( + WidgetTester tester, + Future> Function() load, { + bool retry = false, + TextScaler textScaler = TextScaler.noScaling, + }) async { + await tester.pumpWidget( + _buildTestable( + messages: [root], + textScaler: textScaler, + threadReplyLoaders: {args.rootId: load}, + providerRetry: (_, _) => + retry ? const Duration(milliseconds: 200) : null, + ), + ); + await tester.pumpAndSettle(); + final context = tester.element(find.byType(ChannelDetailPage)); + return (Navigator.of(context), ProviderScope.containerOf(context)); + } + + void open(NavigatorState navigator) { + final head = formatTimeline([root]).single; + navigator.push( + MaterialPageRoute( + builder: (_) => ThreadDetailPage( + threadHead: head, + allMessages: [head], + channelId: _channelId, + currentPubkey: 'self', + isMember: true, + isArchived: false, + ), + ), + ); + } + + testWidgets( + 'thread refresh keeps cached replies through retry and recovery', + (tester) async { + var calls = 0; + final refresh = Completer>(); + final retry = Completer>(); + final (navigator, container) = await mount(tester, () { + calls++; + return switch (calls) { + 1 => Future.value([reply]), + 2 => refresh.future, + _ => retry.future, + }; + }, retry: true); + open(navigator); + await tester.pumpAndSettle(); + expect(find.text('1 reply'), findsOneWidget); + container.invalidate(threadRepliesProvider(args)); + await tester.pump(); + expect(find.text('1 reply'), findsOneWidget); + refresh.completeError(Exception('Transient refresh failure')); + await tester.pump(); + for (var frame = 0; frame < 20; frame++) { + await tester.pump(const Duration(milliseconds: 16)); + expect(find.text('0 replies'), findsNothing, reason: 'frame $frame'); + expect( + find.byKey(const ValueKey('thread-message-group-cached-reply')), + findsOneWidget, + ); + expect(container.read(threadRepliesProvider(args)).value, [reply]); + } + expect(calls, 3); + retry.complete([ + reply, + _textMsg( + id: 'fresh-reply', + pubkey: 'bob', + content: 'Fresh reply', + createdAt: 1002, + extraTags: [ + ['e', args.rootId, '', 'reply'], + ], + ), + ]); + await tester.pumpAndSettle(); + expect(find.text('2 replies'), findsOneWidget); + expect( + find.byKey(const ValueKey('thread-message-group-fresh-reply')), + findsOneWidget, + ); + }, + ); + + testWidgets('thread refresh error keeps cached replies visible', ( + tester, + ) async { + final semantics = tester.ensureSemantics(); + var calls = 0; + final refresh = Completer>(); + tester.view.physicalSize = const Size(320, 844); + tester.view.devicePixelRatio = 1; + addTearDown(tester.view.resetPhysicalSize); + addTearDown(tester.view.resetDevicePixelRatio); + final (navigator, container) = await mount( + tester, + () => ++calls == 1 ? Future.value([reply]) : refresh.future, + textScaler: const TextScaler.linear(2), + ); + open(navigator); + await tester.pumpAndSettle(); + container.invalidate(threadRepliesProvider(args)); + await tester.pump(); + refresh.completeError(Exception('Refresh failed')); + await tester.pumpAndSettle(); + expect( + find.byKey(const ValueKey('thread-message-group-cached-reply')), + findsOneWidget, + ); + expect(find.text('0 replies'), findsNothing); + expect(find.text('1 reply · Couldn’t refresh'), findsOneWidget); + expect( + tester + .getSemantics(find.text('1 reply · Couldn’t refresh')) + .getSemanticsData() + .flagsCollection + .isLiveRegion, + isTrue, + ); + expect(tester.takeException(), isNull); + expect(container.read(threadRepliesProvider(args)).hasError, isTrue); + semantics.dispose(); + }); + + testWidgets('thread reopen shows loading until the fresh query completes', ( + tester, + ) async { + var calls = 0; + final reopened = Completer>(); + final (navigator, _) = await mount( + tester, + () => ++calls == 1 ? Future.value([reply]) : reopened.future, + ); + open(navigator); + await tester.pumpAndSettle(); + navigator.pop(); + await tester.pumpAndSettle(); + open(navigator); + await tester.pumpAndSettle(); + expect(calls, 2); + expect(find.text('0 replies'), findsNothing); + expect(find.text('Loading replies…'), findsOneWidget); + reopened.complete([reply]); + await tester.pumpAndSettle(); + expect(find.text('1 reply'), findsOneWidget); + expect(find.text('Loading replies…'), findsNothing); + }); + + testWidgets('thread initial error is distinct from an empty thread', ( + tester, + ) async { + final query = Completer>(); + final (navigator, _) = await mount(tester, () => query.future); + open(navigator); + await tester.pumpAndSettle(); + query.completeError(Exception('Initial load failed')); + await tester.pumpAndSettle(); + expect(find.text('0 replies'), findsNothing); + expect(find.text('Couldn’t load replies'), findsOneWidget); + }); + testWidgets('thread successful empty query displays zero replies', ( + tester, + ) async { + final (navigator, _) = await mount(tester, () async => []); + open(navigator); + await tester.pumpAndSettle(); + expect(find.text('0 replies'), findsOneWidget); + expect(find.text('Loading replies…'), findsNothing); + expect(find.text('Couldn’t load replies'), findsNothing); + }); +}