Skip to content

Commit b75908b

Browse files
chrisbobbegnprice
authored andcommitted
content [nfc]: Make textStylePlainParagraph complete; cut off inheritance
As Greg points out: #698 (comment) > Avoiding `DefaultTextStyle.merge`, in favor of giving some > already-computed style directly, will be good for explicitness as > well as for potentially a small performance gain. Before this, the inheritance via `DefaultTextStyle.merge` made it kind of tiring to work out what plain-paragraph styling actually was. It was looking up to the nearest DefaultTextStyle, which was actually an AnimatedDefaultTextStyle in the Material widget, that was providing Theme.of(context).textTheme.bodyMedium. That, in turn, was built from various things: - [_M3Typography.englishLike] - [Typography.blackCupertino] or [Typography.blackMountainView] (depending on platform) - `zulipTypography` in lib/widgets/text.dart I've followed these sources in writing down this complete style object, to make this a direct, NFC translation.
1 parent b73962f commit b75908b

File tree

5 files changed

+22
-8
lines changed

5 files changed

+22
-8
lines changed

lib/widgets/content.dart

+16-5
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,22 @@ import 'text.dart';
3232
/// background. For what this is in the message list, see
3333
/// widgets/message_list.dart.
3434
class ContentTheme extends ThemeExtension<ContentTheme> {
35-
ContentTheme() :
35+
ContentTheme(BuildContext context) :
3636
textStylePlainParagraph = TextStyle(
37-
debugLabel: 'ContentTheme.textStylePlainParagraph',
37+
inherit: false,
3838

3939
color: const HSLColor.fromAHSL(1, 0, 0, 0.15).toColor(),
4040
fontSize: kBaseFontSize,
41+
letterSpacing: 0,
42+
textBaseline: TextBaseline.alphabetic,
4143
height: (22 / kBaseFontSize),
42-
);
44+
leadingDistribution: TextLeadingDistribution.even,
45+
decoration: TextDecoration.none,
46+
fontFamily: kDefaultFontFamily,
47+
fontFamilyFallback: defaultFontFamilyFallback,
48+
)
49+
.merge(weightVariableTextStyle(context))
50+
.copyWith(debugLabel: 'ContentTheme.textStylePlainParagraph');
4351

4452
ContentTheme._({
4553
required this.textStylePlainParagraph,
@@ -55,9 +63,12 @@ class ContentTheme extends ThemeExtension<ContentTheme> {
5563
return extension!;
5664
}
5765

58-
/// The [TextStyle] we use for plain, unstyled paragraphs.
66+
/// The complete [TextStyle] we use for plain, unstyled paragraphs.
5967
///
6068
/// Also the base style that all other text content should inherit from.
69+
///
70+
/// This is the complete style for plain paragraphs. Plain-paragraph content
71+
/// should not need styles from other sources, such as Material defaults.
6172
final TextStyle textStylePlainParagraph;
6273

6374
@override
@@ -96,7 +107,7 @@ class MessageContent extends StatelessWidget {
96107
@override
97108
Widget build(BuildContext context) {
98109
return InheritedMessage(message: message,
99-
child: DefaultTextStyle.merge(
110+
child: DefaultTextStyle(
100111
style: ContentTheme.of(context).textStylePlainParagraph,
101112
child: BlockContentList(nodes: content.nodes)));
102113
}

lib/widgets/profile.dart

+1-1
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ class _LinkWidget extends StatelessWidget {
217217
@override
218218
Widget build(BuildContext context) {
219219
final linkNode = LinkNode(url: url, nodes: [TextNode(text)]);
220-
final paragraph = DefaultTextStyle.merge(
220+
final paragraph = DefaultTextStyle(
221221
style: ContentTheme.of(context).textStylePlainParagraph,
222222
child: Paragraph(node: ParagraphNode(nodes: [linkNode], links: [linkNode])));
223223
return Padding(

lib/widgets/text.dart

+3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ import 'package:flutter/material.dart';
1515
/// We often see this in the child of a [Material], for example,
1616
/// since by default [Material] applies an [AnimatedDefaultTextStyle]
1717
/// with the [TextTheme.bodyMedium] that gets its value from here.
18+
/// A notable exception is the base style for message content.
19+
/// That style is self-contained and is not meant to inherit from this;
20+
/// see [ContentTheme.textStylePlainParagraph].
1821
///
1922
/// Applies [kDefaultFontFamily] and [kDefaultFontFamilyFallback],
2023
/// being faithful to the Material-default font weights

lib/widgets/theme.dart

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import 'text.dart';
66
ThemeData zulipThemeData(BuildContext context) {
77
return ThemeData(
88
typography: zulipTypography(context),
9-
extensions: [ContentTheme()],
9+
extensions: [ContentTheme(context)],
1010
appBarTheme: const AppBarTheme(
1111
// Set these two fields to prevent a color change in [AppBar]s when
1212
// there is something scrolled under it. If an app bar hasn't been

test/widgets/content_test.dart

+1-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ void main() {
9494

9595
Widget plainContent(String html) {
9696
return Builder(builder: (context) =>
97-
DefaultTextStyle.merge(
97+
DefaultTextStyle(
9898
style: ContentTheme.of(context).textStylePlainParagraph,
9999
child: BlockContentList(nodes: parseContent(html).nodes)));
100100
}

0 commit comments

Comments
 (0)