Skip to content

content: Show code, mentions, and times properly even in headings etc. #544

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
May 8, 2024
Merged

content: Show code, mentions, and times properly even in headings etc. #544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
4b2dbc9
content test: Add ContentExample.globalTime
chrisbobbe Mar 7, 2024
397fe5e
content test: Add ContentExample.strong, and its widget smoke test
chrisbobbe Mar 8, 2024
3728701
content test: Add ContentExample.emphasis, and its widget smoke test
chrisbobbe Mar 8, 2024
049fa35
content test: Add ContentExample.inlineCode, and its widget smoke test
chrisbobbe Mar 8, 2024
348548d
content test: Add ContentExamples for @-mentions, also widget smoke t…
chrisbobbe Mar 8, 2024
6d0ad45
content [nfc]: Make InlineContent.style required
chrisbobbe Feb 29, 2024
42fc75f
content test: Add zulipTypography in prepareContentBare
chrisbobbe May 1, 2024
14d8e5e
content test: Make some new `group`s to add tests to (coming up)
chrisbobbe May 1, 2024
335d2fb
content test [nfc]: Pull html and a RegExp out to variables
chrisbobbe May 1, 2024
9d951cb
content: Show code/math, mentions, and times properly even in heading…
chrisbobbe Feb 29, 2024
0a01ed1
content: Set error{,Code}Style font size to kBaseFontSize, which is 17
chrisbobbe Feb 28, 2024
89af98c
content: For headings, use weightVariableTextStyle
chrisbobbe Feb 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 59 additions & 20 deletions lib/widgets/content.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,9 @@ class Heading extends StatelessWidget {
child: _buildBlockInlineContainer(
style: TextStyle(
fontSize: kBaseFontSize * emHeight,
fontWeight: FontWeight.w600,
height: 1.4),
height: 1.4,
)
.merge(weightVariableTextStyle(context, wght: 600)),
node: node));
}
}
Expand Down Expand Up @@ -497,7 +498,7 @@ class MathBlock extends StatelessWidget {
//

Widget _buildBlockInlineContainer({
required TextStyle? style,
required TextStyle style,
required BlockInlineContainerNode node,
}) {
if (node.links == null) {
Expand All @@ -513,7 +514,7 @@ class _BlockInlineContainer extends StatefulWidget {
{required this.links, required this.style, required this.nodes});

final List<LinkNode> links;
final TextStyle? style;
final TextStyle style;
final List<InlineContentNode> nodes;

@override
Expand Down Expand Up @@ -571,12 +572,19 @@ class InlineContent extends StatelessWidget {
required this.style,
required this.nodes,
}) {
assert(style.fontSize != null);
_builder = _InlineContentBuilder(this);
}

final GestureRecognizer? recognizer;
final Map<LinkNode, GestureRecognizer>? linkRecognizers;
final TextStyle? style;

/// A [TextStyle] applied to this content and provided to descendants.
///
/// Must set [TextStyle.fontSize]. Some descendant spans will consume it,
/// e.g., to make their content slightly smaller than surrounding text.
final TextStyle style;

final List<InlineContentNode> nodes;

late final _InlineContentBuilder _builder;
Expand Down Expand Up @@ -650,18 +658,21 @@ class _InlineContentBuilder {
return _buildInlineCode(node);
} else if (node is UserMentionNode) {
return WidgetSpan(alignment: PlaceholderAlignment.middle,
child: UserMention(node: node));
child: UserMention(surroundingTextStyle: widget.style, node: node));
} else if (node is UnicodeEmojiNode) {
return TextSpan(text: node.emojiUnicode, recognizer: _recognizer);
} else if (node is ImageEmojiNode) {
return WidgetSpan(alignment: PlaceholderAlignment.middle,
child: MessageImageEmoji(node: node));
} else if (node is MathInlineNode) {
return TextSpan(style: _kInlineMathStyle,
return TextSpan(
style: widget.style
.merge(_kInlineMathStyle)
.apply(fontSizeFactor: _kInlineCodeFontSizeFactor),
children: [TextSpan(text: node.texSource)]);
} else if (node is GlobalTimeNode) {
return WidgetSpan(alignment: PlaceholderAlignment.middle,
child: GlobalTime(node: node));
child: GlobalTime(node: node, surroundingTextStyle: widget.style));
} else if (node is UnimplementedInlineContentNode) {
return _errorUnimplemented(node);
} else {
Expand Down Expand Up @@ -713,8 +724,13 @@ class _InlineContentBuilder {

// TODO `code`: find equivalent of web's `unicode-bidi: embed; direction: ltr`

// Use a light gray background, instead of a border.
return _buildNodes(style: _kInlineCodeStyle, node.nodes);
return _buildNodes(
// Use a light gray background, instead of a border.
style: widget.style
.merge(_kInlineCodeStyle)
.apply(fontSizeFactor: _kInlineCodeFontSizeFactor),
node.nodes,
);

// Another fun solution -- we can in fact have a border! Like so:
// TextStyle(
Expand All @@ -734,17 +750,27 @@ class _InlineContentBuilder {
}
}

/// The [TextStyle] for inline math, excluding font-size adjustment.
///
/// Inline math should use this and also apply [_kInlineCodeFontSizeFactor]
/// to the font size of the surrounding text
/// (which might be a Paragraph, a Heading, etc.).
final _kInlineMathStyle = _kInlineCodeStyle.merge(TextStyle(
backgroundColor: const HSLColor.fromAHSL(1, 240, 0.4, 0.93).toColor()));

const _kInlineCodeFontSizeFactor = 0.825;

/// The [TextStyle] for inline code, excluding font-size adjustment.
///
/// Inline code should use this and also apply [_kInlineCodeFontSizeFactor]
/// to the font size of the surrounding text
/// (which might be a Paragraph, a Heading, etc.).
// Even though [kMonospaceTextStyle] is a variable-weight font,
// it's acceptable to skip [weightVariableTextStyle] here,
// assuming the text gets the effect of [weightVariableTextStyle]
// through inheritance, e.g., from a [DefaultTextStyle].
final _kInlineCodeStyle = kMonospaceTextStyle
.merge(const TextStyle(
backgroundColor: Color(0xffeeeeee),
fontSize: 0.825 * kBaseFontSize));
.merge(const TextStyle(backgroundColor: Color(0xffeeeeee)));

final _kCodeBlockStyle = kMonospaceTextStyle
.merge(const TextStyle(
Expand Down Expand Up @@ -775,8 +801,13 @@ final _kCodeBlockStyle = kMonospaceTextStyle
// const _kInlineCodeRightBracket = '⟩';

class UserMention extends StatelessWidget {
const UserMention({super.key, required this.node});
const UserMention({
super.key,
required this.surroundingTextStyle,
required this.node,
});

final TextStyle surroundingTextStyle;
final UserMentionNode node;

@override
Expand All @@ -790,7 +821,7 @@ class UserMention extends StatelessWidget {
// One hopes an @-mention can't contain an embedded link.
// (The parser on creating a UserMentionNode has a TODO to check that.)
linkRecognizers: null,
style: Paragraph.textStyle,
style: surroundingTextStyle,
nodes: node.nodes));
}

Expand Down Expand Up @@ -857,16 +888,23 @@ class MessageImageEmoji extends StatelessWidget {
}

class GlobalTime extends StatelessWidget {
const GlobalTime({super.key, required this.node});
const GlobalTime({
super.key,
required this.node,
required this.surroundingTextStyle,
});

final GlobalTimeNode node;
final TextStyle surroundingTextStyle;

static final _backgroundColor = const HSLColor.fromAHSL(1, 0, 0, 0.93).toColor();
static final _borderColor = const HSLColor.fromAHSL(1, 0, 0, 0.8).toColor();
static final _dateFormat = DateFormat('EEE, MMM d, y, h:mm a'); // TODO(intl): localize date

@override
Widget build(BuildContext context) {
final surroundingFontSize = surroundingTextStyle.fontSize!;

// Design taken from css for `.rendered_markdown & time` in web,
// see zulip:web/styles/rendered_markdown.css .
final text = _dateFormat.format(node.datetime.toLocal());
Expand All @@ -882,11 +920,11 @@ class GlobalTime extends StatelessWidget {
child: Row(
mainAxisSize: MainAxisSize.min,
children: [
const Icon(ZulipIcons.clock, size: kBaseFontSize),
Icon(ZulipIcons.clock, size: surroundingFontSize),
// Ad-hoc spacing adjustment per feedback:
// https://chat.zulip.org/#narrow/stream/101-design/topic/clock.20icons/near/1729345
const SizedBox(width: 1),
Text(text, style: Paragraph.textStyle),
Text(text, style: surroundingTextStyle),
]))));
}
}
Expand Down Expand Up @@ -1161,8 +1199,9 @@ InlineSpan _errorUnimplemented(UnimplementedNode node) {
}
}

const errorStyle = TextStyle(fontWeight: FontWeight.bold, color: Colors.red);
const errorStyle = TextStyle(
fontSize: kBaseFontSize, fontWeight: FontWeight.bold, color: Colors.red);

final errorCodeStyle = kMonospaceTextStyle
.merge(const TextStyle(color: Colors.red))
.merge(const TextStyle(fontSize: kBaseFontSize, color: Colors.red))
.merge(weightVariableTextStyle(null)); // TODO(a11y) pass a BuildContext
5 changes: 5 additions & 0 deletions test/flutter_checks.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ extension ValueNotifierChecks<T> on Subject<ValueNotifier<T>> {

extension TextChecks on Subject<Text> {
Subject<String?> get data => has((t) => t.data, 'data');
Subject<TextStyle?> get style => has((t) => t.style, 'style');
}

extension TextFieldChecks on Subject<TextField> {
Expand Down Expand Up @@ -101,3 +102,7 @@ extension TypographyChecks on Subject<Typography> {
Subject<TextTheme> get dense => has((t) => t.dense, 'dense');
Subject<TextTheme> get tall => has((t) => t.tall, 'tall');
}

extension InlineSpanChecks on Subject<InlineSpan> {
Subject<TextStyle?> get style => has((x) => x.style, 'style');
}
127 changes: 81 additions & 46 deletions test/model/content_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,69 @@ class ContentExample {
/// to have it defined for each test case right next to [html] and [expectedNodes].
final String? expectedText;

static final strong = ContentExample.inline(
'strong/bold',
'**bold**',
expectedText: 'bold',
'<p><strong>bold</strong></p>',
const StrongNode(nodes: [TextNode('bold')]));

static final emphasis = ContentExample.inline(
'emphasis/italic',
'*italic*',
expectedText: 'italic',
'<p><em>italic</em></p>',
const EmphasisNode(nodes: [TextNode('italic')]));

static final inlineCode = ContentExample.inline(
'inline code',
'`inline code`',
expectedText: 'inline code',
'<p><code>inline code</code></p>',
const InlineCodeNode(nodes: [TextNode('inline code')]));

static final userMentionPlain = ContentExample.inline(
'plain user @-mention',
"@**Greg Price**",
expectedText: '@Greg Price',
'<p><span class="user-mention" data-user-id="2187">@Greg Price</span></p>',
const UserMentionNode(nodes: [TextNode('@Greg Price')]));

static final userMentionSilent = ContentExample.inline(
'silent user @-mention',
"@_**Greg Price**",
expectedText: 'Greg Price',
'<p><span class="user-mention silent" data-user-id="2187">Greg Price</span></p>',
const UserMentionNode(nodes: [TextNode('Greg Price')]));

static final userMentionSilentClassOrderReversed = ContentExample.inline(
'silent user @-mention, class order reversed',
"@_**Greg Price**", // (hypothetical server variation)
expectedText: 'Greg Price',
'<p><span class="silent user-mention" data-user-id="2187">Greg Price</span></p>',
const UserMentionNode(nodes: [TextNode('Greg Price')]));

static final groupMentionPlain = ContentExample.inline(
'plain group @-mention',
"@*test-empty*",
expectedText: '@test-empty',
'<p><span class="user-group-mention" data-user-group-id="186">@test-empty</span></p>',
const UserMentionNode(nodes: [TextNode('@test-empty')]));

static final groupMentionSilent = ContentExample.inline(
'silent group @-mention',
"@_*test-empty*",
expectedText: 'test-empty',
'<p><span class="user-group-mention silent" data-user-group-id="186">test-empty</span></p>',
const UserMentionNode(nodes: [TextNode('test-empty')]));

static final groupMentionSilentClassOrderReversed = ContentExample.inline(
'silent group @-mention, class order reversed',
"@_*test-empty*", // (hypothetical server variation)
expectedText: 'test-empty',
'<p><span class="silent user-group-mention" data-user-group-id="186">test-empty</span></p>',
const UserMentionNode(nodes: [TextNode('test-empty')]));

static final emojiUnicode = ContentExample.inline(
'Unicode emoji, encoded in span element',
":thumbs_up:",
Expand Down Expand Up @@ -125,6 +188,13 @@ class ContentExample {
const ImageEmojiNode(
src: '/static/generated/emoji/images/emoji/unicode/zulip.png', alt: ':zulip:'));

static final globalTime = ContentExample.inline(
'global time',
"<time:2024-03-07T15:00:00-08:00>",
'<p><time datetime="2024-03-07T23:00:00Z">2024-03-07T15:00:00-08:00</time></p>',
GlobalTimeNode(
datetime: DateTime.parse("2024-03-07T23:00:00Z")));

static const spoilerDefaultHeader = ContentExample(
'spoiler with default header',
'```spoiler\nhello world\n```',
Expand Down Expand Up @@ -597,25 +667,16 @@ void main() {
TextNode('\nb'),
])]);

testParseInline('parse strong/bold',
// "**bold**"
'<p><strong>bold</strong></p>',
const StrongNode(nodes: [TextNode('bold')]));
testParseExample(ContentExample.strong);

testParseInline('parse deleted/strike-through',
// "~~strike through~~"
'<p><del>strike through</del></p>',
const DeletedNode(nodes: [TextNode('strike through')]));

testParseInline('parse emphasis/italic',
// "*italic*"
'<p><em>italic</em></p>',
const EmphasisNode(nodes: [TextNode('italic')]));
testParseExample(ContentExample.emphasis);

testParseInline('parse inline code',
// "`inline code`"
'<p><code>inline code</code></p>',
const InlineCodeNode(nodes: [TextNode('inline code')]));
testParseExample(ContentExample.inlineCode);

testParseInline('parse nested del, strong, em, code',
// "~~***`word`***~~"
Expand Down Expand Up @@ -663,35 +724,13 @@ void main() {
nodes: [TextNode('t')])])])]));

group('parse @-mentions', () {
testParseInline('plain user @-mention',
// "@**Greg Price**"
'<p><span class="user-mention" data-user-id="2187">@Greg Price</span></p>',
const UserMentionNode(nodes: [TextNode('@Greg Price')]));

testParseInline('silent user @-mention',
// "@_**Greg Price**"
'<p><span class="user-mention silent" data-user-id="2187">Greg Price</span></p>',
const UserMentionNode(nodes: [TextNode('Greg Price')]));

testParseInline('silent user @-mention, class order reversed',
// "@_**Greg Price**" (hypothetical server variation)
'<p><span class="silent user-mention" data-user-id="2187">Greg Price</span></p>',
const UserMentionNode(nodes: [TextNode('Greg Price')]));

testParseInline('plain group @-mention',
// "@*test-empty*"
'<p><span class="user-group-mention" data-user-group-id="186">@test-empty</span></p>',
const UserMentionNode(nodes: [TextNode('@test-empty')]));

testParseInline('silent group @-mention',
// "@_*test-empty*"
'<p><span class="user-group-mention silent" data-user-group-id="186">test-empty</span></p>',
const UserMentionNode(nodes: [TextNode('test-empty')]));

testParseInline('silent group @-mention, class order reversed',
// "@_*test-empty*" (hypothetical server variation)
'<p><span class="silent user-group-mention" data-user-group-id="186">test-empty</span></p>',
const UserMentionNode(nodes: [TextNode('test-empty')]));
testParseExample(ContentExample.userMentionPlain);
testParseExample(ContentExample.userMentionSilent);
testParseExample(ContentExample.userMentionSilentClassOrderReversed);

testParseExample(ContentExample.groupMentionPlain);
testParseExample(ContentExample.groupMentionSilent);
testParseExample(ContentExample.groupMentionSilentClassOrderReversed);

// TODO test wildcard mentions
});
Expand All @@ -707,11 +746,7 @@ void main() {
testParseExample(ContentExample.mathInline);

group('global times', () {
testParseInline('smoke',
// "<time:2024-01-30T17:33:00Z>"
'<p><time datetime="2024-01-30T17:33:00Z">2024-01-30T17:33:00Z</time></p>',
GlobalTimeNode(datetime: DateTime.parse('2024-01-30T17:33Z')),
);
testParseExample(ContentExample.globalTime);

testParseInline('handles missing attribute',
// No markdown, this is unexpected response
Expand Down
Loading
Loading