Skip to content

Commit 7a2970e

Browse files
chrisbobbechrisbobbe
committed
model [nfc]: Remove StreamColorSwatch; use ColorSwatch<StreamColorVariant>
This indirection was mildly helpful; it let us consume these colors with slightly more concise code, and without needing `!` to satisfy the analyzer. But we get the same correct functionality without the indirection, and a bit more transparently. The more substantive benefit of using ColorSwatch directly is that we now have a ready-made lerp function -- `ColorSwatch.lerp` -- that can operate on our swatch instances. That should be helpful later when we want stream colors to animate smoothly when dark mode is toggled, along with various other style attributes that will naturally animate too.
1 parent 43b2efd commit 7a2970e

12 files changed

+101
-103
lines changed

‎lib/api/model/model.dart

Lines changed: 55 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -416,16 +416,17 @@ class Subscription extends ZulipStream {
416416
return 0xff000000 | int.parse(str.substring(1), radix: 16);
417417
}
418418

419-
StreamColorSwatch? _swatch;
420-
/// A [StreamColorSwatch] for the subscription, memoized.
419+
ColorSwatch<StreamColorVariant>? _swatch;
420+
/// A [ColorSwatch<StreamColorVariant>] for the subscription, memoized.
421421
// TODO I'm not sure this is the right home for this; it seems like we might
422422
// instead have chosen to put it in more UI-centered code, like in a custom
423423
// material [ColorScheme] class or something. But it works for now.
424-
StreamColorSwatch colorSwatch() => _swatch ??= StreamColorSwatch(color);
424+
ColorSwatch<StreamColorVariant> colorSwatch() =>
425+
_swatch ??= streamColorSwatch(color);
425426

426427
@visibleForTesting
427428
@JsonKey(includeToJson: false)
428-
StreamColorSwatch? get debugCachedSwatchValue => _swatch;
429+
ColorSwatch<StreamColorVariant>? get debugCachedSwatchValue => _swatch;
429430

430431
Subscription({
431432
required super.streamId,
@@ -462,70 +463,58 @@ class Subscription extends ZulipStream {
462463
///
463464
/// Use this in UI code for colors related to [Subscription.color],
464465
/// such as the background of an unread count badge.
465-
class StreamColorSwatch extends ColorSwatch<_StreamColorVariant> {
466-
StreamColorSwatch(int base) : super(base, _compute(base));
467-
468-
Color get base => this[_StreamColorVariant.base]!;
469-
470-
Color get unreadCountBadgeBackground => this[_StreamColorVariant.unreadCountBadgeBackground]!;
471-
472-
Color get iconOnPlainBackground => this[_StreamColorVariant.iconOnPlainBackground]!;
473-
474-
Color get iconOnBarBackground => this[_StreamColorVariant.iconOnBarBackground]!;
475-
476-
Color get barBackground => this[_StreamColorVariant.barBackground]!;
477-
478-
static Map<_StreamColorVariant, Color> _compute(int base) {
479-
final baseAsColor = Color(base);
480-
481-
final clamped20to75 = clampLchLightness(baseAsColor, 20, 75);
482-
final clamped20to75AsHsl = HSLColor.fromColor(clamped20to75);
483-
484-
return {
485-
_StreamColorVariant.base: baseAsColor,
486-
487-
// Follows `.unread-count` in Vlad's replit:
488-
// <https://replit.com/@VladKorobov/zulip-sidebar#script.js>
489-
// <https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/design.3A.20.23F117.20.22Inbox.22.20screen/near/1624484>
490-
//
491-
// TODO fix bug where our results differ from the replit's (see unit tests)
492-
_StreamColorVariant.unreadCountBadgeBackground:
493-
clampLchLightness(baseAsColor, 30, 70)
494-
.withOpacity(0.3),
495-
496-
// Follows `.sidebar-row__icon` in Vlad's replit:
497-
// <https://replit.com/@VladKorobov/zulip-topic-feed-colors#script.js>
498-
//
499-
// TODO fix bug where our results differ from the replit's (see unit tests)
500-
_StreamColorVariant.iconOnPlainBackground: clamped20to75,
501-
502-
// Follows `.recepeient__icon` in Vlad's replit:
503-
// <https://replit.com/@VladKorobov/zulip-topic-feed-colors#script.js>
504-
// <https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/design.3A.20.23F117.20.22Inbox.22.20screen/near/1624484>
505-
//
506-
// TODO fix bug where our results differ from the replit's (see unit tests)
507-
_StreamColorVariant.iconOnBarBackground:
508-
clamped20to75AsHsl
509-
.withLightness(clamped20to75AsHsl.lightness - 0.12)
510-
.toColor(),
511-
512-
// Follows `.recepient` in Vlad's replit:
513-
// <https://replit.com/@VladKorobov/zulip-topic-feed-colors#script.js>
514-
//
515-
// TODO I think [LabColor.interpolate] doesn't actually do LAB mixing;
516-
// it just calls up to the superclass method [ColorModel.interpolate]:
517-
// <https://pub.dev/documentation/flutter_color_models/latest/flutter_color_models/ColorModel/interpolate.html>
518-
// which does ordinary RGB mixing. Investigate and send a PR?
519-
// TODO fix bug where our results differ from the replit's (see unit tests)
520-
_StreamColorVariant.barBackground:
521-
LabColor.fromColor(const Color(0xfff9f9f9))
522-
.interpolate(LabColor.fromColor(clamped20to75), 0.22)
523-
.toColor(),
524-
};
525-
}
466+
ColorSwatch<StreamColorVariant> streamColorSwatch(int base) {
467+
final baseAsColor = Color(base);
468+
469+
final clamped20to75 = clampLchLightness(baseAsColor, 20, 75);
470+
final clamped20to75AsHsl = HSLColor.fromColor(clamped20to75);
471+
472+
final map = {
473+
StreamColorVariant.base: baseAsColor,
474+
475+
// Follows `.unread-count` in Vlad's replit:
476+
// <https://replit.com/@VladKorobov/zulip-sidebar#script.js>
477+
// <https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/design.3A.20.23F117.20.22Inbox.22.20screen/near/1624484>
478+
//
479+
// TODO fix bug where our results differ from the replit's (see unit tests)
480+
StreamColorVariant.unreadCountBadgeBackground:
481+
clampLchLightness(baseAsColor, 30, 70)
482+
.withOpacity(0.3),
483+
484+
// Follows `.sidebar-row__icon` in Vlad's replit:
485+
// <https://replit.com/@VladKorobov/zulip-topic-feed-colors#script.js>
486+
//
487+
// TODO fix bug where our results differ from the replit's (see unit tests)
488+
StreamColorVariant.iconOnPlainBackground: clamped20to75,
489+
490+
// Follows `.recepeient__icon` in Vlad's replit:
491+
// <https://replit.com/@VladKorobov/zulip-topic-feed-colors#script.js>
492+
// <https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/design.3A.20.23F117.20.22Inbox.22.20screen/near/1624484>
493+
//
494+
// TODO fix bug where our results differ from the replit's (see unit tests)
495+
StreamColorVariant.iconOnBarBackground:
496+
clamped20to75AsHsl
497+
.withLightness(clamped20to75AsHsl.lightness - 0.12)
498+
.toColor(),
499+
500+
// Follows `.recepient` in Vlad's replit:
501+
// <https://replit.com/@VladKorobov/zulip-topic-feed-colors#script.js>
502+
//
503+
// TODO I think [LabColor.interpolate] doesn't actually do LAB mixing;
504+
// it just calls up to the superclass method [ColorModel.interpolate]:
505+
// <https://pub.dev/documentation/flutter_color_models/latest/flutter_color_models/ColorModel/interpolate.html>
506+
// which does ordinary RGB mixing. Investigate and send a PR?
507+
// TODO fix bug where our results differ from the replit's (see unit tests)
508+
StreamColorVariant.barBackground:
509+
LabColor.fromColor(const Color(0xfff9f9f9))
510+
.interpolate(LabColor.fromColor(clamped20to75), 0.22)
511+
.toColor(),
512+
};
513+
514+
return ColorSwatch<StreamColorVariant>(base, map);
526515
}
527516

528-
enum _StreamColorVariant {
517+
enum StreamColorVariant {
529518
/// The [Subscription.color] int that the swatch is based on.
530519
base,
531520

‎lib/widgets/inbox.dart

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -410,12 +410,14 @@ class _StreamHeaderItem extends _HeaderItem {
410410

411411
@override get title => subscription.name;
412412
@override get icon => iconDataForStream(subscription);
413-
@override get collapsedIconColor => subscription.colorSwatch().iconOnPlainBackground;
414-
@override get uncollapsedIconColor => subscription.colorSwatch().iconOnBarBackground;
413+
@override get collapsedIconColor =>
414+
subscription.colorSwatch()[StreamColorVariant.iconOnPlainBackground]!;
415+
@override get uncollapsedIconColor =>
416+
subscription.colorSwatch()[StreamColorVariant.iconOnBarBackground]!;
415417
@override get uncollapsedBackgroundColor =>
416-
subscription.colorSwatch().barBackground;
418+
subscription.colorSwatch()[StreamColorVariant.barBackground]!;
417419
@override get unreadCountBadgeBackgroundColor =>
418-
subscription.colorSwatch().unreadCountBadgeBackground;
420+
subscription.colorSwatch()[StreamColorVariant.unreadCountBadgeBackground]!;
419421

420422
@override get onCollapseButtonTap => () async {
421423
await super.onCollapseButtonTap();

‎lib/widgets/message_list.dart

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ class _MessageListPageState extends State<MessageListPage> {
6666

6767
case StreamNarrow(:final streamId):
6868
case TopicNarrow(:final streamId):
69-
backgroundColor = store.subscriptions[streamId]?.colorSwatch().barBackground
69+
backgroundColor =
70+
store.subscriptions[streamId]?.colorSwatch()[StreamColorVariant.barBackground]!
7071
?? _kUnsubscribedStreamRecipientHeaderColor;
7172
// All recipient headers will match this color; remove distracting line
7273
// (but are recipient headers even needed for topic narrows?)
@@ -655,12 +656,12 @@ class StreamMessageRecipientHeader extends StatelessWidget {
655656
final Color iconColor;
656657
if (subscription != null) {
657658
final swatch = subscription.colorSwatch();
658-
backgroundColor = swatch.barBackground;
659+
backgroundColor = swatch[StreamColorVariant.barBackground]!;
659660
contrastingColor =
660-
(ThemeData.estimateBrightnessForColor(swatch.barBackground) == Brightness.dark)
661+
(ThemeData.estimateBrightnessForColor(backgroundColor) == Brightness.dark)
661662
? Colors.white
662663
: Colors.black;
663-
iconColor = swatch.iconOnBarBackground;
664+
iconColor = swatch[StreamColorVariant.iconOnBarBackground]!;
664665
} else {
665666
backgroundColor = _kUnsubscribedStreamRecipientHeaderColor;
666667
contrastingColor = Colors.black;

‎lib/widgets/subscription_list.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ class SubscriptionItem extends StatelessWidget {
208208
const SizedBox(width: 16),
209209
Padding(
210210
padding: const EdgeInsets.symmetric(vertical: 11),
211-
child: Icon(size: 18, color: swatch.iconOnPlainBackground,
211+
child: Icon(size: 18, color: swatch[StreamColorVariant.iconOnPlainBackground]!,
212212
iconDataForStream(subscription))),
213213
const SizedBox(width: 5),
214214
Expanded(

‎lib/widgets/unread_count_badge.dart

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,18 @@ class UnreadCountBadge extends StatelessWidget {
2121

2222
/// The badge's background color.
2323
///
24-
/// Pass a [StreamColorSwatch] if this badge represents messages in one
25-
/// specific stream. The appropriate color from the swatch will be used.
24+
/// Pass a [ColorSwatch<StreamColorVariant>] if this badge represents messages
25+
/// in one specific stream. The appropriate color from the swatch will be used.
2626
///
2727
/// If null, the default neutral background will be used.
2828
final Color? backgroundColor;
2929

3030
@override
3131
Widget build(BuildContext context) {
32+
final backgroundColor = this.backgroundColor;
3233
final effectiveBackgroundColor = switch (backgroundColor) {
33-
StreamColorSwatch(unreadCountBadgeBackground: var color) => color,
34+
ColorSwatch<StreamColorVariant>() =>
35+
backgroundColor[StreamColorVariant.unreadCountBadgeBackground]!,
3436
Color() => backgroundColor,
3537
null => const Color.fromRGBO(102, 102, 153, 0.15),
3638
};

‎test/api/model/model_checks.dart

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import 'dart:ui';
2-
31
import 'package:checks/checks.dart';
42
import 'package:zulip/api/model/model.dart';
53

@@ -29,14 +27,6 @@ extension ZulipStreamChecks on Subject<ZulipStream> {
2927
Subject<int?> get canRemoveSubscribersGroup => has((e) => e.canRemoveSubscribersGroup, 'canRemoveSubscribersGroup');
3028
}
3129

32-
extension StreamColorSwatchChecks on Subject<StreamColorSwatch> {
33-
Subject<Color> get base => has((s) => s.base, 'base');
34-
Subject<Color> get unreadCountBadgeBackground => has((s) => s.unreadCountBadgeBackground, 'unreadCountBadgeBackground');
35-
Subject<Color> get iconOnPlainBackground => has((s) => s.iconOnPlainBackground, 'iconOnPlainBackground');
36-
Subject<Color> get iconOnBarBackground => has((s) => s.iconOnBarBackground, 'iconOnBarBackground');
37-
Subject<Color> get barBackground => has((s) => s.barBackground, 'barBackground');
38-
}
39-
4030
extension MessageChecks on Subject<Message> {
4131
Subject<int> get id => has((e) => e.id, 'id');
4232
Subject<String> get content => has((e) => e.content, 'content');

‎test/api/model/model_test.dart

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import 'package:checks/checks.dart';
55
import 'package:test/scaffolding.dart';
66
import 'package:zulip/api/model/model.dart';
77

8+
import '../../flutter_checks.dart';
89
import '../../example_data.dart' as eg;
910
import '../../stdlib_checks.dart';
1011
import 'model_checks.dart';
@@ -120,21 +121,25 @@ void main() {
120121
final sub = eg.subscription(eg.stream(), color: 0xffffffff);
121122
check(sub.debugCachedSwatchValue).isNull();
122123
sub.colorSwatch();
123-
check(sub.debugCachedSwatchValue).isNotNull().base.equals(const Color(0xffffffff));
124+
check(sub.debugCachedSwatchValue).isNotNull()
125+
[StreamColorVariant.base].equals(const Color(0xffffffff));
124126
sub.color = 0xffff0000;
125127
check(sub.debugCachedSwatchValue).isNull();
126128
sub.colorSwatch();
127-
check(sub.debugCachedSwatchValue).isNotNull().base.equals(const Color(0xffff0000));
129+
check(sub.debugCachedSwatchValue).isNotNull()
130+
[StreamColorVariant.base].equals(const Color(0xffff0000));
128131
});
129132

130-
group('StreamColorSwatch', () {
133+
group('streamColorSwatch', () {
131134
test('base', () {
132-
check(StreamColorSwatch(0xffffffff)).base.equals(const Color(0xffffffff));
135+
check(streamColorSwatch(0xffffffff))[StreamColorVariant.base]
136+
.equals(const Color(0xffffffff));
133137
});
134138

135139
test('unreadCountBadgeBackground', () {
136140
void runCheck(int base, Color expected) {
137-
check(StreamColorSwatch(base)).unreadCountBadgeBackground.equals(expected);
141+
check(streamColorSwatch(base))
142+
[StreamColorVariant.unreadCountBadgeBackground].equals(expected);
138143
}
139144

140145
// Check against everything in ZULIP_ASSIGNMENT_COLORS and EXTREME_COLORS
@@ -196,7 +201,8 @@ void main() {
196201

197202
test('iconOnPlainBackground', () {
198203
void runCheck(int base, Color expected) {
199-
check(StreamColorSwatch(base)).iconOnPlainBackground.equals(expected);
204+
check(streamColorSwatch(base))
205+
[StreamColorVariant.iconOnPlainBackground].equals(expected);
200206
}
201207

202208
// Check against everything in ZULIP_ASSIGNMENT_COLORS
@@ -237,7 +243,8 @@ void main() {
237243

238244
test('iconOnBarBackground', () {
239245
void runCheck(int base, Color expected) {
240-
check(StreamColorSwatch(base)).iconOnBarBackground.equals(expected);
246+
check(streamColorSwatch(base))
247+
[StreamColorVariant.iconOnBarBackground].equals(expected);
241248
}
242249

243250
// Check against everything in ZULIP_ASSIGNMENT_COLORS
@@ -278,7 +285,8 @@ void main() {
278285

279286
test('barBackground', () {
280287
void runCheck(int base, Color expected) {
281-
check(StreamColorSwatch(base)).barBackground.equals(expected);
288+
check(streamColorSwatch(base))
289+
[StreamColorVariant.barBackground].equals(expected);
282290
}
283291

284292
// Check against everything in ZULIP_ASSIGNMENT_COLORS

‎test/flutter_checks.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ import 'package:checks/checks.dart';
55
import 'package:flutter/material.dart';
66
import 'package:flutter/services.dart';
77

8+
extension ColorSwatchChecks<T> on Subject<ColorSwatch<T>> {
9+
Subject<Color?> operator [](T index) => has((x) => x[index], '[$index]');
10+
}
11+
812
extension RectChecks on Subject<Rect> {
913
Subject<double> get top => has((d) => d.top, 'top');
1014
Subject<double> get bottom => has((d) => d.bottom, 'bottom');

‎test/widgets/inbox_test.dart

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,8 @@ void main() {
411411
final collapseIcon = findHeaderCollapseIcon(tester, headerRow!);
412412
check(collapseIcon).icon.equals(ZulipIcons.arrow_down);
413413
final streamIcon = findStreamHeaderIcon(tester, streamId);
414-
check(streamIcon).color.equals(subscription.colorSwatch().iconOnBarBackground);
414+
check(streamIcon).color
415+
.equals(subscription.colorSwatch()[StreamColorVariant.iconOnBarBackground]!);
415416
// TODO check bar background color
416417
check(tester.widgetList(findSectionContent)).isNotEmpty();
417418
}
@@ -432,7 +433,8 @@ void main() {
432433
final collapseIcon = findHeaderCollapseIcon(tester, headerRow!);
433434
check(collapseIcon).icon.equals(ZulipIcons.arrow_right);
434435
final streamIcon = findStreamHeaderIcon(tester, streamId);
435-
check(streamIcon).color.equals(subscription.colorSwatch().iconOnPlainBackground);
436+
check(streamIcon).color
437+
.equals(subscription.colorSwatch()[StreamColorVariant.iconOnPlainBackground]!);
436438
// TODO check bar background color
437439
check(tester.widgetList(findSectionContent)).isEmpty();
438440
}

‎test/widgets/message_list_test.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ void main() {
282282
find.descendant(
283283
of: find.byType(StreamMessageRecipientHeader),
284284
matching: find.byType(ColoredBox),
285-
))).color.equals(swatch.barBackground);
285+
))).color.equals(swatch[StreamColorVariant.barBackground]!);
286286
});
287287

288288
testWidgets('color of stream icon', (tester) async {
@@ -294,7 +294,7 @@ void main() {
294294
subscriptions: [subscription]);
295295
await tester.pump();
296296
check(tester.widget<Icon>(find.byIcon(ZulipIcons.globe)))
297-
.color.equals(swatch.iconOnBarBackground);
297+
.color.equals(swatch[StreamColorVariant.iconOnBarBackground]!);
298298
});
299299

300300
testWidgets('normal streams show hash icon', (tester) async {

‎test/widgets/subscription_list_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ void main() {
186186
], unreadMsgs: unreadMsgs);
187187
check(getItemCount()).equals(1);
188188
check(tester.widget<Icon>(find.byType(Icon)).color)
189-
.equals(swatch.iconOnPlainBackground);
189+
.equals(swatch[StreamColorVariant.iconOnPlainBackground]!);
190190
check(tester.widget<UnreadCountBadge>(find.byType(UnreadCountBadge)).backgroundColor)
191191
.equals(swatch);
192192
});

‎test/widgets/unread_count_badge_test.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ void main() {
3838
});
3939

4040
testWidgets('stream color', (WidgetTester tester) async {
41-
final swatch = StreamColorSwatch(0xff76ce90);
41+
final swatch = streamColorSwatch(0xff76ce90);
4242
await prepare(tester, swatch);
43-
check(findBackgroundColor(tester)).equals(swatch.unreadCountBadgeBackground);
43+
check(findBackgroundColor(tester)).equals(swatch[StreamColorVariant.unreadCountBadgeBackground]!);
4444
});
4545
});
4646
});

0 commit comments

Comments
 (0)