Skip to content

Commit

Permalink
text: Set 0 letter spacing throughout zulipTypography
Browse files Browse the repository at this point in the history
This zeroes out the letter spacing in message content (zulip#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 zulip#545 and zulip#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 zulip#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: zulip#545
Fixes-partly: zulip#548
  • Loading branch information
chrisbobbe committed Mar 7, 2024
1 parent 68968ca commit 05e7c54
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 0 deletions.
4 changes: 4 additions & 0 deletions lib/widgets/text.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
1 change: 1 addition & 0 deletions test/flutter_checks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ extension TextFieldChecks on Subject<TextField> {
extension TextStyleChecks on Subject<TextStyle> {
Subject<bool> get inherit => has((t) => t.inherit, 'inherit');
Subject<FontWeight?> get fontWeight => has((t) => t.fontWeight, 'fontWeight');
Subject<double?> get letterSpacing => has((t) => t.letterSpacing, 'letterSpacing');
Subject<List<FontVariation>?> get fontVariations => has((t) => t.fontVariations, 'fontVariations');
Subject<String?> get fontFamily => has((t) => t.fontFamily, 'fontFamily');
Subject<List<String>?> get fontFamilyFallback => has((t) => t.fontFamilyFallback, 'fontFamilyFallback');
Expand Down
7 changes: 7 additions & 0 deletions test/widgets/text_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand Down

0 comments on commit 05e7c54

Please sign in to comment.