Skip to content

Commit c98481d

Browse files
chrisbobbechrisbobbe
committed
action_sheet: Add header param; use it to show message content
See Figma: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3483-29966&m=dev See also a different header variant in Figma: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3481-26993&m=dev At first I'd thought this wouldn't be possible without more work toward issue zulip#488: - zulip#488 "content: Support Zulip content outside messages (even outside per-account contexts)". But it turns out that our content widgets don't assume a MessageList ancestor; it's sufficient that we provide a per-account context, via PerAccountStoreWidget, and a per-message context, via InheritedMessage (created by MessageContent). Fixes: zulip#217
1 parent fc2f449 commit c98481d

File tree

2 files changed

+132
-3
lines changed

2 files changed

+132
-3
lines changed

‎lib/widgets/action_sheet.dart‎

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,14 @@ import '../api/route/channels.dart';
1313
import '../api/route/messages.dart';
1414
import '../generated/l10n/zulip_localizations.dart';
1515
import '../model/binding.dart';
16+
import '../model/content.dart';
1617
import '../model/emoji.dart';
1718
import '../model/internal_link.dart';
1819
import '../model/narrow.dart';
1920
import 'actions.dart';
2021
import 'color.dart';
2122
import 'compose_box.dart';
23+
import 'content.dart';
2224
import 'dialog.dart';
2325
import 'emoji.dart';
2426
import 'emoji_reaction.dart';
@@ -33,6 +35,7 @@ import 'topic_list.dart';
3335

3436
void _showActionSheet(
3537
BuildContext pageContext, {
38+
Widget? header,
3639
required List<Widget> optionButtons,
3740
}) {
3841
// Could omit this if we need _showActionSheet outside a per-account context.
@@ -57,15 +60,31 @@ void _showActionSheet(
5760
child: Column(
5861
mainAxisSize: MainAxisSize.min,
5962
children: [
60-
SizedBox(height: 8),
63+
if (header != null)
64+
Flexible(
65+
// TODO(upstream) Enforce a flex ratio (e.g. 1:3)
66+
// only when the header height plus the buttons' height
67+
// exceeds available space. Otherwise let one or the other
68+
// grow to fill available space even if it breaks the ratio.
69+
// Needs support for separate properties like `flex-grow`
70+
// and `flex-shrink`.
71+
flex: 1,
72+
child: InsetShadowBox(
73+
top: 8, bottom: 8,
74+
color: designVariables.bgContextMenu,
75+
child: SingleChildScrollView(
76+
padding: EdgeInsets.symmetric(vertical: 8),
77+
child: header)))
78+
else
79+
SizedBox(height: 8),
6180
Flexible(
81+
flex: 3,
6282
child: Padding(
6383
padding: const EdgeInsets.fromLTRB(16, 0, 16, 0),
6484
child: Column(
6585
crossAxisAlignment: CrossAxisAlignment.stretch,
6686
mainAxisSize: MainAxisSize.min,
6787
children: [
68-
// TODO(#217): show message text
6988
Flexible(child: InsetShadowBox(
7089
top: 8, bottom: 8,
7190
color: designVariables.bgContextMenu,
@@ -621,7 +640,41 @@ void showMessageActionSheet({required BuildContext context, required Message mes
621640
EditButton(message: message, pageContext: pageContext),
622641
];
623642

624-
_showActionSheet(pageContext, optionButtons: optionButtons);
643+
_showActionSheet(pageContext,
644+
optionButtons: optionButtons,
645+
header: _MessageActionSheetHeader(message: message));
646+
}
647+
648+
class _MessageActionSheetHeader extends StatelessWidget {
649+
const _MessageActionSheetHeader({required this.message});
650+
651+
final Message message;
652+
653+
@override
654+
Widget build(BuildContext context) {
655+
final designVariables = DesignVariables.of(context);
656+
657+
// TODO this seems to lose the hero animation when opening an image;
658+
// investigate.
659+
// TODO should we close the sheet before opening a narrow link?
660+
// On popping the pushed narrow route, the sheet is still open.
661+
662+
return Container(
663+
// TODO(#647) use different color for highlighted messages
664+
// TODO(#681) use different color for DM messages
665+
color: designVariables.bgMessageRegular,
666+
padding: EdgeInsets.symmetric(vertical: 4),
667+
child: Column(
668+
spacing: 4,
669+
children: [
670+
SenderRow(message: message, showTimestamp: true),
671+
Padding(
672+
padding: const EdgeInsets.symmetric(horizontal: 16),
673+
// TODO(#10) offer text selection; the Figma asks for it here:
674+
// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3483-30210&m=dev
675+
child: MessageContent(message: message, content: parseMessageContent(message))),
676+
]));
677+
}
625678
}
626679

627680
abstract class MessageActionSheetMenuItemButton extends ActionSheetMenuItemButton {

‎test/widgets/action_sheet_test.dart‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import '../api/fake_api.dart';
3838
import '../example_data.dart' as eg;
3939
import '../flutter_checks.dart';
4040
import '../model/binding.dart';
41+
import '../model/content_test.dart';
4142
import '../model/test_store.dart';
4243
import '../stdlib_checks.dart';
4344
import '../test_clipboard.dart';
@@ -859,6 +860,81 @@ void main() {
859860
});
860861

861862
group('message action sheet', () {
863+
group('header', () {
864+
void checkSenderAndTimestampShown(WidgetTester tester, {required int senderId}) {
865+
check(find.descendant(
866+
of: find.byType(BottomSheet),
867+
matching: find.byWidgetPredicate(
868+
(widget) => widget is Avatar && widget.userId == senderId))
869+
).findsOne();
870+
final expectedTimestampColor = MessageListTheme.of(
871+
tester.element(find.byType(BottomSheet))).labelTime;
872+
// TODO check the timestamp text itself, when it's convenient to do so:
873+
// https://github.com/zulip/zulip-flutter/pull/1624#discussion_r2181383754
874+
check(find.descendant(
875+
of: find.byType(BottomSheet),
876+
matching: find.byWidgetPredicate((widget) =>
877+
widget is Text
878+
&& widget.style?.color == expectedTimestampColor
879+
&& (widget.style?.fontFeatures?.contains(FontFeature.enable('c2sc')) ?? false)))
880+
).findsOne();
881+
}
882+
883+
testWidgets('message sender and content shown', (tester) async {
884+
final message = eg.streamMessage(
885+
timestamp: 1671409088,
886+
content: ContentExample.userMentionPlain.html);
887+
await setupToMessageActionSheet(tester,
888+
message: message,
889+
narrow: TopicNarrow.ofMessage(message));
890+
checkSenderAndTimestampShown(tester, senderId: message.senderId);
891+
check(find.descendant(
892+
of: find.byType(BottomSheet),
893+
matching: find.byType(UserMention))
894+
).findsOne();
895+
});
896+
897+
testWidgets('muted sender', (tester) async {
898+
final message = eg.streamMessage(
899+
timestamp: 1671409088,
900+
content: ContentExample.userMentionPlain.html);
901+
await setupToMessageActionSheet(tester,
902+
message: message,
903+
narrow: TopicNarrow.ofMessage(message),
904+
mutedUserIds: [message.senderId],
905+
beforeLongPress: () async {
906+
check(find.byType(MessageContent)).findsNothing();
907+
await tester.tap(
908+
find.widgetWithText(ZulipWebUiKitButton, 'Reveal message'));
909+
await tester.pump();
910+
check(find.byType(MessageContent)).findsOne();
911+
},
912+
);
913+
checkSenderAndTimestampShown(tester, senderId: message.senderId);
914+
check(find.descendant(
915+
of: find.byType(BottomSheet),
916+
matching: find.byType(UserMention))
917+
).findsOne();
918+
});
919+
920+
testWidgets('poll is rendered', (tester) async {
921+
final submessageContent = eg.pollWidgetData(
922+
question: 'poll', options: ['First option', 'Second option']);
923+
final message = eg.streamMessage(
924+
timestamp: 1671409088,
925+
sender: eg.selfUser,
926+
submessages: [eg.submessage(content: submessageContent)]);
927+
await setupToMessageActionSheet(tester,
928+
message: message,
929+
narrow: TopicNarrow.ofMessage(message));
930+
checkSenderAndTimestampShown(tester, senderId: message.senderId);
931+
check(find.descendant(
932+
of: find.byType(BottomSheet),
933+
matching: find.text('First option'))
934+
).findsOne();
935+
});
936+
});
937+
862938
group('ReactionButtons', () {
863939
testWidgets('absent if ServerEmojiData not loaded', (tester) async {
864940
final message = eg.streamMessage();

0 commit comments

Comments
 (0)