From 05e7c5428f4e514d2c714789a75c1d294c4308d7 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 6 Mar 2024 17:51:04 -0800 Subject: [PATCH] text: Set 0 letter spacing throughout zulipTypography MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This zeroes out the letter spacing in message content (#545), and also everywhere else that uses styles from zulipTypography without overriding the letter spacing. It's worth mentioning that letter spacing is one of the parameters that are carefully chosen for the 15 TextThemes that make up the default Typography in Material 3: {display,headline,title,label,body} × {small,medium,large} So this might be considered a regression...if the Material spec were our primary or only source of truth for text styles. But it isn't: we have a designer (Vlad), and this change brings us closer to his specified design. See #545 and #548. I also checked to see if the Material 3 default varies the letter spacing among the three [ScriptCategory]ies (englishLike, dense, and tall), which might be a reason to ask Vlad if we want to do the same. But the letter-spacing values are the same across the [ScriptCategory]ies, so no need for that. After this, there's still more to do for #548: - letter spacing of 1% (i.e., 0.01 times `fontSize`) in buttons - perhaps audit for UI code that's meant to follow Figma already, but where we failed to set a non-zero letter spacing that was specified in Figma Fixes: #545 Fixes-partly: #548 --- lib/widgets/text.dart | 4 ++++ test/flutter_checks.dart | 1 + test/widgets/text_test.dart | 7 +++++++ 3 files changed, 12 insertions(+) diff --git a/lib/widgets/text.dart b/lib/widgets/text.dart index 2d144a10a5a..dab42fd8836 100644 --- a/lib/widgets/text.dart +++ b/lib/widgets/text.dart @@ -36,6 +36,10 @@ Typography zulipTypography(BuildContext context) { convertGeometry(TextTheme inputTextTheme) { TextTheme result = _weightVariableTextTheme(context, inputTextTheme); + + result = _convertTextTheme(result, (maybeInputStyle, _) => + maybeInputStyle?.merge(const TextStyle(letterSpacing: 0))); + return result; } diff --git a/test/flutter_checks.dart b/test/flutter_checks.dart index 4344cc7ad1c..eed202ad68b 100644 --- a/test/flutter_checks.dart +++ b/test/flutter_checks.dart @@ -59,6 +59,7 @@ extension TextFieldChecks on Subject { extension TextStyleChecks on Subject { Subject get inherit => has((t) => t.inherit, 'inherit'); Subject get fontWeight => has((t) => t.fontWeight, 'fontWeight'); + Subject get letterSpacing => has((t) => t.letterSpacing, 'letterSpacing'); Subject?> get fontVariations => has((t) => t.fontVariations, 'fontVariations'); Subject get fontFamily => has((t) => t.fontFamily, 'fontFamily'); Subject?> get fontFamilyFallback => has((t) => t.fontFamilyFallback, 'fontFamilyFallback'); diff --git a/test/widgets/text_test.dart b/test/widgets/text_test.dart index e8c98481ed7..c522d8097a0 100644 --- a/test/widgets/text_test.dart +++ b/test/widgets/text_test.dart @@ -47,6 +47,13 @@ void main() { }); } + testWidgets('zero letter spacing', (tester) async { + check(await getZulipTypography(tester, platformRequestsBold: false)) + ..englishLike.bodyMedium.isNotNull().letterSpacing.equals(0) + ..dense.bodyMedium.isNotNull().letterSpacing.equals(0) + ..tall.bodyMedium.isNotNull().letterSpacing.equals(0); + }); + test('Typography has the assumed fields', () { check(Typography().toDiagnosticsNode().getProperties().map((n) => n.name).toList()) .unorderedEquals(['black', 'white', 'englishLike', 'dense', 'tall']);