From 70483dc49f930d4eb6836c0352ad9c8578780c09 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 1 Feb 2024 13:36:38 -0800 Subject: [PATCH 1/3] content: Format **bold text** using weightVariableTextStyle Fixes: #499 --- lib/widgets/content.dart | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index b569f43b55..e62a5fb239 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -454,7 +454,7 @@ class InlineContent extends StatelessWidget { @override Widget build(BuildContext context) { - return Text.rich(_builder.build()); + return Text.rich(_builder.build(context)); } } @@ -463,15 +463,21 @@ class _InlineContentBuilder { final InlineContent widget; - InlineSpan build() { + InlineSpan build(BuildContext context) { + assert(_context == null); + _context = context; assert(_recognizer == widget.recognizer); assert(_recognizerStack == null || _recognizerStack!.isEmpty); final result = _buildNodes(widget.nodes, style: widget.style); + assert(identical(_context, context)); + _context = null; assert(_recognizer == widget.recognizer); assert(_recognizerStack == null || _recognizerStack!.isEmpty); return result; } + BuildContext? _context; + // Why do we have to track `recognizer` here, rather than apply it // once at the top of the affected span? Because the events don't bubble // within a paragraph: @@ -531,7 +537,7 @@ class _InlineContentBuilder { } InlineSpan _buildStrong(StrongNode node) => _buildNodes(node.nodes, - style: const TextStyle(fontWeight: FontWeight.w600)); + style: weightVariableTextStyle(_context, wght: 600, wghtIfPlatformRequestsBold: 900)); InlineSpan _buildEmphasis(EmphasisNode node) => _buildNodes(node.nodes, style: const TextStyle(fontStyle: FontStyle.italic)); From a769a245713ad150821de2eb31802b0dc85ae229 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 1 Feb 2024 12:15:10 -0800 Subject: [PATCH 2/3] content: Fix **`bold code`** rendering with regular weight Our experience with variable-weight fonts is that, whenever we use such a font, we need to specify a "wght" value, even if we want the text to appear in the boring, regular weight. If we don't specify a "wght", the text will appear in an extremely light weight. We use a variable-weight font (Source Code Pro) for rendering inline code spans, so that's why we were using weightVariableTextStyle here. As an unfortunate result, when a code span appears inside a bold ("strong") span, the outer span's bold-weight style has been getting clobbered, and the code span appears in regular weight. Fortunately, removing the `weightVariableTextStyle` from the inline code style doesn't actually cause any inline spans to appear at minimum weight. This is a relief, and it follows from the presence of a `weightVariableTextStyle` applied widely across the app; see zulipTypography and 6784ef9ca. We apply it widely across the app because we apply Source Sans 3 widely across the app, and that is also a variable-weight font. Moreover, Source Code Pro and Source Sans 3 seem closely related (and were designed by the same person), so it's likely that their "wght" axes are identical. Fixes: #498 --- lib/widgets/content.dart | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index e62a5fb239..bc43f0b84b 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -600,9 +600,18 @@ class _InlineContentBuilder { final _kInlineMathStyle = _kInlineCodeStyle.merge(TextStyle( backgroundColor: const HSLColor.fromAHSL(1, 240, 0.4, 0.93).toColor())); +// 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)); + +final _kCodeBlockStyle = kMonospaceTextStyle + .merge(const TextStyle( + backgroundColor: Color.fromRGBO(255, 255, 255, 1), fontSize: 0.825 * kBaseFontSize)) .merge( // TODO(a11y) pass a BuildContext, to handle platform request for bold text. @@ -612,14 +621,6 @@ final _kInlineCodeStyle = kMonospaceTextStyle // frequently. Then consumers can just look it up on the InheritedWidget. weightVariableTextStyle(null)); -final _kCodeBlockStyle = kMonospaceTextStyle - .merge(const TextStyle( - backgroundColor: Color.fromRGBO(255, 255, 255, 1), - fontSize: 0.825 * kBaseFontSize)) - .merge( - // TODO(a11y) pass a BuildContext; see comment in _kInlineCodeStyle above. - weightVariableTextStyle(null)); - // const _kInlineCodeLeftBracket = '⸤'; // const _kInlineCodeRightBracket = '⸣'; // Some alternatives: From 4c0c6cd0534b90004dbbc29480294a47e43022ea Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 1 Feb 2024 14:43:39 -0800 Subject: [PATCH 3/3] =?UTF-8?q?text=20[nfc]:=20Update=20comment=20on=20wei?= =?UTF-8?q?ghtVariableTextStyle(=E2=80=A6).fontWeight?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Related: #500 --- lib/widgets/text.dart | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/widgets/text.dart b/lib/widgets/text.dart index 4fb9e1cec7..5cc659abd1 100644 --- a/lib/widgets/text.dart +++ b/lib/widgets/text.dart @@ -200,8 +200,9 @@ TextStyle weightVariableTextStyle(BuildContext? context, { fontVariations: [FontVariation('wght', value)], // This use of `fontWeight` shouldn't affect glyphs in the preferred, - // "wght"-axis font. If it does, see for debugging: - // https://github.com/zulip/zulip-flutter/issues/65#issuecomment-1550666764 + // "wght"-axis font. But it can; see upstream bug: + // https://github.com/flutter/flutter/issues/136779 + // TODO(#500) await/send upstream bugfix? fontWeight: clampVariableFontWeight(value), inherit: true);