Skip to content

Commit 71949f1

Browse files
committed
ui: Set letter spacing to 1% in buttons
TextTheme.labelLarge is the default label style in all the various Material buttons; see: [ElevatedButton.defaultStyleOf], [FilledButton.defaultStyleOf], [MenuItemButton.defaultStyleOf], [MenuItemButton.defaultStyleOf], [OutlinedButton.defaultStyleOf], `defaults` in [SegmentedButton.build], [TextButton.defaultStyleOf] . Since our buttons are all Material buttons, changing this will effectively "set letter spacing to 1% in buttons", which is #548. (...Actually, I guess there's one kind of button that's not a Material button: reaction chips in the message list. So, adjust those separately, also in this commit.) Greg mentioned, when suggesting this approach, > In principle that has a slightly broader effect than specified in > #548, in that it affects any other Material widgets that use the > `labelLarge` text style. But fundamentally those are widgets that > the Material designers intended to have a similar text style to > that of buttons (even though they're not called "buttons"), so > it's likely that that'll be the right answer anyway. > If we end up with such a widget where Vlad wants a different > spacing, we can address that then… and I think it's fairly likely > that in that case he'd want something that wasn't the Material > widget in any event. Which makes sense and works for me. Fixes: #548
1 parent 68b2f15 commit 71949f1

File tree

6 files changed

+103
-4
lines changed

6 files changed

+103
-4
lines changed

lib/widgets/emoji_reaction.dart

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ class ReactionChip extends StatelessWidget {
153153
// which we learn about especially late).
154154
final maxLabelWidth = (maxRowWidth - 6) * 0.75; // 6 is padding
155155

156+
final labelScaler = _labelTextScalerClamped(context);
156157
return Row(
157158
mainAxisSize: MainAxisSize.min,
158159
crossAxisAlignment: CrossAxisAlignment.center,
@@ -168,9 +169,13 @@ class ReactionChip extends StatelessWidget {
168169
constraints: BoxConstraints(maxWidth: maxLabelWidth),
169170
child: Text(
170171
textWidthBasis: TextWidthBasis.longestLine,
171-
textScaler: _labelTextScalerClamped(context),
172+
textScaler: labelScaler,
172173
style: TextStyle(
173174
fontSize: (14 * 0.90),
175+
letterSpacing: proportionalLetterSpacing(context,
176+
kButtonTextLetterSpacingProportion,
177+
baseFontSize: (14 * 0.90),
178+
textScaler: labelScaler),
174179
height: 13 / (14 * 0.90),
175180
color: labelColor,
176181
).merge(weightVariableTextStyle(context,

lib/widgets/message_list.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,8 +484,10 @@ class MarkAsReadWidget extends StatelessWidget {
484484
// [zulipTypography]…
485485
Theme.of(context).textTheme.labelLarge!
486486
// …then clobber some attributes to follow Figma:
487-
.merge(const TextStyle(
487+
.merge(TextStyle(
488488
fontSize: 18,
489+
letterSpacing: proportionalLetterSpacing(context,
490+
kButtonTextLetterSpacingProportion, baseFontSize: 18),
489491
height: (23 / 18))
490492
.merge(weightVariableTextStyle(context, wght: 400))),
491493
shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(7)),

lib/widgets/text.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,14 @@ Typography zulipTypography(BuildContext context) {
4040
result = _convertTextTheme(result, (maybeInputStyle, _) =>
4141
maybeInputStyle?.merge(const TextStyle(letterSpacing: 0)));
4242

43+
result = result.copyWith(
44+
labelLarge: result.labelLarge!.copyWith(
45+
fontSize: 14.0, // (should be unchanged; restated here for explicitness)
46+
letterSpacing: proportionalLetterSpacing(context,
47+
kButtonTextLetterSpacingProportion, baseFontSize: 14.0),
48+
),
49+
);
50+
4351
return result;
4452
}
4553

@@ -167,6 +175,8 @@ final TextStyle kMonospaceTextStyle = TextStyle(
167175
inherit: true,
168176
);
169177

178+
const kButtonTextLetterSpacingProportion = 0.01;
179+
170180
/// A mergeable [TextStyle] to use when the preferred font has a "wght" axis.
171181
///
172182
/// Some variable fonts can be controlled on a "wght" axis.

test/flutter_checks.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ extension TextFieldChecks on Subject<TextField> {
6565

6666
extension TextStyleChecks on Subject<TextStyle> {
6767
Subject<bool> get inherit => has((t) => t.inherit, 'inherit');
68+
Subject<double?> get fontSize => has((t) => t.fontSize, 'fontSize');
6869
Subject<FontWeight?> get fontWeight => has((t) => t.fontWeight, 'fontWeight');
6970
Subject<double?> get letterSpacing => has((t) => t.letterSpacing, 'letterSpacing');
7071
Subject<List<FontVariation>?> get fontVariations => has((t) => t.fontVariations, 'fontVariations');

test/widgets/text_test.dart

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,14 @@ void main() {
4646
});
4747
}
4848

49-
testWidgets('zero letter spacing', (tester) async {
49+
testWidgets('letter spacing', (tester) async {
5050
check(await getZulipTypography(tester, platformRequestsBold: false))
5151
..englishLike.bodyMedium.isNotNull().letterSpacing.equals(0)
52+
..englishLike.labelLarge.isNotNull().letterSpacing.equals(0.14)
5253
..dense.bodyMedium.isNotNull().letterSpacing.equals(0)
53-
..tall.bodyMedium.isNotNull().letterSpacing.equals(0);
54+
..dense.labelLarge.isNotNull().letterSpacing.equals(0.14)
55+
..tall.bodyMedium.isNotNull().letterSpacing.equals(0)
56+
..tall.labelLarge.isNotNull().letterSpacing.equals(0.14);
5457
});
5558

5659
test('Typography has the assumed fields', () {

test/widgets/theme_test.dart

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import 'package:checks/checks.dart';
2+
import 'package:flutter/material.dart';
3+
import 'package:flutter/rendering.dart';
4+
import 'package:flutter_test/flutter_test.dart';
5+
import 'package:zulip/widgets/text.dart';
6+
import 'package:zulip/widgets/theme.dart';
7+
8+
import '../flutter_checks.dart';
9+
10+
void main() {
11+
group('button text size and letter spacing', () {
12+
Future<void> doCheck(
13+
String description, {
14+
required Widget Function(BuildContext context, String text) buttonBuilder,
15+
double? ambientTextScaleFactor,
16+
}) async {
17+
testWidgets(description, (WidgetTester tester) async {
18+
if (ambientTextScaleFactor != null) {
19+
tester.platformDispatcher.textScaleFactorTestValue = ambientTextScaleFactor;
20+
addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue);
21+
}
22+
const buttonText = 'Zulip';
23+
double? expectedFontSize;
24+
double? expectedLetterSpacing;
25+
await tester.pumpWidget(
26+
Builder(builder: (context) => MaterialApp(
27+
theme: zulipThemeData(context),
28+
home: Builder(builder: (context) {
29+
expectedFontSize = Theme.of(context).textTheme.labelLarge!.fontSize!;
30+
expectedLetterSpacing = proportionalLetterSpacing(context,
31+
0.01, baseFontSize: expectedFontSize!);
32+
return Builder(builder: (context) =>
33+
buttonBuilder(context, buttonText));
34+
}))));
35+
36+
final text = tester.renderObject<RenderParagraph>(find.text(buttonText)).text;
37+
check(text.style!)
38+
..fontSize.equals(expectedFontSize)
39+
..letterSpacing.equals(expectedLetterSpacing);
40+
});
41+
}
42+
43+
doCheck('with device text size adjusted',
44+
ambientTextScaleFactor: 2.0,
45+
buttonBuilder: (context, text) => ElevatedButton(onPressed: () {},
46+
child: Text(text)));
47+
48+
doCheck('ElevatedButton',
49+
buttonBuilder: (context, text) => ElevatedButton(onPressed: () {},
50+
child: Text(text)));
51+
52+
doCheck('FilledButton',
53+
buttonBuilder: (context, text) => FilledButton(onPressed: () {},
54+
child: Text(text)));
55+
56+
// IconButton can't have text; skip
57+
58+
doCheck('MenuItemButton',
59+
buttonBuilder: (context, text) => MenuItemButton(onPressed: () {},
60+
child: Text(text)));
61+
62+
doCheck('SubmenuButton',
63+
buttonBuilder: (context, text) => SubmenuButton(menuChildren: const [],
64+
child: Text(text)));
65+
66+
doCheck('OutlinedButton',
67+
buttonBuilder: (context, text) => OutlinedButton(onPressed: () {},
68+
child: Text(text)));
69+
70+
doCheck('SegmentedButton',
71+
buttonBuilder: (context, text) => SegmentedButton(selected: const {1},
72+
segments: [ButtonSegment(value: 1, label: Text(text))]));
73+
74+
doCheck('TextButton',
75+
buttonBuilder: (context, text) => TextButton(onPressed: () {},
76+
child: Text(text)));
77+
});
78+
}

0 commit comments

Comments
 (0)