From b6dd0f5105cbc8d040f4b2ef89ee0dd0721700aa Mon Sep 17 00:00:00 2001 From: kenny lopez Date: Mon, 3 Aug 2026 16:46:09 +0100 Subject: [PATCH] fix mobile tablet review feedback Signed-off-by: kenny lopez --- .../lib/features/activity/activity_page.dart | 2 +- .../channels/channels_page/community.dart | 32 +- .../features/channels/thread_detail_page.dart | 291 +----------------- .../lib/features/channels/thread_message.dart | 290 +++++++++++++++++ .../home/home_page/wide_navigation.dart | 7 +- .../features/activity/activity_page_test.dart | 42 +++ mobile/test/features/home/home_page_test.dart | 60 ++++ 7 files changed, 425 insertions(+), 299 deletions(-) create mode 100644 mobile/lib/features/channels/thread_message.dart diff --git a/mobile/lib/features/activity/activity_page.dart b/mobile/lib/features/activity/activity_page.dart index bd2105027..8bd8bcfb2 100644 --- a/mobile/lib/features/activity/activity_page.dart +++ b/mobile/lib/features/activity/activity_page.dart @@ -124,7 +124,7 @@ class ActivityPage extends HookConsumerWidget { selectedItemTarget.value = null; } return null; - }, [isWideInbox, visibleItemIdsKey]); + }, [isWideInbox, visibleItemIdsKey, filter.value, unreadOnly.value]); final selectedItem = visibleItems.cast().firstWhere( (item) => item?.id == selectedItemId.value, orElse: () => null, diff --git a/mobile/lib/features/channels/channels_page/community.dart b/mobile/lib/features/channels/channels_page/community.dart index 6c6cb6879..08a199035 100644 --- a/mobile/lib/features/channels/channels_page/community.dart +++ b/mobile/lib/features/channels/channels_page/community.dart @@ -2,16 +2,22 @@ part of '../channels_page.dart'; /// Opens the community switcher from either the compact channel header or the /// expanded iPad sidebar identity row. -Future showCommunitySwitcherSheet(BuildContext context) { +Future showCommunitySwitcherSheet( + BuildContext context, { + ValueChanged? onCommunitySwitchStart, +}) { return showModalBottomSheet( context: context, showDragHandle: true, - builder: (_) => const _CommunitySwitcherSheet(), + builder: (_) => + _CommunitySwitcherSheet(onCommunitySwitchStart: onCommunitySwitchStart), ); } class _CommunitySwitcherSheet extends HookConsumerWidget { - const _CommunitySwitcherSheet(); + final ValueChanged? onCommunitySwitchStart; + + const _CommunitySwitcherSheet({this.onCommunitySwitchStart}); @override Widget build(BuildContext context, WidgetRef ref) { @@ -117,11 +123,21 @@ class _CommunitySwitcherSheet extends HookConsumerWidget { : () async { final community = communities[index]; if (community.id != activeId) { - await ref - .read( - communityListProvider.notifier, - ) - .switchCommunity(community.id); + onCommunitySwitchStart?.call( + community.id, + ); + try { + await ref + .read( + communityListProvider.notifier, + ) + .switchCommunity(community.id); + } catch (error) { + debugPrint( + '[CommunitySwitcherSheet] failed to switch community: $error', + ); + onCommunitySwitchStart?.call(null); + } } if (context.mounted) { Navigator.of(context).pop(); diff --git a/mobile/lib/features/channels/thread_detail_page.dart b/mobile/lib/features/channels/thread_detail_page.dart index 9095b9332..5f9a71637 100644 --- a/mobile/lib/features/channels/thread_detail_page.dart +++ b/mobile/lib/features/channels/thread_detail_page.dart @@ -33,6 +33,8 @@ import 'send_message_provider.dart'; import 'small_avatar.dart'; import 'timeline_message.dart'; +part 'thread_message.dart'; + /// Full-screen thread detail page. /// /// Shows the thread head message, direct replies, typing indicators scoped to @@ -710,292 +712,3 @@ class _ThreadTailMetricsObserver with WidgetsBindingObserver { @override void didChangeMetrics() => onMetricsChanged(); } - -class _ThreadMessage extends ConsumerWidget { - final TimelineMessage message; - final Map channelNames; - final String channelId; - final String? currentPubkey; - final bool showAuthor; - final bool isHighlighted; - final List? allMessages; - final bool isMember; - final bool isArchived; - - /// Whether this is the message the thread hangs off, which keeps a standing - /// "+" where replies only get one once they carry a reaction. - final bool isThreadHead; - - const _ThreadMessage({ - required this.message, - required this.channelNames, - required this.channelId, - required this.currentPubkey, - required this.showAuthor, - this.isHighlighted = false, - this.allMessages, - this.isMember = false, - this.isArchived = false, - this.isThreadHead = false, - }); - - @override - Widget build(BuildContext context, WidgetRef ref) { - final isTabletLayout = MediaQuery.sizeOf(context).width >= 840; - final pk = message.pubkey.toLowerCase(); - final profile = - ref.watch(userCacheProvider.select((cache) => cache[pk])) ?? - ref.read(userCacheProvider.notifier).get(pk); - final displayName = profile?.label ?? shortPubkey(message.pubkey); - final canManageMessage = - currentPubkey?.toLowerCase() == pk || - (profile?.ownerPubkey != null && - profile?.ownerPubkey == currentPubkey?.toLowerCase()); - - final userCache = ref.watch(userCacheProvider); - final knownAgentPubkeys = agentPubkeysWithProfileOwners( - knownAgentPubkeys: ref.watch(agentMentionPubkeysProvider(channelId)), - profileOwnedAgentPubkeys: [ - for (final profile in userCache.values) - if (profile.ownerPubkey != null) profile.pubkey, - ], - ); - final mentionNames = {}; - final agentMentionPubkeys = {}; - for (final mpk in message.mentionPubkeys) { - final normalizedPubkey = mpk.toLowerCase(); - final p = userCache[normalizedPubkey]; - if (p?.displayName != null) { - mentionNames[normalizedPubkey] = p!.displayName!; - } - if (knownAgentPubkeys.contains(normalizedPubkey)) { - agentMentionPubkeys.add(normalizedPubkey); - } - } - final resolvedMentionNames = mentionNamesWithDirectoryLabels( - mentionPubkeys: message.mentionPubkeys, - profileMentionNames: mentionNames, - directoryDisplayNames: ref.watch(agentDirectoryDisplayNamesProvider), - agentMentionPubkeys: agentMentionPubkeys, - ); - - return Padding( - // In the tablet's wider thread pane, the first author header follows a - // day divider. Halve that handoff gap so the timestamp sits with the - // message rather than floating below the divider. - padding: EdgeInsets.only( - top: showAuthor ? (isTabletLayout ? Grid.half : Grid.xs) : 0, - ), - child: DecoratedBox( - key: ValueKey('thread-message-${message.id}'), - decoration: BoxDecoration( - color: isHighlighted - ? context.colors.primary.withValues(alpha: 0.12) - : Colors.transparent, - borderRadius: BorderRadius.circular(Radii.md), - ), - child: Material( - color: Colors.transparent, - borderRadius: BorderRadius.circular(Radii.md), - // The media carousel intentionally continues through the list's - // trailing gutter. InkWell still clips its ink to [borderRadius], - // while leaving overflowing message content visible. - clipBehavior: Clip.none, - child: InkWell( - key: ValueKey('thread-message-row-${message.id}'), - borderRadius: BorderRadius.circular(Radii.md), - highlightColor: context.colors.primary.withValues(alpha: 0.1), - onLongPress: () => showMessageActions( - context: context, - ref: ref, - message: message, - channelId: channelId, - canManageMessage: canManageMessage, - allMessages: allMessages, - currentPubkey: currentPubkey, - isMember: isMember, - isArchived: isArchived, - ), - child: Padding( - padding: EdgeInsets.only( - top: showAuthor ? 0 : Grid.xxs, - bottom: showAuthor ? 0 : Grid.xxs, - ), - child: Row( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - if (showAuthor) - GestureDetector( - onTap: () => - showUserProfileSheet(context, message.pubkey), - child: _Avatar(profile: profile, pubkey: message.pubkey), - ) - else - const SizedBox(width: messageAvatarSize), - const SizedBox(width: messageAvatarContentGap), - Expanded( - child: Padding( - padding: EdgeInsets.only(top: showAuthor ? Grid.half : 0), - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - if (showAuthor) - Padding( - padding: const EdgeInsets.only( - bottom: Grid.quarter, - ), - child: Row( - children: [ - Expanded( - child: MessageAuthorMeta( - displayName: displayName, - username: messageUsernameLabel(profile), - timestamp: formatMessageTime( - message.createdAt, - ), - nameColor: context.colors.onSurface, - metadataColor: - context.colors.onSurfaceVariant, - onAuthorTap: () => showUserProfileSheet( - context, - message.pubkey, - ), - displayNameKey: ValueKey( - 'thread-message-author-${message.id}', - ), - usernameKey: ValueKey( - 'thread-message-username-${message.id}', - ), - timestampKey: ValueKey( - 'thread-message-timestamp-${message.id}', - ), - ), - ), - if (message.edited) ...[ - const SizedBox(width: Grid.half), - Text( - '(edited)', - style: context.textTheme.labelSmall - ?.copyWith( - color: - context.colors.onSurfaceVariant, - fontStyle: FontStyle.italic, - ), - ), - ], - ], - ), - ), - MessageContent( - content: message.content, - mentionNames: resolvedMentionNames, - agentMentionPubkeys: agentMentionPubkeys, - channelNames: channelNames, - tags: message.tags, - baseStyle: messageBodyTextStyle.copyWith( - color: context.colors.onSurface, - ), - scaleEmojiOnly: true, - mediaCarouselTrailingOverflow: Grid.gutter, - onMediaReply: allMessages == null - ? null - : () { - if (!context.mounted) return; - Navigator.of(context).push( - MaterialPageRoute( - builder: (_) => ThreadDetailPage( - threadHead: message, - allMessages: allMessages!, - channelId: channelId, - currentPubkey: currentPubkey, - isMember: isMember, - isArchived: isArchived, - ), - ), - ); - }, - onMediaMore: (viewerContext, imageUrl) => - showImageActions( - context: viewerContext, - ref: ref, - message: message, - channelId: channelId, - imageUrl: imageUrl, - canManageMessage: canManageMessage, - onDeleted: () { - if (viewerContext.mounted) { - Navigator.of(viewerContext).maybePop(); - } - }, - ), - onChannelTap: (targetChannelId) { - openChannelLink( - context: context, - ref: ref, - channelId: targetChannelId, - currentChannelId: channelId, - ); - }, - onMentionTap: (pubkey) => - showUserProfileSheet(context, pubkey), - ), - ReactionRow( - messageId: message.id, - reactions: message.reactions, - onToggle: (emoji) => toggleReaction( - ref, - message, - emoji, - channelId: channelId, - ), - showAddButton: - isMember && - !isArchived && - (isThreadHead || message.reactions.isNotEmpty), - onAddReaction: () => showAddReactionPicker( - context: context, - ref: ref, - message: message, - channelId: channelId, - ), - ), - ], - ), - ), - ), - ], - ), - ), - ), - ), - ), - ); - } -} - -class _Avatar extends StatelessWidget { - final UserProfile? profile; - final String pubkey; - - const _Avatar({required this.profile, required this.pubkey}); - - @override - Widget build(BuildContext context) { - final initial = - profile?.initial ?? (pubkey.isNotEmpty ? pubkey[0].toUpperCase() : '?'); - final avatarUrl = profile?.avatarUrl; - - return AvatarImage( - imageUrl: avatarUrl, - radius: messageAvatarSize / 2, - backgroundColor: context.colors.primaryContainer, - fallback: Text( - initial, - style: context.textTheme.labelMedium?.copyWith( - color: context.colors.onPrimaryContainer, - fontWeight: FontWeight.w600, - ), - ), - ); - } -} diff --git a/mobile/lib/features/channels/thread_message.dart b/mobile/lib/features/channels/thread_message.dart new file mode 100644 index 000000000..3a3725ba8 --- /dev/null +++ b/mobile/lib/features/channels/thread_message.dart @@ -0,0 +1,290 @@ +part of 'thread_detail_page.dart'; + +class _ThreadMessage extends ConsumerWidget { + final TimelineMessage message; + final Map channelNames; + final String channelId; + final String? currentPubkey; + final bool showAuthor; + final bool isHighlighted; + final List? allMessages; + final bool isMember; + final bool isArchived; + + /// Whether this is the message the thread hangs off, which keeps a standing + /// "+" where replies only get one once they carry a reaction. + final bool isThreadHead; + + const _ThreadMessage({ + required this.message, + required this.channelNames, + required this.channelId, + required this.currentPubkey, + required this.showAuthor, + this.isHighlighted = false, + this.allMessages, + this.isMember = false, + this.isArchived = false, + this.isThreadHead = false, + }); + + @override + Widget build(BuildContext context, WidgetRef ref) { + final isTabletLayout = MediaQuery.sizeOf(context).width >= 840; + final pk = message.pubkey.toLowerCase(); + final profile = + ref.watch(userCacheProvider.select((cache) => cache[pk])) ?? + ref.read(userCacheProvider.notifier).get(pk); + final displayName = profile?.label ?? shortPubkey(message.pubkey); + final canManageMessage = + currentPubkey?.toLowerCase() == pk || + (profile?.ownerPubkey != null && + profile?.ownerPubkey == currentPubkey?.toLowerCase()); + + final userCache = ref.watch(userCacheProvider); + final knownAgentPubkeys = agentPubkeysWithProfileOwners( + knownAgentPubkeys: ref.watch(agentMentionPubkeysProvider(channelId)), + profileOwnedAgentPubkeys: [ + for (final profile in userCache.values) + if (profile.ownerPubkey != null) profile.pubkey, + ], + ); + final mentionNames = {}; + final agentMentionPubkeys = {}; + for (final mpk in message.mentionPubkeys) { + final normalizedPubkey = mpk.toLowerCase(); + final p = userCache[normalizedPubkey]; + if (p?.displayName != null) { + mentionNames[normalizedPubkey] = p!.displayName!; + } + if (knownAgentPubkeys.contains(normalizedPubkey)) { + agentMentionPubkeys.add(normalizedPubkey); + } + } + final resolvedMentionNames = mentionNamesWithDirectoryLabels( + mentionPubkeys: message.mentionPubkeys, + profileMentionNames: mentionNames, + directoryDisplayNames: ref.watch(agentDirectoryDisplayNamesProvider), + agentMentionPubkeys: agentMentionPubkeys, + ); + + return Padding( + // In the tablet's wider thread pane, the first author header follows a + // day divider. Halve that handoff gap so the timestamp sits with the + // message rather than floating below the divider. + padding: EdgeInsets.only( + top: showAuthor ? (isTabletLayout ? Grid.half : Grid.xs) : 0, + ), + child: DecoratedBox( + key: ValueKey('thread-message-${message.id}'), + decoration: BoxDecoration( + color: isHighlighted + ? context.colors.primary.withValues(alpha: 0.12) + : Colors.transparent, + borderRadius: BorderRadius.circular(Radii.md), + ), + child: Material( + color: Colors.transparent, + borderRadius: BorderRadius.circular(Radii.md), + // The media carousel intentionally continues through the list's + // trailing gutter. InkWell still clips its ink to [borderRadius], + // while leaving overflowing message content visible. + clipBehavior: Clip.none, + child: InkWell( + key: ValueKey('thread-message-row-${message.id}'), + borderRadius: BorderRadius.circular(Radii.md), + highlightColor: context.colors.primary.withValues(alpha: 0.1), + onLongPress: () => showMessageActions( + context: context, + ref: ref, + message: message, + channelId: channelId, + canManageMessage: canManageMessage, + allMessages: allMessages, + currentPubkey: currentPubkey, + isMember: isMember, + isArchived: isArchived, + ), + child: Padding( + padding: EdgeInsets.only( + top: showAuthor ? 0 : Grid.xxs, + bottom: showAuthor ? 0 : Grid.xxs, + ), + child: Row( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + if (showAuthor) + GestureDetector( + onTap: () => + showUserProfileSheet(context, message.pubkey), + child: _Avatar(profile: profile, pubkey: message.pubkey), + ) + else + const SizedBox(width: messageAvatarSize), + const SizedBox(width: messageAvatarContentGap), + Expanded( + child: Padding( + padding: EdgeInsets.only(top: showAuthor ? Grid.half : 0), + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + if (showAuthor) + Padding( + padding: const EdgeInsets.only( + bottom: Grid.quarter, + ), + child: Row( + children: [ + Expanded( + child: MessageAuthorMeta( + displayName: displayName, + username: messageUsernameLabel(profile), + timestamp: formatMessageTime( + message.createdAt, + ), + nameColor: context.colors.onSurface, + metadataColor: + context.colors.onSurfaceVariant, + onAuthorTap: () => showUserProfileSheet( + context, + message.pubkey, + ), + displayNameKey: ValueKey( + 'thread-message-author-${message.id}', + ), + usernameKey: ValueKey( + 'thread-message-username-${message.id}', + ), + timestampKey: ValueKey( + 'thread-message-timestamp-${message.id}', + ), + ), + ), + if (message.edited) ...[ + const SizedBox(width: Grid.half), + Text( + '(edited)', + style: context.textTheme.labelSmall + ?.copyWith( + color: + context.colors.onSurfaceVariant, + fontStyle: FontStyle.italic, + ), + ), + ], + ], + ), + ), + MessageContent( + content: message.content, + mentionNames: resolvedMentionNames, + agentMentionPubkeys: agentMentionPubkeys, + channelNames: channelNames, + tags: message.tags, + baseStyle: messageBodyTextStyle.copyWith( + color: context.colors.onSurface, + ), + scaleEmojiOnly: true, + mediaCarouselTrailingOverflow: Grid.gutter, + onMediaReply: allMessages == null + ? null + : () { + if (!context.mounted) return; + Navigator.of(context).push( + MaterialPageRoute( + builder: (_) => ThreadDetailPage( + threadHead: message, + allMessages: allMessages!, + channelId: channelId, + currentPubkey: currentPubkey, + isMember: isMember, + isArchived: isArchived, + ), + ), + ); + }, + onMediaMore: (viewerContext, imageUrl) => + showImageActions( + context: viewerContext, + ref: ref, + message: message, + channelId: channelId, + imageUrl: imageUrl, + canManageMessage: canManageMessage, + onDeleted: () { + if (viewerContext.mounted) { + Navigator.of(viewerContext).maybePop(); + } + }, + ), + onChannelTap: (targetChannelId) { + openChannelLink( + context: context, + ref: ref, + channelId: targetChannelId, + currentChannelId: channelId, + ); + }, + onMentionTap: (pubkey) => + showUserProfileSheet(context, pubkey), + ), + ReactionRow( + messageId: message.id, + reactions: message.reactions, + onToggle: (emoji) => toggleReaction( + ref, + message, + emoji, + channelId: channelId, + ), + showAddButton: + isMember && + !isArchived && + (isThreadHead || message.reactions.isNotEmpty), + onAddReaction: () => showAddReactionPicker( + context: context, + ref: ref, + message: message, + channelId: channelId, + ), + ), + ], + ), + ), + ), + ], + ), + ), + ), + ), + ), + ); + } +} + +class _Avatar extends StatelessWidget { + final UserProfile? profile; + final String pubkey; + + const _Avatar({required this.profile, required this.pubkey}); + + @override + Widget build(BuildContext context) { + final initial = + profile?.initial ?? (pubkey.isNotEmpty ? pubkey[0].toUpperCase() : '?'); + final avatarUrl = profile?.avatarUrl; + + return AvatarImage( + imageUrl: avatarUrl, + radius: messageAvatarSize / 2, + backgroundColor: context.colors.primaryContainer, + fallback: Text( + initial, + style: context.textTheme.labelMedium?.copyWith( + color: context.colors.onPrimaryContainer, + fontWeight: FontWeight.w600, + ), + ), + ); + } +} diff --git a/mobile/lib/features/home/home_page/wide_navigation.dart b/mobile/lib/features/home/home_page/wide_navigation.dart index 4aa9b94d8..6a8b7aa58 100644 --- a/mobile/lib/features/home/home_page/wide_navigation.dart +++ b/mobile/lib/features/home/home_page/wide_navigation.dart @@ -97,7 +97,12 @@ class _WideNavigationSidebar extends HookConsumerWidget { crossAxisAlignment: CrossAxisAlignment.stretch, children: [ _WideNavigationCommunitySwitcher( - onTap: () => unawaited(showCommunitySwitcherSheet(context)), + onTap: () => unawaited( + showCommunitySwitcherSheet( + context, + onCommunitySwitchStart: onCommunitySwitchStart, + ), + ), onCommunitySwitchStart: onCommunitySwitchStart, ), const SizedBox(height: Grid.xxs), diff --git a/mobile/test/features/activity/activity_page_test.dart b/mobile/test/features/activity/activity_page_test.dart index 66344f21c..fc9904764 100644 --- a/mobile/test/features/activity/activity_page_test.dart +++ b/mobile/test/features/activity/activity_page_test.dart @@ -211,6 +211,48 @@ void main() { ); }); + testWidgets('keeps a wide inbox detail selected across same-row filters', ( + tester, + ) async { + tester.view.physicalSize = const Size(1180, 820); + tester.view.devicePixelRatio = 1; + addTearDown(() { + tester.view.resetPhysicalSize(); + tester.view.resetDevicePixelRatio(); + }); + final secondMention = FeedItem( + id: 'm2', + kind: 9, + pubkey: 'bob_pk', + content: 'Another mention', + createdAt: now - 60, + channelId: 'ch2', + channelName: 'engineering', + tags: const [], + category: 'mention', + ); + + await tester.pumpWidget( + await buildTestable( + feed: HomeFeedResponse( + mentions: [testMention, secondMention], + needsAction: const [], + activity: const [], + agentActivity: const [], + ), + ), + ); + await tester.pumpAndSettle(); + + expect(find.byType(ChannelDetailPage), findsOneWidget); + await tester.tap(find.byKey(const ValueKey('activity-filter-menu'))); + await tester.pumpAndSettle(); + await tester.tap(find.text('Mentions')); + await tester.pumpAndSettle(); + + expect(find.byType(ChannelDetailPage), findsOneWidget); + }); + testWidgets( 'keeps the oldest unread deep link after a wide inbox row is read', (tester) async { diff --git a/mobile/test/features/home/home_page_test.dart b/mobile/test/features/home/home_page_test.dart index fc8cf847c..ad6fa66c7 100644 --- a/mobile/test/features/home/home_page_test.dart +++ b/mobile/test/features/home/home_page_test.dart @@ -658,6 +658,66 @@ void main() { ); }); + testWidgets('shows the workspace skeleton while sheet switching community', ( + tester, + ) async { + tester.view.physicalSize = const Size(1180, 820); + tester.view.devicePixelRatio = 1; + addTearDown(() { + tester.view.resetPhysicalSize(); + tester.view.resetDevicePixelRatio(); + }); + final communities = [ + Community( + id: 'alpha', + name: 'Alpha', + relayUrl: 'wss://alpha.example.com', + addedAt: DateTime(2026), + ), + Community( + id: 'bravo', + name: 'Bravo', + relayUrl: 'wss://bravo.example.com', + addedAt: DateTime(2026), + ), + ]; + final communityListNotifier = _FakeCommunityListNotifier(communities); + + await tester.pumpWidget( + await buildHome( + communityListNotifier: communityListNotifier, + activeCommunity: communities.first, + ), + ); + await tester.pump(); + + await tester.tap( + find.byKey(const Key('wide-navigation-community-switcher')), + ); + await tester.pump(const Duration(milliseconds: 300)); + tester + .widget( + find.ancestor(of: find.text('Bravo'), matching: find.byType(InkWell)), + ) + .onTap!(); + await tester.pump(); + + expect(communityListNotifier.switchedToId, 'bravo'); + expect( + tester + .widget( + find.ancestor( + of: find.byKey( + const Key('wide-navigation-community-switch-skeleton'), + ), + matching: find.byType(SkeletonShimmer), + ), + ) + .enabled, + isTrue, + ); + }); + testWidgets('a failed community swipe restores the workspace', ( tester, ) async {