Skip to content

Commit

Permalink
inbox: Add label for archived channels in headers
Browse files Browse the repository at this point in the history
Fixes #800
  • Loading branch information
chimnayajith committed Feb 11, 2025
1 parent 5a5bb5b commit 432b103
Show file tree
Hide file tree
Showing 13 changed files with 173 additions and 22 deletions.
4 changes: 4 additions & 0 deletions assets/l10n/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,10 @@
"@unknownChannelName": {
"description": "Replacement name for channel when it cannot be found in the store."
},
"channelArchivedLabel": "(archived)",
"@channelArchivedLabel": {
"description": "Label shown next to an archived channel's name in headers."
},
"composeBoxTopicHintText": "Topic",
"@composeBoxTopicHintText": {
"description": "Hint text for topic input widget in compose box."
Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,12 @@ abstract class ZulipLocalizations {
/// **'(unknown channel)'**
String get unknownChannelName;

/// Label shown next to an archived channel's name in headers.
///
/// In en, this message translates to:
/// **'(archived)'**
String get channelArchivedLabel;

/// Hint text for topic input widget in compose box.
///
/// In en, this message translates to:
Expand Down
3 changes: 3 additions & 0 deletions lib/generated/l10n/zulip_localizations_ar.dart
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,9 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
@override
String get unknownChannelName => '(unknown channel)';

@override
String get channelArchivedLabel => '(archived)';

@override
String get composeBoxTopicHintText => 'Topic';

Expand Down
3 changes: 3 additions & 0 deletions lib/generated/l10n/zulip_localizations_en.dart
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,9 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
@override
String get unknownChannelName => '(unknown channel)';

@override
String get channelArchivedLabel => '(archived)';

@override
String get composeBoxTopicHintText => 'Topic';

Expand Down
3 changes: 3 additions & 0 deletions lib/generated/l10n/zulip_localizations_ja.dart
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,9 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
@override
String get unknownChannelName => '(unknown channel)';

@override
String get channelArchivedLabel => '(archived)';

@override
String get composeBoxTopicHintText => 'Topic';

Expand Down
3 changes: 3 additions & 0 deletions lib/generated/l10n/zulip_localizations_nb.dart
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,9 @@ class ZulipLocalizationsNb extends ZulipLocalizations {
@override
String get unknownChannelName => '(unknown channel)';

@override
String get channelArchivedLabel => '(archived)';

@override
String get composeBoxTopicHintText => 'Topic';

Expand Down
3 changes: 3 additions & 0 deletions lib/generated/l10n/zulip_localizations_pl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,9 @@ class ZulipLocalizationsPl extends ZulipLocalizations {
@override
String get unknownChannelName => '(nieznany kanał)';

@override
String get channelArchivedLabel => '(archived)';

@override
String get composeBoxTopicHintText => 'Wątek';

Expand Down
3 changes: 3 additions & 0 deletions lib/generated/l10n/zulip_localizations_ru.dart
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,9 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
@override
String get unknownChannelName => '(unknown channel)';

@override
String get channelArchivedLabel => '(archived)';

@override
String get composeBoxTopicHintText => 'Тема';

Expand Down
3 changes: 3 additions & 0 deletions lib/generated/l10n/zulip_localizations_sk.dart
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,9 @@ class ZulipLocalizationsSk extends ZulipLocalizations {
@override
String get unknownChannelName => '(unknown channel)';

@override
String get channelArchivedLabel => '(archived)';

@override
String get composeBoxTopicHintText => 'Topic';

Expand Down
38 changes: 29 additions & 9 deletions lib/widgets/inbox.dart
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ abstract class _HeaderItem extends StatelessWidget {
final _InboxPageState pageState;
final int count;
final bool hasMention;

final bool isArchived;
/// A build context within the [_StreamSection] or [_AllDmsSection].
///
/// Used to ensure the [_StreamSection] or [_AllDmsSection] that encloses the
Expand All @@ -236,6 +236,7 @@ abstract class _HeaderItem extends StatelessWidget {
required this.count,
required this.hasMention,
required this.sectionContext,
this.isArchived = false,
});

String title(ZulipLocalizations zulipLocalizations);
Expand Down Expand Up @@ -284,16 +285,33 @@ abstract class _HeaderItem extends StatelessWidget {
const SizedBox(width: 5),
Expanded(child: Padding(
padding: const EdgeInsets.symmetric(vertical: 4),
child: Text(
style: TextStyle(
fontSize: 17,
height: (20 / 17),
// TODO(design) check if this is the right variable
color: designVariables.labelMenuButton,
).merge(weightVariableTextStyle(context, wght: 600)),
child: RichText(
maxLines: 1,
overflow: TextOverflow.ellipsis,
title(zulipLocalizations)))),
text: TextSpan(
children: [
TextSpan(
text: title(zulipLocalizations),
style: TextStyle(
fontSize: 17,
height: (20 / 17),
color: designVariables.labelMenuButton,
).merge(weightVariableTextStyle(context, wght: 600)),
),
if (isArchived)
TextSpan(
text: ' ${zulipLocalizations.channelArchivedLabel}',
style: TextStyle(
fontSize: 17,
height: (20 / 17),
color: MessageListTheme.of(context).streamRecipientHeaderChevronRight,
fontStyle: FontStyle.italic,
),
),
],
),
),
)),
const SizedBox(width: 12),
if (hasMention) const _IconMarker(icon: ZulipIcons.at_sign),
Padding(padding: const EdgeInsetsDirectional.only(end: 16),
Expand Down Expand Up @@ -441,6 +459,7 @@ class _StreamHeaderItem extends _HeaderItem {
required super.count,
required super.hasMention,
required super.sectionContext,
required super.isArchived,
});

@override String title(ZulipLocalizations zulipLocalizations) =>
Expand Down Expand Up @@ -487,6 +506,7 @@ class _StreamSection extends StatelessWidget {
collapsed: collapsed,
pageState: pageState,
sectionContext: context,
isArchived: subscription.isArchived,
);
return StickyHeaderItem(
header: header,
Expand Down
26 changes: 26 additions & 0 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ class MessageListAppBarTitle extends StatelessWidget {
ZulipStream? stream,
}) {
final zulipLocalizations = ZulipLocalizations.of(context);
final messageListTheme = MessageListTheme.of(context);
// A null [Icon.icon] makes a blank space.
final icon = stream != null ? iconDataForStream(stream) : null;
return Row(
Expand All @@ -351,6 +352,18 @@ class MessageListAppBarTitle extends StatelessWidget {
const SizedBox(width: 4),
Flexible(child: Text(
stream?.name ?? zulipLocalizations.unknownChannelName)),
if (stream?.isArchived ?? false)
Padding(
padding: const EdgeInsets.symmetric(horizontal: 4),
child: Text(
zulipLocalizations.channelArchivedLabel,
style: TextStyle(
fontSize: 18,
color: messageListTheme.streamRecipientHeaderChevronRight,
fontStyle: FontStyle.italic,
),
),
),
]);
}

Expand Down Expand Up @@ -1101,6 +1114,19 @@ class StreamMessageRecipientHeader extends StatelessWidget {
style: recipientHeaderTextStyle(context),
overflow: TextOverflow.ellipsis),
),
if (stream?.isArchived ?? false)
Padding(
padding: const EdgeInsets.symmetric(vertical: 11, horizontal: 4),
child: Text(
zulipLocalizations.channelArchivedLabel,
style: recipientHeaderTextStyle(context).copyWith(
color: messageListTheme.streamRecipientHeaderChevronRight,
fontStyle: FontStyle.italic,
),
overflow: TextOverflow.ellipsis,
maxLines: 1,
),
),
Padding(
// Figma has 5px horizontal padding around an 8px wide icon.
// Icon is 16px wide here so horizontal padding is 1px.
Expand Down
75 changes: 63 additions & 12 deletions test/widgets/inbox_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import 'package:flutter_checks/flutter_checks.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:zulip/api/model/events.dart';
import 'package:zulip/api/model/model.dart';
import 'package:zulip/model/localizations.dart';
import 'package:zulip/model/store.dart';
import 'package:zulip/widgets/color.dart';
import 'package:zulip/widgets/home.dart';
Expand Down Expand Up @@ -133,7 +134,14 @@ void main() {

/// Find the all-DMs header element.
Widget? findAllDmsHeaderRow(WidgetTester tester) {
return findRowByLabel(tester, 'Direct messages');
final finder = find.ancestor(
of: find.byWidgetPredicate((widget) =>
widget is RichText &&
widget.text.toPlainText().contains('Direct messages')
),
matching: find.byType(Row),
);
return finder.evaluate().isNotEmpty ? tester.widget(finder.first) : null;
}

Color? allDmsHeaderBackgroundColor(WidgetTester tester) {
Expand All @@ -149,7 +157,15 @@ void main() {
/// For the given stream ID, find the stream header element.
Widget? findStreamHeaderRow(WidgetTester tester, int streamId) {
final stream = store.streams[streamId]!;
return findRowByLabel(tester, stream.name);
return tester.widget<Row>(
find.ancestor(
of: find.byWidgetPredicate((widget) =>
widget is RichText &&
widget.text.toPlainText().contains(stream.name)
),
matching: find.byType(Row),
).first
);
}

Color? streamHeaderBackgroundColor(WidgetTester tester, int streamId) {
Expand Down Expand Up @@ -259,8 +275,13 @@ void main() {
final subscription = eg.subscription(stream);
const topic = 'lunch';

bool hasAtSign(WidgetTester tester, Widget? parent) =>
hasIcon(tester, parent: parent, icon: ZulipIcons.at_sign);
bool hasAtSign(WidgetTester tester, {required Widget? parent}) {
if (parent == null) return false;
return tester.widgetList(find.descendant(
of: find.byWidget(parent),
matching: find.byIcon(ZulipIcons.at_sign),
)).isNotEmpty;
}

testWidgets('topic with a mention', (tester) async {
await setupPage(tester,
Expand All @@ -269,9 +290,9 @@ void main() {
unreadMessages: [eg.streamMessage(stream: stream, topic: topic,
flags: [MessageFlag.mentioned])]);

check(hasAtSign(tester, findStreamHeaderRow(tester, stream.streamId)))
check(hasAtSign(tester, parent: findStreamHeaderRow(tester, stream.streamId)))
.isTrue();
check(hasAtSign(tester, findRowByLabel(tester, topic))).isTrue();
check(hasAtSign(tester, parent: findRowByLabel(tester, topic))).isTrue();
});

testWidgets('topic without a mention', (tester) async {
Expand All @@ -281,9 +302,9 @@ void main() {
unreadMessages: [eg.streamMessage(stream: stream, topic: topic,
flags: [])]);

check(hasAtSign(tester, findStreamHeaderRow(tester, stream.streamId)))
check(hasAtSign(tester, parent: findStreamHeaderRow(tester, stream.streamId)))
.isFalse();
check(hasAtSign(tester, findRowByLabel(tester, topic))).isFalse();
check(hasAtSign(tester, parent: findRowByLabel(tester, topic))).isFalse();
});

testWidgets('dm with a mention', (tester) async {
Expand All @@ -292,8 +313,8 @@ void main() {
unreadMessages: [eg.dmMessage(from: eg.otherUser, to: [eg.selfUser],
flags: [MessageFlag.mentioned])]);

check(hasAtSign(tester, findAllDmsHeaderRow(tester))).isTrue();
check(hasAtSign(tester, findRowByLabel(tester, eg.otherUser.fullName))).isTrue();
check(hasAtSign(tester, parent: findAllDmsHeaderRow(tester))).isTrue();
check(hasAtSign(tester, parent: findRowByLabel(tester, eg.otherUser.fullName))).isTrue();
});

testWidgets('dm without mention', (tester) async {
Expand All @@ -302,8 +323,8 @@ void main() {
unreadMessages: [eg.dmMessage(from: eg.otherUser, to: [eg.selfUser],
flags: [])]);

check(hasAtSign(tester, findAllDmsHeaderRow(tester))).isFalse();
check(hasAtSign(tester, findRowByLabel(tester, eg.otherUser.fullName))).isFalse();
check(hasAtSign(tester, parent: findAllDmsHeaderRow(tester))).isFalse();
check(hasAtSign(tester, parent: findRowByLabel(tester, eg.otherUser.fullName))).isFalse();
});
});

Expand Down Expand Up @@ -595,6 +616,36 @@ void main() {
check(rectAfterTap).equals(rectBeforeTap);
});

testWidgets('shows archived label for archived streams', (tester) async {
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
final stream = eg.stream(streamId: 1, isArchived: true);
final subscription = eg.subscription(stream);

await setupPage(tester,
streams: [stream],
subscriptions: [subscription],
unreadMessages: [eg.streamMessage(stream: stream)]);
await tester.pumpAndSettle();

final headerRowFinder = findStreamHeaderRow(tester, stream.streamId);
check(headerRowFinder).isNotNull();

final richTextFinder = find.descendant(
of: find.byWidget(headerRowFinder!),
matching: find.byWidgetPredicate((widget) =>
widget is RichText &&
widget.text.toPlainText().contains(stream.name) &&
widget.text.toPlainText().contains(zulipLocalizations.channelArchivedLabel)
),
);
expect(richTextFinder, findsOneWidget);

final richText = tester.widget<RichText>(richTextFinder);
final textSpan = richText.text as TextSpan;
final archivedSpan = textSpan.children![1] as TextSpan;
expect(archivedSpan.style?.fontStyle, FontStyle.italic);
});

// TODO check it remains collapsed even if you scroll far away and back

// TODO check that it's always uncollapsed when it appears after being
Expand Down
25 changes: 24 additions & 1 deletion test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -975,6 +975,29 @@ void main() {
await tester.pump();
check(pushedRoutes).isEmpty();
});

testWidgets('shows archived label for archived streams', (tester) async {
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
final stream = eg.stream(streamId: 1, name: 'stream name', isArchived: true);
final message = eg.streamMessage(stream: stream, topic: 'topic');

await setupMessageListPage(tester,
narrow: const CombinedFeedNarrow(),
messages: [message],
streams: [stream],
subscriptions: [eg.subscription(stream)]);
await tester.pump();

check(findInMessageList('stream name')).length.equals(1);
check(findInMessageList(zulipLocalizations.channelArchivedLabel)).length.equals(1);

final archivedLabelFinder = find.descendant(
of: find.byType(MessageList),
matching: find.text(zulipLocalizations.channelArchivedLabel),
);
final textWidget = tester.widget<Text>(archivedLabelFinder);
expect(textWidget.style?.fontStyle, FontStyle.italic);
});
});

group('DmRecipientHeader', () {
Expand Down Expand Up @@ -1062,7 +1085,7 @@ void main() {
.initNarrow.equals(DmNarrow.withUser(eg.otherUser.userId, selfUserId: eg.selfUser.userId));
await tester.pumpAndSettle();
});

testWidgets('does not navigate on tapping recipient header in DmNarrow', (tester) async {
final pushedRoutes = <Route<void>>[];
final navObserver = TestNavigatorObserver()
Expand Down

0 comments on commit 432b103

Please sign in to comment.