Skip to content

Commit e81f84e

Browse files
committed
ui: Provide [StreamColorSwatch]es via DesignVariables, not api/model/
Instead of putting them in Subscription model objects. This helps toward zulip#95 by bundling this with our other ThemeExtensions that will switch together between light and dark variants when the theme changes. It's also just nicer to separate this very UI-focused code out of api/model/ (zulip#393). Not marking [nfc] just because of differences in caching logic, although I don't expect a user-visible perf effect. Related: zulip#95 Fixes: zulip#393
1 parent 7fcdd69 commit e81f84e

12 files changed

+210
-51
lines changed

lib/api/model/model.dart

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
import 'package:flutter/foundation.dart';
21
import 'package:json_annotation/json_annotation.dart';
32

4-
import '../../widgets/stream_colors.dart';
53
import 'events.dart';
64
import 'initial_snapshot.dart';
75
import 'reaction.dart';
@@ -403,29 +401,13 @@ class Subscription extends ZulipStream {
403401
/// As an int that dart:ui's Color constructor will take:
404402
/// <https://api.flutter.dev/flutter/dart-ui/Color/Color.html>
405403
@JsonKey(readValue: _readColor)
406-
int get color => _color;
407-
int _color;
408-
set color(int value) {
409-
_color = value;
410-
_swatch = null;
411-
}
404+
int color;
412405
static Object? _readColor(Map<dynamic, dynamic> json, String key) {
413406
final str = (json[key] as String);
414407
assert(RegExp(r'^#[0-9a-f]{6}$').hasMatch(str));
415408
return 0xff000000 | int.parse(str.substring(1), radix: 16);
416409
}
417410

418-
StreamColorSwatch? _swatch;
419-
/// A [StreamColorSwatch] for the subscription, memoized.
420-
// TODO I'm not sure this is the right home for this; it seems like we might
421-
// instead have chosen to put it in more UI-centered code, like in a custom
422-
// material [ColorScheme] class or something. But it works for now.
423-
StreamColorSwatch colorSwatch() => _swatch ??= StreamColorSwatch.light(color);
424-
425-
@visibleForTesting
426-
@JsonKey(includeToJson: false)
427-
StreamColorSwatch? get debugCachedSwatchValue => _swatch;
428-
429411
Subscription({
430412
required super.streamId,
431413
required super.name,
@@ -447,8 +429,8 @@ class Subscription extends ZulipStream {
447429
required this.audibleNotifications,
448430
required this.pinToTop,
449431
required this.isMuted,
450-
required int color,
451-
}) : _color = color;
432+
required this.color,
433+
});
452434

453435
factory Subscription.fromJson(Map<String, dynamic> json) =>
454436
_$SubscriptionFromJson(json);

lib/widgets/inbox.dart

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import 'page.dart';
1010
import 'sticky_header.dart';
1111
import 'store.dart';
1212
import 'text.dart';
13+
import 'theme.dart';
1314
import 'unread_count_badge.dart';
1415

1516
class InboxPage extends StatefulWidget {
@@ -423,12 +424,14 @@ class _StreamHeaderItem extends _HeaderItem {
423424

424425
@override String get title => subscription.name;
425426
@override IconData get icon => iconDataForStream(subscription);
426-
@override Color collapsedIconColor(context) => subscription.colorSwatch().iconOnPlainBackground;
427-
@override Color uncollapsedIconColor(context) => subscription.colorSwatch().iconOnBarBackground;
427+
@override Color collapsedIconColor(context) =>
428+
colorSwatchFor(context, subscription).iconOnPlainBackground;
429+
@override Color uncollapsedIconColor(context) =>
430+
colorSwatchFor(context, subscription).iconOnBarBackground;
428431
@override Color uncollapsedBackgroundColor(context) =>
429-
subscription.colorSwatch().barBackground;
432+
colorSwatchFor(context, subscription).barBackground;
430433
@override Color? unreadCountBadgeBackgroundColor(context) =>
431-
subscription.colorSwatch().unreadCountBadgeBackground;
434+
colorSwatchFor(context, subscription).unreadCountBadgeBackground;
432435

433436
@override Future<void> onCollapseButtonTap() async {
434437
await super.onCollapseButtonTap();
@@ -525,7 +528,8 @@ class _TopicItem extends StatelessWidget {
525528
const SizedBox(width: 12),
526529
if (hasMention) const _AtMentionMarker(),
527530
Padding(padding: const EdgeInsetsDirectional.only(end: 16),
528-
child: UnreadCountBadge(backgroundColor: subscription.colorSwatch(),
531+
child: UnreadCountBadge(
532+
backgroundColor: colorSwatchFor(context, subscription),
529533
count: count)),
530534
]))));
531535
}

lib/widgets/message_list.dart

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import 'profile.dart';
2222
import 'sticky_header.dart';
2323
import 'store.dart';
2424
import 'text.dart';
25+
import 'theme.dart';
2526

2627
class MessageListPage extends StatefulWidget {
2728
const MessageListPage({super.key, required this.narrow});
@@ -67,8 +68,9 @@ class _MessageListPageState extends State<MessageListPage> {
6768
case StreamNarrow(:final streamId):
6869
case TopicNarrow(:final streamId):
6970
final subscription = store.subscriptions[streamId];
70-
appBarBackgroundColor = subscription?.colorSwatch().barBackground
71-
?? _kUnsubscribedStreamRecipientHeaderColor;
71+
appBarBackgroundColor = subscription != null
72+
? colorSwatchFor(context, subscription).barBackground
73+
: _kUnsubscribedStreamRecipientHeaderColor;
7274
// All recipient headers will match this color; remove distracting line
7375
// (but are recipient headers even needed for topic narrows?)
7476
removeAppBarBottomBorder = true;
@@ -664,7 +666,7 @@ class StreamMessageRecipientHeader extends StatelessWidget {
664666
final Color backgroundColor;
665667
final Color iconColor;
666668
if (subscription != null) {
667-
final swatch = subscription.colorSwatch();
669+
final swatch = colorSwatchFor(context, subscription);
668670
backgroundColor = swatch.barBackground;
669671
iconColor = swatch.iconOnBarBackground;
670672
} else {

lib/widgets/stream_colors.dart

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,72 @@ import 'package:flutter_color_models/flutter_color_models.dart';
66
import '../api/model/model.dart';
77
import 'color.dart';
88

9+
/// A provider for [StreamColorSwatch]es that computes and caches them.
10+
abstract class StreamColorSwatches {
11+
static final StreamColorSwatches light = _StreamColorSwatchesLight._();
12+
static final StreamColorSwatches dark = _StreamColorSwatchesDark._();
13+
14+
final Map<int, StreamColorSwatch> _cache = {};
15+
16+
/// Gives the cached [StreamColorSwatch] for a [subscription.color].
17+
StreamColorSwatch forBaseColor(int base) =>
18+
_cache[base] ??= computeForBaseColor(base);
19+
20+
StreamColorSwatch computeForBaseColor(int base);
21+
22+
/// Gives a [StreamColorSwatches], lerped to [other] at [t].
23+
///
24+
/// If [this] and [other] are [identical], returns [this].
25+
///
26+
/// Else returns an instance whose [forBaseColor] will call
27+
/// [this.forBaseColor] and [other.forBaseColor]
28+
/// and return [StreamColorSwatch.lerp]'s result on those.
29+
/// This computation is cached on the instance
30+
/// in order to save work building [t]'s animation frame when there are
31+
/// multiple UI elements using the same [subscription.color].
32+
StreamColorSwatches lerp(StreamColorSwatches? other, double t) {
33+
// This short-circuit helps when [a] and [b]
34+
// are both [StreamColorSwatches.light]
35+
// or both [StreamColorSwatches.dark].
36+
// Empirically, [lerp] is called even when the theme hasn't changed,
37+
// so this is an important optimization.
38+
if (identical(this, other)) {
39+
return this;
40+
}
41+
42+
return _StreamColorSwatchesLerped(this, other, t);
43+
}
44+
}
45+
46+
/// The [StreamColorSwatches] for the light theme.
47+
class _StreamColorSwatchesLight extends StreamColorSwatches {
48+
_StreamColorSwatchesLight._();
49+
50+
@override
51+
StreamColorSwatch computeForBaseColor(int base) => StreamColorSwatch.light(base);
52+
}
53+
54+
/// The [StreamColorSwatches] for the dark theme.
55+
class _StreamColorSwatchesDark extends StreamColorSwatches {
56+
_StreamColorSwatchesDark._();
57+
58+
@override
59+
StreamColorSwatch computeForBaseColor(int base) => StreamColorSwatch.dark(base);
60+
}
61+
62+
class _StreamColorSwatchesLerped extends StreamColorSwatches {
63+
_StreamColorSwatchesLerped(this.a, this.b, this.t);
64+
65+
final StreamColorSwatches a;
66+
final StreamColorSwatches? b;
67+
final double t;
68+
69+
@override
70+
StreamColorSwatch computeForBaseColor(int base) =>
71+
StreamColorSwatch.lerp(a.forBaseColor(base), b?.forBaseColor(base), t)!;
72+
}
73+
74+
975
/// A [ColorSwatch] with colors related to a base stream color.
1076
///
1177
/// Use this in UI code for colors related to [Subscription.color],

lib/widgets/subscription_list.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import 'message_list.dart';
99
import 'page.dart';
1010
import 'store.dart';
1111
import 'text.dart';
12+
import 'theme.dart';
1213
import 'unread_count_badge.dart';
1314

1415
/// Scrollable listing of subscribed streams.
@@ -199,7 +200,7 @@ class SubscriptionItem extends StatelessWidget {
199200

200201
@override
201202
Widget build(BuildContext context) {
202-
final swatch = subscription.colorSwatch();
203+
final swatch = colorSwatchFor(context, subscription);
203204
final hasUnreads = (unreadCount > 0);
204205
return Material(
205206
// TODO(#95) need dark-theme color

lib/widgets/theme.dart

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import 'package:flutter/material.dart';
22

3+
import '../api/model/model.dart';
34
import 'content.dart';
5+
import 'stream_colors.dart';
46
import 'text.dart';
57

68
ThemeData zulipThemeData(BuildContext context) {
@@ -78,14 +80,16 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
7880
bgTopBar = const Color(0xfff5f5f5),
7981
borderBar = const Color(0x33000000),
8082
icon = const Color(0xff666699),
81-
title = const Color(0xff1a1a1a);
83+
title = const Color(0xff1a1a1a),
84+
streamColorSwatches = StreamColorSwatches.light;
8285

8386
DesignVariables._({
8487
required this.bgMain,
8588
required this.bgTopBar,
8689
required this.borderBar,
8790
required this.icon,
8891
required this.title,
92+
required this.streamColorSwatches,
8993
});
9094

9195
/// The [DesignVariables] from the context's active theme.
@@ -104,20 +108,25 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
104108
final Color icon;
105109
final Color title;
106110

111+
// Not exactly from the Figma design, but from Vlad anyway.
112+
final StreamColorSwatches streamColorSwatches;
113+
107114
@override
108115
DesignVariables copyWith({
109116
Color? bgMain,
110117
Color? bgTopBar,
111118
Color? borderBar,
112119
Color? icon,
113120
Color? title,
121+
StreamColorSwatches? streamColorSwatches,
114122
}) {
115123
return DesignVariables._(
116124
bgMain: bgMain ?? this.bgMain,
117125
bgTopBar: bgTopBar ?? this.bgTopBar,
118126
borderBar: borderBar ?? this.borderBar,
119127
icon: icon ?? this.icon,
120128
title: title ?? this.title,
129+
streamColorSwatches: streamColorSwatches ?? this.streamColorSwatches,
121130
);
122131
}
123132

@@ -132,6 +141,15 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
132141
borderBar: Color.lerp(borderBar, other?.borderBar, t)!,
133142
icon: Color.lerp(icon, other?.icon, t)!,
134143
title: Color.lerp(title, other?.title, t)!,
144+
streamColorSwatches: streamColorSwatches.lerp(other?.streamColorSwatches, t),
135145
);
136146
}
137147
}
148+
149+
/// The theme-appropriate [StreamColorSwatch] based on [subscription.color].
150+
///
151+
/// For how this value is cached, see [StreamColorSwatches.forBaseColor].
152+
StreamColorSwatch colorSwatchFor(BuildContext context, Subscription subscription) {
153+
return DesignVariables.of(context)
154+
.streamColorSwatches.forBaseColor(subscription.color);
155+
}

test/api/model/model_test.dart

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
import 'dart:convert';
2-
import 'dart:ui';
32

43
import 'package:checks/checks.dart';
54
import 'package:test/scaffolding.dart';
65
import 'package:zulip/api/model/model.dart';
76

87
import '../../example_data.dart' as eg;
98
import '../../stdlib_checks.dart';
10-
import '../../widgets/stream_colors_checks.dart';
119
import 'model_checks.dart';
1210

1311
void main() {
@@ -117,17 +115,6 @@ void main() {
117115
check(subWithColor('#ffffff').color).equals(0xffffffff);
118116
check(subWithColor('#000000').color).equals(0xff000000);
119117
});
120-
121-
test('colorSwatch caching', () {
122-
final sub = eg.subscription(eg.stream(), color: 0xffffffff);
123-
check(sub.debugCachedSwatchValue).isNull();
124-
sub.colorSwatch();
125-
check(sub.debugCachedSwatchValue).isNotNull().base.equals(const Color(0xffffffff));
126-
sub.color = 0xffff0000;
127-
check(sub.debugCachedSwatchValue).isNull();
128-
sub.colorSwatch();
129-
check(sub.debugCachedSwatchValue).isNotNull().base.equals(const Color(0xffff0000));
130-
});
131118
});
132119

133120
group('Message', () {

test/widgets/inbox_test.dart

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -423,9 +423,10 @@ void main() {
423423
final collapseIcon = findHeaderCollapseIcon(tester, headerRow!);
424424
check(collapseIcon).icon.equals(ZulipIcons.arrow_down);
425425
final streamIcon = findStreamHeaderIcon(tester, streamId);
426-
check(streamIcon).color.equals(subscription.colorSwatch().iconOnBarBackground);
426+
check(streamIcon).color.equals(
427+
StreamColorSwatch.light(subscription.color).iconOnBarBackground);
427428
check(streamHeaderBackgroundColor(tester, streamId))
428-
.isNotNull().equals(subscription.colorSwatch().barBackground);
429+
.isNotNull().equals(StreamColorSwatch.light(subscription.color).barBackground);
429430
check(tester.widgetList(findSectionContent)).isNotEmpty();
430431
}
431432

@@ -445,7 +446,8 @@ void main() {
445446
final collapseIcon = findHeaderCollapseIcon(tester, headerRow!);
446447
check(collapseIcon).icon.equals(ZulipIcons.arrow_right);
447448
final streamIcon = findStreamHeaderIcon(tester, streamId);
448-
check(streamIcon).color.equals(subscription.colorSwatch().iconOnPlainBackground);
449+
check(streamIcon).color.equals(
450+
StreamColorSwatch.light(subscription.color).iconOnPlainBackground);
449451
check(streamHeaderBackgroundColor(tester, streamId))
450452
.isNotNull().equals(Colors.white);
451453
check(tester.widgetList(findSectionContent)).isEmpty();

test/widgets/message_list_test.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import 'package:zulip/widgets/content.dart';
1919
import 'package:zulip/widgets/icons.dart';
2020
import 'package:zulip/widgets/message_list.dart';
2121
import 'package:zulip/widgets/store.dart';
22+
import 'package:zulip/widgets/stream_colors.dart';
2223
import 'package:zulip/widgets/theme.dart';
2324

2425
import '../api/fake_api.dart';
@@ -293,7 +294,7 @@ void main() {
293294

294295
testWidgets('color of recipient header background', (tester) async {
295296
final subscription = eg.subscription(stream, color: Colors.red.value);
296-
final swatch = subscription.colorSwatch();
297+
final swatch = StreamColorSwatch.light(subscription.color);
297298
await setupMessageListPage(tester,
298299
messages: [eg.streamMessage(stream: subscription)],
299300
subscriptions: [subscription]);
@@ -308,7 +309,7 @@ void main() {
308309
testWidgets('color of stream icon', (tester) async {
309310
final stream = eg.stream(isWebPublic: true);
310311
final subscription = eg.subscription(stream, color: Colors.red.value);
311-
final swatch = subscription.colorSwatch();
312+
final swatch = StreamColorSwatch.light(subscription.color);
312313
await setupMessageListPage(tester,
313314
messages: [eg.streamMessage(stream: subscription)],
314315
subscriptions: [subscription]);

0 commit comments

Comments
 (0)