Skip to content

Commit 806ea79

Browse files
committed
theme: Compute colorSwatchFor when subscription is null
Looking into the design requirement a bit more, we initially fall back to this color because we didn't have the logic to generate channel colors: https://chat.zulip.org/#narrow/channel/48-mobile/topic/All.20channels.20screen.20-.20.23F832/near/1904009 This replaces the ad-hoc colors with generated colors based on a fallback base color. The base color we fall back to is still a placeholder. Ideally, we should generate different colors within constraints for the unsubscribed channels (since only subscribed channels have color data from the server), as shown in the Figma design linked in zulip#188. That issue is out-of-scope for this, though. For now, it's sufficient to just remove the ad-hoc color on MessageListTheme. This way we don't have to reuse it in places where colors for unsubscribed channels are needed, namely the topic list.
1 parent 79324c9 commit 806ea79

File tree

3 files changed

+25
-29
lines changed

3 files changed

+25
-29
lines changed

lib/widgets/message_list.dart

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,6 @@ class MessageListTheme extends ThemeExtension<MessageListTheme> {
4343
unreadMarker: const HSLColor.fromAHSL(1, 227, 0.78, 0.59).toColor(),
4444

4545
unreadMarkerGap: Colors.white.withValues(alpha: 0.6),
46-
47-
// TODO(design) this seems ad-hoc; is there a better color?
48-
unsubscribedStreamRecipientHeaderBg: const Color(0xfff5f5f5),
4946
);
5047

5148
static final dark = MessageListTheme._(
@@ -63,9 +60,6 @@ class MessageListTheme extends ThemeExtension<MessageListTheme> {
6360
unreadMarker: const HSLColor.fromAHSL(0.75, 227, 0.78, 0.59).toColor(),
6461

6562
unreadMarkerGap: Colors.transparent,
66-
67-
// TODO(design) this is ad-hoc and untested; is there a better color?
68-
unsubscribedStreamRecipientHeaderBg: const Color(0xff0a0a0a),
6963
);
7064

7165
MessageListTheme._({
@@ -76,7 +70,6 @@ class MessageListTheme extends ThemeExtension<MessageListTheme> {
7670
required this.streamRecipientHeaderChevronRight,
7771
required this.unreadMarker,
7872
required this.unreadMarkerGap,
79-
required this.unsubscribedStreamRecipientHeaderBg,
8073
});
8174

8275
/// The [MessageListTheme] from the context's active theme.
@@ -96,7 +89,6 @@ class MessageListTheme extends ThemeExtension<MessageListTheme> {
9689
final Color streamRecipientHeaderChevronRight;
9790
final Color unreadMarker;
9891
final Color unreadMarkerGap;
99-
final Color unsubscribedStreamRecipientHeaderBg;
10092

10193
@override
10294
MessageListTheme copyWith({
@@ -107,7 +99,6 @@ class MessageListTheme extends ThemeExtension<MessageListTheme> {
10799
Color? streamRecipientHeaderChevronRight,
108100
Color? unreadMarker,
109101
Color? unreadMarkerGap,
110-
Color? unsubscribedStreamRecipientHeaderBg,
111102
}) {
112103
return MessageListTheme._(
113104
bgMessageRegular: bgMessageRegular ?? this.bgMessageRegular,
@@ -117,7 +108,6 @@ class MessageListTheme extends ThemeExtension<MessageListTheme> {
117108
streamRecipientHeaderChevronRight: streamRecipientHeaderChevronRight ?? this.streamRecipientHeaderChevronRight,
118109
unreadMarker: unreadMarker ?? this.unreadMarker,
119110
unreadMarkerGap: unreadMarkerGap ?? this.unreadMarkerGap,
120-
unsubscribedStreamRecipientHeaderBg: unsubscribedStreamRecipientHeaderBg ?? this.unsubscribedStreamRecipientHeaderBg,
121111
);
122112
}
123113

@@ -134,7 +124,6 @@ class MessageListTheme extends ThemeExtension<MessageListTheme> {
134124
streamRecipientHeaderChevronRight: Color.lerp(streamRecipientHeaderChevronRight, other.streamRecipientHeaderChevronRight, t)!,
135125
unreadMarker: Color.lerp(unreadMarker, other.unreadMarker, t)!,
136126
unreadMarkerGap: Color.lerp(unreadMarkerGap, other.unreadMarkerGap, t)!,
137-
unsubscribedStreamRecipientHeaderBg: Color.lerp(unsubscribedStreamRecipientHeaderBg, other.unsubscribedStreamRecipientHeaderBg, t)!,
138127
);
139128
}
140129
}
@@ -225,9 +214,8 @@ class _MessageListPageState extends State<MessageListPage> implements MessageLis
225214
case ChannelNarrow(:final streamId):
226215
case TopicNarrow(:final streamId):
227216
final subscription = store.subscriptions[streamId];
228-
appBarBackgroundColor = subscription != null
229-
? colorSwatchFor(context, subscription).barBackground
230-
: messageListTheme.unsubscribedStreamRecipientHeaderBg;
217+
appBarBackgroundColor =
218+
colorSwatchFor(context, subscription).barBackground;
231219
// All recipient headers will match this color; remove distracting line
232220
// (but are recipient headers even needed for topic narrows?)
233221
removeAppBarBottomBorder = true;
@@ -1045,24 +1033,15 @@ class StreamMessageRecipientHeader extends StatelessWidget {
10451033
// https://github.com/zulip/zulip-mobile/issues/5511
10461034
final store = PerAccountStoreWidget.of(context);
10471035
final designVariables = DesignVariables.of(context);
1036+
final messageListTheme = MessageListTheme.of(context);
10481037
final zulipLocalizations = ZulipLocalizations.of(context);
10491038

10501039
final streamId = message.conversation.streamId;
10511040
final topic = message.conversation.topic;
10521041

1053-
final messageListTheme = MessageListTheme.of(context);
1054-
1055-
final subscription = store.subscriptions[streamId];
1056-
final Color backgroundColor;
1057-
final Color iconColor;
1058-
if (subscription != null) {
1059-
final swatch = colorSwatchFor(context, subscription);
1060-
backgroundColor = swatch.barBackground;
1061-
iconColor = swatch.iconOnBarBackground;
1062-
} else {
1063-
backgroundColor = messageListTheme.unsubscribedStreamRecipientHeaderBg;
1064-
iconColor = designVariables.title;
1065-
}
1042+
final swatch = colorSwatchFor(context, store.subscriptions[streamId]);
1043+
final backgroundColor = swatch.barBackground;
1044+
final iconColor = swatch.iconOnBarBackground;
10661045

10671046
final Widget streamWidget;
10681047
if (!_containsDifferentChannels(narrow)) {

lib/widgets/theme.dart

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -554,10 +554,19 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
554554
}
555555
}
556556

557+
// This is taken from:
558+
// https://github.com/zulip/zulip/blob/b248e2d93/web/src/stream_data.ts#L40
559+
const kDefaultChannelColorSwatchBaseColor = 0xffc2c2c2;
560+
557561
/// The theme-appropriate [ChannelColorSwatch] based on [subscription.color].
558562
///
563+
/// If [subscription] is null, [ChannelColorSwatch] will be based on
564+
/// [kDefaultChannelColorSwatchBaseColor].
565+
///
559566
/// For how this value is cached, see [ChannelColorSwatches.forBaseColor].
560-
ChannelColorSwatch colorSwatchFor(BuildContext context, Subscription subscription) {
567+
// TODO(#188) pick different colors for unsubscribed channels
568+
ChannelColorSwatch colorSwatchFor(BuildContext context, Subscription? subscription) {
561569
return DesignVariables.of(context)
562-
.channelColorSwatches.forBaseColor(subscription.color);
570+
.channelColorSwatches.forBaseColor(
571+
subscription?.color ?? kDefaultChannelColorSwatchBaseColor);
563572
}

test/widgets/theme_test.dart

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,5 +178,13 @@ void main() {
178178
check(colorSwatchFor(element, subscription))
179179
.isSameColorSwatchAs(ChannelColorSwatch.dark(baseColor));
180180
});
181+
182+
testWidgets('fallback to default base color when no subscription', (tester) async {
183+
await tester.pumpWidget(const TestZulipApp());
184+
await tester.pump();
185+
final element = tester.element(find.byType(Placeholder));
186+
check(colorSwatchFor(element, null)).isSameColorSwatchAs(
187+
ChannelColorSwatch.light(kDefaultChannelColorSwatchBaseColor));
188+
});
181189
});
182190
}

0 commit comments

Comments
 (0)