From 9f0757d570087088fdfe11048739e706fc3b83ae Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 19 Aug 2025 17:05:02 -0700 Subject: [PATCH 1/4] katex [nfc]: Consolidate where we cancel out ambient text styles In the inline-math case, we're about to pass the actual ambient text style, which may have italic, strikethrough, bold, or link formatting. Consolidate the code that overrides each of those, so that it reads as intentional, and add TODOs for overrides that we might not want to keep. --- lib/widgets/katex.dart | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/lib/widgets/katex.dart b/lib/widgets/katex.dart index 4b4f39aa3f..a4715398ff 100644 --- a/lib/widgets/katex.dart +++ b/lib/widgets/katex.dart @@ -10,12 +10,35 @@ import 'content.dart'; /// Creates a base text style for rendering KaTeX content. /// -/// This applies the CSS styles defined in .katex class in katex.scss : +/// This cancels out some attributes that may be ambient from Zulip content +/// (italic, bold, etc.) +/// and applies the CSS styles defined in .katex class in katex.scss : /// https://github.com/KaTeX/KaTeX/blob/613c3da8/src/styles/katex.scss#L13-L15 /// /// Requires the [style.fontSize] to be non-null. -TextStyle mkBaseKatexTextStyle(TextStyle style) { +TextStyle mkBaseKatexTextStyle(TextStyle style, Color baseColor) { return style.copyWith( + ////// Overrides of our own styles: + + // Bold formatting is removed below by setting FontWeight.normal… + // Just for completeness, remove "wght", but it wouldn't do anything anyway + // since KaTeX_Main is not a variable-weight font. + fontVariations: [], + // Remove link color, but + // TODO(#1823) do we want to do that? Web doesn't. + color: baseColor, + // Italic is removed below. + + // Strikethrough is removed below, which affects formatting of rendered + // KatexSpanNodes…but a single strikethrough on the whole KatexWidget will + // be visible as long as it doesn't paint an opaque background. (The line + // "shows through" from an ancestor span.) I think we're happy with this: + // the message author asked for a strikethrough by wrapping the math in ~~, + // but we should render it as one unbroken line, not separate lines on each + // KaTeX span. + + ////// From the .katex class in katex.scss: + fontSize: style.fontSize! * 1.21, fontFamily: 'KaTeX_Main', height: 1.2, @@ -45,8 +68,8 @@ class KatexWidget extends StatelessWidget { return Directionality( textDirection: TextDirection.ltr, child: DefaultTextStyle( - style: mkBaseKatexTextStyle(textStyle).copyWith( - color: ContentTheme.of(context).textStylePlainParagraph.color), + style: mkBaseKatexTextStyle(textStyle, + ContentTheme.of(context).textStylePlainParagraph.color!), child: widget)); } } From 48d4cd5e93fd130ab2bdc2f9ec85378f5a0428e1 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 19 Aug 2025 17:12:42 -0700 Subject: [PATCH 2/4] katex [nfc]: s/textStyle/ambientTextStyle/, like UserMention and GlobalTime --- lib/widgets/content.dart | 4 ++-- lib/widgets/katex.dart | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 5851222a1c..a25e0ee27d 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -822,7 +822,7 @@ class MathBlock extends StatelessWidget { child: SingleChildScrollViewWithScrollbar( scrollDirection: Axis.horizontal, child: KatexWidget( - textStyle: ContentTheme.of(context).textStylePlainParagraph, + ambientTextStyle: ContentTheme.of(context).textStylePlainParagraph, nodes: nodes)))); } } @@ -1145,7 +1145,7 @@ class _InlineContentBuilder { : WidgetSpan( alignment: PlaceholderAlignment.baseline, baseline: TextBaseline.alphabetic, - child: KatexWidget(textStyle: widget.style, nodes: nodes)); + child: KatexWidget(ambientTextStyle: widget.style, nodes: nodes)); case GlobalTimeNode(): return WidgetSpan(alignment: PlaceholderAlignment.middle, diff --git a/lib/widgets/katex.dart b/lib/widgets/katex.dart index a4715398ff..9292db8460 100644 --- a/lib/widgets/katex.dart +++ b/lib/widgets/katex.dart @@ -15,9 +15,9 @@ import 'content.dart'; /// and applies the CSS styles defined in .katex class in katex.scss : /// https://github.com/KaTeX/KaTeX/blob/613c3da8/src/styles/katex.scss#L13-L15 /// -/// Requires the [style.fontSize] to be non-null. -TextStyle mkBaseKatexTextStyle(TextStyle style, Color baseColor) { - return style.copyWith( +/// Requires the [ambientStyle.fontSize] to be non-null. +TextStyle mkBaseKatexTextStyle(TextStyle ambientStyle, Color baseColor) { + return ambientStyle.copyWith( ////// Overrides of our own styles: // Bold formatting is removed below by setting FontWeight.normal… @@ -39,7 +39,7 @@ TextStyle mkBaseKatexTextStyle(TextStyle style, Color baseColor) { ////// From the .katex class in katex.scss: - fontSize: style.fontSize! * 1.21, + fontSize: ambientStyle.fontSize! * 1.21, fontFamily: 'KaTeX_Main', height: 1.2, fontWeight: FontWeight.normal, @@ -54,11 +54,11 @@ TextStyle mkBaseKatexTextStyle(TextStyle style, Color baseColor) { class KatexWidget extends StatelessWidget { const KatexWidget({ super.key, - required this.textStyle, + required this.ambientTextStyle, required this.nodes, }); - final TextStyle textStyle; + final TextStyle ambientTextStyle; final List nodes; @override @@ -68,7 +68,7 @@ class KatexWidget extends StatelessWidget { return Directionality( textDirection: TextDirection.ltr, child: DefaultTextStyle( - style: mkBaseKatexTextStyle(textStyle, + style: mkBaseKatexTextStyle(ambientTextStyle, ContentTheme.of(context).textStylePlainParagraph.color!), child: widget)); } From 61a118bc88e0a73193499d1cb8978669a447ca2f Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 19 Aug 2025 16:57:16 -0700 Subject: [PATCH 3/4] content: Include inline styles (italic etc.) in what we pass to WidgetSpans Fixes #1813. Fixes #1819. This is NFC for inline math, since all the inline styles are overridden there. But see the previous commit for where we consolidated those overrides, making a natural place to add more if needed in the future, with TODOs for some existing ones we might not want to keep. --- lib/widgets/content.dart | 23 +++++++++++++++++------ test/widgets/content_test.dart | 29 +++++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index a25e0ee27d..5304c1de33 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -1081,10 +1081,21 @@ class _InlineContentBuilder { _recognizer = _recognizerStack!.removeLast(); } - InlineSpan _buildNodes(List nodes, {required TextStyle? style}) { + final List _styleStack = []; + + TextStyle _resolveStyleStack() { + assert(_styleStack.isNotEmpty); // first item is `widget.style` + return _styleStack.reduce((value, element) => value.merge(element)); + } + + InlineSpan _buildNodes(List nodes, {required TextStyle style}) { + _styleStack.add(style); + final children = nodes.map(_buildNode).toList(growable: false); + _styleStack.removeLast(); + return TextSpan( style: style, - children: nodes.map(_buildNode).toList(growable: false)); + children: children); } InlineSpan _buildNode(InlineContentNode node) { @@ -1123,7 +1134,7 @@ class _InlineContentBuilder { case UserMentionNode(): return WidgetSpan(alignment: PlaceholderAlignment.middle, - child: UserMention(ambientTextStyle: widget.style, node: node)); + child: UserMention(ambientTextStyle: _resolveStyleStack(), node: node)); case UnicodeEmojiNode(): return TextSpan(text: node.emojiUnicode, recognizer: _recognizer, @@ -1145,11 +1156,11 @@ class _InlineContentBuilder { : WidgetSpan( alignment: PlaceholderAlignment.baseline, baseline: TextBaseline.alphabetic, - child: KatexWidget(ambientTextStyle: widget.style, nodes: nodes)); + child: KatexWidget(ambientTextStyle: _resolveStyleStack(), nodes: nodes)); case GlobalTimeNode(): return WidgetSpan(alignment: PlaceholderAlignment.middle, - child: GlobalTime(node: node, ambientTextStyle: widget.style)); + child: GlobalTime(node: node, ambientTextStyle: _resolveStyleStack())); case UnimplementedInlineContentNode(): return _errorUnimplemented(node, context: _context!); @@ -1341,7 +1352,7 @@ class GlobalTime extends StatelessWidget { size: ambientTextStyle.fontSize!, // (When GlobalTime appears in a link, it should be blue // like the text.) - color: DefaultTextStyle.of(context).style.color!, + color: ambientTextStyle.color, ZulipIcons.clock), // Ad-hoc spacing adjustment per feedback: // https://chat.zulip.org/#narrow/stream/101-design/topic/clock.20icons/near/1729345 diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index aef5ef3cc0..32c746eb6b 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -763,6 +763,14 @@ void main() { }); }); + testWidgets('is italic in italic span', (tester) async { + // Regression test for: https://github.com/zulip/zulip-flutter/issues/1813 + await prepareContent(tester, + plainContent('

@Chris Bobbe

')); + final style = mergedStyleOf(tester, '@Chris Bobbe'); + check(style!.fontStyle).equals(FontStyle.italic); + }); + testFontWeight('silent or non-self mention in plain paragraph', expectedWght: 400, // @_**Greg Price** @@ -1081,7 +1089,19 @@ void main() { check(find.textContaining(renderedTextRegexpTwelveHour)).findsOne(); }); - void testIconAndTextSameColor(String description, String html) { + testWidgets('is italic in italic span', (tester) async { + // Regression test for: https://github.com/zulip/zulip-flutter/issues/1813 + await prepareContent(tester, + // We use the self-account's time-format setting. + wrapWithPerAccountStoreWidget: true, + initialSnapshot: eg.initialSnapshot(), + plainContent('

$timeSpanHtml

')); + final style = mergedStyleOf(tester, + findAncestor: find.byType(GlobalTime), renderedTextRegexp); + check(style!.fontStyle).equals(FontStyle.italic); + }); + + void testIconAndTextSameColor(String description, String html, {Color? expectedColor}) { testWidgets('clock icon and text are the same color: $description', (tester) async { await prepareContent(tester, // We use the self-account's time-format setting. @@ -1097,11 +1117,16 @@ void main() { check(textColor).isNotNull(); check(icon).color.isNotNull().isSameColorAs(textColor!); + if (expectedColor != null) { + check(icon).color.equals(expectedColor); + } }); } testIconAndTextSameColor('common case', '

$timeSpanHtml

'); - testIconAndTextSameColor('inside link', '

$timeSpanHtml

'); + // Regression test for: https://github.com/zulip/zulip-flutter/issues/1819 + testIconAndTextSameColor('inside link', '

$timeSpanHtml

', + expectedColor: const HSLColor.fromAHSL(1, 200, 1, 0.4).toColor()); group('maintains font-size ratio with surrounding text', () { Future doCheck(WidgetTester tester, double Function(GlobalTime widget) sizeFromWidget) async { From 8310d37aa7c9b2bd02816cb92c827e0f4918a6d7 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 20 Aug 2025 16:02:32 -0700 Subject: [PATCH 4/4] content: Make TeX in link spans link-colored Fixes #1823. --- lib/widgets/katex.dart | 9 ++------- test/widgets/content_test.dart | 19 ++++++++++++++----- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/lib/widgets/katex.dart b/lib/widgets/katex.dart index 9292db8460..b85b3e7f0d 100644 --- a/lib/widgets/katex.dart +++ b/lib/widgets/katex.dart @@ -6,7 +6,6 @@ import 'package:flutter/rendering.dart'; import '../model/content.dart'; import '../model/katex.dart'; -import 'content.dart'; /// Creates a base text style for rendering KaTeX content. /// @@ -16,7 +15,7 @@ import 'content.dart'; /// https://github.com/KaTeX/KaTeX/blob/613c3da8/src/styles/katex.scss#L13-L15 /// /// Requires the [ambientStyle.fontSize] to be non-null. -TextStyle mkBaseKatexTextStyle(TextStyle ambientStyle, Color baseColor) { +TextStyle mkBaseKatexTextStyle(TextStyle ambientStyle) { return ambientStyle.copyWith( ////// Overrides of our own styles: @@ -24,9 +23,6 @@ TextStyle mkBaseKatexTextStyle(TextStyle ambientStyle, Color baseColor) { // Just for completeness, remove "wght", but it wouldn't do anything anyway // since KaTeX_Main is not a variable-weight font. fontVariations: [], - // Remove link color, but - // TODO(#1823) do we want to do that? Web doesn't. - color: baseColor, // Italic is removed below. // Strikethrough is removed below, which affects formatting of rendered @@ -68,8 +64,7 @@ class KatexWidget extends StatelessWidget { return Directionality( textDirection: TextDirection.ltr, child: DefaultTextStyle( - style: mkBaseKatexTextStyle(ambientTextStyle, - ContentTheme.of(context).textStylePlainParagraph.color!), + style: mkBaseKatexTextStyle(ambientTextStyle), child: widget)); } } diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 32c746eb6b..a1126938d5 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -997,13 +997,22 @@ void main() { testContentSmoke(ContentExample.mathInline); + final mathInlineHtml = '' + 'λ' + ' \\lambda ' + ''; + + testWidgets('is link-colored in link span', (tester) async { + // Regression test for: https://github.com/zulip/zulip-flutter/issues/1823 + await prepareContent(tester, + plainContent('

$mathInlineHtml

')); + final style = mergedStyleOf(tester, 'λ'); + check(style!.color).equals(const HSLColor.fromAHSL(1, 200, 1, 0.4).toColor()); + }); + testWidgets('maintains font-size ratio with surrounding text', (tester) async { - const html = '' - 'λ' - ' \\lambda ' - ''; await checkFontSizeRatio(tester, - targetHtml: html, + targetHtml: mathInlineHtml, targetFontSizeFinder: (rootSpan) { late final double result; rootSpan.visitChildren((span) {