Repository navigation
fix(theme): resolve a monospace family the platform actually knows - #29
Merged
Merged
Conversation
Flutter passes fontFamily straight to the platform font manager. Android's font config and fontconfig on Linux resolve the CSS generic 'monospace'; CoreText (iOS, macOS) and DirectWrite (Windows) do not, so inline code and fenced blocks silently rendered in the proportional body face on both. Substitute a real face on exactly those two (Menlo, Consolas) and keep the generic everywhere it already worked, web included: its renderer cannot reach arbitrary system fonts, so naming one could only be worse than the generic the browser can answer. Rendering on Android, Fuchsia, Linux and web is unchanged. Also makes the face replaceable. Inline code went through textStyleFor, so subclassing the theme moved that half; fenced blocks did not, because BlockPainter$Code built its base style in a static method reading a global. The only ways out were abusing SyntaxHighlighter.baseStyleFor to set a font family or reimplementing the fence painter through theme.builder — and the two halves could not be changed by the same mechanism. MarkdownThemeData .monospaceFontFamily / .monospaceFontFamilyFallback now feed both. Split out of #27, where this arrived inside a commit about hit-testing, hardcoded Menlo-first for every platform, and was mentioned nowhere in the description or changelog. NOTE: font resolution is not observable in flutter_test (the harness uses a stub font), so the tests pin the family string we pass, not that it resolves. Needs a look on a real iOS / macOS / Windows build before merge.
12 tasks done
pana docks 10 points off the pub.dev score for an unformatted file, so this is not cosmetic.
`kMonospaceFontFamily`, `kMonospaceFontFamilyFallback` and the two `effectiveMonospace…` getters were six new public symbols for what is one knob. The platform table is an implementation detail — a host either accepts the default or names its own face. Privatise both globals and resolve the default in the constructor, so `monospaceFontFamily` / `monospaceFontFamilyFallback` are plain non-nullable fields like `surfaceColor` and `linkColor`. Painters (and `BlockPainter$Code`) read them directly; the effective-value getters go away. New public surface: two fields. Also corrects the changelog's "rendering is unchanged" claim for Android / Fuchsia / Linux / web: code spans now carry a `fontFamilyFallback` everywhere, and it replaces any chain the host set on `textStyle` (verified against `origin/master`). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDVBM7Eu7JTmar3LUC96Vm
Two follow-ups from the review of this PR. `textStyleFor` and `BlockPainter$Code` passed the monospace chain as `fontFamilyFallback`, and `TextStyle.copyWith` *replaces* that list. So a host whose `textStyle.fontFamilyFallback` carried the coverage a body face needs — CJK, emoji, right-to-left scripts — silently lost it inside `code` and fenced blocks, on every platform including the ones whose primary face this PR does not touch. Verified against `origin/master`: the chain went from the host's `[Noto Sans SC, Noto Color Emoji]` to `[Menlo, Consolas, …, monospace]`. Append the host's chain after the monospace one instead: monospace faces still win wherever they have the glyph, and everything they do not cover falls back to what the host already chose. The composed list lives in `monospaceFallbackChain` (`@meta.internal`, computed once per theme) so both the inline and the fenced path read the same value. `mergeTheme` now resolves the default face from `ThemeData.platform` rather than the global `defaultTargetPlatform`. An app that adapts its Material theme to another platform is stating which platform it is imitating, and the factory had that answer in hand. `ThemeData.platform` defaults to `defaultTargetPlatform`, so nothing moves unless an app deliberately overrode it. Tests: the fenced-block case now reads the style `BlockPainter$Code` actually builds instead of only asserting that it renders. 991 pass; each new case was checked to fail without its fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDVBM7Eu7JTmar3LUC96Vm
Building the example and measuring in a real engine contradicts the platform table this PR was written against. On web, CanvasKit / skwasm reach no system fonts at all: `'monospace'`, `'Courier New'`, `'DejaVu Sans Mono'` and a deliberately nonexistent family all lay out byte-identically to the default proportional face (W×10 = 142.0 for all four). Code has never rendered monospace on web — this PR neither fixes nor regresses that, but the table claimed web was already correct. Linux, measured the same way, is exactly as described: `'monospace'` resolves to a real monospace face (i×10 == W×10 == 96.3), an unknown family falls to the proportional default (161.2), and the fallback chain rescues a missing primary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDVBM7Eu7JTmar3LUC96Vm
itsezlife
pushed a commit
to itsezlife/md
that referenced
this pull request
Oct 17, 2026
…octorinaAI#29) * fix(theme): resolve a monospace family the platform actually knows Flutter passes fontFamily straight to the platform font manager. Android's font config and fontconfig on Linux resolve the CSS generic 'monospace'; CoreText (iOS, macOS) and DirectWrite (Windows) do not, so inline code and fenced blocks silently rendered in the proportional body face on both. Substitute a real face on exactly those two (Menlo, Consolas) and keep the generic everywhere it already worked, web included: its renderer cannot reach arbitrary system fonts, so naming one could only be worse than the generic the browser can answer. Rendering on Android, Fuchsia, Linux and web is unchanged. Also makes the face replaceable. Inline code went through textStyleFor, so subclassing the theme moved that half; fenced blocks did not, because BlockPainter$Code built its base style in a static method reading a global. The only ways out were abusing SyntaxHighlighter.baseStyleFor to set a font family or reimplementing the fence painter through theme.builder — and the two halves could not be changed by the same mechanism. MarkdownThemeData .monospaceFontFamily / .monospaceFontFamilyFallback now feed both. Split out of DoctorinaAI#27, where this arrived inside a commit about hit-testing, hardcoded Menlo-first for every platform, and was mentioned nowhere in the description or changelog. NOTE: font resolution is not observable in flutter_test (the harness uses a stub font), so the tests pin the family string we pass, not that it resolves. Needs a look on a real iOS / macOS / Windows build before merge. * style: dart format pana docks 10 points off the pub.dev score for an unformatted file, so this is not cosmetic. * refactor(theme): keep the monospace defaults out of the public API `kMonospaceFontFamily`, `kMonospaceFontFamilyFallback` and the two `effectiveMonospace…` getters were six new public symbols for what is one knob. The platform table is an implementation detail — a host either accepts the default or names its own face. Privatise both globals and resolve the default in the constructor, so `monospaceFontFamily` / `monospaceFontFamilyFallback` are plain non-nullable fields like `surfaceColor` and `linkColor`. Painters (and `BlockPainter$Code`) read them directly; the effective-value getters go away. New public surface: two fields. Also corrects the changelog's "rendering is unchanged" claim for Android / Fuchsia / Linux / web: code spans now carry a `fontFamilyFallback` everywhere, and it replaces any chain the host set on `textStyle` (verified against `origin/master`). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDVBM7Eu7JTmar3LUC96Vm * fix(theme): keep the body face's fallback chain in code spans Two follow-ups from the review of this PR. `textStyleFor` and `BlockPainter$Code` passed the monospace chain as `fontFamilyFallback`, and `TextStyle.copyWith` *replaces* that list. So a host whose `textStyle.fontFamilyFallback` carried the coverage a body face needs — CJK, emoji, right-to-left scripts — silently lost it inside `code` and fenced blocks, on every platform including the ones whose primary face this PR does not touch. Verified against `origin/master`: the chain went from the host's `[Noto Sans SC, Noto Color Emoji]` to `[Menlo, Consolas, …, monospace]`. Append the host's chain after the monospace one instead: monospace faces still win wherever they have the glyph, and everything they do not cover falls back to what the host already chose. The composed list lives in `monospaceFallbackChain` (`@meta.internal`, computed once per theme) so both the inline and the fenced path read the same value. `mergeTheme` now resolves the default face from `ThemeData.platform` rather than the global `defaultTargetPlatform`. An app that adapts its Material theme to another platform is stating which platform it is imitating, and the factory had that answer in hand. `ThemeData.platform` defaults to `defaultTargetPlatform`, so nothing moves unless an app deliberately overrode it. Tests: the fenced-block case now reads the style `BlockPainter$Code` actually builds instead of only asserting that it renders. 991 pass; each new case was checked to fail without its fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDVBM7Eu7JTmar3LUC96Vm * docs(theme): correct the web claim, measured in a browser Building the example and measuring in a real engine contradicts the platform table this PR was written against. On web, CanvasKit / skwasm reach no system fonts at all: `'monospace'`, `'Courier New'`, `'DejaVu Sans Mono'` and a deliberately nonexistent family all lay out byte-identically to the default proportional face (W×10 = 142.0 for all four). Code has never rendered monospace on web — this PR neither fixes nor regresses that, but the table claimed web was already correct. Linux, measured the same way, is exactly as described: `'monospace'` resolves to a real monospace face (i×10 == W×10 == 96.3), an unknown family falls to the proportional default (161.2), and the fallback chain rescues a missing primary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDVBM7Eu7JTmar3LUC96Vm --------- Co-authored-by: Mikhail Matiunin <plugfox@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 12e068f)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of #27, where this arrived inside a commit about hit-testing (
93a8791), hardcodedMenlofirst for every platform, and was mentioned nowhere in the description or the changelog.The bug it fixes
Flutter hands
fontFamilystraight to the platform font manager, and only some of them know the CSS generic'monospace':'monospace'So inline
`code`and fenced blocks silently stopped looking like code on Apple and Windows.What this does
Substitute a real face on exactly the two platforms that need it, and keep the generic everywhere it already worked:
kMonospaceFontFamilyFallbackfollows and ends in'monospace'again, so a platform missing its primary face still lands on a real monospace one instead of the proportional default.Web deliberately keeps the generic: its renderer cannot reach arbitrary system fonts, so naming one could not help, and this guarantees mobile web is byte-identical to 0.2.0.
Rendering on Android, Fuchsia, Linux and web is unchanged. Only Apple and Windows move, and only in the direction of correctness.
It also makes the face replaceable
There was no supported way to change it. Inline code goes through
MarkdownThemeData.textStyleFor, so subclassing the theme moved that half — butBlockPainter$Codebuilds its base style in a static method reading the global directly, so fenced blocks did not follow. The only ways out were abusingSyntaxHighlighter.baseStyleForto set a font family, or reimplementing the whole fence painter throughtheme.builder. The two halves could not even be changed by the same mechanism.nullkeeps the platform default; an empty fallback list means "no fallbacks", not "unset". Both paths readeffectiveMonospaceFontFamily/effectiveMonospaceFontFamilyFallback, which are public so a custom block painter can use them too. Threaded throughmergeTheme,copyWithandlerp(a family cannot be interpolated — it snaps att < 0.5).Font resolution is not observable in
flutter_test— the harness substitutes a stub font, so the 15 tests here pin the family string we pass, not thatMenlo/Consolasactually resolve. Please eyeball a real iOS, macOS and Windows build.Tests
15 cases in
test/theme/theme_test.dart: the generic survives on Android / Fuchsia / Linux, substitution happens on iOS / macOS / Windows, every platform's chain ends in'monospace'and contains its primary, non-monospace spans keep the body face, a host override wins on every platform, an empty fallback list is honoured,copyWithcarries it, and a fenced block renders.flutter test test/unit_test.dart→ 987 passed.dart analyze --fatal-infos --fatal-warnings lib/ test/→ clean.One nit:
copyWith carries the overrideneeds a cast, because onmasterMarkdownThemeData.copyWithstill returns the baseThemeExtension<MarkdownThemeData>. #27 narrows that return type, so I left the cast here with a comment rather than have two PRs touch the same line.Credit: @itsezlife spotted the underlying bug in #27 and wrote the first fix. What changed here is the shape: platform-aware instead of Menlo-first everywhere, plus the theme knob.