-
Notifications
You must be signed in to change notification settings - Fork 308
@-mention markers #509
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
@-mention markers #509
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -94,18 +94,22 @@ class _InboxPageState extends State<InboxPage> with PerAccountStoreAwareStateMix | |
// TODO efficiently include DM conversations that aren't recent enough | ||
// to appear in recentDmConversationsView, but still have unreads in | ||
// unreadsModel. | ||
final dmItems = <(DmNarrow, int)>[]; | ||
final dmItems = <(DmNarrow, int, bool)>[]; | ||
int allDmsCount = 0; | ||
bool allDmsHasMention = false; | ||
for (final dmNarrow in recentDmConversationsModel!.sorted) { | ||
final countInNarrow = unreadsModel!.countInDmNarrow(dmNarrow); | ||
if (countInNarrow == 0) { | ||
continue; | ||
} | ||
dmItems.add((dmNarrow, countInNarrow)); | ||
final hasMention = unreadsModel!.dms[dmNarrow]!.any( | ||
(messageId) => unreadsModel!.mentions.contains(messageId)); | ||
if (hasMention) allDmsHasMention = true; | ||
dmItems.add((dmNarrow, countInNarrow, hasMention)); | ||
allDmsCount += countInNarrow; | ||
} | ||
if (allDmsCount > 0) { | ||
sections.add(_AllDmsSectionData(allDmsCount, dmItems)); | ||
sections.add(_AllDmsSectionData(allDmsCount, allDmsHasMention, dmItems)); | ||
} | ||
|
||
final sortedUnreadStreams = unreadsModel!.streams.entries | ||
|
@@ -132,23 +136,26 @@ class _InboxPageState extends State<InboxPage> with PerAccountStoreAwareStateMix | |
}); | ||
|
||
for (final MapEntry(key: streamId, value: topics) in sortedUnreadStreams) { | ||
final topicItems = <(String, int, int)>[]; | ||
final topicItems = <(String, int, bool, int)>[]; | ||
int countInStream = 0; | ||
bool streamHasMention = false; | ||
for (final MapEntry(key: topic, value: messageIds) in topics.entries) { | ||
if (!store.isTopicVisible(streamId, topic)) continue; | ||
final countInTopic = messageIds.length; | ||
topicItems.add((topic, countInTopic, messageIds.last)); | ||
final hasMention = messageIds.any((messageId) => unreadsModel!.mentions.contains(messageId)); | ||
if (hasMention) streamHasMention = true; | ||
topicItems.add((topic, countInTopic, hasMention, messageIds.last)); | ||
countInStream += countInTopic; | ||
} | ||
if (countInStream == 0) { | ||
continue; | ||
} | ||
topicItems.sort((a, b) { | ||
final (_, _, aLastUnreadId) = a; | ||
final (_, _, bLastUnreadId) = b; | ||
final (_, _, _, aLastUnreadId) = a; | ||
final (_, _, _, bLastUnreadId) = b; | ||
return bLastUnreadId.compareTo(aLastUnreadId); | ||
}); | ||
sections.add(_StreamSectionData(streamId, countInStream, topicItems)); | ||
sections.add(_StreamSectionData(streamId, countInStream, streamHasMention, topicItems)); | ||
} | ||
|
||
return Scaffold( | ||
|
@@ -181,28 +188,32 @@ sealed class _InboxSectionData { | |
|
||
class _AllDmsSectionData extends _InboxSectionData { | ||
final int count; | ||
final List<(DmNarrow, int)> items; | ||
final bool hasMention; | ||
final List<(DmNarrow, int, bool)> items; | ||
|
||
const _AllDmsSectionData(this.count, this.items); | ||
const _AllDmsSectionData(this.count, this.hasMention, this.items); | ||
} | ||
|
||
class _StreamSectionData extends _InboxSectionData { | ||
final int streamId; | ||
final int count; | ||
final List<(String, int, int)> items; | ||
final bool hasMention; | ||
final List<(String, int, bool, int)> items; | ||
|
||
const _StreamSectionData(this.streamId, this.count, this.items); | ||
const _StreamSectionData(this.streamId, this.count, this.hasMention, this.items); | ||
} | ||
|
||
abstract class _HeaderItem extends StatelessWidget { | ||
final bool collapsed; | ||
final _InboxPageState pageState; | ||
final int count; | ||
final bool hasMention; | ||
|
||
const _HeaderItem({ | ||
required this.collapsed, | ||
required this.pageState, | ||
required this.count, | ||
required this.hasMention, | ||
}); | ||
|
||
String get title; | ||
|
@@ -246,7 +257,7 @@ abstract class _HeaderItem extends StatelessWidget { | |
overflow: TextOverflow.ellipsis, | ||
title))), | ||
const SizedBox(width: 12), | ||
// TODO(#384) for streams, show @-mention indicator when it applies | ||
if (hasMention) const _AtMentionMarker(), | ||
Padding(padding: const EdgeInsetsDirectional.only(end: 16), | ||
child: UnreadCountBadge(backgroundColor: unreadCountBadgeBackgroundColor, bold: true, | ||
count: count)), | ||
|
@@ -259,6 +270,7 @@ class _AllDmsHeaderItem extends _HeaderItem { | |
required super.collapsed, | ||
required super.pageState, | ||
required super.count, | ||
required super.hasMention, | ||
}); | ||
|
||
@override get title => 'Direct messages'; // TODO(i18n) | ||
|
@@ -289,6 +301,7 @@ class _AllDmsSection extends StatelessWidget { | |
Widget build(BuildContext context) { | ||
final header = _AllDmsHeaderItem( | ||
count: data.count, | ||
hasMention: data.hasMention, | ||
collapsed: collapsed, | ||
pageState: pageState, | ||
); | ||
|
@@ -297,10 +310,11 @@ class _AllDmsSection extends StatelessWidget { | |
child: Column(children: [ | ||
header, | ||
if (!collapsed) ...data.items.map((item) { | ||
final (narrow, count) = item; | ||
final (narrow, count, hasMention) = item; | ||
return _DmItem( | ||
narrow: narrow, | ||
count: count, | ||
hasMention: hasMention, | ||
); | ||
}), | ||
])); | ||
|
@@ -311,10 +325,12 @@ class _DmItem extends StatelessWidget { | |
const _DmItem({ | ||
required this.narrow, | ||
required this.count, | ||
required this.hasMention, | ||
}); | ||
|
||
final DmNarrow narrow; | ||
final int count; | ||
final bool hasMention; | ||
|
||
@override | ||
Widget build(BuildContext context) { | ||
|
@@ -353,6 +369,7 @@ class _DmItem extends StatelessWidget { | |
overflow: TextOverflow.ellipsis, | ||
title))), | ||
const SizedBox(width: 12), | ||
if (hasMention) const _AtMentionMarker(), | ||
Padding(padding: const EdgeInsetsDirectional.only(end: 16), | ||
child: UnreadCountBadge(backgroundColor: null, | ||
count: count)), | ||
|
@@ -368,6 +385,7 @@ class _StreamHeaderItem extends _HeaderItem { | |
required super.collapsed, | ||
required super.pageState, | ||
required super.count, | ||
required super.hasMention, | ||
}); | ||
|
||
@override get title => subscription.name; | ||
|
@@ -406,6 +424,7 @@ class _StreamSection extends StatelessWidget { | |
final header = _StreamHeaderItem( | ||
subscription: subscription, | ||
count: data.count, | ||
hasMention: data.hasMention, | ||
collapsed: collapsed, | ||
pageState: pageState, | ||
); | ||
|
@@ -414,11 +433,12 @@ class _StreamSection extends StatelessWidget { | |
child: Column(children: [ | ||
header, | ||
if (!collapsed) ...data.items.map((item) { | ||
final (topic, count, _) = item; | ||
final (topic, count, hasMention, _) = item; | ||
return _TopicItem( | ||
streamId: data.streamId, | ||
topic: topic, | ||
count: count, | ||
hasMention: hasMention, | ||
); | ||
}), | ||
])); | ||
|
@@ -430,11 +450,13 @@ class _TopicItem extends StatelessWidget { | |
required this.streamId, | ||
required this.topic, | ||
required this.count, | ||
required this.hasMention, | ||
}); | ||
|
||
final int streamId; | ||
final String topic; | ||
final int count; | ||
final bool hasMention; | ||
|
||
@override | ||
Widget build(BuildContext context) { | ||
|
@@ -464,10 +486,25 @@ class _TopicItem extends StatelessWidget { | |
overflow: TextOverflow.ellipsis, | ||
topic))), | ||
const SizedBox(width: 12), | ||
// TODO(#384) show @-mention indicator when it applies | ||
if (hasMention) const _AtMentionMarker(), | ||
Padding(padding: const EdgeInsetsDirectional.only(end: 16), | ||
child: UnreadCountBadge(backgroundColor: subscription.colorSwatch(), | ||
count: count)), | ||
])))); | ||
} | ||
} | ||
|
||
class _AtMentionMarker extends StatelessWidget { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These two commits are best squashed together: because the latter basically amounts to rethinking how to arrange some of the code added by the former, and so it's simplest to go straight to the revised way of arranging it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In fact let's squash those together with the next commit, handling streams: That way there isn't an intermediate state where we have @-mention markers on the individual topics but missing on the streams, which feels a little glitchy given expectations set by web. And conversely the squashed commit is still a quite reasonable size. |
||
const _AtMentionMarker(); | ||
|
||
static final markerColor = const HSLColor.fromAHSL(0.5, 0, 0, 0.2).toColor(); | ||
|
||
@override | ||
Widget build(BuildContext context) { | ||
// Design for at-mention marker based on Figma screen: | ||
// https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?type=design&node-id=224-16386&mode=design&t=JsNndFQ8fKFH0SjS-0 | ||
return Padding( | ||
padding: const EdgeInsetsDirectional.only(end: 4), | ||
child: Icon(ZulipIcons.at_sign, size: 14, color: markerColor)); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit in commit message:
Should say "topics and streams", since the same commit does it for streams too.