diff --git a/CHANGELOG.md b/CHANGELOG.md index 0402849..de8cac3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,36 @@ +## Unreleased + +### Monospace font resolution +- **FIXED**: Inline `` `code` `` and fenced blocks pick a monospace family the + host platform can actually resolve. Flutter hands the family straight to the + platform font manager, and CoreText (iOS / macOS) and DirectWrite (Windows) + do not know the CSS generic `'monospace'` — code silently rendered in the + proportional body face there. Those two now get `Menlo` / `Consolas`. + **Android, Fuchsia, Linux and web keep `'monospace'`**, so the primary face + there is unchanged. Note that on web it is unchanged but still wrong: + CanvasKit / skwasm reach no system fonts, so no family name resolves there + and code has always rendered in the proportional default. Bundling a + monospace asset and pointing `monospaceFontFamily` at it is the only fix, + and that is a host decision. +- **CHANGED**: code spans now carry a `fontFamilyFallback` on every platform: + the monospace chain followed by whatever `textStyle` already carried. A + monospace face covers Latin and little else, so the body face's chain is + appended rather than replaced — a host's CJK / emoji coverage keeps working + inside `` `code` `` and fenced blocks. +- **ADDED**: `MarkdownThemeData.monospaceFontFamily` / + `monospaceFontFamilyFallback` to pin a bundled face. Both default to the + platform-resolved chain above and are resolved once, in the constructor. One + knob covers both inline `` `code` `` and fenced blocks — there was previously + no supported way to change the fenced-block face at all: `BlockPainter$Code` + built its style in a static method reading a global, so even overriding + `textStyleFor` moved inline code only. An empty fallback list means "no + monospace fallbacks", not "unset" — `textStyle`'s own chain still applies. +- **CHANGED**: `MarkdownThemeData.mergeTheme` resolves the default face from + `ThemeData.platform` rather than the global `defaultTargetPlatform`, so an + app that adapts its Material theme to another platform gets that platform's + face. `ThemeData.platform` defaults to `defaultTargetPlatform`, so this only + moves for an app that deliberately overrode it. + ## 0.2.0 > **Upgrading from 0.0.x?** See the diff --git a/README.md b/README.md index 1ede646..c07df4c 100644 --- a/README.md +++ b/README.md @@ -441,6 +441,12 @@ MarkdownTheme( linkStyle: const TextStyle( decoration: TextDecoration.underline, ), + // Monospace face for inline `code` AND fenced blocks. Leave it unset to + // get a family the host platform can actually resolve: `monospace` on + // Android / Linux / web, Menlo on iOS + macOS, Consolas on Windows + // (CoreText and DirectWrite do not know the CSS generic, so code would + // otherwise render in the proportional body face there). + monospaceFontFamily: 'JetBrains Mono', // Per-type accent colors for GitHub alert blocks alertColors: const { MD$AlertType.warning: Color(0xFF9A6700), diff --git a/docs/rendering.md b/docs/rendering.md index 9a3869b..c0564ab 100644 --- a/docs/rendering.md +++ b/docs/rendering.md @@ -194,7 +194,12 @@ Styling: `textStyle`, per-level `h1Style..h6Style` (+ cached `headingStyleFor`), `textStyleFor(MD$Style)` (cached mapping of the bitmask → bold/italic/underline/ strikethrough/monospace + highlight/monospace backgrounds + link color), `linkColor`/`linkStyle`, `surfaceColor` (code/table/quote backgrounds), -`highlightBackgroundColor`, `monospaceBackgroundColor`, `dividerColor`, +`highlightBackgroundColor`, `monospaceBackgroundColor`, +`monospaceFontFamily`/`monospaceFontFamilyFallback` (platform-resolved by +default — `monospace` on Android/Fuchsia/Linux/web, `Menlo` on iOS/macOS, +`Consolas` on Windows, from `ThemeData.platform` under `mergeTheme`; one knob +for both inline `` `code` `` and fenced blocks, and `textStyle`'s own fallback +chain is appended after it), `dividerColor`, `alertColors` (+ built-in GitHub palette fallback via `alertColorFor`), `textDirection`, `textScaler`, and `onLinkTap`. diff --git a/lib/src/render/blocks/code.dart b/lib/src/render/blocks/code.dart index d44733c..d65689c 100644 --- a/lib/src/render/blocks/code.dart +++ b/lib/src/render/blocks/code.dart @@ -41,7 +41,8 @@ class BlockPainter$Code with SelectableTextBlock implements BlockPainter { MarkdownThemeData theme, ) { final baseStyle = theme.textStyle.copyWith( - fontFamily: 'monospace', + fontFamily: theme.monospaceFontFamily, + fontFamilyFallback: theme.monospaceFallbackChain, fontSize: theme.textStyle.fontSize ?? kDefaultFontSize, ); final highlighter = theme.highlighter; diff --git a/lib/src/theme.dart b/lib/src/theme.dart index bca6016..263566d 100644 --- a/lib/src/theme.dart +++ b/lib/src/theme.dart @@ -1,10 +1,58 @@ import 'dart:collection'; +import 'package:flutter/foundation.dart' show defaultTargetPlatform, kIsWeb; import 'package:flutter/material.dart'; +import 'package:meta/meta.dart' as meta show internal; import '../flutter_md.dart'; import 'highlight/engine.dart'; +/// Primary monospace face for [platform] — the default for +/// [MarkdownThemeData.monospaceFontFamily]. +/// +/// Flutter passes the family straight to the platform font manager, and only +/// some of them know the CSS generic `'monospace'`: Android's font config and +/// fontconfig on Linux resolve it, CoreText (iOS / macOS) and DirectWrite +/// (Windows) do not — there a `'monospace'` span silently falls back to the +/// proportional body face, so inline code stops looking like code. +/// +/// So a real system family is substituted on exactly those two platforms, and +/// every other target keeps the generic it already resolved. +/// +/// Web keeps the generic as well, but not because it works: CanvasKit and +/// skwasm reach no system fonts at all, so `'monospace'` resolves no better +/// than `'Courier New'` or a name nobody has — measured in a browser, all +/// three lay out identically to the default proportional face. Naming a +/// family cannot fix that; only bundling a monospace asset and setting +/// [MarkdownThemeData.monospaceFontFamily] to it can. +String _defaultMonospaceFontFamily(TargetPlatform platform) { + if (kIsWeb) return 'monospace'; + return switch (platform) { + // Ships with every iOS / macOS release; SF Mono is not user-installable. + TargetPlatform.iOS || TargetPlatform.macOS => 'Menlo', + // Ships with Windows since Vista. + TargetPlatform.windows => 'Consolas', + TargetPlatform.android || + TargetPlatform.fuchsia || + TargetPlatform.linux => + 'monospace', + }; +} + +/// Families tried after the primary face, for a platform whose own is +/// missing (a trimmed Linux image, a custom Android ROM). +/// +/// Ends in the CSS generic so anything that resolves it still gets a real +/// monospace face rather than the proportional default. +const List _defaultMonospaceFontFamilyFallback = [ + 'Menlo', // Apple + 'Consolas', // Windows + 'DejaVu Sans Mono', // Linux + 'Roboto Mono', // Android + 'Courier New', + 'monospace', +]; + /// {@template markdown_theme_data} /// Theme data for Markdown widgets. /// {@endtemplate} @@ -27,6 +75,8 @@ class MarkdownThemeData implements ThemeExtension { this.surfaceColor = const Color.fromARGB(255, 235, 235, 235), this.highlightBackgroundColor = const Color(0x40FF5722), this.monospaceBackgroundColor = const Color(0x409E9E9E), + String? monospaceFontFamily, + List? monospaceFontFamilyFallback, this.dividerColor, this.alertColors, this.blockFilter, @@ -34,7 +84,11 @@ class MarkdownThemeData implements ThemeExtension { this.builder, this.onLinkTap, this.highlighter, - }) : _headingStyles = List.filled(8, null), + }) : monospaceFontFamily = monospaceFontFamily ?? + _defaultMonospaceFontFamily(defaultTargetPlatform), + monospaceFontFamilyFallback = + monospaceFontFamilyFallback ?? _defaultMonospaceFontFamilyFallback, + _headingStyles = List.filled(8, null), _textStyles = HashMap(); /// Creates a [MarkdownThemeData] from the given [ThemeData]. @@ -55,6 +109,8 @@ class MarkdownThemeData implements ThemeExtension { Color? surfaceColor, Color? highlightBackgroundColor, Color? monospaceBackgroundColor, + String? monospaceFontFamily, + List? monospaceFontFamilyFallback, Color? dividerColor, Map? alertColors, bool Function(MD$Block block)? blockFilter, @@ -86,6 +142,12 @@ class MarkdownThemeData implements ThemeExtension { highlightBackgroundColor ?? theme.colorScheme.errorContainer, monospaceBackgroundColor: monospaceBackgroundColor ?? theme.colorScheme.surfaceContainerHigh, + // ThemeData.platform is the app's own answer to "which platform are + // we adapting to". It defaults to defaultTargetPlatform, so this only + // diverges for an app that deliberately overrode it. + monospaceFontFamily: + monospaceFontFamily ?? _defaultMonospaceFontFamily(theme.platform), + monospaceFontFamilyFallback: monospaceFontFamilyFallback, dividerColor: dividerColor ?? theme.dividerColor.withValues(alpha: 0.12), alertColors: alertColors, blockFilter: blockFilter, @@ -145,6 +207,40 @@ class MarkdownThemeData implements ThemeExtension { /// The color to use for the background of monospace text. final Color? monospaceBackgroundColor; + /// Font family for inline `` `code` `` and fenced blocks. One knob covers + /// both — [BlockPainter$Code] reads it too. + /// + /// Defaults to a family the host platform can actually resolve: the CSS + /// generic `'monospace'` on Android, Fuchsia, Linux and web, `Menlo` on + /// iOS / macOS and `Consolas` on Windows, because CoreText and DirectWrite + /// do not know the generic and would fall back to the proportional body + /// face. Pass a bundled face (`JetBrains Mono`, `Fira Code`, …) to pin it. + /// + /// Resolved once, when the theme is constructed. + final String monospaceFontFamily; + + /// Families tried after [monospaceFontFamily]. + /// + /// Defaults to a chain ending in the CSS generic, so a platform missing its + /// primary face still lands on a real monospace one. Whatever [textStyle] + /// carries is appended after it — see [monospaceFallbackChain] — so pass an + /// empty list to fall straight through to the body face's own chain. + final List monospaceFontFamilyFallback; + + /// The fallback chain code spans actually use: [monospaceFontFamilyFallback] + /// followed by [textStyle]'s own. + /// + /// A monospace face covers Latin and little else, so dropping the body + /// face's chain would lose the host's coverage for everything it was there + /// for — CJK, emoji, right-to-left scripts — and lose it inside `` `code` `` + /// and fenced blocks only. Appending keeps the monospace faces winning + /// wherever they have the glyph and hands the rest back to the host. + @meta.internal + late final List monospaceFallbackChain = [ + ...monospaceFontFamilyFallback, + ...?textStyle.fontFamilyFallback, + ]; + /// The color to use for the divider. final Color? dividerColor; @@ -267,7 +363,11 @@ class MarkdownThemeData implements ThemeExtension { TextDecoration.lineThrough, _ => null, }, - fontFamily: style.contains(MD$Style.monospace) ? 'monospace' : null, + fontFamily: + style.contains(MD$Style.monospace) ? monospaceFontFamily : null, + fontFamilyFallback: style.contains(MD$Style.monospace) + ? monospaceFallbackChain + : null, color: switch (style) { var s when s.contains(MD$Style.link) => linkColor, _ => null, @@ -305,6 +405,8 @@ class MarkdownThemeData implements ThemeExtension { Color? surfaceColor, Color? highlightBackgroundColor, Color? monospaceBackgroundColor, + String? monospaceFontFamily, + List? monospaceFontFamilyFallback, Color? dividerColor, Map? alertColors, bool Function(MD$Block block)? blockFilter, @@ -331,6 +433,9 @@ class MarkdownThemeData implements ThemeExtension { highlightBackgroundColor ?? this.highlightBackgroundColor, monospaceBackgroundColor: monospaceBackgroundColor ?? this.monospaceBackgroundColor, + monospaceFontFamily: monospaceFontFamily ?? this.monospaceFontFamily, + monospaceFontFamilyFallback: + monospaceFontFamilyFallback ?? this.monospaceFontFamilyFallback, dividerColor: dividerColor ?? this.dividerColor, alertColors: alertColors ?? this.alertColors, blockFilter: blockFilter ?? this.blockFilter, @@ -367,6 +472,13 @@ class MarkdownThemeData implements ThemeExtension { highlightBackgroundColor, other?.highlightBackgroundColor, t), monospaceBackgroundColor: Color.lerp( monospaceBackgroundColor, other?.monospaceBackgroundColor, t), + // A font family cannot be interpolated — snap at the midpoint. + monospaceFontFamily: t < 0.5 + ? monospaceFontFamily + : other?.monospaceFontFamily ?? monospaceFontFamily, + monospaceFontFamilyFallback: t < 0.5 + ? monospaceFontFamilyFallback + : other?.monospaceFontFamilyFallback ?? monospaceFontFamilyFallback, dividerColor: Color.lerp(dividerColor, other?.dividerColor, t), alertColors: t < 0.5 ? alertColors : other?.alertColors, blockFilter: t < 0.5 ? blockFilter : other?.blockFilter, diff --git a/test/theme/theme_test.dart b/test/theme/theme_test.dart index aedde4f..367142f 100644 --- a/test/theme/theme_test.dart +++ b/test/theme/theme_test.dart @@ -1,3 +1,4 @@ +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter_md/flutter_md.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -208,4 +209,276 @@ void main() => group('MarkdownThemeData', () { expect(a.updateShouldNotify(different), isTrue); }); }); + + group('Monospace family', () { + /// Reads the resolved inline-code family under [platform]. + /// + /// Reset via try/finally rather than `addTearDown` — the framework's + /// `debugAssertAllFoundationVarsUnset` check runs first. The theme is + /// rebuilt inside the override because the family is resolved once, + /// in the constructor. + (String?, List?) familyOn(TargetPlatform platform) { + debugDefaultTargetPlatformOverride = platform; + try { + final style = base().textStyleFor(MD$Style.monospace); + return (style.fontFamily, style.fontFamilyFallback); + } finally { + debugDefaultTargetPlatformOverride = null; + } + } + + test('keeps the CSS generic where the platform resolves it', () { + // Android's font config and fontconfig on Linux both map + // `monospace`; substituting a named face there could only make the + // result worse than 0.2.x. + for (final platform in const [ + TargetPlatform.android, + TargetPlatform.fuchsia, + TargetPlatform.linux, + ]) { + expect(familyOn(platform).$1, 'monospace', reason: '$platform'); + } + }); + + test('substitutes a real face where the generic does not resolve', () { + // CoreText and DirectWrite do not know `monospace`, so inline code + // silently fell back to the proportional body face. + expect(familyOn(TargetPlatform.iOS).$1, 'Menlo'); + expect(familyOn(TargetPlatform.macOS).$1, 'Menlo'); + expect(familyOn(TargetPlatform.windows).$1, 'Consolas'); + }); + + test('every platform falls back through the generic last', () { + for (final platform in TargetPlatform.values) { + final (family, fallback) = familyOn(platform); + expect(fallback, isNotNull, reason: '$platform'); + expect(fallback!.last, 'monospace', reason: '$platform'); + expect( + fallback.contains(family) || family == 'monospace', + isTrue, + reason: '$platform primary must also appear in the chain', + ); + } + }); + + test('non-monospace spans keep the body face', () { + for (final platform in TargetPlatform.values) { + debugDefaultTargetPlatformOverride = platform; + try { + final style = base().textStyleFor(MD$Style.bold); + expect(style.fontFamily, isNull, reason: '$platform'); + expect(style.fontFamilyFallback, isNull, reason: '$platform'); + } finally { + debugDefaultTargetPlatformOverride = null; + } + } + }); + + test('a host face overrides the platform default everywhere', () { + // Both the inline span styles and BlockPainter$Code read the + // effective getters, so one knob covers `code` and ``` fences. + final theme = MarkdownThemeData( + textStyle: const TextStyle(fontSize: 14), + monospaceFontFamily: 'JetBrains Mono', + monospaceFontFamilyFallback: const ['Fira Code'], + ); + + expect(theme.monospaceFontFamily, 'JetBrains Mono'); + expect( + theme.monospaceFontFamilyFallback, + const ['Fira Code'], + ); + + final inline = theme.textStyleFor(MD$Style.monospace); + expect(inline.fontFamily, 'JetBrains Mono'); + expect(inline.fontFamilyFallback, const ['Fira Code']); + }); + + test('an override survives on every platform', () { + for (final platform in TargetPlatform.values) { + debugDefaultTargetPlatformOverride = platform; + try { + final theme = MarkdownThemeData( + textStyle: const TextStyle(fontSize: 14), + monospaceFontFamily: 'JetBrains Mono', + ); + expect( + theme.textStyleFor(MD$Style.monospace).fontFamily, + 'JetBrains Mono', + reason: '$platform', + ); + // Fallbacks stay on the shared chain unless overridden too. + expect( + theme.monospaceFontFamilyFallback, + base().monospaceFontFamilyFallback, + reason: '$platform', + ); + } finally { + debugDefaultTargetPlatformOverride = null; + } + } + }); + + test('an empty fallback list is honored, not treated as unset', () { + final theme = MarkdownThemeData( + textStyle: const TextStyle(fontSize: 14), + monospaceFontFamilyFallback: const [], + ); + expect(theme.monospaceFontFamilyFallback, isEmpty); + expect( + theme.textStyleFor(MD$Style.monospace).fontFamilyFallback, + isEmpty, + ); + }); + + test('an empty fallback list falls through to the body chain', () { + // "No monospace fallbacks" means exactly that — it does not also + // throw away the coverage the host put on textStyle. + final theme = MarkdownThemeData( + textStyle: const TextStyle( + fontSize: 14, + fontFamilyFallback: ['Noto Sans SC'], + ), + monospaceFontFamilyFallback: const [], + ); + expect( + theme.textStyleFor(MD$Style.monospace).fontFamilyFallback, + const ['Noto Sans SC'], + ); + }); + + test("the body face's fallback chain is appended, not replaced", () { + // A monospace face covers Latin and little else. Before this was + // appended, a host chain carrying CJK / emoji coverage was dropped + // for code spans only — on every platform, generic or not. + final theme = MarkdownThemeData( + textStyle: const TextStyle( + fontSize: 14, + fontFamily: 'Inter', + fontFamilyFallback: ['Noto Sans SC', 'Noto Color Emoji'], + ), + monospaceFontFamily: 'JetBrains Mono', + monospaceFontFamilyFallback: const ['Fira Code'], + ); + + const expected = [ + 'Fira Code', + 'Noto Sans SC', + 'Noto Color Emoji', + ]; + // Monospace faces still win wherever they have the glyph. + expect(theme.monospaceFallbackChain, expected); + expect( + theme.textStyleFor(MD$Style.monospace).fontFamilyFallback, + expected, + ); + // A non-monospace span is untouched by any of this. + expect( + theme.textStyleFor(MD$Style.bold).fontFamilyFallback, + const ['Noto Sans SC', 'Noto Color Emoji'], + ); + }); + + test('copyWith carries the override', () { + // The cast is only needed because copyWith still returns the base + // ThemeExtension here; DoctorinaAI/md#27 narrows it. + final theme = MarkdownThemeData(textStyle: const TextStyle()) + .copyWith(monospaceFontFamily: 'Fira Code') as MarkdownThemeData; + expect(theme.monospaceFontFamily, 'Fira Code'); + }); + + test('a fenced block uses the same family and chain', () { + // BlockPainter$Code builds its base style in a static method, so it + // used to be the half that no theme knob could move. Read the + // painter's own span rather than trusting that it shares the code + // path with textStyleFor. + final theme = MarkdownThemeData( + textStyle: const TextStyle( + fontSize: 14, + fontFamilyFallback: ['Noto Sans SC'], + ), + monospaceFontFamily: 'JetBrains Mono', + monospaceFontFamilyFallback: const ['Fira Code'], + ); + final painter = BlockPainter$Code( + text: 'void main() {}', + language: 'dart', + theme: theme, + ); + + final style = painter.painter.text?.style; + expect(style?.fontFamily, 'JetBrains Mono'); + expect( + style?.fontFamilyFallback, + const ['Fira Code', 'Noto Sans SC'], + ); + expect( + style?.fontFamily, + theme.textStyleFor(MD$Style.monospace).fontFamily, + reason: 'inline code and fences must not drift apart', + ); + }); + + testWidgets('a fenced block renders on the platform default', + (tester) async { + debugDefaultTargetPlatformOverride = TargetPlatform.macOS; + try { + expect(base().monospaceFontFamily, 'Menlo'); + await tester.pumpWidget(MaterialApp( + home: Align( + alignment: Alignment.topLeft, + child: SizedBox( + width: 400, + child: MarkdownWidget( + markdown: Markdown.fromString( + '```dart\nvoid main() {}\n```', + ), + theme: base(), + ), + ), + ), + )); + await tester.pumpAndSettle(); + expect(tester.takeException(), isNull); + } finally { + debugDefaultTargetPlatformOverride = null; + } + }); + + testWidgets('mergeTheme follows ThemeData.platform', (tester) async { + // An app that adapts its Material theme to another platform is + // stating which platform it is imitating; the global + // defaultTargetPlatform is not that answer. + debugDefaultTargetPlatformOverride = TargetPlatform.android; + try { + expect( + MarkdownThemeData.mergeTheme( + ThemeData(platform: TargetPlatform.iOS), + ).monospaceFontFamily, + 'Menlo', + ); + expect( + MarkdownThemeData.mergeTheme( + ThemeData(platform: TargetPlatform.windows), + ).monospaceFontFamily, + 'Consolas', + ); + // Unset, ThemeData.platform *is* defaultTargetPlatform. + expect( + MarkdownThemeData.mergeTheme(ThemeData()).monospaceFontFamily, + 'monospace', + ); + // An explicit face still wins over both. + expect( + MarkdownThemeData.mergeTheme( + ThemeData(platform: TargetPlatform.iOS), + monospaceFontFamily: 'JetBrains Mono', + ).monospaceFontFamily, + 'JetBrains Mono', + ); + } finally { + debugDefaultTargetPlatformOverride = null; + } + }); + }); });