diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 3c2ad856f7..818e0c221a 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -1,7 +1,5 @@ -import 'package:flutter/foundation.dart'; import 'package:json_annotation/json_annotation.dart'; -import '../../widgets/stream_colors.dart'; import 'events.dart'; import 'initial_snapshot.dart'; import 'reaction.dart'; @@ -403,29 +401,13 @@ class Subscription extends ZulipStream { /// As an int that dart:ui's Color constructor will take: /// @JsonKey(readValue: _readColor) - int get color => _color; - int _color; - set color(int value) { - _color = value; - _swatch = null; - } + int color; static Object? _readColor(Map json, String key) { final str = (json[key] as String); assert(RegExp(r'^#[0-9a-f]{6}$').hasMatch(str)); return 0xff000000 | int.parse(str.substring(1), radix: 16); } - StreamColorSwatch? _swatch; - /// A [StreamColorSwatch] for the subscription, memoized. - // TODO I'm not sure this is the right home for this; it seems like we might - // instead have chosen to put it in more UI-centered code, like in a custom - // material [ColorScheme] class or something. But it works for now. - StreamColorSwatch colorSwatch() => _swatch ??= StreamColorSwatch.light(color); - - @visibleForTesting - @JsonKey(includeToJson: false) - StreamColorSwatch? get debugCachedSwatchValue => _swatch; - Subscription({ required super.streamId, required super.name, @@ -447,8 +429,8 @@ class Subscription extends ZulipStream { required this.audibleNotifications, required this.pinToTop, required this.isMuted, - required int color, - }) : _color = color; + required this.color, + }); factory Subscription.fromJson(Map json) => _$SubscriptionFromJson(json); diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index 18648afab2..1d237fa909 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -10,6 +10,7 @@ import 'page.dart'; import 'sticky_header.dart'; import 'store.dart'; import 'text.dart'; +import 'theme.dart'; import 'unread_count_badge.dart'; class InboxPage extends StatefulWidget { @@ -423,12 +424,14 @@ class _StreamHeaderItem extends _HeaderItem { @override String get title => subscription.name; @override IconData get icon => iconDataForStream(subscription); - @override Color collapsedIconColor(context) => subscription.colorSwatch().iconOnPlainBackground; - @override Color uncollapsedIconColor(context) => subscription.colorSwatch().iconOnBarBackground; + @override Color collapsedIconColor(context) => + colorSwatchFor(context, subscription).iconOnPlainBackground; + @override Color uncollapsedIconColor(context) => + colorSwatchFor(context, subscription).iconOnBarBackground; @override Color uncollapsedBackgroundColor(context) => - subscription.colorSwatch().barBackground; + colorSwatchFor(context, subscription).barBackground; @override Color? unreadCountBadgeBackgroundColor(context) => - subscription.colorSwatch().unreadCountBadgeBackground; + colorSwatchFor(context, subscription).unreadCountBadgeBackground; @override Future onCollapseButtonTap() async { await super.onCollapseButtonTap(); @@ -525,7 +528,8 @@ class _TopicItem extends StatelessWidget { const SizedBox(width: 12), if (hasMention) const _AtMentionMarker(), Padding(padding: const EdgeInsetsDirectional.only(end: 16), - child: UnreadCountBadge(backgroundColor: subscription.colorSwatch(), + child: UnreadCountBadge( + backgroundColor: colorSwatchFor(context, subscription), count: count)), ])))); } diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index f2bf3ad0a4..e4334e70ad 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -22,6 +22,7 @@ import 'profile.dart'; import 'sticky_header.dart'; import 'store.dart'; import 'text.dart'; +import 'theme.dart'; class MessageListPage extends StatefulWidget { const MessageListPage({super.key, required this.narrow}); @@ -67,8 +68,9 @@ class _MessageListPageState extends State { case StreamNarrow(:final streamId): case TopicNarrow(:final streamId): final subscription = store.subscriptions[streamId]; - appBarBackgroundColor = subscription?.colorSwatch().barBackground - ?? _kUnsubscribedStreamRecipientHeaderColor; + appBarBackgroundColor = subscription != null + ? colorSwatchFor(context, subscription).barBackground + : _kUnsubscribedStreamRecipientHeaderColor; // All recipient headers will match this color; remove distracting line // (but are recipient headers even needed for topic narrows?) removeAppBarBottomBorder = true; @@ -655,7 +657,7 @@ class StreamMessageRecipientHeader extends StatelessWidget { final Color backgroundColor; final Color iconColor; if (subscription != null) { - final swatch = subscription.colorSwatch(); + final swatch = colorSwatchFor(context, subscription); backgroundColor = swatch.barBackground; iconColor = swatch.iconOnBarBackground; } else { diff --git a/lib/widgets/stream_colors.dart b/lib/widgets/stream_colors.dart index 1034197c44..aa195b971a 100644 --- a/lib/widgets/stream_colors.dart +++ b/lib/widgets/stream_colors.dart @@ -6,6 +6,101 @@ import 'package:flutter_color_models/flutter_color_models.dart'; import '../api/model/model.dart'; import 'color.dart'; +/// A caching factory for [StreamColorSwatch]es, from [subscription.color] bases. +/// +/// See subclasses: +/// [StreamColorSwatchesLight] +/// [StreamColorSwatchesDark] +abstract class StreamColorSwatches { + /// Gives the cached [StreamColorSwatch] for a [subscription.color]. + /// + /// For caching details, see subclasses. + StreamColorSwatch forBaseColor(int base); + + /// Gives a [StreamColorSwatches], lerped to [other] at [t]. + /// + /// If [this] and [other] are [identical], returns [this]. + /// + /// Else returns an instance whose [forBaseColor] will call + /// [this.forBaseColor] and [other.forBaseColor] + /// and return [StreamColorSwatch.lerp]'s result on those. + /// This computation is cached on the instance + /// in order to save work building [t]'s animation frame when there are + /// multiple UI elements using the same [subscription.color]. + StreamColorSwatches lerp(StreamColorSwatches? other, double t) { + // This short-circuit helps a lot when [a] and [b] + // are both [StreamColorSwatchesLight.instance] + // or both [StreamColorSwatchesDark.instance]. + if (identical(this, other)) { + return this; + } + + return _StreamColorSwatchesLerped(this, other, t); + } +} + +/// The [StreamColorSwatches] for the light theme. +class StreamColorSwatchesLight extends StreamColorSwatches { + // No public constructor; always use [instance]. Empirically, + // [StreamColorSwatches.lerp] is called even when the theme has not changed, + // and the short-circuit on [identical] cuts redundant work when that happens. + static StreamColorSwatchesLight get instance => + (_instance ??= StreamColorSwatchesLight._()); + static StreamColorSwatchesLight? _instance; + + StreamColorSwatchesLight._(); + + final Map _cache = {}; + + /// Gives the cached [StreamColorSwatch.light] for a [subscription.color]. + /// + /// When this is first called, the result is added to a global cache + /// keyed by [subscription.color]. + /// Subsequent calls return the value from the cache. + @override + StreamColorSwatch forBaseColor(int base) => + _cache[base] ??= StreamColorSwatch.light(base); +} + +/// The [StreamColorSwatches] for the dark theme. +class StreamColorSwatchesDark extends StreamColorSwatches { + // No public constructor, like [StreamColorSwatchesLight]. + static StreamColorSwatchesDark get instance => + (_instance ??= StreamColorSwatchesDark._()); + static StreamColorSwatchesDark? _instance; + + StreamColorSwatchesDark._(); + + final Map _cache = {}; + + /// Like [StreamColorSwatchesLight.forBaseColor], + /// but using [StreamColorSwatch.dark]. + @override + StreamColorSwatch forBaseColor(int base) => + _cache[base] ??= StreamColorSwatch.dark(base); +} + +class _StreamColorSwatchesLerped extends StreamColorSwatches { + _StreamColorSwatchesLerped(this.a, this.b, this.t); + + final StreamColorSwatches a; + final StreamColorSwatches? b; + final double t; + + final Map _cache = {}; + + /// Gives the cached [StreamColorSwatch] for a [subscription.color]. + /// + /// Caching is per-instance, not global, in order to save work building + /// [t]'s animation frame when there are multiple UI elements using the same + /// [subscription.color]. + @override + StreamColorSwatch forBaseColor(int base) => + _cache[base] + ??= StreamColorSwatch.lerp(a.forBaseColor(base), b?.forBaseColor(base), t)!; +} + + /// A [ColorSwatch] with colors related to a base stream color. /// /// Use this in UI code for colors related to [Subscription.color], diff --git a/lib/widgets/subscription_list.dart b/lib/widgets/subscription_list.dart index 7b69374b60..7aa4822a47 100644 --- a/lib/widgets/subscription_list.dart +++ b/lib/widgets/subscription_list.dart @@ -9,6 +9,7 @@ import 'message_list.dart'; import 'page.dart'; import 'store.dart'; import 'text.dart'; +import 'theme.dart'; import 'unread_count_badge.dart'; /// Scrollable listing of subscribed streams. @@ -199,7 +200,7 @@ class SubscriptionItem extends StatelessWidget { @override Widget build(BuildContext context) { - final swatch = subscription.colorSwatch(); + final swatch = colorSwatchFor(context, subscription); final hasUnreads = (unreadCount > 0); return Material( // TODO(#95) need dark-theme color diff --git a/lib/widgets/theme.dart b/lib/widgets/theme.dart index c2e61cca98..f3c997d987 100644 --- a/lib/widgets/theme.dart +++ b/lib/widgets/theme.dart @@ -1,6 +1,8 @@ import 'package:flutter/material.dart'; +import '../api/model/model.dart'; import 'content.dart'; +import 'stream_colors.dart'; import 'text.dart'; ThemeData zulipThemeData(BuildContext context) { @@ -78,7 +80,8 @@ class DesignVariables extends ThemeExtension { bgTopBar = const Color(0xfff5f5f5), borderBar = const Color(0x33000000), icon = const Color(0xff666699), - title = const Color(0xff1a1a1a); + title = const Color(0xff1a1a1a), + streamColorSwatches = StreamColorSwatchesLight.instance; DesignVariables._({ required this.bgMain, @@ -86,6 +89,7 @@ class DesignVariables extends ThemeExtension { required this.borderBar, required this.icon, required this.title, + required this.streamColorSwatches, }); /// The [DesignVariables] from the context's active theme. @@ -104,6 +108,9 @@ class DesignVariables extends ThemeExtension { final Color icon; final Color title; + // Not exactly from the Figma design, but from Vlad anyway. + final StreamColorSwatches streamColorSwatches; + @override DesignVariables copyWith({ Color? bgMain, @@ -111,6 +118,7 @@ class DesignVariables extends ThemeExtension { Color? borderBar, Color? icon, Color? title, + StreamColorSwatches? streamColorSwatches, }) { return DesignVariables._( bgMain: bgMain ?? this.bgMain, @@ -118,6 +126,7 @@ class DesignVariables extends ThemeExtension { borderBar: borderBar ?? this.borderBar, icon: icon ?? this.icon, title: title ?? this.title, + streamColorSwatches: streamColorSwatches ?? this.streamColorSwatches, ); } @@ -132,6 +141,15 @@ class DesignVariables extends ThemeExtension { borderBar: Color.lerp(borderBar, other?.borderBar, t)!, icon: Color.lerp(icon, other?.icon, t)!, title: Color.lerp(title, other?.title, t)!, + streamColorSwatches: streamColorSwatches.lerp(other?.streamColorSwatches, t), ); } } + +/// The theme-appropriate [StreamColorSwatch] based on [subscription.color]. +/// +/// For how this value is cached, see [StreamColorSwatches.forBaseColor]. +StreamColorSwatch colorSwatchFor(BuildContext context, Subscription subscription) { + return DesignVariables.of(context) + .streamColorSwatches.forBaseColor(subscription.color); +} diff --git a/test/api/model/model_test.dart b/test/api/model/model_test.dart index 0a911614e5..ef9e92238d 100644 --- a/test/api/model/model_test.dart +++ b/test/api/model/model_test.dart @@ -1,5 +1,4 @@ import 'dart:convert'; -import 'dart:ui'; import 'package:checks/checks.dart'; import 'package:test/scaffolding.dart'; @@ -7,7 +6,6 @@ import 'package:zulip/api/model/model.dart'; import '../../example_data.dart' as eg; import '../../stdlib_checks.dart'; -import '../../widgets/stream_colors_checks.dart'; import 'model_checks.dart'; void main() { @@ -117,17 +115,6 @@ void main() { check(subWithColor('#ffffff').color).equals(0xffffffff); check(subWithColor('#000000').color).equals(0xff000000); }); - - test('colorSwatch caching', () { - final sub = eg.subscription(eg.stream(), color: 0xffffffff); - check(sub.debugCachedSwatchValue).isNull(); - sub.colorSwatch(); - check(sub.debugCachedSwatchValue).isNotNull().base.equals(const Color(0xffffffff)); - sub.color = 0xffff0000; - check(sub.debugCachedSwatchValue).isNull(); - sub.colorSwatch(); - check(sub.debugCachedSwatchValue).isNotNull().base.equals(const Color(0xffff0000)); - }); }); group('Message', () { diff --git a/test/widgets/inbox_test.dart b/test/widgets/inbox_test.dart index a4d02a049d..920bf25996 100644 --- a/test/widgets/inbox_test.dart +++ b/test/widgets/inbox_test.dart @@ -423,9 +423,10 @@ void main() { final collapseIcon = findHeaderCollapseIcon(tester, headerRow!); check(collapseIcon).icon.equals(ZulipIcons.arrow_down); final streamIcon = findStreamHeaderIcon(tester, streamId); - check(streamIcon).color.equals(subscription.colorSwatch().iconOnBarBackground); + check(streamIcon).color.equals( + StreamColorSwatch.light(subscription.color).iconOnBarBackground); check(streamHeaderBackgroundColor(tester, streamId)) - .isNotNull().equals(subscription.colorSwatch().barBackground); + .isNotNull().equals(StreamColorSwatch.light(subscription.color).barBackground); check(tester.widgetList(findSectionContent)).isNotEmpty(); } @@ -445,7 +446,8 @@ void main() { final collapseIcon = findHeaderCollapseIcon(tester, headerRow!); check(collapseIcon).icon.equals(ZulipIcons.arrow_right); final streamIcon = findStreamHeaderIcon(tester, streamId); - check(streamIcon).color.equals(subscription.colorSwatch().iconOnPlainBackground); + check(streamIcon).color.equals( + StreamColorSwatch.light(subscription.color).iconOnPlainBackground); check(streamHeaderBackgroundColor(tester, streamId)) .isNotNull().equals(Colors.white); check(tester.widgetList(findSectionContent)).isEmpty(); diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index aa7cd1fa3f..3e3c31f188 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -19,6 +19,7 @@ import 'package:zulip/widgets/content.dart'; import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/message_list.dart'; import 'package:zulip/widgets/store.dart'; +import 'package:zulip/widgets/stream_colors.dart'; import 'package:zulip/widgets/theme.dart'; import '../api/fake_api.dart'; @@ -277,7 +278,7 @@ void main() { testWidgets('color of recipient header background', (tester) async { final subscription = eg.subscription(stream, color: Colors.red.value); - final swatch = subscription.colorSwatch(); + final swatch = StreamColorSwatch.light(subscription.color); await setupMessageListPage(tester, messages: [eg.streamMessage(stream: subscription)], subscriptions: [subscription]); @@ -292,7 +293,7 @@ void main() { testWidgets('color of stream icon', (tester) async { final stream = eg.stream(isWebPublic: true); final subscription = eg.subscription(stream, color: Colors.red.value); - final swatch = subscription.colorSwatch(); + final swatch = StreamColorSwatch.light(subscription.color); await setupMessageListPage(tester, messages: [eg.streamMessage(stream: subscription)], subscriptions: [subscription]); diff --git a/test/widgets/stream_colors_test.dart b/test/widgets/stream_colors_test.dart index 1b7e5f0e76..117cd8aed0 100644 --- a/test/widgets/stream_colors_test.dart +++ b/test/widgets/stream_colors_test.dart @@ -1,12 +1,92 @@ import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; -import 'package:flutter/rendering.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:zulip/widgets/stream_colors.dart'; import 'stream_colors_checks.dart'; void main() { + group('StreamColorSwatches', () { + test('StreamColorSwatchesLight', () { + final instance = StreamColorSwatchesLight.instance; + + const base1 = 0xff76ce90; + final swatch1 = instance.forBaseColor(base1); + check(swatch1).equals(StreamColorSwatch.light(base1)); + check(instance.forBaseColor(base1)).identicalTo(swatch1); + + const base2 = 0xfffae589; + final swatch2 = instance.forBaseColor(base2); + check(swatch2).equals(StreamColorSwatch.light(base2)); + check(instance.forBaseColor(base2)).identicalTo(swatch2); + check(instance.forBaseColor(base1)).identicalTo(swatch1); + }); + + // TODO deduplicate with corresponding StreamColorSwatchesLight test? + test('StreamColorSwatchesDark', () { + final instance = StreamColorSwatchesDark.instance; + + const base1 = 0xff76ce90; + final swatch1 = instance.forBaseColor(base1); + check(swatch1).equals(StreamColorSwatch.dark(base1)); + check(instance.forBaseColor(base1)).identicalTo(swatch1); + + const base2 = 0xfffae589; + final swatch2 = instance.forBaseColor(base2); + check(swatch2).equals(StreamColorSwatch.dark(base2)); + check(instance.forBaseColor(base2)).identicalTo(swatch2); + check(instance.forBaseColor(base1)).identicalTo(swatch1); + }); + + group('lerp', () { + test('on identical instances', () { + check( + StreamColorSwatchesLight.instance.lerp(StreamColorSwatchesLight.instance, 0.5) + ).identicalTo(StreamColorSwatchesLight.instance); + + check( + StreamColorSwatchesDark.instance.lerp(StreamColorSwatchesDark.instance, 0.5) + ).identicalTo(StreamColorSwatchesDark.instance); + }); + + test('from light to dark', () { + final instance = StreamColorSwatchesLight.instance + .lerp(StreamColorSwatchesDark.instance, 0.4); + + const base1 = 0xff76ce90; + final swatch1 = instance.forBaseColor(base1); + check(swatch1).equals(StreamColorSwatch.lerp( + StreamColorSwatch.light(base1), StreamColorSwatch.dark(base1), 0.4)!); + check(instance.forBaseColor(base1)).identicalTo(swatch1); + + const base2 = 0xfffae589; + final swatch2 = instance.forBaseColor(base2); + check(swatch2).equals(StreamColorSwatch.lerp( + StreamColorSwatch.light(base2), StreamColorSwatch.dark(base2), 0.4)!); + check(instance.forBaseColor(base2)).identicalTo(swatch2); + check(instance.forBaseColor(base1)).identicalTo(swatch1); + }); + + test('from dark to light', () { + final instance = StreamColorSwatchesDark.instance + .lerp(StreamColorSwatchesLight.instance, 0.4); + + const base1 = 0xff76ce90; + final swatch1 = instance.forBaseColor(base1); + check(swatch1).equals(StreamColorSwatch.lerp( + StreamColorSwatch.dark(base1), StreamColorSwatch.light(base1), 0.4)!); + check(instance.forBaseColor(base1)).identicalTo(swatch1); + + const base2 = 0xfffae589; + final swatch2 = instance.forBaseColor(base2); + check(swatch2).equals(StreamColorSwatch.lerp( + StreamColorSwatch.dark(base2), StreamColorSwatch.light(base2), 0.4)!); + check(instance.forBaseColor(base2)).identicalTo(swatch2); + check(instance.forBaseColor(base1)).identicalTo(swatch1); + }); + }); + }); + group('StreamColorSwatch', () { group('light', () { test('base', () { diff --git a/test/widgets/subscription_list_test.dart b/test/widgets/subscription_list_test.dart index 04d36ca3ef..a13ae5cb31 100644 --- a/test/widgets/subscription_list_test.dart +++ b/test/widgets/subscription_list_test.dart @@ -5,6 +5,7 @@ import 'package:zulip/api/model/initial_snapshot.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/widgets/app.dart'; import 'package:zulip/widgets/icons.dart'; +import 'package:zulip/widgets/stream_colors.dart'; import 'package:zulip/widgets/subscription_list.dart'; import 'package:zulip/widgets/unread_count_badge.dart'; @@ -179,7 +180,7 @@ void main() { UnreadStreamSnapshot(streamId: stream.streamId, topic: 'a', unreadMessageIds: [1, 2]), ]); final subscription = eg.subscription(stream, color: Colors.red.value); - final swatch = subscription.colorSwatch(); + final swatch = StreamColorSwatch.light(subscription.color); await setupStreamListPage(tester, subscriptions: [ subscription, ], unreadMsgs: unreadMsgs); diff --git a/test/widgets/theme_test.dart b/test/widgets/theme_test.dart index 8bc32e94d9..b75fdf5c49 100644 --- a/test/widgets/theme_test.dart +++ b/test/widgets/theme_test.dart @@ -2,12 +2,19 @@ import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/widgets/app.dart'; +import 'package:zulip/widgets/page.dart'; +import 'package:zulip/widgets/stream_colors.dart'; import 'package:zulip/widgets/text.dart'; import 'package:zulip/widgets/theme.dart'; +import '../example_data.dart' as eg; import '../flutter_checks.dart'; +import '../model/binding.dart'; void main() { + TestZulipBinding.ensureInitialized(); + group('button text size and letter spacing', () { const buttonText = 'Zulip'; @@ -68,4 +75,40 @@ void main() { doCheck('TextButton', button: TextButton(onPressed: () {}, child: const Text(buttonText))); }); + + group('colorSwatchFor', () { + void doTest(int baseColor, Brightness brightness) { + final description = '${Color(baseColor).toString()}, $brightness'; + testWidgets(description, (WidgetTester tester) async { + addTearDown(testBinding.reset); + + tester.platformDispatcher.platformBrightnessTestValue = brightness; + addTearDown(tester.platformDispatcher.clearPlatformBrightnessTestValue); + + final subscription = eg.subscription(eg.stream(), color: baseColor); + + await tester.pumpWidget(const ZulipApp()); + await tester.pump(); + + late StreamColorSwatch actualSwatch; + final navigator = await ZulipApp.navigator; + navigator.push(MaterialWidgetRoute(page: Builder(builder: (context) { + actualSwatch = colorSwatchFor(context, subscription); + return const Placeholder(); + }))); + await tester.pumpAndSettle(); + + final expectedSwatch = switch (brightness) { + Brightness.light => StreamColorSwatch.light(baseColor), + Brightness.dark => StreamColorSwatch.dark(baseColor), + }; + + // Compares all the swatch's members; see [ColorSwatch]'s `operator ==`. + check(actualSwatch).equals(expectedSwatch); + }); + } + + doTest(0xff76ce90, Brightness.light); + // TODO(#95) test with Brightness.dark and lerping between light/dark + }); }