Skip to content

msglist: Colorize channel icon in the app bar, following Figma #1524

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 29, 2025
Merged

msglist: Colorize channel icon in the app bar, following Figma #1524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions lib/widgets/message_list.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,17 +329,26 @@ class MessageListAppBarTitle extends StatelessWidget {
Widget _buildStreamRow(BuildContext context, {
ZulipStream? stream,
}) {
final store = PerAccountStoreWidget.of(context);
final zulipLocalizations = ZulipLocalizations.of(context);

// A null [Icon.icon] makes a blank space.
final icon = stream != null ? iconDataForStream(stream) : null;
IconData? icon;
Color? iconColor;
if (stream != null) {
icon = iconDataForStream(stream);
iconColor = colorSwatchFor(context, store.subscriptions[stream.streamId])
.iconOnBarBackground;
Comment on lines +340 to +341
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we fall back to the default base color kDefaultChannelColorSwatchBaseColor (by passing a possibly null subscription) until we get to fully implementing #188?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's what this code already does; am I reading too quickly? store.subscriptions[stream.streamId] might be null, and if it is, colorSwatchFor uses a swatch based on kDefaultChannelColorSwatchBaseColor.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right. I think I meant the case when stream is null, i.e. we got a "(unknown channel)".

}

return Row(
mainAxisSize: MainAxisSize.min,
// TODO(design): The vertical alignment of the stream privacy icon is a bit ad hoc.
// For screenshots of some experiments, see:
// https://github.com/zulip/zulip-flutter/pull/219#discussion_r1281024746
crossAxisAlignment: CrossAxisAlignment.center,
children: [
Icon(size: 16, icon),
Icon(size: 16, color: iconColor, icon),
const SizedBox(width: 4),
Flexible(child: Text(
stream?.name ?? zulipLocalizations.unknownChannelName)),
Expand Down
31 changes: 31 additions & 0 deletions test/widgets/message_list_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import 'package:zulip/model/message_list.dart';
import 'package:zulip/model/narrow.dart';
import 'package:zulip/model/store.dart';
import 'package:zulip/model/typing_status.dart';
import 'package:zulip/widgets/app_bar.dart';
import 'package:zulip/widgets/autocomplete.dart';
import 'package:zulip/widgets/color.dart';
import 'package:zulip/widgets/compose_box.dart';
Expand Down Expand Up @@ -211,6 +212,36 @@ void main() {
channel.name, eg.defaultRealmEmptyTopicDisplayName);
});

void testChannelIconInChannelRow(IconData expectedIcon, {
required bool isWebPublic,
required bool inviteOnly,
}) {
final description = 'channel icon in channel row; '
'web-public: $isWebPublic, invite-only: $inviteOnly';
testWidgets(description, (tester) async {
final color = 0xff95a5fd;

final channel = eg.stream(isWebPublic: isWebPublic, inviteOnly: inviteOnly);
final subscription = eg.subscription(channel, color: color);

await setupMessageListPage(tester,
narrow: ChannelNarrow(channel.streamId),
streams: [channel],
subscriptions: [subscription],
messages: [eg.streamMessage(stream: channel)]);

final iconElement = tester.element(find.descendant(
of: find.byType(ZulipAppBar),
matching: find.byIcon(expectedIcon)));

check(Theme.brightnessOf(iconElement)).equals(Brightness.light);
check(iconElement.widget as Icon).color.equals(Color(0xff5972fc));
});
}
testChannelIconInChannelRow(ZulipIcons.globe, isWebPublic: true, inviteOnly: false);
testChannelIconInChannelRow(ZulipIcons.lock, isWebPublic: false, inviteOnly: true);
testChannelIconInChannelRow(ZulipIcons.hash_sign, isWebPublic: false, inviteOnly: false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! It's nice to have this tested.


testWidgets('has channel-feed action for topic narrows', (tester) async {
final pushedRoutes = <Route<void>>[];
final navObserver = TestNavigatorObserver()
Expand Down