From 3041bebfac83f6dae6707d613cbeb7d252337134 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 30 Aug 2024 17:45:47 -0700 Subject: [PATCH] color [nfc]: Add/use extension with `argbInt`, replacing deprecated `value` The `value` getter is deprecated in flutter/engine@eff1b76cf (rolled in flutter/flutter@8c1a93508), and I think that's because it could lose precision for colors in different color spaces that are now possible to represent. We're about to bump the Flutter minimum version to take that change. We're happy with the color space we've been using (sRGB), and this integer representation has been helpful for us. So, add an extension for it, allowing consumers to keep using it without each one needing a `deprecated_member_use`. --- lib/notifications/display.dart | 5 +++-- lib/widgets/channel_colors.dart | 2 +- lib/widgets/color.dart | 23 +++++++++++++++++++++++ test/notifications/display_test.dart | 5 +++-- test/widgets/color_test.dart | 18 ++++++++++++++++++ test/widgets/inbox_test.dart | 5 +++-- test/widgets/message_list_test.dart | 5 +++-- test/widgets/subscription_list_test.dart | 3 ++- 8 files changed, 56 insertions(+), 10 deletions(-) create mode 100644 test/widgets/color_test.dart diff --git a/lib/notifications/display.dart b/lib/notifications/display.dart index 6b517e5159..2e8288bdc4 100644 --- a/lib/notifications/display.dart +++ b/lib/notifications/display.dart @@ -15,6 +15,7 @@ import '../model/binding.dart'; import '../model/localizations.dart'; import '../model/narrow.dart'; import '../widgets/app.dart'; +import '../widgets/color.dart'; import '../widgets/message_list.dart'; import '../widgets/page.dart'; import '../widgets/store.dart'; @@ -151,7 +152,7 @@ class NotificationDisplayManager { channelId: NotificationChannelManager.kChannelId, groupKey: groupKey, - color: kZulipBrandColor.value, + color: kZulipBrandColor.argbInt, // TODO vary notification icon for debug smallIconResourceName: 'zulip_notification', // This name must appear in keep.xml too: https://github.com/zulip/zulip-flutter/issues/528 @@ -193,7 +194,7 @@ class NotificationDisplayManager { groupKey: groupKey, isGroupSummary: true, - color: kZulipBrandColor.value, + color: kZulipBrandColor.argbInt, // TODO vary notification icon for debug smallIconResourceName: 'zulip_notification', // This name must appear in keep.xml too: https://github.com/zulip/zulip-flutter/issues/528 inboxStyle: InboxStyle( diff --git a/lib/widgets/channel_colors.dart b/lib/widgets/channel_colors.dart index 9896b71e1e..764ac6697f 100644 --- a/lib/widgets/channel_colors.dart +++ b/lib/widgets/channel_colors.dart @@ -209,7 +209,7 @@ class ChannelColorSwatch extends ColorSwatch { swatch = a._swatch.map((key, color) => MapEntry(key, Color.lerp(color, b[key], t)!)); } } - return ChannelColorSwatch._(Color.lerp(a, b, t)!.value, swatch); + return ChannelColorSwatch._(Color.lerp(a, b, t)!.argbInt, swatch); } } diff --git a/lib/widgets/color.dart b/lib/widgets/color.dart index 7ef325e40e..59c75826ba 100644 --- a/lib/widgets/color.dart +++ b/lib/widgets/color.dart @@ -16,3 +16,26 @@ Color clampLchLightness(Color color, num lowerLimit, num upperLimit) { .copyWith(lightness: asLab.lightness.clamp(lowerLimit, upperLimit)) .toColor(); } + +extension ColorExtension on Color { + /// A 32 bit integer representing this sRGB color. + /// + /// If [colorSpace] is not [ColorSpace.sRGB], do not use this. + /// + /// The bits are assigned as follows: + /// + /// * Bits 24-31 are the alpha value. + /// * Bits 16-23 are the red value. + /// * Bits 8-15 are the green value. + /// * Bits 0-7 are the blue value. + /// + /// This is the same form that [Color.new] takes. + int get argbInt { + assert(colorSpace == ColorSpace.sRGB); + + return ((a * 255.0).round() & 0xff) << 24 | + ((r * 255.0).round() & 0xff) << 16 | + ((g * 255.0).round() & 0xff) << 8 | + ((b * 255.0).round() & 0xff) << 0; + } +} diff --git a/test/notifications/display_test.dart b/test/notifications/display_test.dart index 2cf45d8cf9..dbaef45f9f 100644 --- a/test/notifications/display_test.dart +++ b/test/notifications/display_test.dart @@ -20,6 +20,7 @@ import 'package:zulip/model/store.dart'; import 'package:zulip/notifications/display.dart'; import 'package:zulip/notifications/receive.dart'; import 'package:zulip/widgets/app.dart'; +import 'package:zulip/widgets/color.dart'; import 'package:zulip/widgets/inbox.dart'; import 'package:zulip/widgets/message_list.dart'; import 'package:zulip/widgets/page.dart'; @@ -183,7 +184,7 @@ void main() { ..conversationTitle.equals(expectedTitle) ..messages.deepEquals(messageStyleMessagesChecks)) ..number.equals(messageStyleMessages.length) - ..color.equals(kZulipBrandColor.value) + ..color.equals(kZulipBrandColor.argbInt) ..smallIconResourceName.equals('zulip_notification') ..extras.which((it) => it.isNotNull() ..deepEquals({ @@ -203,7 +204,7 @@ void main() { ..channelId.equals(NotificationChannelManager.kChannelId) ..contentTitle.isNull() ..contentText.isNull() - ..color.equals(kZulipBrandColor.value) + ..color.equals(kZulipBrandColor.argbInt) ..smallIconResourceName.equals('zulip_notification') ..extras.isNull() ..groupKey.equals(expectedGroupKey) diff --git a/test/widgets/color_test.dart b/test/widgets/color_test.dart new file mode 100644 index 0000000000..22f24a78b5 --- /dev/null +++ b/test/widgets/color_test.dart @@ -0,0 +1,18 @@ +import 'dart:ui'; + +import 'package:checks/checks.dart'; +import 'package:test/scaffolding.dart'; +import 'package:zulip/widgets/color.dart'; + +void main() { + group('ColorExtension', () { + test('argbInt smoke', () { + const testCases = [ + 0xffffffff, 0x00000000, 0x12345678, 0x87654321, 0xfedcba98, 0x89abcdef]; + + for (final testCase in testCases) { + check(Color(testCase).argbInt).equals(testCase); + } + }); + }); +} diff --git a/test/widgets/inbox_test.dart b/test/widgets/inbox_test.dart index 551c57da11..b75e219cc9 100644 --- a/test/widgets/inbox_test.dart +++ b/test/widgets/inbox_test.dart @@ -4,6 +4,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/model/store.dart'; +import 'package:zulip/widgets/color.dart'; import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/inbox.dart'; import 'package:zulip/widgets/channel_colors.dart'; @@ -470,7 +471,7 @@ void main() { }); testWidgets('uncollapsed header changes background color when [subscription.color] changes', (tester) async { - final initialColor = Colors.indigo.value; + final initialColor = Colors.indigo.argbInt; final stream = eg.stream(streamId: 1); await setupPage(tester, @@ -483,7 +484,7 @@ void main() { check(streamHeaderBackgroundColor(tester, 1)) .equals(ChannelColorSwatch.light(initialColor).barBackground); - final newColor = Colors.orange.value; + final newColor = Colors.orange.argbInt; store.handleEvent(SubscriptionUpdateEvent(id: 1, streamId: 1, property: SubscriptionProperty.color, value: newColor)); await tester.pump(); diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 0319b93c94..af9bbfc12c 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -15,6 +15,7 @@ import 'package:zulip/model/localizations.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/autocomplete.dart'; +import 'package:zulip/widgets/color.dart'; import 'package:zulip/widgets/content.dart'; import 'package:zulip/widgets/emoji_reaction.dart'; import 'package:zulip/widgets/icons.dart'; @@ -755,7 +756,7 @@ void main() { }); testWidgets('color of recipient header background', (tester) async { - final subscription = eg.subscription(stream, color: Colors.red.value); + final subscription = eg.subscription(stream, color: Colors.red.argbInt); final swatch = ChannelColorSwatch.light(subscription.color); await setupMessageListPage(tester, messages: [eg.streamMessage(stream: subscription)], @@ -770,7 +771,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 subscription = eg.subscription(stream, color: Colors.red.argbInt); final swatch = ChannelColorSwatch.light(subscription.color); await setupMessageListPage(tester, messages: [eg.streamMessage(stream: subscription)], diff --git a/test/widgets/subscription_list_test.dart b/test/widgets/subscription_list_test.dart index b1c7b89137..ac327a2894 100644 --- a/test/widgets/subscription_list_test.dart +++ b/test/widgets/subscription_list_test.dart @@ -3,6 +3,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:zulip/api/model/initial_snapshot.dart'; import 'package:zulip/api/model/model.dart'; +import 'package:zulip/widgets/color.dart'; import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/channel_colors.dart'; import 'package:zulip/widgets/subscription_list.dart'; @@ -232,7 +233,7 @@ void main() { final unreadMsgs = eg.unreadMsgs(channels: [ UnreadChannelSnapshot(streamId: stream.streamId, topic: 'a', unreadMessageIds: [1, 2]), ]); - final subscription = eg.subscription(stream, color: Colors.red.value); + final subscription = eg.subscription(stream, color: Colors.red.argbInt); final swatch = ChannelColorSwatch.light(subscription.color); await setupStreamListPage(tester, subscriptions: [ subscription,