Skip to content

content: Fix **bold code** rendering with regular weight #501

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 3 commits into from
Feb 2, 2024
Merged

content: Fix **bold code** rendering with regular weight #501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
70483dc
content: Format **bold text** using weightVariableTextStyle
chrisbobbe Feb 1, 2024
a769a24
content: Fix **`bold code`** rendering with regular weight
chrisbobbe Feb 1, 2024
4c0c6cd
text [nfc]: Update comment on weightVariableTextStyle(…).fontWeight
chrisbobbe Feb 1, 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
29 changes: 18 additions & 11 deletions lib/widgets/content.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ class InlineContent extends StatelessWidget {

@override
Widget build(BuildContext context) {
return Text.rich(_builder.build());
return Text.rich(_builder.build(context));
}
}

Expand All @@ -463,15 +463,21 @@ class _InlineContentBuilder {

final InlineContent widget;

InlineSpan build() {
InlineSpan build(BuildContext context) {
assert(_context == null);
_context = context;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_context = 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:
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -594,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.
Expand All @@ -606,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:
Expand Down
5 changes: 3 additions & 2 deletions lib/widgets/text.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down