From 117670afd55a8131c7fa30964075a671507e6677 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 5 Dec 2023 17:37:39 -0800 Subject: [PATCH 01/11] text [nfc]: Pull out kDefaultFontFamily constant --- lib/widgets/content.dart | 2 +- lib/widgets/emoji_reaction.dart | 4 ++-- lib/widgets/inbox.dart | 6 +++--- lib/widgets/message_list.dart | 8 ++++---- lib/widgets/recent_dm_conversations.dart | 2 +- lib/widgets/subscription_list.dart | 6 +++--- lib/widgets/text.dart | 6 ++++++ lib/widgets/unread_count_badge.dart | 2 +- 8 files changed, 21 insertions(+), 15 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 1954b3931f..0ac4c96c1c 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -101,7 +101,7 @@ class Paragraph extends StatelessWidget { final ParagraphNode node; static TextStyle getTextStyle(BuildContext context) => const TextStyle( - fontFamily: 'Source Sans 3', + fontFamily: kDefaultFontFamily, fontSize: 14, height: (17 / 14), ).merge(weightVariableTextStyle(context)); diff --git a/lib/widgets/emoji_reaction.dart b/lib/widgets/emoji_reaction.dart index 7487140367..3f8afb7c90 100644 --- a/lib/widgets/emoji_reaction.dart +++ b/lib/widgets/emoji_reaction.dart @@ -173,7 +173,7 @@ class ReactionChip extends StatelessWidget { textWidthBasis: TextWidthBasis.longestLine, textScaler: _labelTextScalerClamped(context), style: TextStyle( - fontFamily: 'Source Sans 3', + fontFamily: kDefaultFontFamily, fontSize: (14 * 0.90), height: 13 / (14 * 0.90), color: labelColor, @@ -352,7 +352,7 @@ class _TextEmoji extends StatelessWidget { textScaler: _textEmojiScalerClamped(context), textWidthBasis: TextWidthBasis.longestLine, style: TextStyle( - fontFamily: 'Source Sans 3', + fontFamily: kDefaultFontFamily, fontSize: 14 * 0.8, height: 1, // to be denser when we have to wrap color: selected ? _textColorSelected : _textColorUnselected, diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index e8e343aa91..e65b2ef57c 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -238,7 +238,7 @@ abstract class _HeaderItem extends StatelessWidget { padding: const EdgeInsets.symmetric(vertical: 4), child: Text( style: const TextStyle( - fontFamily: 'Source Sans 3', + fontFamily: kDefaultFontFamily, fontSize: 17, height: (20 / 17), color: Color(0xFF222222), @@ -359,7 +359,7 @@ class _DmItem extends StatelessWidget { padding: const EdgeInsets.symmetric(vertical: 4), child: Text( style: const TextStyle( - fontFamily: 'Source Sans 3', + fontFamily: kDefaultFontFamily, fontSize: 17, height: (20 / 17), color: Color(0xFF222222), @@ -485,7 +485,7 @@ class _TopicItem extends StatelessWidget { padding: const EdgeInsets.symmetric(vertical: 4), child: Text( style: const TextStyle( - fontFamily: 'Source Sans 3', + fontFamily: kDefaultFontFamily, fontSize: 17, height: (20 / 17), color: Color(0xFF222222), diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 7358749fdc..ca2f96f3f0 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -450,7 +450,7 @@ class MarkAsReadWidget extends StatelessWidget { backgroundColor: _UnreadMarker.color, minimumSize: const Size.fromHeight(38), textStyle: const TextStyle( - fontFamily: 'Source Sans 3', + fontFamily: kDefaultFontFamily, fontSize: 18, height: (23 / 18), ).merge(weightVariableTextStyle(context)), @@ -631,7 +631,7 @@ class StreamMessageRecipientHeader extends StatelessWidget { } final textStyle = TextStyle( color: contrastingColor, - fontFamily: 'Source Sans 3', + fontFamily: kDefaultFontFamily, fontSize: 16, letterSpacing: 0.02 * 16, height: (18 / 16), @@ -742,7 +742,7 @@ class DmRecipientHeader extends StatelessWidget { Expanded( child: Text(title, style: const TextStyle( - fontFamily: 'Source Sans 3', + fontFamily: kDefaultFontFamily, fontSize: 16, letterSpacing: 0.02 * 16, height: (18 / 16), @@ -801,7 +801,7 @@ class DateText extends StatelessWidget { return Text( style: TextStyle( color: color, - fontFamily: 'Source Sans 3', + fontFamily: kDefaultFontFamily, fontSize: fontSize, height: height, // This is equivalent to css `all-small-caps`, see: diff --git a/lib/widgets/recent_dm_conversations.dart b/lib/widgets/recent_dm_conversations.dart index f8a84bf7b9..f1f36ca33a 100644 --- a/lib/widgets/recent_dm_conversations.dart +++ b/lib/widgets/recent_dm_conversations.dart @@ -127,7 +127,7 @@ class RecentDmConversationsItem extends StatelessWidget { padding: const EdgeInsets.symmetric(vertical: 4), child: Text( style: const TextStyle( - fontFamily: 'Source Sans 3', + fontFamily: kDefaultFontFamily, fontSize: 17, height: (20 / 17), color: Color(0xFF222222), diff --git a/lib/widgets/subscription_list.dart b/lib/widgets/subscription_list.dart index 935da57280..65d94fb50a 100644 --- a/lib/widgets/subscription_list.dart +++ b/lib/widgets/subscription_list.dart @@ -119,7 +119,7 @@ class _NoSubscriptionsItem extends StatelessWidget { textAlign: TextAlign.center, style: TextStyle( color: const HSLColor.fromAHSL(1.0, 240, 0.1, 0.5).toColor(), - fontFamily: 'Source Sans 3', + fontFamily: kDefaultFontFamily, fontSize: 18, height: (20 / 18), ).merge(weightVariableTextStyle(context))))); @@ -148,7 +148,7 @@ class _SubscriptionListHeader extends StatelessWidget { textAlign: TextAlign.center, style: TextStyle( color: const HSLColor.fromAHSL(1.0, 240, 0.1, 0.5).toColor(), - fontFamily: 'Source Sans 3', + fontFamily: kDefaultFontFamily, fontSize: 14, letterSpacing: 0.04 * 14, height: (16 / 14), @@ -221,7 +221,7 @@ class SubscriptionItem extends StatelessWidget { // https://github.com/zulip/zulip-flutter/pull/397#pullrequestreview-1742524205 child: Text( style: const TextStyle( - fontFamily: 'Source Sans 3', + fontFamily: kDefaultFontFamily, fontSize: 18, height: (20 / 18), color: Color(0xFF262626), diff --git a/lib/widgets/text.dart b/lib/widgets/text.dart index 1a80cee4bc..600f695ed6 100644 --- a/lib/widgets/text.dart +++ b/lib/widgets/text.dart @@ -1,6 +1,12 @@ import 'dart:io'; import 'package:flutter/widgets.dart'; +/// The [TextStyle.fontFamily] to use in most of the app. +/// +/// This is a variable-weight font, so any [TextStyle] that uses this should be +/// merged with the result of calling [weightVariableTextStyle]. +const kDefaultFontFamily = 'Source Sans 3'; + /// A mergeable [TextStyle] with 'Source Code Pro' and platform-aware fallbacks. /// /// Callers should also call [weightVariableTextStyle] and merge that in too, diff --git a/lib/widgets/unread_count_badge.dart b/lib/widgets/unread_count_badge.dart index dd7d97127f..8f44c8aec8 100644 --- a/lib/widgets/unread_count_badge.dart +++ b/lib/widgets/unread_count_badge.dart @@ -44,7 +44,7 @@ class UnreadCountBadge extends StatelessWidget { padding: const EdgeInsets.fromLTRB(4, 0, 4, 1), child: Text( style: const TextStyle( - fontFamily: 'Source Sans 3', + fontFamily: kDefaultFontFamily, fontSize: 16, height: (18 / 16), fontFeatures: [FontFeature.enable('smcp')], // small caps From 51eadddee19a649da0699b5db928b174b9a6c04e Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 5 Dec 2023 17:44:23 -0800 Subject: [PATCH 02/11] text: On Android, render emojis in message content with Noto Color Emoji In #436, we brought back Noto Color Emoji, but just for reaction chips. This brings it to message content in the message list, and all the other places where we've been specifying Source Sans 3. This leaves some edge cases where we'll still fall back to a system emoji font, like if there's an emoji in a user's name and you're looking at the profile screen. (Or in the "sender" area of a message in the message list, for that matter, checking again just now.) Those will be fixed later in this series, along with a cleanup that eliminates the repetition of [kDefaultFontFamily] and [kDefaultFontFamilyFallback] that we see in this commit. Fixes-partly: #438 --- lib/widgets/content.dart | 3 ++- lib/widgets/emoji_reaction.dart | 2 ++ lib/widgets/inbox.dart | 15 +++++++++------ lib/widgets/message_list.dart | 8 ++++++-- lib/widgets/recent_dm_conversations.dart | 5 +++-- lib/widgets/subscription_list.dart | 7 +++++-- lib/widgets/text.dart | 12 ++++++++++++ lib/widgets/unread_count_badge.dart | 7 ++++--- 8 files changed, 43 insertions(+), 16 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 0ac4c96c1c..318d1324f5 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -100,8 +100,9 @@ class Paragraph extends StatelessWidget { final ParagraphNode node; - static TextStyle getTextStyle(BuildContext context) => const TextStyle( + static TextStyle getTextStyle(BuildContext context) => TextStyle( fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback, fontSize: 14, height: (17 / 14), ).merge(weightVariableTextStyle(context)); diff --git a/lib/widgets/emoji_reaction.dart b/lib/widgets/emoji_reaction.dart index 3f8afb7c90..350fe2e7f6 100644 --- a/lib/widgets/emoji_reaction.dart +++ b/lib/widgets/emoji_reaction.dart @@ -174,6 +174,7 @@ class ReactionChip extends StatelessWidget { textScaler: _labelTextScalerClamped(context), style: TextStyle( fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback, fontSize: (14 * 0.90), height: 13 / (14 * 0.90), color: labelColor, @@ -353,6 +354,7 @@ class _TextEmoji extends StatelessWidget { textWidthBasis: TextWidthBasis.longestLine, style: TextStyle( fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback, fontSize: 14 * 0.8, height: 1, // to be denser when we have to wrap color: selected ? _textColorSelected : _textColorUnselected, diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index e65b2ef57c..69d59ac4dc 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -237,11 +237,12 @@ abstract class _HeaderItem extends StatelessWidget { Expanded(child: Padding( padding: const EdgeInsets.symmetric(vertical: 4), child: Text( - style: const TextStyle( + style: TextStyle( fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback, fontSize: 17, height: (20 / 17), - color: Color(0xFF222222), + color: const Color(0xFF222222), ).merge(weightVariableTextStyle(context, wght: 600, wghtIfPlatformRequestsBold: 900)), maxLines: 1, overflow: TextOverflow.ellipsis, @@ -358,11 +359,12 @@ class _DmItem extends StatelessWidget { Expanded(child: Padding( padding: const EdgeInsets.symmetric(vertical: 4), child: Text( - style: const TextStyle( + style: TextStyle( fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback, fontSize: 17, height: (20 / 17), - color: Color(0xFF222222), + color: const Color(0xFF222222), ).merge(weightVariableTextStyle(context)), maxLines: 2, overflow: TextOverflow.ellipsis, @@ -484,11 +486,12 @@ class _TopicItem extends StatelessWidget { Expanded(child: Padding( padding: const EdgeInsets.symmetric(vertical: 4), child: Text( - style: const TextStyle( + style: TextStyle( fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback, fontSize: 17, height: (20 / 17), - color: Color(0xFF222222), + color: const Color(0xFF222222), ).merge(weightVariableTextStyle(context)), maxLines: 2, overflow: TextOverflow.ellipsis, diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index ca2f96f3f0..d8727389fe 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -449,8 +449,9 @@ class MarkAsReadWidget extends StatelessWidget { style: FilledButton.styleFrom( backgroundColor: _UnreadMarker.color, minimumSize: const Size.fromHeight(38), - textStyle: const TextStyle( + textStyle: TextStyle( fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback, fontSize: 18, height: (23 / 18), ).merge(weightVariableTextStyle(context)), @@ -632,6 +633,7 @@ class StreamMessageRecipientHeader extends StatelessWidget { final textStyle = TextStyle( color: contrastingColor, fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback, fontSize: 16, letterSpacing: 0.02 * 16, height: (18 / 16), @@ -741,8 +743,9 @@ class DmRecipientHeader extends StatelessWidget { child: Icon(size: 16, ZulipIcons.user)), Expanded( child: Text(title, - style: const TextStyle( + style: TextStyle( fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback, fontSize: 16, letterSpacing: 0.02 * 16, height: (18 / 16), @@ -802,6 +805,7 @@ class DateText extends StatelessWidget { style: TextStyle( color: color, fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback, fontSize: fontSize, height: height, // This is equivalent to css `all-small-caps`, see: diff --git a/lib/widgets/recent_dm_conversations.dart b/lib/widgets/recent_dm_conversations.dart index f1f36ca33a..b87e8822b0 100644 --- a/lib/widgets/recent_dm_conversations.dart +++ b/lib/widgets/recent_dm_conversations.dart @@ -126,11 +126,12 @@ class RecentDmConversationsItem extends StatelessWidget { Expanded(child: Padding( padding: const EdgeInsets.symmetric(vertical: 4), child: Text( - style: const TextStyle( + style: TextStyle( fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback, fontSize: 17, height: (20 / 17), - color: Color(0xFF222222), + color: const Color(0xFF222222), ).merge(weightVariableTextStyle(context)), maxLines: 2, overflow: TextOverflow.ellipsis, diff --git a/lib/widgets/subscription_list.dart b/lib/widgets/subscription_list.dart index 65d94fb50a..c88cbc57f6 100644 --- a/lib/widgets/subscription_list.dart +++ b/lib/widgets/subscription_list.dart @@ -120,6 +120,7 @@ class _NoSubscriptionsItem extends StatelessWidget { style: TextStyle( color: const HSLColor.fromAHSL(1.0, 240, 0.1, 0.5).toColor(), fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback, fontSize: 18, height: (20 / 18), ).merge(weightVariableTextStyle(context))))); @@ -149,6 +150,7 @@ class _SubscriptionListHeader extends StatelessWidget { style: TextStyle( color: const HSLColor.fromAHSL(1.0, 240, 0.1, 0.5).toColor(), fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback, fontSize: 14, letterSpacing: 0.04 * 14, height: (16 / 14), @@ -220,11 +222,12 @@ class SubscriptionItem extends StatelessWidget { // or only those with unreads: // https://github.com/zulip/zulip-flutter/pull/397#pullrequestreview-1742524205 child: Text( - style: const TextStyle( + style: TextStyle( fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback, fontSize: 18, height: (20 / 18), - color: Color(0xFF262626), + color: const Color(0xFF262626), ).merge(hasUnreads ? weightVariableTextStyle(context, wght: 600, wghtIfPlatformRequestsBold: 900) : weightVariableTextStyle(context)), diff --git a/lib/widgets/text.dart b/lib/widgets/text.dart index 600f695ed6..759f569f6d 100644 --- a/lib/widgets/text.dart +++ b/lib/widgets/text.dart @@ -1,12 +1,24 @@ import 'dart:io'; +import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; /// The [TextStyle.fontFamily] to use in most of the app. /// +/// The same [TextStyle] should also specify [defaultFontFamilyFallback] +/// for [TextStyle.fontFamilyFallback]. +/// /// This is a variable-weight font, so any [TextStyle] that uses this should be /// merged with the result of calling [weightVariableTextStyle]. const kDefaultFontFamily = 'Source Sans 3'; +/// The [TextStyle.fontFamilyFallback] for use with [kDefaultFontFamily]. +List get defaultFontFamilyFallback => [ + // iOS doesn't support any of the formats this font is available in. + // If we use it on iOS, we'll get blank spaces where we could have had Apple- + // style emojis. + if (defaultTargetPlatform == TargetPlatform.android) 'Noto Color Emoji', +]; + /// A mergeable [TextStyle] with 'Source Code Pro' and platform-aware fallbacks. /// /// Callers should also call [weightVariableTextStyle] and merge that in too, diff --git a/lib/widgets/unread_count_badge.dart b/lib/widgets/unread_count_badge.dart index 8f44c8aec8..a511d44be2 100644 --- a/lib/widgets/unread_count_badge.dart +++ b/lib/widgets/unread_count_badge.dart @@ -43,12 +43,13 @@ class UnreadCountBadge extends StatelessWidget { child: Padding( padding: const EdgeInsets.fromLTRB(4, 0, 4, 1), child: Text( - style: const TextStyle( + style: TextStyle( fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback, fontSize: 16, height: (18 / 16), - fontFeatures: [FontFeature.enable('smcp')], // small caps - color: Color(0xFF222222), + fontFeatures: const [FontFeature.enable('smcp')], // small caps + color: const Color(0xFF222222), ).merge(bold ? weightVariableTextStyle(context, wght: 600, wghtIfPlatformRequestsBold: 900) : weightVariableTextStyle(context)), From f47ce05f96e0fdb343198205b80a17bcbf33cbbd Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 4 Dec 2023 11:36:34 -0800 Subject: [PATCH 03/11] content [nfc]: Use kBaseFontSize in a place we were using literal value --- lib/widgets/content.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 318d1324f5..cc0c08b069 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -103,8 +103,8 @@ class Paragraph extends StatelessWidget { static TextStyle getTextStyle(BuildContext context) => TextStyle( fontFamily: kDefaultFontFamily, fontFamilyFallback: defaultFontFamilyFallback, - fontSize: 14, - height: (17 / 14), + fontSize: kBaseFontSize, + height: (17 / kBaseFontSize), ).merge(weightVariableTextStyle(context)); @override From cdc2688cfd2164c3a9b340c65da6e8c664ac1f36 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 5 Dec 2023 10:54:34 -0800 Subject: [PATCH 04/11] text: Use more efficient MediaQuery.boldTextOf in weightVariableTextStyle Now this should only call for work when the bold text setting is changed, and not when other parts of [MediaQueryData] change. --- lib/widgets/text.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/widgets/text.dart b/lib/widgets/text.dart index 759f569f6d..8a8ca7a391 100644 --- a/lib/widgets/text.dart +++ b/lib/widgets/text.dart @@ -72,7 +72,7 @@ TextStyle weightVariableTextStyle(BuildContext? context, { }) { assert((wght != null) == (wghtIfPlatformRequestsBold != null)); double value = wght ?? FontWeight.normal.value.toDouble(); - if (context != null && MediaQuery.of(context).boldText) { + if (context != null && MediaQuery.boldTextOf(context)) { // The framework has a condition on [MediaQueryData.boldText] // in the [Text] widget, but that only affects `fontWeight`. // [Text] doesn't know where to land on the chosen font's "wght" axis if any, From 806a3c30118f7b4eaafcf21db8509c7184f34451 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 4 Dec 2023 14:26:38 -0800 Subject: [PATCH 05/11] text [nfc]: Take out helper constants kWght{Min,Max} While we're at it, make them doubles. --- lib/widgets/text.dart | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/widgets/text.dart b/lib/widgets/text.dart index 8a8ca7a391..f26505c8f3 100644 --- a/lib/widgets/text.dart +++ b/lib/widgets/text.dart @@ -79,7 +79,7 @@ TextStyle weightVariableTextStyle(BuildContext? context, { // and indeed it doesn't seem updated to be aware of variable fonts at all. value = wghtIfPlatformRequestsBold ?? FontWeight.bold.value.toDouble(); } - assert(value >= 1 && value <= 1000); // https://fonts.google.com/variablefonts#axis-definitions + assert(value >= kWghtMin && value <= kWghtMax); return TextStyle( fontVariations: [FontVariation('wght', value)], @@ -92,6 +92,16 @@ TextStyle weightVariableTextStyle(BuildContext? context, { inherit: true); } +/// The minimum that a [FontVariation] "wght" value can be. +/// +/// See . +const kWghtMin = 1.0; + +/// The maximum that a [FontVariation] "wght" value can be. +/// +/// See . +const kWghtMax = 1000.0; + /// Find the nearest [FontWeight] constant for a variable-font "wght"-axis value. /// /// Use this for a reasonable [TextStyle.fontWeight] for glyphs that need to be From bff20714fc3a1e94ed0f02661d47fda4244acb57 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 4 Dec 2023 14:54:50 -0800 Subject: [PATCH 06/11] text test [nfc]: Reorder some lines to be like surrounding code --- test/widgets/text_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/widgets/text_test.dart b/test/widgets/text_test.dart index 584950c736..b75b650850 100644 --- a/test/widgets/text_test.dart +++ b/test/widgets/text_test.dart @@ -56,8 +56,8 @@ void main() { expectedFontVariations: const [FontVariation('wght', 475)], expectedFontWeight: FontWeight.w500); testWeights('specific values; platform requests bold', - platformRequestsBold: true, styleBuilder: (context) => weightVariableTextStyle(context, wght: 475, wghtIfPlatformRequestsBold: 675), + platformRequestsBold: true, expectedFontVariations: const [FontVariation('wght', 675)], expectedFontWeight: FontWeight.w700); }); From 139fc3425e92834c35a98795cf4b6492caeeaccc Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 4 Dec 2023 14:26:05 -0800 Subject: [PATCH 07/11] text [nfc]: Don't force any callers to specify wghtIfPlatformRequestsBold In particular, remove this assert: assert((wght != null) == (wghtIfPlatformRequestsBold != null)); and default the effective `wghtIfPlatformRequestsBold`, when absent, to a consistent formula that gives the same result for all the callers so far. (Including callers that haven't been passing either `wght` or `wghtIfPlatformRequestsBold`: those have been getting [FontWeight.normal.value] / [FontWeight.bold.value], which is 400 / 700; and our new helper gives 700 for 400.) --- lib/widgets/emoji_reaction.dart | 10 ++++------ lib/widgets/inbox.dart | 2 +- lib/widgets/message_list.dart | 4 ++-- lib/widgets/subscription_list.dart | 5 ++--- lib/widgets/text.dart | 9 +++++++-- lib/widgets/unread_count_badge.dart | 5 ++--- test/widgets/text_test.dart | 30 +++++++++++++++++++++++++++++ 7 files changed, 48 insertions(+), 17 deletions(-) diff --git a/lib/widgets/emoji_reaction.dart b/lib/widgets/emoji_reaction.dart index 350fe2e7f6..b6d2d235f1 100644 --- a/lib/widgets/emoji_reaction.dart +++ b/lib/widgets/emoji_reaction.dart @@ -178,9 +178,8 @@ class ReactionChip extends StatelessWidget { fontSize: (14 * 0.90), height: 13 / (14 * 0.90), color: labelColor, - ).merge(selfVoted - ? weightVariableTextStyle(context, wght: 600, wghtIfPlatformRequestsBold: 900) - : weightVariableTextStyle(context)), + ).merge(weightVariableTextStyle(context, + wght: selfVoted ? 600 : null)), label), )), ]); @@ -358,9 +357,8 @@ class _TextEmoji extends StatelessWidget { fontSize: 14 * 0.8, height: 1, // to be denser when we have to wrap color: selected ? _textColorSelected : _textColorUnselected, - ).merge(selected - ? weightVariableTextStyle(context, wght: 600, wghtIfPlatformRequestsBold: 900) - : weightVariableTextStyle(context)), + ).merge(weightVariableTextStyle(context, + wght: selected ? 600 : null)), // Encourage line breaks before "_" (common in these), but try not // to leave a colon alone on a line. See: // diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index 69d59ac4dc..ef24228bdc 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -243,7 +243,7 @@ abstract class _HeaderItem extends StatelessWidget { fontSize: 17, height: (20 / 17), color: const Color(0xFF222222), - ).merge(weightVariableTextStyle(context, wght: 600, wghtIfPlatformRequestsBold: 900)), + ).merge(weightVariableTextStyle(context, wght: 600)), maxLines: 1, overflow: TextOverflow.ellipsis, title))), diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index d8727389fe..06e0a13fcf 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -637,7 +637,7 @@ class StreamMessageRecipientHeader extends StatelessWidget { fontSize: 16, letterSpacing: 0.02 * 16, height: (18 / 16), - ).merge(weightVariableTextStyle(context, wght: 600, wghtIfPlatformRequestsBold: 900)); + ).merge(weightVariableTextStyle(context, wght: 600)); final Widget streamWidget; if (!showStream) { @@ -749,7 +749,7 @@ class DmRecipientHeader extends StatelessWidget { fontSize: 16, letterSpacing: 0.02 * 16, height: (18 / 16), - ).merge(weightVariableTextStyle(context, wght: 600, wghtIfPlatformRequestsBold: 900)), + ).merge(weightVariableTextStyle(context, wght: 600)), overflow: TextOverflow.ellipsis)), RecipientHeaderDate(message: message, color: _kDmRecipientHeaderDateColor), diff --git a/lib/widgets/subscription_list.dart b/lib/widgets/subscription_list.dart index c88cbc57f6..6e30e3f64c 100644 --- a/lib/widgets/subscription_list.dart +++ b/lib/widgets/subscription_list.dart @@ -228,9 +228,8 @@ class SubscriptionItem extends StatelessWidget { fontSize: 18, height: (20 / 18), color: const Color(0xFF262626), - ).merge(hasUnreads - ? weightVariableTextStyle(context, wght: 600, wghtIfPlatformRequestsBold: 900) - : weightVariableTextStyle(context)), + ).merge(weightVariableTextStyle(context, + wght: hasUnreads ? 600 : null)), maxLines: 1, overflow: TextOverflow.ellipsis, subscription.name))), diff --git a/lib/widgets/text.dart b/lib/widgets/text.dart index f26505c8f3..eb7fefc448 100644 --- a/lib/widgets/text.dart +++ b/lib/widgets/text.dart @@ -70,14 +70,13 @@ TextStyle weightVariableTextStyle(BuildContext? context, { double? wght, double? wghtIfPlatformRequestsBold, }) { - assert((wght != null) == (wghtIfPlatformRequestsBold != null)); double value = wght ?? FontWeight.normal.value.toDouble(); if (context != null && MediaQuery.boldTextOf(context)) { // The framework has a condition on [MediaQueryData.boldText] // in the [Text] widget, but that only affects `fontWeight`. // [Text] doesn't know where to land on the chosen font's "wght" axis if any, // and indeed it doesn't seem updated to be aware of variable fonts at all. - value = wghtIfPlatformRequestsBold ?? FontWeight.bold.value.toDouble(); + value = wghtIfPlatformRequestsBold ?? bolderWght(value); } assert(value >= kWghtMin && value <= kWghtMax); @@ -102,6 +101,12 @@ const kWghtMin = 1.0; /// See . const kWghtMax = 1000.0; +/// A [FontVariation] "wght" value that's 300 above a given, clamped to [kWghtMax]. +double bolderWght(double baseWght) { + assert(kWghtMin <= baseWght && baseWght <= kWghtMax); + return clampDouble(baseWght + 300, kWghtMin, kWghtMax); +} + /// Find the nearest [FontWeight] constant for a variable-font "wght"-axis value. /// /// Use this for a reasonable [TextStyle.fontWeight] for glyphs that need to be diff --git a/lib/widgets/unread_count_badge.dart b/lib/widgets/unread_count_badge.dart index a511d44be2..11f8ea0604 100644 --- a/lib/widgets/unread_count_badge.dart +++ b/lib/widgets/unread_count_badge.dart @@ -50,9 +50,8 @@ class UnreadCountBadge extends StatelessWidget { height: (18 / 16), fontFeatures: const [FontFeature.enable('smcp')], // small caps color: const Color(0xFF222222), - ).merge(bold - ? weightVariableTextStyle(context, wght: 600, wghtIfPlatformRequestsBold: 900) - : weightVariableTextStyle(context)), + ).merge(weightVariableTextStyle(context, + wght: bold ? 600 : null)), count.toString()))); } } diff --git a/test/widgets/text_test.dart b/test/widgets/text_test.dart index b75b650850..95b01aa22e 100644 --- a/test/widgets/text_test.dart +++ b/test/widgets/text_test.dart @@ -50,6 +50,7 @@ void main() { platformRequestsBold: true, expectedFontVariations: const [FontVariation('wght', 700)], expectedFontWeight: FontWeight.bold); + testWeights('specific values; platform does not request bold', styleBuilder: (context) => weightVariableTextStyle(context, wght: 475, wghtIfPlatformRequestsBold: 675), platformRequestsBold: false, @@ -60,6 +61,35 @@ void main() { platformRequestsBold: true, expectedFontVariations: const [FontVariation('wght', 675)], expectedFontWeight: FontWeight.w700); + + testWeights('specific `wght`, default `wghtIfPlatformRequestsBold`; platform does not request bold', + styleBuilder: (context) => weightVariableTextStyle(context, wght: 475), + platformRequestsBold: false, + expectedFontVariations: const [FontVariation('wght', 475)], + expectedFontWeight: FontWeight.w500); + testWeights('specific `wght`, default `wghtIfPlatformRequestsBold`; platform requests bold', + styleBuilder: (context) => weightVariableTextStyle(context, wght: 475), + platformRequestsBold: true, + expectedFontVariations: const [FontVariation('wght', 775)], + expectedFontWeight: FontWeight.w800); + + testWeights('default `wght`, specific `wghtIfPlatformRequestsBold`; platform does not request bold', + styleBuilder: (context) => weightVariableTextStyle(context, wghtIfPlatformRequestsBold: 775), + platformRequestsBold: false, + expectedFontVariations: const [FontVariation('wght', 400)], + expectedFontWeight: FontWeight.normal); + testWeights('default `wght`, specific `wghtIfPlatformRequestsBold`; platform requests bold', + styleBuilder: (context) => weightVariableTextStyle(context, wghtIfPlatformRequestsBold: 775), + platformRequestsBold: true, + expectedFontVariations: const [FontVariation('wght', 775)], + expectedFontWeight: FontWeight.w800); + }); + + test('bolderWght', () { + check(bolderWght(1)).equals(301); + check(bolderWght(400)).equals(700); + check(bolderWght(600)).equals(900); + check(bolderWght(900)).equals(1000); }); test('clampVariableFontWeight: FontWeight has the assumed list of values', () { From 8567fdb710049162cb22bef28b54581310f28bbc Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 8 Jan 2024 16:00:24 -0800 Subject: [PATCH 08/11] text [nfc]: In dev, add `debugLabel` to result of weightVariableTextStyle This will take effect thanks to my PR for an upstream issue: https://github.com/flutter/flutter/issues/141140 https://github.com/flutter/flutter/pull/141141 --- lib/widgets/text.dart | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/widgets/text.dart b/lib/widgets/text.dart index eb7fefc448..2939859ff3 100644 --- a/lib/widgets/text.dart +++ b/lib/widgets/text.dart @@ -80,7 +80,7 @@ TextStyle weightVariableTextStyle(BuildContext? context, { } assert(value >= kWghtMin && value <= kWghtMax); - return TextStyle( + TextStyle result = TextStyle( fontVariations: [FontVariation('wght', value)], // This use of `fontWeight` shouldn't affect glyphs in the preferred, @@ -89,6 +89,18 @@ TextStyle weightVariableTextStyle(BuildContext? context, { fontWeight: clampVariableFontWeight(value), inherit: true); + + assert(() { + final attributes = [ + if (wght != null) 'wght: $wght', + if (wghtIfPlatformRequestsBold != null) 'wghtIfPlatformRequestsBold: $wghtIfPlatformRequestsBold', + ]; + result = result.copyWith( + debugLabel: 'weightVariableTextStyle(${attributes.join(', ')})'); + return true; + }()); + + return result; } /// The minimum that a [FontVariation] "wght" value can be. From 16d925ff1ea40b0cef58b305f704bc270e55a07a Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 4 Dec 2023 14:15:26 -0800 Subject: [PATCH 09/11] text: Use font-family constants for text built on Typography styles With plumbing to make kDefaultFamily work, since it's a variable-weight font. As it happens, most -- perhaps all -- of the app's text is built on Typography styles. See the next commit for some thoughts on that. Since that's the case, I'll mark this commit as fixing these issues: - #294 Use "Source Sans 3" font for most UI text - #438 Consistently use "Noto Color Emoji" for emojis on Android If there's some corner of the app where the two fonts aren't getting applied, we'll eventually find it, and apply kDefaultFontFamily and defaultFontFamilyFallback according to their doc comments, to fix. Fixes: #294 Fixes: #438 --- lib/widgets/app.dart | 2 + lib/widgets/text.dart | 118 +++++++++++++++++++++++++++++++++++- test/flutter_checks.dart | 31 +++++++++- test/widgets/text_test.dart | 57 +++++++++++++++++ 4 files changed, 206 insertions(+), 2 deletions(-) diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index 3ff1eee459..281e0ffbee 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -15,6 +15,7 @@ import 'page.dart'; import 'recent_dm_conversations.dart'; import 'store.dart'; import 'subscription_list.dart'; +import 'text.dart'; class ZulipApp extends StatelessWidget { const ZulipApp({super.key, this.navigatorObservers}); @@ -81,6 +82,7 @@ class ZulipApp extends StatelessWidget { @override Widget build(BuildContext context) { final theme = ThemeData( + typography: zulipTypography(context), appBarTheme: const AppBarTheme( // This prevents an elevation change in [AppBar]s so they stop turning // darker if there is something scrolled underneath it. See docs: diff --git a/lib/widgets/text.dart b/lib/widgets/text.dart index 2939859ff3..ce8eb611fa 100644 --- a/lib/widgets/text.dart +++ b/lib/widgets/text.dart @@ -1,6 +1,113 @@ import 'dart:io'; import 'package:flutter/foundation.dart'; -import 'package:flutter/widgets.dart'; +import 'package:flutter/material.dart'; + +/// An app-wide [Typography] for Zulip, customized from the Material default. +/// +/// Include this in the app-wide [MaterialApp.theme]. +/// +/// We expect these text styles to be the basis of all the styles chosen by the +/// Material library's widgets, such as the default styling of +/// an [AppBar]'s title, of an [ElevatedButton]'s label, and so on. +/// +/// Applies [kDefaultFontFamily] and [kDefaultFontFamilyFallback], +/// being faithful to the Material-default font weights +/// by running them through [weightVariableTextStyle]. +/// (That is needed because [kDefaultFontFamily] is a variable-weight font). +/// +/// When building on top of these [TextStyles], callers that wish to specify +/// a different font weight are still responsible for reprocessing the style +/// with [weightVariableTextStyle] before passing it to a [Text]. +/// (Widgets in the Material library won't do this; they aren't yet equipped +/// to set font weights on variable-weight fonts. If this causes visible bugs, +/// we should investigate and fix, but such bugs should become less likely as +/// we transition from Material's widgets to our own bespoke ones.) +Typography zulipTypography(BuildContext context) { + final typography = Theme.of(context).typography; + + Typography result = typography.copyWith( + black: typography.black.apply( + fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback), + white: typography.white.apply( + fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback), + + dense: _weightVariableTextTheme(context, typography.dense), + englishLike: _weightVariableTextTheme(context, typography.englishLike), + tall: _weightVariableTextTheme(context, typography.tall), + ); + + assert(() { + // Set [TextStyle.debugLabel] for all styles, like: + // "zulipTypography black titleMedium" + + mkAddLabel(String debugTextThemeLabel) + => (TextStyle? maybeInputStyle, String debugStyleLabel) + => maybeInputStyle?.copyWith(debugLabel: '$debugTextThemeLabel $debugStyleLabel'); + + result = result.copyWith( + black: _convertTextTheme(result.black, mkAddLabel('zulipTypography black')), + white: _convertTextTheme(result.white, mkAddLabel('zulipTypography white')), + englishLike: _convertTextTheme(result.englishLike, mkAddLabel('zulipTypography englishLike')), + dense: _convertTextTheme(result.dense, mkAddLabel('zulipTypography dense')), + tall: _convertTextTheme(result.tall, mkAddLabel('zulipTypography tall')), + ); + return true; + }()); + + return result; +} + +/// Convert a geometry [TextTheme] to one that works with "wght"-variable fonts. +/// +/// A "geometry [TextTheme]" is a [TextTheme] that's meant to specify +/// font weight and other parameters about shape, size, distance, etc. +/// See [Typography]. +/// +/// This looks at each of the [TextStyle]s found on the input [TextTheme] +/// (such as [TextTheme.bodyMedium]), +/// and uses [weightVariableTextStyle] to adjust the [TextStyle]. +/// Fields that are null in the input [TextTheme] remain null in the output. +/// +/// For each input [TextStyle], the `wght` value passed +/// to [weightVariableTextStyle] is based on the input's [TextStyle.fontWeight]. +/// A null [TextStyle.fontWeight] is interpreted as the normal font weight. +TextTheme _weightVariableTextTheme(BuildContext context, TextTheme input) { + TextStyle? convert(TextStyle? maybeInputStyle, _) { + if (maybeInputStyle == null) { + return null; + } + final inputFontWeight = maybeInputStyle.fontWeight; + return maybeInputStyle.merge(weightVariableTextStyle(context, + wght: inputFontWeight != null + ? wghtFromFontWeight(inputFontWeight) + : null)); + } + + return _convertTextTheme(input, convert); +} + +TextTheme _convertTextTheme( + TextTheme input, + TextStyle? Function(TextStyle?, String debugStyleLabel) converter, +) => TextTheme( + displayLarge: converter(input.displayLarge, 'displayLarge'), + displayMedium: converter(input.displayMedium, 'displayMedium'), + displaySmall: converter(input.displaySmall, 'displaySmall'), + headlineLarge: converter(input.headlineLarge, 'headlineLarge'), + headlineMedium: converter(input.headlineMedium, 'headlineMedium'), + headlineSmall: converter(input.headlineSmall, 'headlineSmall'), + titleLarge: converter(input.titleLarge, 'titleLarge'), + titleMedium: converter(input.titleMedium, 'titleMedium'), + titleSmall: converter(input.titleSmall, 'titleSmall'), + bodyLarge: converter(input.bodyLarge, 'bodyLarge'), + bodyMedium: converter(input.bodyMedium, 'bodyMedium'), + bodySmall: converter(input.bodySmall, 'bodySmall'), + labelLarge: converter(input.labelLarge, 'labelLarge'), + labelMedium: converter(input.labelMedium, 'labelMedium'), + labelSmall: converter(input.labelSmall, 'labelSmall'), +); /// The [TextStyle.fontFamily] to use in most of the app. /// @@ -145,3 +252,12 @@ FontWeight clampVariableFontWeight(double wght) { } } } + +/// A good guess at a font's "wght" value to match a given [FontWeight]. +/// +/// Returns [FontWeight.value] as a double. +/// +/// This might not be exactly where the font designer would land on their +/// font's own custom-defined "wght" axis. But it's a great guess, +/// at least without knowledge of the particular font. +double wghtFromFontWeight(FontWeight fontWeight) => fontWeight.value.toDouble(); diff --git a/test/flutter_checks.dart b/test/flutter_checks.dart index 6fcd4a1a18..81977e9933 100644 --- a/test/flutter_checks.dart +++ b/test/flutter_checks.dart @@ -54,8 +54,37 @@ extension TextFieldChecks on Subject { extension TextStyleChecks on Subject { Subject get inherit => has((t) => t.inherit, 'inherit'); - Subject?> get fontVariations => has((t) => t.fontVariations, 'fontVariations'); Subject get fontWeight => has((t) => t.fontWeight, 'fontWeight'); + Subject?> get fontVariations => has((t) => t.fontVariations, 'fontVariations'); + Subject get fontFamily => has((t) => t.fontFamily, 'fontFamily'); + Subject?> get fontFamilyFallback => has((t) => t.fontFamilyFallback, 'fontFamilyFallback'); // TODO others } + + +extension TextThemeChecks on Subject { + Subject get displayLarge => has((t) => t.displayLarge, 'displayLarge'); + Subject get displayMedium => has((t) => t.displayMedium, 'displayMedium'); + Subject get displaySmall => has((t) => t.displaySmall, 'displaySmall'); + Subject get headlineLarge => has((t) => t.headlineLarge, 'headlineLarge'); + Subject get headlineMedium => has((t) => t.headlineMedium, 'headlineMedium'); + Subject get headlineSmall => has((t) => t.headlineSmall, 'headlineSmall'); + Subject get titleLarge => has((t) => t.titleLarge, 'titleLarge'); + Subject get titleMedium => has((t) => t.titleMedium, 'titleMedium'); + Subject get titleSmall => has((t) => t.titleSmall, 'titleSmall'); + Subject get bodyLarge => has((t) => t.bodyLarge, 'bodyLarge'); + Subject get bodyMedium => has((t) => t.bodyMedium, 'bodyMedium'); + Subject get bodySmall => has((t) => t.bodySmall, 'bodySmall'); + Subject get labelLarge => has((t) => t.labelLarge, 'labelLarge'); + Subject get labelMedium => has((t) => t.labelMedium, 'labelMedium'); + Subject get labelSmall => has((t) => t.labelSmall, 'labelSmall'); +} + +extension TypographyChecks on Subject { + Subject get black => has((t) => t.black, 'black'); + Subject get white => has((t) => t.white, 'white'); + Subject get englishLike => has((t) => t.englishLike, 'englishLike'); + Subject get dense => has((t) => t.dense, 'dense'); + Subject get tall => has((t) => t.tall, 'tall'); +} diff --git a/test/widgets/text_test.dart b/test/widgets/text_test.dart index 95b01aa22e..e8c98481ed 100644 --- a/test/widgets/text_test.dart +++ b/test/widgets/text_test.dart @@ -7,6 +7,63 @@ import 'package:zulip/widgets/text.dart'; import '../flutter_checks.dart'; void main() { + group('zulipTypography', () { + Future getZulipTypography(WidgetTester tester, { + required bool platformRequestsBold, + }) async { + late final Typography result; + await tester.pumpWidget( + MediaQuery(data: MediaQueryData(boldText: platformRequestsBold), + child: Builder(builder: (context) { + result = zulipTypography(context); + return const SizedBox.shrink(); + }))); + return result; + } + + matchesFontFamilies(Subject it) => it + ..fontFamily.equals(kDefaultFontFamily) + ..fontFamilyFallback.isNotNull().deepEquals(defaultFontFamilyFallback); + + matchesWeight(FontWeight weight) => (Subject it) => it + ..fontWeight.equals(weight) + ..fontVariations.isNotNull().contains( + FontVariation('wght', wghtFromFontWeight(weight))); + + for (final platformRequestsBold in [false, true]) { + final description = platformRequestsBold + ? 'platform requests bold' + : 'platform does not request bold'; + testWidgets(description, (tester) async { + check(await getZulipTypography(tester, platformRequestsBold: platformRequestsBold)) + ..black.bodyMedium.isNotNull().which(matchesFontFamilies) + ..white.bodyMedium.isNotNull().which(matchesFontFamilies) + ..englishLike.bodyMedium.isNotNull().which( + matchesWeight(platformRequestsBold ? FontWeight.w700 : FontWeight.w400)) + ..dense.bodyMedium.isNotNull().which( + matchesWeight(platformRequestsBold ? FontWeight.w700 : FontWeight.w400)) + ..tall.bodyMedium.isNotNull().which( + matchesWeight(platformRequestsBold ? FontWeight.w700 : FontWeight.w400)); + }); + } + + test('Typography has the assumed fields', () { + check(Typography().toDiagnosticsNode().getProperties().map((n) => n.name).toList()) + .unorderedEquals(['black', 'white', 'englishLike', 'dense', 'tall']); + }); + }); + + test('_convertTextTheme: TextTheme has the assumed fields', () { + check(const TextTheme().toDiagnosticsNode().getProperties().map((n) => n.name).toList()) + .unorderedEquals([ + 'displayLarge', 'displayMedium', 'displaySmall', + 'headlineLarge', 'headlineMedium', 'headlineSmall', + 'titleLarge', 'titleMedium', 'titleSmall', + 'bodyLarge', 'bodyMedium', 'bodySmall', + 'labelLarge', 'labelMedium', 'labelSmall', + ]); + }); + group('weightVariableTextStyle', () { Future testWeights( String description, { From 6784ef9ca370c1e52692e3ac4d14f7fa54c69f8d Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 5 Dec 2023 11:48:54 -0800 Subject: [PATCH 10/11] text [nfc]: Remove some now-redundant fontFamily{,Fallback} attributes --- lib/widgets/content.dart | 6 ++---- lib/widgets/emoji_reaction.dart | 4 ---- lib/widgets/inbox.dart | 22 ++++++++------------- lib/widgets/message_list.dart | 25 +++++++++++------------- lib/widgets/recent_dm_conversations.dart | 9 +++------ lib/widgets/subscription_list.dart | 14 ++++--------- lib/widgets/text.dart | 9 +++++++++ lib/widgets/unread_count_badge.dart | 8 +++----- 8 files changed, 40 insertions(+), 57 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index cc0c08b069..03d351005f 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -100,12 +100,10 @@ class Paragraph extends StatelessWidget { final ParagraphNode node; - static TextStyle getTextStyle(BuildContext context) => TextStyle( - fontFamily: kDefaultFontFamily, - fontFamilyFallback: defaultFontFamilyFallback, + static TextStyle getTextStyle(BuildContext context) => const TextStyle( fontSize: kBaseFontSize, height: (17 / kBaseFontSize), - ).merge(weightVariableTextStyle(context)); + ); @override Widget build(BuildContext context) { diff --git a/lib/widgets/emoji_reaction.dart b/lib/widgets/emoji_reaction.dart index b6d2d235f1..12aed292e7 100644 --- a/lib/widgets/emoji_reaction.dart +++ b/lib/widgets/emoji_reaction.dart @@ -173,8 +173,6 @@ class ReactionChip extends StatelessWidget { textWidthBasis: TextWidthBasis.longestLine, textScaler: _labelTextScalerClamped(context), style: TextStyle( - fontFamily: kDefaultFontFamily, - fontFamilyFallback: defaultFontFamilyFallback, fontSize: (14 * 0.90), height: 13 / (14 * 0.90), color: labelColor, @@ -352,8 +350,6 @@ class _TextEmoji extends StatelessWidget { textScaler: _textEmojiScalerClamped(context), textWidthBasis: TextWidthBasis.longestLine, style: TextStyle( - fontFamily: kDefaultFontFamily, - fontFamilyFallback: defaultFontFamilyFallback, fontSize: 14 * 0.8, height: 1, // to be denser when we have to wrap color: selected ? _textColorSelected : _textColorUnselected, diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index ef24228bdc..efd28c042b 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -237,12 +237,10 @@ abstract class _HeaderItem extends StatelessWidget { Expanded(child: Padding( padding: const EdgeInsets.symmetric(vertical: 4), child: Text( - style: TextStyle( - fontFamily: kDefaultFontFamily, - fontFamilyFallback: defaultFontFamilyFallback, + style: const TextStyle( fontSize: 17, height: (20 / 17), - color: const Color(0xFF222222), + color: Color(0xFF222222), ).merge(weightVariableTextStyle(context, wght: 600)), maxLines: 1, overflow: TextOverflow.ellipsis, @@ -359,13 +357,11 @@ class _DmItem extends StatelessWidget { Expanded(child: Padding( padding: const EdgeInsets.symmetric(vertical: 4), child: Text( - style: TextStyle( - fontFamily: kDefaultFontFamily, - fontFamilyFallback: defaultFontFamilyFallback, + style: const TextStyle( fontSize: 17, height: (20 / 17), - color: const Color(0xFF222222), - ).merge(weightVariableTextStyle(context)), + color: Color(0xFF222222), + ), maxLines: 2, overflow: TextOverflow.ellipsis, title))), @@ -486,13 +482,11 @@ class _TopicItem extends StatelessWidget { Expanded(child: Padding( padding: const EdgeInsets.symmetric(vertical: 4), child: Text( - style: TextStyle( - fontFamily: kDefaultFontFamily, - fontFamilyFallback: defaultFontFamilyFallback, + style: const TextStyle( fontSize: 17, height: (20 / 17), - color: const Color(0xFF222222), - ).merge(weightVariableTextStyle(context)), + color: Color(0xFF222222), + ), maxLines: 2, overflow: TextOverflow.ellipsis, topic))), diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 06e0a13fcf..625dd393c2 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -449,12 +449,15 @@ class MarkAsReadWidget extends StatelessWidget { style: FilledButton.styleFrom( backgroundColor: _UnreadMarker.color, minimumSize: const Size.fromHeight(38), - textStyle: TextStyle( - fontFamily: kDefaultFontFamily, - fontFamilyFallback: defaultFontFamilyFallback, - fontSize: 18, - height: (23 / 18), - ).merge(weightVariableTextStyle(context)), + textStyle: + // Restate [FilledButton]'s default, which inherits from + // [zulipTypography]… + Theme.of(context).textTheme.labelLarge! + // …then clobber some attributes to follow Figma: + .merge(const TextStyle( + fontSize: 18, + height: (23 / 18)) + .merge(weightVariableTextStyle(context, wght: 400))), shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(7)), ), onPressed: () => _handlePress(context), @@ -632,8 +635,6 @@ class StreamMessageRecipientHeader extends StatelessWidget { } final textStyle = TextStyle( color: contrastingColor, - fontFamily: kDefaultFontFamily, - fontFamilyFallback: defaultFontFamilyFallback, fontSize: 16, letterSpacing: 0.02 * 16, height: (18 / 16), @@ -743,9 +744,7 @@ class DmRecipientHeader extends StatelessWidget { child: Icon(size: 16, ZulipIcons.user)), Expanded( child: Text(title, - style: TextStyle( - fontFamily: kDefaultFontFamily, - fontFamilyFallback: defaultFontFamilyFallback, + style: const TextStyle( fontSize: 16, letterSpacing: 0.02 * 16, height: (18 / 16), @@ -804,14 +803,12 @@ class DateText extends StatelessWidget { return Text( style: TextStyle( color: color, - fontFamily: kDefaultFontFamily, - fontFamilyFallback: defaultFontFamilyFallback, fontSize: fontSize, height: height, // This is equivalent to css `all-small-caps`, see: // https://developer.mozilla.org/en-US/docs/Web/CSS/font-variant-caps#all-small-caps fontFeatures: const [FontFeature.enable('c2sc'), FontFeature.enable('smcp')], - ).merge(weightVariableTextStyle(context)), + ), formatHeaderDate( zulipLocalizations, DateTime.fromMillisecondsSinceEpoch(timestamp * 1000), diff --git a/lib/widgets/recent_dm_conversations.dart b/lib/widgets/recent_dm_conversations.dart index b87e8822b0..4791bd4b5c 100644 --- a/lib/widgets/recent_dm_conversations.dart +++ b/lib/widgets/recent_dm_conversations.dart @@ -8,7 +8,6 @@ import 'icons.dart'; import 'message_list.dart'; import 'page.dart'; import 'store.dart'; -import 'text.dart'; import 'unread_count_badge.dart'; class RecentDmConversationsPage extends StatefulWidget { @@ -126,13 +125,11 @@ class RecentDmConversationsItem extends StatelessWidget { Expanded(child: Padding( padding: const EdgeInsets.symmetric(vertical: 4), child: Text( - style: TextStyle( - fontFamily: kDefaultFontFamily, - fontFamilyFallback: defaultFontFamilyFallback, + style: const TextStyle( fontSize: 17, height: (20 / 17), - color: const Color(0xFF222222), - ).merge(weightVariableTextStyle(context)), + color: Color(0xFF222222), + ), maxLines: 2, overflow: TextOverflow.ellipsis, title))), diff --git a/lib/widgets/subscription_list.dart b/lib/widgets/subscription_list.dart index 6e30e3f64c..97c75f8eca 100644 --- a/lib/widgets/subscription_list.dart +++ b/lib/widgets/subscription_list.dart @@ -119,11 +119,9 @@ class _NoSubscriptionsItem extends StatelessWidget { textAlign: TextAlign.center, style: TextStyle( color: const HSLColor.fromAHSL(1.0, 240, 0.1, 0.5).toColor(), - fontFamily: kDefaultFontFamily, - fontFamilyFallback: defaultFontFamilyFallback, fontSize: 18, height: (20 / 18), - ).merge(weightVariableTextStyle(context))))); + )))); } } @@ -149,12 +147,10 @@ class _SubscriptionListHeader extends StatelessWidget { textAlign: TextAlign.center, style: TextStyle( color: const HSLColor.fromAHSL(1.0, 240, 0.1, 0.5).toColor(), - fontFamily: kDefaultFontFamily, - fontFamilyFallback: defaultFontFamilyFallback, fontSize: 14, letterSpacing: 0.04 * 14, height: (16 / 14), - ).merge(weightVariableTextStyle(context)))), + ))), const SizedBox(width: 8), Expanded(child: Divider( color: const HSLColor.fromAHSL(0.2, 240, 0.1, 0.5).toColor())), @@ -222,12 +218,10 @@ class SubscriptionItem extends StatelessWidget { // or only those with unreads: // https://github.com/zulip/zulip-flutter/pull/397#pullrequestreview-1742524205 child: Text( - style: TextStyle( - fontFamily: kDefaultFontFamily, - fontFamilyFallback: defaultFontFamilyFallback, + style: const TextStyle( fontSize: 18, height: (20 / 18), - color: const Color(0xFF262626), + color: Color(0xFF262626), ).merge(weightVariableTextStyle(context, wght: hasUnreads ? 600 : null)), maxLines: 1, diff --git a/lib/widgets/text.dart b/lib/widgets/text.dart index ce8eb611fa..4fb9e1cec7 100644 --- a/lib/widgets/text.dart +++ b/lib/widgets/text.dart @@ -10,6 +10,12 @@ import 'package:flutter/material.dart'; /// Material library's widgets, such as the default styling of /// an [AppBar]'s title, of an [ElevatedButton]'s label, and so on. /// +/// As of writing, it turns out that these styles also flow naturally into +/// most of our own widgets' text styles. +/// We often see this in the child of a [Material], for example, +/// since by default [Material] applies an [AnimatedDefaultTextStyle] +/// with the [TextTheme.bodyMedium] that gets its value from here. +/// /// Applies [kDefaultFontFamily] and [kDefaultFontFamilyFallback], /// being faithful to the Material-default font weights /// by running them through [weightVariableTextStyle]. @@ -22,6 +28,9 @@ import 'package:flutter/material.dart'; /// to set font weights on variable-weight fonts. If this causes visible bugs, /// we should investigate and fix, but such bugs should become less likely as /// we transition from Material's widgets to our own bespoke ones.) +// TODO decide if we like this data flow for our own widgets' text styles. +// Does our design fit well with the fields of a [TextTheme]? +// (That's [TextTheme.titleLarge], [TextTheme.bodyMedium], etc.) Typography zulipTypography(BuildContext context) { final typography = Theme.of(context).typography; diff --git a/lib/widgets/unread_count_badge.dart b/lib/widgets/unread_count_badge.dart index 11f8ea0604..45df67e80d 100644 --- a/lib/widgets/unread_count_badge.dart +++ b/lib/widgets/unread_count_badge.dart @@ -43,13 +43,11 @@ class UnreadCountBadge extends StatelessWidget { child: Padding( padding: const EdgeInsets.fromLTRB(4, 0, 4, 1), child: Text( - style: TextStyle( - fontFamily: kDefaultFontFamily, - fontFamilyFallback: defaultFontFamilyFallback, + style: const TextStyle( fontSize: 16, height: (18 / 16), - fontFeatures: const [FontFeature.enable('smcp')], // small caps - color: const Color(0xFF222222), + fontFeatures: [FontFeature.enable('smcp')], // small caps + color: Color(0xFF222222), ).merge(weightVariableTextStyle(context, wght: bold ? 600 : null)), count.toString()))); From 97afdcdd758fe9a5b6f2e0d59c60fe0c70d7a768 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 5 Jan 2024 14:20:42 -0800 Subject: [PATCH 11/11] content [nfc]: Make paragraph text style `const` --- lib/widgets/content.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 03d351005f..b569f43b55 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -100,7 +100,7 @@ class Paragraph extends StatelessWidget { final ParagraphNode node; - static TextStyle getTextStyle(BuildContext context) => const TextStyle( + static const textStyle = TextStyle( fontSize: kBaseFontSize, height: (17 / kBaseFontSize), ); @@ -113,7 +113,7 @@ class Paragraph extends StatelessWidget { final text = _buildBlockInlineContainer( node: node, - style: getTextStyle(context), + style: textStyle, ); // If the paragraph didn't actually have a `p` element in the HTML, @@ -646,7 +646,7 @@ class UserMention extends StatelessWidget { // One hopes an @-mention can't contain an embedded link. // (The parser on creating a UserMentionNode has a TODO to check that.) linkRecognizers: null, - style: Paragraph.getTextStyle(context), + style: Paragraph.textStyle, nodes: node.nodes)); }