diff --git a/assets/icons/ZulipIcons.ttf b/assets/icons/ZulipIcons.ttf index 31f1ddd0c5..6ca113431c 100644 Binary files a/assets/icons/ZulipIcons.ttf and b/assets/icons/ZulipIcons.ttf differ diff --git a/assets/icons/chevron_right.svg b/assets/icons/chevron_right.svg new file mode 100644 index 0000000000..dc422e86cf --- /dev/null +++ b/assets/icons/chevron_right.svg @@ -0,0 +1,3 @@ + + + diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index d421823742..e91f7c5075 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -81,6 +81,12 @@ class ZulipApp extends StatelessWidget { @override Widget build(BuildContext context) { final theme = ThemeData( + appBarTheme: const AppBarTheme( + // This prevents an elevation change in [AppBar]s so they stop turning + // darker if there is something scrolled underneath it. See docs: + // https://api.flutter.dev/flutter/material/AppBar/elevation.html + scrolledUnderElevation: 0, + ), // This applies Material 3's color system to produce a palette of // appropriately matching and contrasting colors for use in a UI. // The Zulip brand color is a starting point, but doesn't end up as diff --git a/lib/widgets/icons.dart b/lib/widgets/icons.dart index 8f3113e5fb..fe6ddf30cf 100644 --- a/lib/widgets/icons.dart +++ b/lib/widgets/icons.dart @@ -33,35 +33,38 @@ abstract final class ZulipIcons { /// The Zulip custom icon "bot". static const IconData bot = IconData(0xf103, fontFamily: "Zulip Icons"); + /// The Zulip custom icon "chevron_right". + static const IconData chevron_right = IconData(0xf104, fontFamily: "Zulip Icons"); + /// The Zulip custom icon "globe". - static const IconData globe = IconData(0xf104, fontFamily: "Zulip Icons"); + static const IconData globe = IconData(0xf105, fontFamily: "Zulip Icons"); /// The Zulip custom icon "group_dm". - static const IconData group_dm = IconData(0xf105, fontFamily: "Zulip Icons"); + static const IconData group_dm = IconData(0xf106, fontFamily: "Zulip Icons"); /// The Zulip custom icon "hash_sign". - static const IconData hash_sign = IconData(0xf106, fontFamily: "Zulip Icons"); + static const IconData hash_sign = IconData(0xf107, fontFamily: "Zulip Icons"); /// The Zulip custom icon "language". - static const IconData language = IconData(0xf107, fontFamily: "Zulip Icons"); + static const IconData language = IconData(0xf108, fontFamily: "Zulip Icons"); /// The Zulip custom icon "lock". - static const IconData lock = IconData(0xf108, fontFamily: "Zulip Icons"); + static const IconData lock = IconData(0xf109, fontFamily: "Zulip Icons"); /// The Zulip custom icon "mute". - static const IconData mute = IconData(0xf109, fontFamily: "Zulip Icons"); + static const IconData mute = IconData(0xf10a, fontFamily: "Zulip Icons"); /// The Zulip custom icon "read_receipts". - static const IconData read_receipts = IconData(0xf10a, fontFamily: "Zulip Icons"); + static const IconData read_receipts = IconData(0xf10b, fontFamily: "Zulip Icons"); /// The Zulip custom icon "topic". - static const IconData topic = IconData(0xf10b, fontFamily: "Zulip Icons"); + static const IconData topic = IconData(0xf10c, fontFamily: "Zulip Icons"); /// The Zulip custom icon "unmute". - static const IconData unmute = IconData(0xf10c, fontFamily: "Zulip Icons"); + static const IconData unmute = IconData(0xf10d, fontFamily: "Zulip Icons"); /// The Zulip custom icon "user". - static const IconData user = IconData(0xf10d, fontFamily: "Zulip Icons"); + static const IconData user = IconData(0xf10e, fontFamily: "Zulip Icons"); // END GENERATED ICON DATA } diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 4cfc642cce..32d34ca147 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -1,4 +1,5 @@ import 'dart:math'; +import 'dart:ui'; import 'package:collection/collection.dart'; import 'package:flutter/material.dart'; @@ -46,13 +47,30 @@ class MessageListPage extends StatefulWidget { State createState() => _MessageListPageState(); } +const _kFallbackStreamColor = Color(0xfff5f5f5); + class _MessageListPageState extends State { final GlobalKey _composeBoxKey = GlobalKey(); @override Widget build(BuildContext context) { + final store = PerAccountStoreWidget.of(context); + + final Color backgroundColor; + switch(widget.narrow) { + case AllMessagesNarrow(): + backgroundColor = _kFallbackStreamColor; + case StreamNarrow(:final streamId): + case TopicNarrow(:final streamId): + backgroundColor = store.subscriptions[streamId]?.colorSwatch().barBackground + ?? _kFallbackStreamColor; + case DmNarrow(): + backgroundColor = _kFallbackStreamColor; + } + return Scaffold( - appBar: AppBar(title: MessageListAppBarTitle(narrow: widget.narrow)), + appBar: AppBar(title: MessageListAppBarTitle(narrow: widget.narrow), + backgroundColor: backgroundColor), // TODO question for Vlad: for a stream view, should we set // [backgroundColor] based on stream color, as in this frame: // https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=132%3A9684&mode=dev @@ -324,12 +342,14 @@ class _MessageListState extends State with PerAccountStoreAwareStat padding: EdgeInsets.symmetric(vertical: 16.0), child: CircularProgressIndicator())); // TODO perhaps a different indicator case MessageListRecipientHeaderItem(): - final header = RecipientHeader(message: data.message); + final header = RecipientHeader(message: data.message, narrow: widget.narrow); return StickyHeaderItem(allowOverflow: true, header: header, child: header); case MessageListMessageItem(): + final header = RecipientHeader(message: data.message, narrow: widget.narrow); return MessageItem( key: ValueKey(data.message.id), + header: header, trailingWhitespace: i == 1 ? 8 : 11, item: data); } @@ -428,16 +448,17 @@ class MarkAsReadWidget extends StatelessWidget { } class RecipientHeader extends StatelessWidget { - const RecipientHeader({super.key, required this.message}); + const RecipientHeader({super.key, required this.message, required this.narrow}); final Message message; + final Narrow narrow; @override Widget build(BuildContext context) { - // TODO recipient headings depend on narrow final message = this.message; return switch (message) { - StreamMessage() => StreamTopicRecipientHeader(message: message), + StreamMessage() => StreamMessageRecipientHeader(message: message, + showStream: narrow is AllMessagesNarrow), DmMessage() => DmRecipientHeader(message: message), }; } @@ -447,10 +468,12 @@ class MessageItem extends StatelessWidget { const MessageItem({ super.key, required this.item, + required this.header, this.trailingWhitespace, }); final MessageListMessageItem item; + final Widget header; final double? trailingWhitespace; @override @@ -458,7 +481,7 @@ class MessageItem extends StatelessWidget { final message = item.message; return StickyHeaderItem( allowOverflow: !item.isLastInBlock, - header: RecipientHeader(message: message), + header: header, child: _UnreadMarker( isRead: message.flags.contains(MessageFlag.read), child: ColoredBox( @@ -511,60 +534,117 @@ class _UnreadMarker extends StatelessWidget { } } -class StreamTopicRecipientHeader extends StatelessWidget { - const StreamTopicRecipientHeader({super.key, required this.message}); +class StreamMessageRecipientHeader extends StatelessWidget { + const StreamMessageRecipientHeader({ + super.key, + required this.message, + required this.showStream, + }); final StreamMessage message; + final bool showStream; @override Widget build(BuildContext context) { + // For design specs, see: + // https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=538%3A20849&mode=dev + // https://github.com/zulip/zulip-mobile/issues/5511 final store = PerAccountStoreWidget.of(context); - final stream = store.streams[message.streamId]; - final streamName = stream?.name ?? message.displayRecipient; // TODO(log) if missing final topic = message.subject; final subscription = store.subscriptions[message.streamId]; - final streamColor = Color(subscription?.color ?? 0x00c2c2c2); - final contrastingColor = - ThemeData.estimateBrightnessForColor(streamColor) == Brightness.dark - ? Colors.white - : Colors.black; + final Color backgroundColor; + final Color contrastingColor; + final Color iconColor; + if (subscription != null) { + final swatch = subscription.colorSwatch(); + backgroundColor = swatch.barBackground; + contrastingColor = + (ThemeData.estimateBrightnessForColor(swatch.barBackground) == Brightness.dark) + ? Colors.white + : Colors.black; + iconColor = swatch.iconOnBarBackground; + } else { + backgroundColor = _kFallbackStreamColor; + contrastingColor = Colors.black; + iconColor = Colors.black; + } + final textStyle = TextStyle( + color: contrastingColor, + fontFamily: 'Source Sans 3', + fontSize: 16, + letterSpacing: 0.02 * 16, + height: (18 / 16), + ).merge(weightVariableTextStyle(context, wght: 600, wghtIfPlatformRequestsBold: 900)); + + final Widget streamWidget; + if (!showStream) { + streamWidget = const SizedBox(width: 16); + } else { + final stream = store.streams[message.streamId]; + final streamName = stream?.name ?? message.displayRecipient; // TODO(log) if missing + + streamWidget = GestureDetector( + onTap: () => Navigator.push(context, + MessageListPage.buildRoute(context: context, + narrow: StreamNarrow(message.streamId))), + child: Row( + crossAxisAlignment: CrossAxisAlignment.center, + children: [ + Padding( + // Figma specifies 5px horizontal spacing around an icon that's + // 18x18 and includes 1px padding. The icon SVG is flush with + // the edges, so make it 16x16 with 6px horizontal padding. + // Bottom padding added here to shift icon up to + // match alignment with text visually. + padding: const EdgeInsets.only(left: 6, right: 6, bottom: 3), + child: Icon(size: 16, color: iconColor, + // A null [Icon.icon] makes a blank space. + (stream != null) ? iconDataForStream(stream) : null)), + Padding( + padding: const EdgeInsets.symmetric(vertical: 11), + child: Text(streamName, + style: textStyle, + overflow: TextOverflow.ellipsis), + ), + Padding( + // Figma has 5px horizontal padding around an 8px wide icon. + // Icon is 16px wide here so horizontal padding is 1px. + padding: const EdgeInsets.symmetric(horizontal: 1), + child: Icon(size: 16, + color: contrastingColor.withOpacity(0.6), + ZulipIcons.chevron_right)), + ])); + } return GestureDetector( onTap: () => Navigator.push(context, MessageListPage.buildRoute(context: context, narrow: TopicNarrow.ofMessage(message))), child: ColoredBox( - color: _kStreamMessageBorderColor, - child: Row(mainAxisAlignment: MainAxisAlignment.start, children: [ - // TODO(#282): Long stream name will break layout; find a fix. - GestureDetector( - onTap: () => Navigator.push(context, - MessageListPage.buildRoute(context: context, - narrow: StreamNarrow(message.streamId))), - child: RecipientHeaderChevronContainer( - color: streamColor, - // TODO globe/lock icons for web-public and private streams - child: Text(streamName, style: TextStyle(color: contrastingColor)))), - Expanded( - child: Padding( - // Web has padding 9, 3, 3, 2 here; but 5px is the chevron. - padding: const EdgeInsets.fromLTRB(4, 3, 3, 2), - child: Text(topic, - // TODO: Give a way to see the whole topic (maybe a - // long-press interaction?) - overflow: TextOverflow.ellipsis, - style: const TextStyle(fontWeight: FontWeight.w600)))), - // TODO topic links? - // Then web also has edit/resolve/mute buttons. Skip those for mobile. - RecipientHeaderDate(message: message), - ]))); + color: backgroundColor, + child: Row( + crossAxisAlignment: CrossAxisAlignment.center, + children: [ + // TODO(#282): Long stream name will break layout; find a fix. + streamWidget, + Expanded( + child: Padding( + padding: const EdgeInsets.symmetric(vertical: 11), + child: Text(topic, + // TODO: Give a way to see the whole topic (maybe a + // long-press interaction?) + overflow: TextOverflow.ellipsis, + style: textStyle))), + // TODO topic links? + // Then web also has edit/resolve/mute buttons. Skip those for mobile. + RecipientHeaderDate(message: message, + color: contrastingColor.withOpacity(0.4)), + ]))); } } -final _kStreamMessageBorderColor = const HSLColor.fromAHSL(1, 0, 0, 0.88).toColor(); - class DmRecipientHeader extends StatelessWidget { const DmRecipientHeader({super.key, required this.message}); @@ -599,7 +679,8 @@ class DmRecipientHeader extends StatelessWidget { color: _kDmRecipientHeaderColor, child: Text(style: const TextStyle(color: Colors.white), title)), - RecipientHeaderDate(message: message), + RecipientHeaderDate(message: message, + color: _kRecipientHeaderDateColor), ]))); } } @@ -607,25 +688,34 @@ class DmRecipientHeader extends StatelessWidget { final _kDmRecipientHeaderColor = const HSLColor.fromAHSL(1, 0, 0, 0.27).toColor(); class RecipientHeaderDate extends StatelessWidget { - const RecipientHeaderDate({super.key, required this.message}); + const RecipientHeaderDate({super.key, required this.message, required this.color}); final Message message; + final Color color; @override Widget build(BuildContext context) { return Padding( - padding: const EdgeInsets.fromLTRB(10, 0, 10, 0), + padding: const EdgeInsets.fromLTRB(10, 0, 16, 0), child: Text( - style: _kRecipientHeaderDateStyle, + style: TextStyle( + color: color, + fontFamily: 'Source Sans 3', + fontSize: 16, + // In Figma this has a line-height of 19, but using 18 + // here to match the stream/topic text widgets helps + // to align all the text to the same baseline. + height: (18 / 16), + // This is equivalent to css `all-small-caps`, see: + // https://developer.mozilla.org/en-US/docs/Web/CSS/font-variant-caps#all-small-caps + fontFeatures: const [FontFeature.enable('c2sc'), FontFeature.enable('smcp')], + ).merge(weightVariableTextStyle(context)), _kRecipientHeaderDateFormat.format( DateTime.fromMillisecondsSinceEpoch(message.timestamp * 1000)))); } } -final _kRecipientHeaderDateStyle = TextStyle( - fontWeight: FontWeight.w600, - color: const HSLColor.fromAHSL(0.75, 0, 0, 0.15).toColor(), -); +final _kRecipientHeaderDateColor = const HSLColor.fromAHSL(0.75, 0, 0, 0.15).toColor(); final _kRecipientHeaderDateFormat = DateFormat('y-MM-dd', 'en_US'); // TODO(#278) @@ -648,7 +738,7 @@ class RecipientHeaderChevronContainer extends StatelessWidget { decoration: ShapeDecoration(color: color, shape: recipientBorderShape), padding: const EdgeInsets.only(right: chevronLength), child: Padding( - padding: const EdgeInsets.fromLTRB(6, 4, 6, 3), child: child)); + padding: const EdgeInsets.fromLTRB(16, 4, 6, 3), child: child)); } } @@ -699,7 +789,7 @@ class MessageWithPossibleSender extends StatelessWidget { ])), Container( width: 80, - padding: const EdgeInsets.only(top: 4, right: 2), + padding: const EdgeInsets.only(top: 4, right: 16 - 8), alignment: Alignment.topRight, child: Text(time, style: _kMessageTimestampStyle)), ]))); diff --git a/test/flutter_checks.dart b/test/flutter_checks.dart index 7e24ed4782..92e62e04a0 100644 --- a/test/flutter_checks.dart +++ b/test/flutter_checks.dart @@ -16,6 +16,10 @@ extension ClipboardDataChecks on Subject { Subject get text => has((d) => d.text, 'text'); } +extension ColoredBoxChecks on Subject { + Subject get color => has((d) => d.color, 'color'); +} + extension GlobalKeyChecks> on Subject> { Subject get currentContext => has((k) => k.currentContext, 'currentContext'); Subject get currentWidget => has((k) => k.currentWidget, 'currentWidget'); diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index c8555e2b74..048233c24d 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -14,6 +14,7 @@ import 'package:zulip/model/localizations.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/content.dart'; +import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/message_list.dart'; import 'package:zulip/widgets/sticky_header.dart'; import 'package:zulip/widgets/store.dart'; @@ -40,10 +41,14 @@ void main() { bool foundOldest = true, int? messageCount, List? messages, + List? streams, + List? subscriptions, UnreadMessagesSnapshot? unreadMsgs, }) async { addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot(unreadMsgs: unreadMsgs)); + streams ??= subscriptions; + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot( + streams: streams, subscriptions: subscriptions, unreadMsgs: unreadMsgs)); store = await testBinding.globalStore.perAccount(eg.selfAccount.id); connection = store.connection as FakeApiConnection; @@ -194,22 +199,127 @@ void main() { }); group('recipient headers', () { + group('StreamMessageRecipientHeader', () { + final stream = eg.stream(name: 'stream name'); + final message = eg.streamMessage(stream: stream, topic: 'topic name'); + + FinderResult findInMessageList(String text) { + // Stream name shows up in [AppBar] so need to avoid matching that + return find.descendant( + of: find.byType(MessageList), + matching: find.text(text)).evaluate(); + } + + testWidgets('show stream name in AllMessagesNarrow', (tester) async { + await setupMessageListPage(tester, + narrow: const AllMessagesNarrow(), + messages: [message], streams: [stream]); + await tester.pump(); + check(findInMessageList('stream name')).length.equals(1); + check(findInMessageList('topic name')).length.equals(1); + }); + + testWidgets('do not show stream name in StreamNarrow', (tester) async { + await setupMessageListPage(tester, + narrow: StreamNarrow(stream.streamId), + messages: [message], streams: [stream]); + await tester.pump(); + check(findInMessageList('stream name')).length.equals(0); + check(findInMessageList('topic name')).length.equals(1); + }); + + testWidgets('do not show stream name in TopicNarrow', (tester) async { + await setupMessageListPage(tester, + narrow: TopicNarrow.ofMessage(message), + messages: [message], streams: [stream]); + await tester.pump(); + check(findInMessageList('stream name')).length.equals(0); + check(findInMessageList('topic name')).length.equals(1); + }); + + testWidgets('color of recipient header background', (tester) async { + final subscription = eg.subscription(stream, color: Colors.red.value); + final swatch = subscription.colorSwatch(); + await setupMessageListPage(tester, + messages: [eg.streamMessage(stream: subscription)], + subscriptions: [subscription]); + await tester.pump(); + check(tester.widget( + find.descendant( + of: find.byType(StreamMessageRecipientHeader), + matching: find.byType(ColoredBox), + ))).color.equals(swatch.barBackground); + }); + + testWidgets('color of stream icon', (tester) async { + final stream = eg.stream(isWebPublic: true); + final subscription = eg.subscription(stream, color: Colors.red.value); + final swatch = subscription.colorSwatch(); + await setupMessageListPage(tester, + messages: [eg.streamMessage(stream: subscription)], + subscriptions: [subscription]); + await tester.pump(); + check(tester.widget(find.byIcon(ZulipIcons.globe))) + .color.equals(swatch.iconOnBarBackground); + }); + + testWidgets('normal streams show hash icon', (tester) async { + final stream = eg.stream(isWebPublic: false, inviteOnly: false); + await setupMessageListPage(tester, + messages: [eg.streamMessage(stream: stream)], + streams: [stream]); + await tester.pump(); + check(find.descendant( + of: find.byType(StreamMessageRecipientHeader), + matching: find.byIcon(ZulipIcons.hash_sign), + ).evaluate()).length.equals(1); + }); + + testWidgets('public streams show globe icon', (tester) async { + final stream = eg.stream(isWebPublic: true); + await setupMessageListPage(tester, + messages: [eg.streamMessage(stream: stream)], + streams: [stream]); + await tester.pump(); + check(find.descendant( + of: find.byType(StreamMessageRecipientHeader), + matching: find.byIcon(ZulipIcons.globe), + ).evaluate()).length.equals(1); + }); + + testWidgets('private streams show lock icon', (tester) async { + final stream = eg.stream(inviteOnly: true); + await setupMessageListPage(tester, + messages: [eg.streamMessage(stream: stream)], + streams: [stream]); + await tester.pump(); + check(find.descendant( + of: find.byType(StreamMessageRecipientHeader), + matching: find.byIcon(ZulipIcons.lock), + ).evaluate()).length.equals(1); + }); + }); + testWidgets('show stream name from message when stream unknown', (tester) async { // This can perfectly well happen, because message fetches can race // with events. final stream = eg.stream(name: 'stream name'); - await setupMessageListPage(tester, messages: [ - eg.streamMessage(stream: stream), - ]); + await setupMessageListPage(tester, + narrow: const AllMessagesNarrow(), + messages: [ + eg.streamMessage(stream: stream), + ]); await tester.pump(); tester.widget(find.text('stream name')); }); testWidgets('show stream name from stream data when known', (tester) async { final stream = eg.stream(name: 'old stream name'); - await setupMessageListPage(tester, messages: [ - eg.streamMessage(stream: stream), - ]); + await setupMessageListPage(tester, + narrow: const AllMessagesNarrow(), + messages: [ + eg.streamMessage(stream: stream), + ]); // TODO(#182) this test would be more realistic using a StreamUpdateEvent store.handleEvent(StreamCreateEvent(id: stream.streamId, streams: [ ZulipStream.fromJson({