Skip to content

Commit 16d925f

Browse files
chrisbobbechrisbobbe
committed
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
1 parent 8567fdb commit 16d925f

File tree

4 files changed

+206
-2
lines changed

4 files changed

+206
-2
lines changed

‎lib/widgets/app.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import 'page.dart';
1515
import 'recent_dm_conversations.dart';
1616
import 'store.dart';
1717
import 'subscription_list.dart';
18+
import 'text.dart';
1819

1920
class ZulipApp extends StatelessWidget {
2021
const ZulipApp({super.key, this.navigatorObservers});
@@ -81,6 +82,7 @@ class ZulipApp extends StatelessWidget {
8182
@override
8283
Widget build(BuildContext context) {
8384
final theme = ThemeData(
85+
typography: zulipTypography(context),
8486
appBarTheme: const AppBarTheme(
8587
// This prevents an elevation change in [AppBar]s so they stop turning
8688
// darker if there is something scrolled underneath it. See docs:

‎lib/widgets/text.dart

Lines changed: 117 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,113 @@
11
import 'dart:io';
22
import 'package:flutter/foundation.dart';
3-
import 'package:flutter/widgets.dart';
3+
import 'package:flutter/material.dart';
4+
5+
/// An app-wide [Typography] for Zulip, customized from the Material default.
6+
///
7+
/// Include this in the app-wide [MaterialApp.theme].
8+
///
9+
/// We expect these text styles to be the basis of all the styles chosen by the
10+
/// Material library's widgets, such as the default styling of
11+
/// an [AppBar]'s title, of an [ElevatedButton]'s label, and so on.
12+
///
13+
/// Applies [kDefaultFontFamily] and [kDefaultFontFamilyFallback],
14+
/// being faithful to the Material-default font weights
15+
/// by running them through [weightVariableTextStyle].
16+
/// (That is needed because [kDefaultFontFamily] is a variable-weight font).
17+
///
18+
/// When building on top of these [TextStyles], callers that wish to specify
19+
/// a different font weight are still responsible for reprocessing the style
20+
/// with [weightVariableTextStyle] before passing it to a [Text].
21+
/// (Widgets in the Material library won't do this; they aren't yet equipped
22+
/// to set font weights on variable-weight fonts. If this causes visible bugs,
23+
/// we should investigate and fix, but such bugs should become less likely as
24+
/// we transition from Material's widgets to our own bespoke ones.)
25+
Typography zulipTypography(BuildContext context) {
26+
final typography = Theme.of(context).typography;
27+
28+
Typography result = typography.copyWith(
29+
black: typography.black.apply(
30+
fontFamily: kDefaultFontFamily,
31+
fontFamilyFallback: defaultFontFamilyFallback),
32+
white: typography.white.apply(
33+
fontFamily: kDefaultFontFamily,
34+
fontFamilyFallback: defaultFontFamilyFallback),
35+
36+
dense: _weightVariableTextTheme(context, typography.dense),
37+
englishLike: _weightVariableTextTheme(context, typography.englishLike),
38+
tall: _weightVariableTextTheme(context, typography.tall),
39+
);
40+
41+
assert(() {
42+
// Set [TextStyle.debugLabel] for all styles, like:
43+
// "zulipTypography black titleMedium"
44+
45+
mkAddLabel(String debugTextThemeLabel)
46+
=> (TextStyle? maybeInputStyle, String debugStyleLabel)
47+
=> maybeInputStyle?.copyWith(debugLabel: '$debugTextThemeLabel $debugStyleLabel');
48+
49+
result = result.copyWith(
50+
black: _convertTextTheme(result.black, mkAddLabel('zulipTypography black')),
51+
white: _convertTextTheme(result.white, mkAddLabel('zulipTypography white')),
52+
englishLike: _convertTextTheme(result.englishLike, mkAddLabel('zulipTypography englishLike')),
53+
dense: _convertTextTheme(result.dense, mkAddLabel('zulipTypography dense')),
54+
tall: _convertTextTheme(result.tall, mkAddLabel('zulipTypography tall')),
55+
);
56+
return true;
57+
}());
58+
59+
return result;
60+
}
61+
62+
/// Convert a geometry [TextTheme] to one that works with "wght"-variable fonts.
63+
///
64+
/// A "geometry [TextTheme]" is a [TextTheme] that's meant to specify
65+
/// font weight and other parameters about shape, size, distance, etc.
66+
/// See [Typography].
67+
///
68+
/// This looks at each of the [TextStyle]s found on the input [TextTheme]
69+
/// (such as [TextTheme.bodyMedium]),
70+
/// and uses [weightVariableTextStyle] to adjust the [TextStyle].
71+
/// Fields that are null in the input [TextTheme] remain null in the output.
72+
///
73+
/// For each input [TextStyle], the `wght` value passed
74+
/// to [weightVariableTextStyle] is based on the input's [TextStyle.fontWeight].
75+
/// A null [TextStyle.fontWeight] is interpreted as the normal font weight.
76+
TextTheme _weightVariableTextTheme(BuildContext context, TextTheme input) {
77+
TextStyle? convert(TextStyle? maybeInputStyle, _) {
78+
if (maybeInputStyle == null) {
79+
return null;
80+
}
81+
final inputFontWeight = maybeInputStyle.fontWeight;
82+
return maybeInputStyle.merge(weightVariableTextStyle(context,
83+
wght: inputFontWeight != null
84+
? wghtFromFontWeight(inputFontWeight)
85+
: null));
86+
}
87+
88+
return _convertTextTheme(input, convert);
89+
}
90+
91+
TextTheme _convertTextTheme(
92+
TextTheme input,
93+
TextStyle? Function(TextStyle?, String debugStyleLabel) converter,
94+
) => TextTheme(
95+
displayLarge: converter(input.displayLarge, 'displayLarge'),
96+
displayMedium: converter(input.displayMedium, 'displayMedium'),
97+
displaySmall: converter(input.displaySmall, 'displaySmall'),
98+
headlineLarge: converter(input.headlineLarge, 'headlineLarge'),
99+
headlineMedium: converter(input.headlineMedium, 'headlineMedium'),
100+
headlineSmall: converter(input.headlineSmall, 'headlineSmall'),
101+
titleLarge: converter(input.titleLarge, 'titleLarge'),
102+
titleMedium: converter(input.titleMedium, 'titleMedium'),
103+
titleSmall: converter(input.titleSmall, 'titleSmall'),
104+
bodyLarge: converter(input.bodyLarge, 'bodyLarge'),
105+
bodyMedium: converter(input.bodyMedium, 'bodyMedium'),
106+
bodySmall: converter(input.bodySmall, 'bodySmall'),
107+
labelLarge: converter(input.labelLarge, 'labelLarge'),
108+
labelMedium: converter(input.labelMedium, 'labelMedium'),
109+
labelSmall: converter(input.labelSmall, 'labelSmall'),
110+
);
4111

5112
/// The [TextStyle.fontFamily] to use in most of the app.
6113
///
@@ -145,3 +252,12 @@ FontWeight clampVariableFontWeight(double wght) {
145252
}
146253
}
147254
}
255+
256+
/// A good guess at a font's "wght" value to match a given [FontWeight].
257+
///
258+
/// Returns [FontWeight.value] as a double.
259+
///
260+
/// This might not be exactly where the font designer would land on their
261+
/// font's own custom-defined "wght" axis. But it's a great guess,
262+
/// at least without knowledge of the particular font.
263+
double wghtFromFontWeight(FontWeight fontWeight) => fontWeight.value.toDouble();

‎test/flutter_checks.dart

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,37 @@ extension TextFieldChecks on Subject<TextField> {
5454

5555
extension TextStyleChecks on Subject<TextStyle> {
5656
Subject<bool> get inherit => has((t) => t.inherit, 'inherit');
57-
Subject<List<FontVariation>?> get fontVariations => has((t) => t.fontVariations, 'fontVariations');
5857
Subject<FontWeight?> get fontWeight => has((t) => t.fontWeight, 'fontWeight');
58+
Subject<List<FontVariation>?> get fontVariations => has((t) => t.fontVariations, 'fontVariations');
59+
Subject<String?> get fontFamily => has((t) => t.fontFamily, 'fontFamily');
60+
Subject<List<String>?> get fontFamilyFallback => has((t) => t.fontFamilyFallback, 'fontFamilyFallback');
5961

6062
// TODO others
6163
}
64+
65+
66+
extension TextThemeChecks on Subject<TextTheme> {
67+
Subject<TextStyle?> get displayLarge => has((t) => t.displayLarge, 'displayLarge');
68+
Subject<TextStyle?> get displayMedium => has((t) => t.displayMedium, 'displayMedium');
69+
Subject<TextStyle?> get displaySmall => has((t) => t.displaySmall, 'displaySmall');
70+
Subject<TextStyle?> get headlineLarge => has((t) => t.headlineLarge, 'headlineLarge');
71+
Subject<TextStyle?> get headlineMedium => has((t) => t.headlineMedium, 'headlineMedium');
72+
Subject<TextStyle?> get headlineSmall => has((t) => t.headlineSmall, 'headlineSmall');
73+
Subject<TextStyle?> get titleLarge => has((t) => t.titleLarge, 'titleLarge');
74+
Subject<TextStyle?> get titleMedium => has((t) => t.titleMedium, 'titleMedium');
75+
Subject<TextStyle?> get titleSmall => has((t) => t.titleSmall, 'titleSmall');
76+
Subject<TextStyle?> get bodyLarge => has((t) => t.bodyLarge, 'bodyLarge');
77+
Subject<TextStyle?> get bodyMedium => has((t) => t.bodyMedium, 'bodyMedium');
78+
Subject<TextStyle?> get bodySmall => has((t) => t.bodySmall, 'bodySmall');
79+
Subject<TextStyle?> get labelLarge => has((t) => t.labelLarge, 'labelLarge');
80+
Subject<TextStyle?> get labelMedium => has((t) => t.labelMedium, 'labelMedium');
81+
Subject<TextStyle?> get labelSmall => has((t) => t.labelSmall, 'labelSmall');
82+
}
83+
84+
extension TypographyChecks on Subject<Typography> {
85+
Subject<TextTheme> get black => has((t) => t.black, 'black');
86+
Subject<TextTheme> get white => has((t) => t.white, 'white');
87+
Subject<TextTheme> get englishLike => has((t) => t.englishLike, 'englishLike');
88+
Subject<TextTheme> get dense => has((t) => t.dense, 'dense');
89+
Subject<TextTheme> get tall => has((t) => t.tall, 'tall');
90+
}

‎test/widgets/text_test.dart

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,63 @@ import 'package:zulip/widgets/text.dart';
77
import '../flutter_checks.dart';
88

99
void main() {
10+
group('zulipTypography', () {
11+
Future<Typography> getZulipTypography(WidgetTester tester, {
12+
required bool platformRequestsBold,
13+
}) async {
14+
late final Typography result;
15+
await tester.pumpWidget(
16+
MediaQuery(data: MediaQueryData(boldText: platformRequestsBold),
17+
child: Builder(builder: (context) {
18+
result = zulipTypography(context);
19+
return const SizedBox.shrink();
20+
})));
21+
return result;
22+
}
23+
24+
matchesFontFamilies(Subject<TextStyle> it) => it
25+
..fontFamily.equals(kDefaultFontFamily)
26+
..fontFamilyFallback.isNotNull().deepEquals(defaultFontFamilyFallback);
27+
28+
matchesWeight(FontWeight weight) => (Subject<TextStyle> it) => it
29+
..fontWeight.equals(weight)
30+
..fontVariations.isNotNull().contains(
31+
FontVariation('wght', wghtFromFontWeight(weight)));
32+
33+
for (final platformRequestsBold in [false, true]) {
34+
final description = platformRequestsBold
35+
? 'platform requests bold'
36+
: 'platform does not request bold';
37+
testWidgets(description, (tester) async {
38+
check(await getZulipTypography(tester, platformRequestsBold: platformRequestsBold))
39+
..black.bodyMedium.isNotNull().which(matchesFontFamilies)
40+
..white.bodyMedium.isNotNull().which(matchesFontFamilies)
41+
..englishLike.bodyMedium.isNotNull().which(
42+
matchesWeight(platformRequestsBold ? FontWeight.w700 : FontWeight.w400))
43+
..dense.bodyMedium.isNotNull().which(
44+
matchesWeight(platformRequestsBold ? FontWeight.w700 : FontWeight.w400))
45+
..tall.bodyMedium.isNotNull().which(
46+
matchesWeight(platformRequestsBold ? FontWeight.w700 : FontWeight.w400));
47+
});
48+
}
49+
50+
test('Typography has the assumed fields', () {
51+
check(Typography().toDiagnosticsNode().getProperties().map((n) => n.name).toList())
52+
.unorderedEquals(['black', 'white', 'englishLike', 'dense', 'tall']);
53+
});
54+
});
55+
56+
test('_convertTextTheme: TextTheme has the assumed fields', () {
57+
check(const TextTheme().toDiagnosticsNode().getProperties().map((n) => n.name).toList())
58+
.unorderedEquals([
59+
'displayLarge', 'displayMedium', 'displaySmall',
60+
'headlineLarge', 'headlineMedium', 'headlineSmall',
61+
'titleLarge', 'titleMedium', 'titleSmall',
62+
'bodyLarge', 'bodyMedium', 'bodySmall',
63+
'labelLarge', 'labelMedium', 'labelSmall',
64+
]);
65+
});
66+
1067
group('weightVariableTextStyle', () {
1168
Future<void> testWeights(
1269
String description, {

0 commit comments

Comments
 (0)