Skip to content

Commit

Permalink
content [nfc]: Make textStylePlainParagraph complete; cut off inherit…
Browse files Browse the repository at this point in the history
…ance

As Greg points out:

zulip#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.
  • Loading branch information
chrisbobbe authored and gnprice committed May 24, 2024
1 parent b73962f commit b75908b
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 8 deletions.
21 changes: 16 additions & 5 deletions lib/widgets/content.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,22 @@ import 'text.dart';
/// background. For what this is in the message list, see
/// widgets/message_list.dart.
class ContentTheme extends ThemeExtension<ContentTheme> {
ContentTheme() :
ContentTheme(BuildContext context) :
textStylePlainParagraph = TextStyle(
debugLabel: 'ContentTheme.textStylePlainParagraph',
inherit: false,

color: const HSLColor.fromAHSL(1, 0, 0, 0.15).toColor(),
fontSize: kBaseFontSize,
letterSpacing: 0,
textBaseline: TextBaseline.alphabetic,
height: (22 / kBaseFontSize),
);
leadingDistribution: TextLeadingDistribution.even,
decoration: TextDecoration.none,
fontFamily: kDefaultFontFamily,
fontFamilyFallback: defaultFontFamilyFallback,
)
.merge(weightVariableTextStyle(context))
.copyWith(debugLabel: 'ContentTheme.textStylePlainParagraph');

ContentTheme._({
required this.textStylePlainParagraph,
Expand All @@ -55,9 +63,12 @@ class ContentTheme extends ThemeExtension<ContentTheme> {
return extension!;
}

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

@override
Expand Down Expand Up @@ -96,7 +107,7 @@ class MessageContent extends StatelessWidget {
@override
Widget build(BuildContext context) {
return InheritedMessage(message: message,
child: DefaultTextStyle.merge(
child: DefaultTextStyle(
style: ContentTheme.of(context).textStylePlainParagraph,
child: BlockContentList(nodes: content.nodes)));
}
Expand Down
2 changes: 1 addition & 1 deletion lib/widgets/profile.dart
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ class _LinkWidget extends StatelessWidget {
@override
Widget build(BuildContext context) {
final linkNode = LinkNode(url: url, nodes: [TextNode(text)]);
final paragraph = DefaultTextStyle.merge(
final paragraph = DefaultTextStyle(
style: ContentTheme.of(context).textStylePlainParagraph,
child: Paragraph(node: ParagraphNode(nodes: [linkNode], links: [linkNode])));
return Padding(
Expand Down
3 changes: 3 additions & 0 deletions lib/widgets/text.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import 'package:flutter/material.dart';
/// We often see this in the child of a [Material], for example,
/// since by default [Material] applies an [AnimatedDefaultTextStyle]
/// with the [TextTheme.bodyMedium] that gets its value from here.
/// A notable exception is the base style for message content.
/// That style is self-contained and is not meant to inherit from this;
/// see [ContentTheme.textStylePlainParagraph].
///
/// Applies [kDefaultFontFamily] and [kDefaultFontFamilyFallback],
/// being faithful to the Material-default font weights
Expand Down
2 changes: 1 addition & 1 deletion lib/widgets/theme.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import 'text.dart';
ThemeData zulipThemeData(BuildContext context) {
return ThemeData(
typography: zulipTypography(context),
extensions: [ContentTheme()],
extensions: [ContentTheme(context)],
appBarTheme: const AppBarTheme(
// Set these two fields to prevent a color change in [AppBar]s when
// there is something scrolled under it. If an app bar hasn't been
Expand Down
2 changes: 1 addition & 1 deletion test/widgets/content_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ void main() {

Widget plainContent(String html) {
return Builder(builder: (context) =>
DefaultTextStyle.merge(
DefaultTextStyle(
style: ContentTheme.of(context).textStylePlainParagraph,
child: BlockContentList(nodes: parseContent(html).nodes)));
}
Expand Down

0 comments on commit b75908b

Please sign in to comment.