Skip to content

Commit d33dece

Browse files
committed
text: Use kDefaultFontFamily{,Fallback} 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 kDefaultFontFamilyFallback according to their doc comments, to fix. Fixes: #294 Fixes: #438
1 parent fe906dd commit d33dece

File tree

3 files changed

+155
-1
lines changed

3 files changed

+155
-1
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: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,44 @@
11
import 'dart:io';
22
import 'dart:ui';
33
import 'package:flutter/foundation.dart';
4-
import 'package:flutter/widgets.dart';
4+
import 'package:flutter/material.dart';
5+
6+
/// An app-wide [Typography] for Zulip, customized from the Material default.
7+
///
8+
/// Include this in the app-wide [MaterialApp.theme].
9+
///
10+
/// We expect these text styles to be the basis of all the styles chosen by the
11+
/// Material library's widgets, such as the default styling of
12+
/// an [AppBar]'s title, of an [ElevatedButton]'s label, and so on.
13+
///
14+
/// In all the comprised [TextStyle]s, this sets:
15+
///
16+
/// - [TextStyle.fontFamily] to [kDefaultFontFamily], and
17+
/// - [TextStyle.fontFamilyFallback] to [kDefaultFontFamilyFallback].
18+
///
19+
/// Since [kDefaultFontFamily] is a variable-weight font,
20+
/// each [TextStyle] needs some processing, which this provides,
21+
/// in order to correctly specify the font weight.
22+
///
23+
/// When building on top of these [TextStyles], callers that wish to specify
24+
/// a different font weight are still responsible for reprocessing the style
25+
/// with [weightVariableTextStyle] before passing it to a [Text].
26+
/// (Widgets in the Material library won't do this; they aren't yet equipped
27+
/// to set font weights on variable-weight fonts. If this causes visible bugs,
28+
/// we should investigate and fix, but they should become less likely as
29+
/// we transition from Material's widgets to our own bespoke ones.)
30+
Typography zulipTypography(BuildContext context) {
31+
final typography = Theme.of(context).typography;
32+
return weightVariableTypography(context, typography)
33+
.copyWith(
34+
black: typography.black.apply(
35+
fontFamily: kDefaultFontFamily,
36+
fontFamilyFallback: kDefaultFontFamilyFallback),
37+
white: typography.white.apply(
38+
fontFamily: kDefaultFontFamily,
39+
fontFamilyFallback: kDefaultFontFamilyFallback),
40+
);
41+
}
542

643
/// The [TextStyle.fontFamily] to use in most of the app.
744
///
@@ -134,3 +171,75 @@ FontWeight clampVariableFontWeight(double wght) {
134171
}
135172
}
136173
}
174+
175+
/// Convert an input [Typography] to one that works with "wght"-variable fonts.
176+
///
177+
/// Processes the "geometry" [TextTheme] fields
178+
/// ([Typography.dense], [Typography.englishLike], and [Typography.tall])
179+
/// with [weightVariableTextTheme], passing along the [BuildContext].
180+
Typography weightVariableTypography(BuildContext context, Typography input) =>
181+
input.copyWith(
182+
dense: weightVariableTextTheme(context, input.dense),
183+
englishLike: weightVariableTextTheme(context, input.englishLike),
184+
tall: weightVariableTextTheme(context, input.tall),
185+
);
186+
187+
/// Convert a geometry [TextTheme] to one that works with "wght"-variable fonts.
188+
///
189+
/// A "geometry [TextTheme]" is a [TextTheme] that's meant to specify
190+
/// font weight and other parameters about shape, size, distance, etc.
191+
/// See [Typography].
192+
///
193+
/// This looks at each of `input`'s fields, such as [TextTheme.bodyMedium], and:
194+
///
195+
/// If the field is non-null, calls [TextStyle.merge], passing the result of
196+
/// [weightVariableTextStyle] called with:
197+
/// - this function's [BuildContext] param, and
198+
/// - for `wght`:
199+
/// - if present, the field's [TextStyle.fontWeight]
200+
/// (via [wghtFromFontWeight]), else
201+
/// - null.
202+
///
203+
/// If the field is null, gives null.
204+
///
205+
/// This assumes that if a non-null field omits [TextStyle.fontWeight], then
206+
/// the normal font weight is implied, and it will be expressed in the output.
207+
TextTheme weightVariableTextTheme(BuildContext context, TextTheme input) {
208+
TextStyle? convert(TextStyle? maybeInputStyle) {
209+
if (maybeInputStyle == null) {
210+
return null;
211+
}
212+
final maybeInputFontWeight = maybeInputStyle.fontWeight;
213+
return maybeInputStyle.merge(weightVariableTextStyle(context,
214+
wght: maybeInputFontWeight != null
215+
? wghtFromFontWeight(maybeInputFontWeight)
216+
: null));
217+
}
218+
219+
return TextTheme(
220+
displayLarge: convert(input.displayLarge),
221+
displayMedium: convert(input.displayMedium),
222+
displaySmall: convert(input.displaySmall),
223+
headlineLarge: convert(input.headlineLarge),
224+
headlineMedium: convert(input.headlineMedium),
225+
headlineSmall: convert(input.headlineSmall),
226+
titleLarge: convert(input.titleLarge),
227+
titleMedium: convert(input.titleMedium),
228+
titleSmall: convert(input.titleSmall),
229+
bodyLarge: convert(input.bodyLarge),
230+
bodyMedium: convert(input.bodyMedium),
231+
bodySmall: convert(input.bodySmall),
232+
labelLarge: convert(input.labelLarge),
233+
labelMedium: convert(input.labelMedium),
234+
labelSmall: convert(input.labelSmall),
235+
);
236+
}
237+
238+
/// A good guess at a font's "wght" value to match a given [FontWeight].
239+
///
240+
/// Returns [FontWeight.value] as a double.
241+
///
242+
/// This might not be exactly where the font designer would land on their
243+
/// font's own custom-defined "wght" axis. But it's a great guess,
244+
/// at least without knowledge of the particular font.
245+
double wghtFromFontWeight(FontWeight fontWeight) => fontWeight.value.toDouble();

test/widgets/text_test.dart

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,4 +144,47 @@ void main() {
144144
check(clampVariableFontWeight(999)) .equals(FontWeight.w900);
145145
check(clampVariableFontWeight(1000)) .equals(FontWeight.w900);
146146
});
147+
148+
test('weightVariableTypography: ScriptCategory has the assumed values', () {
149+
// If Flutter adds a new one, check [Typography] for a corresponding
150+
// "geometry [TextStyle]" and update weightVariableTypography to process it.
151+
check(ScriptCategory.values).deepEquals(
152+
[ScriptCategory.englishLike, ScriptCategory.dense, ScriptCategory.tall]);
153+
});
154+
155+
// TODO exercise/check weightVariableTypography
156+
157+
test('weightVariableTextTheme: TextTheme has the assumed fields', () {
158+
const family = '123';
159+
const withFamily = TextStyle(fontFamily: family);
160+
161+
const base = Typography.englishLike2021;
162+
163+
// Keep a lookout for new [TextTheme] fields that didn't exist at the time
164+
// of writing. If there are new fields, we should update
165+
// [weightVariableTextTheme] so it handles them.
166+
check(
167+
base.merge(const TextTheme(
168+
displayLarge: withFamily,
169+
displayMedium: withFamily,
170+
displaySmall: withFamily,
171+
headlineLarge: withFamily,
172+
headlineMedium: withFamily,
173+
headlineSmall: withFamily,
174+
titleLarge: withFamily,
175+
titleMedium: withFamily,
176+
titleSmall: withFamily,
177+
bodyLarge: withFamily,
178+
bodyMedium: withFamily,
179+
bodySmall: withFamily,
180+
labelLarge: withFamily,
181+
labelMedium: withFamily,
182+
labelSmall: withFamily,
183+
))
184+
).equals(
185+
base.apply(fontFamily: family),
186+
);
187+
});
188+
189+
// TODO exercise/check weightVariableTextTheme
147190
}

0 commit comments

Comments
 (0)