Skip to content

Commit

Permalink
color [nfc]: Add/use extension with argbInt, replacing deprecated `…
Browse files Browse the repository at this point in the history
…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`.
  • Loading branch information
chrisbobbe authored and gnprice committed Sep 7, 2024
1 parent 459717e commit e200a09
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 10 deletions.
5 changes: 3 additions & 2 deletions lib/notifications/display.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion lib/widgets/channel_colors.dart
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ class ChannelColorSwatch extends ColorSwatch<ChannelColorVariant> {
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);
}
}

Expand Down
23 changes: 23 additions & 0 deletions lib/widgets/color.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
5 changes: 3 additions & 2 deletions test/notifications/display_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(<String, String>{
Expand All @@ -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)
Expand Down
18 changes: 18 additions & 0 deletions test/widgets/color_test.dart
Original file line number Diff line number Diff line change
@@ -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);
}
});
});
}
5 changes: 3 additions & 2 deletions test/widgets/inbox_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand All @@ -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();
Expand Down
5 changes: 3 additions & 2 deletions test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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)],
Expand All @@ -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)],
Expand Down
3 changes: 2 additions & 1 deletion test/widgets/subscription_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit e200a09

Please sign in to comment.