Skip to content

Selection chrome parity, registry fixes, autoscroll, and line clamp - #27

Closed
itsezlife wants to merge 11 commits into
DoctorinaAI:masterfrom
itsezlife:feat/selection-chrome
Closed

itsezlife wants to merge 11 commits into
DoctorinaAI:masterfrom
itsezlife:feat/selection-chrome

Conversation

@itsezlife

@itsezlife itsezlife commented Sep 11, 2026 •

Copy link
Copy Markdown

Before / after

This is high-level preview of how the package behaves before the changes and after, in fact changes are broader than what's can be just visually noticed.

Before (pre-change selection)

before.mp4

After (this branch)

after.mp4

Summary

Follow-up on the 0.2.0 selection work. The controller path now behaves like Flutter SelectionArea / SelectableRegion without wrapping markdown in SelectionArea.

Registry. removeDocument waits until the surface detaches when the body is still mounted. A later putDocument cancels that pending remove. rangeFor and endpoint ordering skip unregistered ids instead of treating them as index -1 (which used to paint every body from the start of the registry through the other endpoint). Mounted surfaces heal-register with putDocument on attach.

Chrome & desktop parity. Gesture starts need a hit on mounted selectable markdown. Empty chrome does not nearest-neighbor clamp into content; clamp still applies when extending across gaps. Soft-wrap affinity on handles, directed base/extent edges with reverse handle types, coincident-caret separation, and handle proxies when an endpoint surface unmounts. Touch keeps native SelectionOverlay handles and magnifier.

  • Desktop toolbar parity: On desktop (macOS, Linux, Windows), mouse drags, double/triple clicks, keyboard shortcuts (Cmd/Ctrl+A), and programmatic selection updates (selectAll, selection = ...) do not pop up the toolbar. Mobile touch interactions and programmatic selection continue to show the toolbar with action items.
  • Right-click on desktop: Secondary click shows the context toolbar at the click coordinates without mutating or selecting text (does not select word or collapse to caret).
  • Desktop anchor stability: Context menu preserves its right-click anchor position when triggering actions such as "Select all", rather than jumping to selection endpoints.
  • Desktop scroll dismissal: Context menu dismisses immediately upon ancestor/host scroll.
  • Focus loss during an active pointer drag does not clear the selection (list rebuilds during autoscroll).

Host gates for chat mounts.

  • MarkdownSelectionScope.enableTouchGestures — when false, touch / stylus / trackpad selection recognizers stay off; mouse multi-click, handles, toolbar, and keyboard remain.
  • MarkdownSelectionScope.enableTouchConsecutiveTaps — with touch gestures on, long-press → word → drag still arms, but touch multi-tap / horizontal-drag selection does not (viewport keeps taps).
  • MarkdownSelectionScope.ownsSelectionChrome / canStartSelectionAt — multi-scope hosts can refuse chrome ownership or selection starts on chrome (links, code headers).
  • FIXED: Disabled / non-owning sibling scopes no longer clear handle leaders on every shared-controller surface. Clearing is scoped to that scope’s own LayerLinks, so chat dual mounts keep handles after non-collapsed range commits (including repeated handle-drag settles).

Glyph-tight hit testing. Hover I-beam and link hit-testing use rendered line/glyph boxes, not the full max-width layout. Empty horizontal gutter beside a short line is no longer I-beam / click-to-open. Gesture starts stay surface-bounds; drag-extend may still clamp outside glyphs. Mouse single-click that misses selectable content while a non-collapsed range is active clears the selection.

Dynamic cursor resolution & span bounding boxes.

  • MarkdownThemeData.cursorResolver and MarkdownWidget.cursorResolver (MarkdownCursorResolver) — unopinionated hook to dynamically resolve hover mouse cursors per local offset, block index, and block model, falling back to default link/text/defer cursors when returning null.
  • MarkdownSelectionSurface.localBoxesForRange and MarkdownPainter.localBoxesForRange — fast query returning content-local bounding boxes for an arbitrary character range within a block, directly from cached block painters without re-layout.

Highlight paint. Selection stays outside the glyph Picture cache. Default pass is under the glyphs. A second pass goes above opaque chrome (selectionHighlightAboveCachedContent) for code fences, table zebra rows, and inline monospace / highlight backgrounds.

Autoscroll. Edge-zone scrolling while dragging near the padded viewport (MarkdownSelectionScope.autoscroll, default edgeZone: 48). Hard stop is the host union of mounted selectable bodies, with a direction arming gate after a flush stop.

Line clamp. New public clampMarkdownToLines / MarkdownLineClamp measures where the first N visual text lines end at a width so a host can size a clipped box. Spacers and dividers add height without spending the line budget. The renderer still has no line budget of its own.

Quote / alert nested fenced code.

  • Fenced code (```lang / ~~~) inside a blockquote or GitHub alert is re-parsed as nested MD$Code in MD$Quote.blocks / MD$Alert.blocks, instead of being consumed as inline monospace by backtick pairing.
  • Optional blocks on MD$Quote / MD$Alert (empty for leaf inline bodies). Render and markdownBlockRenderedText walk nested children when present.
  • Prose nested beside a fence inherits quoteStyle via BlockPainter$Quote.inheritFrom; code/table chrome keeps the document theme.

Example. SDK / Flutter floor bump, drop stale smoke test, tidy chat and highlight tabs.

Docs and CHANGELOG Unreleased cover the above. CONTEXT.md stays out of this PR.

Test plan

  • flutter test test/unit_test.dart (1079+ passed)
  • dart analyze --fatal-infos --fatal-warnings lib/ test/ (clean)
  • Smoke the example Chat tab on a touch device: long-press, drag across messages, handles, magnifier, edge autoscroll, toolbar hide/show
  • Same on desktop: mouse drag, Shift-extend, right-click context menu (no select side-effect), Copy / Select all, Esc, scroll dismissal
  • Dual-body chat mounts: selection handles survive settle / repeated handle-drag with ownsSelectionChrome siblings
  • Dynamic cursor resolution via cursorResolver with fallback to default text/link cursors
  • Query localBoxesForRange across blocks and soft-wrapped text lines
  • Glyph-tight hover: empty gutter beside a short line is not I-beam; click-miss clears selection
  • Virtualized list: scroll a selected endpoint off-screen and back; confirm handle proxy and registry remove/remount do not leave a stuck highlight
  • Code fence / table / `inline code` selection tint visible above opaque fills
  • Quote with nested fenced code renders as a real code block; surrounding quote prose keeps quoteStyle
  • Call clampMarkdownToLines from a host clip and confirm cuts land on line boundaries

Made with Cursor

itsezlife and others added 8 commits September 11, 2026 20:13
…oscroll

Bring read-only selection up to SelectableRegion quality: content-gated
gestures, directed handles with soft-wrap affinity, live toolbar anchors,
deferred registry remove/heal, dual-pass highlight paint, and host-union
edge autoscroll while dragging.
Expose a public measure-then-clip helper that cuts on real text line
boundaries so hosts can size overflow chrome without half-line crops.
Raise the example app floor to current Flutter tooling, drop the stale
smoke test, and tidy chat/highlight demo formatting.
Co-authored-by: Cursor <cursoragent@cursor.com>
…ith stock Flutter

Co-authored-by: Cursor <cursoragent@cursor.com>
…onScope

Introduce a new boolean parameter, enableTouchGestures, to the MarkdownSelectionScope widget, allowing control over touch, stylus, and trackpad selection gestures. This enhances flexibility for scenarios where touch input should be disabled, while maintaining keyboard and mouse interactions. Update the focus handling logic to respect this new parameter.
Separate selectable surface bounds from glyph-tight ink for pan/cursor
arbitration, expose isLinkAtGlobal for painter-owned copy chrome, and clear
mouse selection on non-text taps when a live range exists.

Co-authored-by: Cursor <cursoragent@cursor.com>
Re-parse quote/alert bodies that contain fences into nested blocks so
code stays MD$Code (not inline monospace), and apply quoteStyle to nested
prose. Also clear only this scope's handle LayerLinks so sibling mounts
sharing a controller keep leaders after settle.

Co-authored-by: Cursor <cursoragent@cursor.com>
@mike-doctorina
mike-doctorina self-requested a review September 14, 2026 10:42
@mike-doctorina mike-doctorina self-assigned this Sep 14, 2026
@mike-doctorina mike-doctorina added the enhancement New feature or request label Sep 14, 2026
Lock in boot, theme toggle, tab switching, editor refresh, and chat
stream/stop so example regressions fail in CI-less local runs.

@mike-doctorina mike-doctorina left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed at dcc4f3a, re-checked against 6284c9b — that commit only adds example/test/smoke_test.dart and does not touch lib/, so everything below still stands.

How: flutter test test/unit_test.dart, dart analyze --fatal-infos --fatal-warnings lib/ test/, flutter build web on the example, plus ~140 new tests written against the areas below. My branch is at 1222 tests vs 1080 here (after moving the line-clamp and monospace tests out, see Please split below).

Verdict: the selection work is good and I want it in. Eight defects block the merge — all are fixed with regression tests on a branch I can push wherever you prefer. Two features should leave this PR entirely; those branches are ready too.


Blockers

Each of these had a test that failed before the fix.

  1. Registry corruption when a list recycles a widget — updateRenderObject pushes the new model before rewiring the id, so a ListView.builder reusing a slot writes message B's body under message A's id. Scrolled-off messages then copy and highlight the wrong text. This is the one I'd call critical: it is silent and it hits the exact virtualized-chat case the PR is built for.
  2. setState() called during build — onSelectionChanged fires synchronously from inside build/layout on two ordinary paths: a streaming putDocument from MarkdownWidget.updateRenderObject, and a deferred removeDocument flushing from RenderObject.detach. Any host that rebuilds from the callback throws. Both reproduce in a widget test.
  3. canStartSelectionAt refusal leaves the drag armed — the recognizers correctly stay in the arena while a range is live so a chrome tap can dismiss it, but _beginSelection returning false does not un-arm _dragGlobal. A long press on a refused link then keeps extending the active selection from that point (observed: the range grew to the whole document).
  4. Heal-registration jumps to the front of the conversation — putDocument with no order assigns _docs.length, which sorts ahead of sparse explicit orders. Chat hosts key order on the message id, so a healed body lands at the top and reverses extracted text.
  5. Unstable order tie-break — _docs.sort compares order only, and List.sort is not stable in Dart. Documents sharing an order can swap on any re-sort.
  6. Glyph boxes recomputed on every hover event — _painterHitsGlyphBoxes calls getBoxesForSelection over the whole block. Mouse-move over a long fenced block allocates one box per run per line, every event.
  7. maxVelocity: 0 spins the frame ticker forever — past the outer band the code returns scrolled on a zero velocity, so the ticker keeps running on a drag that can never scroll.
  8. Nested quote/alert children bypass MarkdownThemeData.builder — a host that replaces the code painter (fence with a copy button) silently gets the default one inside > ….

Autoscroll should stop speaking the sliver protocol

This is the one structural change I'd ask for. applyMarkdownSelectionAutoscroll resolves a ScrollableState and drives ScrollPosition.jumpTo, so it cannot drive our own chat viewport, which owns an anchor offset and has no Scrollable anywhere in the tree.

I reworked it behind a MarkdownAutoscrollTarget — viewport, canScroll, applyScrollDelta, all in screen-space deltas so reverse axes and inverted anchors are the adapter's problem. MarkdownScrollableAutoscrollTarget keeps today's behaviour (and fixes reverse: true lists, which the current sign handling gets wrong for a ListView whose axis is AxisDirection.up), and a targetResolver on the config covers everything else. Also added useHostUnionGate: false: the union gate assumes a cache extent, so a viewport that builds only what is visible would have its own paging vetoed.

Side effect worth having on its own: the scroll surface is now resolved once per drag instead of two full element-subtree walks per frame.

Please split out

Neither belongs in a PR about selection chrome, and nothing inside the package calls either one:

  • clampMarkdownToLines / MarkdownLineClamp / textLineBottoms — zero callers in lib/ and example/, so the only thing exercising the line-counting heuristic is its own unit tests. Branch is ready.
  • The monospace family — see the inline note; it changes rendering for every user of the package and font resolution is not observable in flutter_test at all, so it needs a device pass this PR cannot give it. Branch is ready (platform-aware, plus a theme knob, because right now the fenced-block face cannot be changed by any supported means).

Coverage gap I closed

The streaming corpus had quote, quote-multi and two alerts but no body containing a fence — the one construct that makes the decoder re-enter itself, and the one the chat streams straight into. Added six documents (closed, unclosed, tilde, empty body, nested quote beside the fence, alert form); incremental matches batch on every prefix across all six chunkings.

Still needs a real device before merge

Not blockers, just genuinely unverifiable in widget tests:

  • Handle chrome — affinity, directed base/extent, coincident-caret separation, handle proxies, multi-scope LayerLink ownership. This is the most intricate code in the PR and its failure modes are visual: two handles stacked at one caret, a ghost handle after remount, a handle on the wrong side.
  • Desktop toolbar anchors — toolbarHeight = 48.0, aboveGap = 8.0, _kTallSelectionSecondaryOffset = 48 are pinned by tests numerically but nothing checks the menu does not leave the screen.
  • Autoscroll feel — the quadratic ramp and the edgeZone / maxVelocity defaults.

Changelog

## Unreleased is missing every breaking change. At minimum: removed BlockPainter$Quote.painter and BlockPainter$Alert.bodyPainter; both painters switching from SelectableTextBlock to MultiPainterSelectable; four new members on SelectableBlockPainter and seven on MarkdownSelectionSurface (breaking for anyone implementing either directly — the mixins cover the rest); MarkdownThemeData.copyWith narrowing its return type. I've written that section out on my branch.

Great PR otherwise — the SelectionArea parity work and the registry reasoning are exactly right.

Comment thread lib/src/widget.dart
theme: _resolveTheme(context),
cursorResolver: cursorResolver,
)
..updateSelection(_resolveController(context), documentId);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocker — registry corruption on element recycling.

update() reads _documentId, which is still the outgoing id at this point, so it writes the incoming markdown under the previous document. A ListView.builder recycling a slot changes documentId and markdown together, which is precisely this path: message B's body lands in message A's registry entry, and A's copied text and highlight geometry are wrong from then on, silently.

Fix is the order plus giving the rewire the incoming model, so the write in update() lands on the new id:

renderObject
  ..updateSelection(_resolveController(context), documentId, markdown: markdown)
  ..update(markdown: markdown, theme: _resolveTheme(context), cursorResolver: cursorResolver);

(updateSelection needs the markdown: param because the painter still holds the outgoing model when it heals.) Failing test on my branch: recycling a widget onto another id keeps both models intact.

Comment thread lib/src/selection.dart
final idx = _orderIndex(id);
if (idx < 0) {
_docs.add(_DocEntry(id, model, order ?? _docs.length));
final assigned = order ?? _docs.length;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocker — _docs.length is not "append".

With sparse explicit orders it sorts ahead of them. Chat hosts key order on the message id, so registering {a: 10, b: 20} and then healing c with no order yields [c, a, b] — the healed body jumps to the top of the conversation and cross-message extraction comes out reversed. Needs max(order) + 1.

Related, a few lines down: _sort compares order only, and List.sort is not stable in Dart, so two documents sharing an order can swap places on any re-sort (a model update, a new mount). Worth a monotonic registration counter as the tie-break — silent text reversal is a nasty way to find out.

@@ -204,19 +348,58 @@ class MarkdownSelectionScopeState extends State<MarkdownSelectionScope> {
if (sel != _lastSelection) {
_lastSelection = sel;
widget.onSelectionChanged?.call(sel);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocker — this can run inside build/layout.

The controller notifies from the build phase on two ordinary paths: a streaming putDocument reaches here from MarkdownWidget.updateRenderObject, and a deferred removeDocument flushes from RenderObject.detach. Hosts routinely setState from onSelectionChanged, so both throw setState() or markNeedsBuild() called during build. Reproduced in a widget test for each.

Coalescing the delivery to the end of the frame while in persistentCallbacks / midFrameMicrotasks (and reporting the settled selection) fixes both without changing the synchronous case.

Feedback.forLongPress(context);
// Arm drag before beginSelection so overlay sync is pointer-sync.
_dragGlobal = d.globalPosition;
_beginSelection(d.globalPosition, 2);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocker — the host gate is not honoured here.

_shouldArmTouchSelectionGesture deliberately arms the recognizer when a range is live so a chrome tap can dismiss it, which means this handler does get called on points the host refused. _dragGlobal is already set above, and _beginSelection returning false does not clear it, so onLongPressMoveUpdate happily keeps extending from the refused point at the previous granularity.

Observed in a test: long-pressing a refused right half after selecting a word grew the range to the whole document. Same shape in _handleDragStart. Two things needed — re-check widget.canStartSelectionAt before the side effects (focus, haptics, hideToolbar), and un-arm _dragGlobal when _beginSelection refuses.

clear();
return MarkdownAutoscrollResult.idle;
}
final scrollable = _scrollableFor(context);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Design — this is what stops us adopting autoscroll.

Everything below is written against ScrollableState / ScrollPosition, so the whole feature is unavailable to a host with its own scroll implementation. Our chat viewport is a RenderBox that owns an anchor offset and positions children itself; there is no Scrollable anywhere in the tree for _scrollableFor to find.

The band, host-union and arming logic do not actually need the sliver protocol — they need "where is the viewport", "can it still move", "move it". I pulled those behind a MarkdownAutoscrollTarget and kept a MarkdownScrollableAutoscrollTarget as the default, with a targetResolver on the config for everything else. Deltas become screen-space content movement, which also fixes reverse axes: today a ListView(reverse: true) (AxisDirection.up) is what most chats use, and the sign handling here does not survive it.

Happy to hand this over as a commit rather than have you redo it.


/// Whether [local] (text-painter coordinates) lies inside any glyph line box
/// of [painter]'s full rendered range.
bool _painterHitsGlyphBoxes(TextPainter painter, Offset local) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Perf — this runs on every hover event.

isSelectableAtLocal → hitsRenderedTextAt → here, on every PointerHoverEvent for the I-beam, and again for link hit-testing. getBoxesForSelection over the whole block allocates one box per run per line, so a long fenced code block rebuilds thousands of rects per mouse move on desktop and web.

Caching them per TextPainter layout is enough — key on the laid-out size plus identity of plainText (the painter memoises that string, so the check is O(1)).

Comment thread lib/src/theme.dart
/// Prefer a real family over the CSS generic `'monospace'`, which Flutter does
/// not map on most platforms (so spans fall back to the body proportional
/// font).
const String kMonospaceFontFamily = 'Menlo';

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please take this out of the PR.

I did not know this was in here — it arrived inside 93a8791 feat(selection): surface vs glyph hit-testing and fenced chrome links, is mentioned nowhere in the description or the changelog, and has no test. It changes rendering for every user of the package, in two places (textStyleFor here and BlockPainter$Code._buildSpan).

The premise is right on two platforms and wrong on the rest. CoreText (iOS/macOS) and DirectWrite (Windows) genuinely do not know the CSS generic 'monospace', so inline code fell back to the proportional body face there — a real bug worth fixing. But Android's font config and fontconfig on Linux resolve it natively, and Menlo/Monaco/Consolas exist on none of them, so putting an Apple font first makes those platforms worse than 0.2.0.

Two more things while it is out: the face is currently unoverridable — BlockPainter$Code builds its style in a static method reading this global, so even subclassing the theme and overriding textStyleFor moves inline code only and leaves fences behind. And font resolution is not observable in flutter_test at all (the harness substitutes a stub font), so no test here can prove Menlo resolves.

I have a branch that substitutes a real face on exactly the two platforms that need it, keeps the generic everywhere it already worked (web included — its renderer cannot reach arbitrary system fonts), and adds MarkdownThemeData.monospaceFontFamily so both paths are settable from one knob.

spans: a.spans,
theme: theme,
children: [
for (final child in a.blocks) _defaultBlockBuilder(child, theme),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Bug — nested children skip the builder hook.

Top-level blocks go through _theme.builder (line ~132), these do not. A host that replaces the code painter — a fence with a copy button, line numbers — gets its painter everywhere except inside a quote or an alert, which is surprising and silent.

Routing children through theme.builder?.call(block, theme) ?? _defaultBlockBuilder(block, theme) fixes it; no recursion risk, the block tree is finite. Same for the quote branch above (keeping the inheritFrom theme it already passes).

@mike-doctorina

Copy link
Copy Markdown
Collaborator

Follow-up to the review above: I've carved the two unrelated features out into their own branches so this PR can stay about selection chrome. Both are off master, self-contained, and green.

Nothing else was touched, and there is no dependency between the two or on this PR — they overlap only in CHANGELOG.md and docs/rendering.md, in different sections.

Suggested order: land #28 and #29 first (both small, ~10 minutes of reading each), then rebase this PR on master. After that there is nothing to dedupe here — I've already removed both features and their changelog sections from my working branch, and the selection-only suite sits at 1222 tests.

Note on #29: font resolution is not observable in flutter_test at all, so its tests pin the family string we pass, not that Menlo / Consolas resolve. It wants an eyeball on a real iOS / macOS / Windows build before merge — which is the main reason it should not ride along here.

The eight blockers from the review are fixed with regression tests on a branch on my side; say the word and I'll push it as commits on top of feat/selection-chrome or open it as a PR against your branch, whichever you prefer.

@mike-doctorina

Copy link
Copy Markdown
Collaborator

@itsezlife — rather than have you cycle on the review above, I've taken the work the rest of the way in our own repo so it isn't blocked on a fork branch. Your commits are preserved verbatim at the base of each branch; the review fixes sit on top.

Three PRs against master, all green:

PR What
#28 clampMarkdownToLines / MarkdownLineClamp — split out, nothing in the package calls it
#29 Monospace family — split out, platform-aware now, plus a theme knob so the face is actually replaceable
#30 Everything else: the selection chrome parity, host gates, registry fixes, and the autoscroll rework

#30 carries the eight fixes from the review, each with a regression test that failed beforehand, plus the autoscroll rework so it can drive a non-Scrollable viewport. 1222 tests, analyzer and formatter clean, example web build passes.

No action needed from you — this is just so the work lands. Happy to keep this PR open for discussion, or close it in favour of #30, whichever you prefer. Thanks for the work, the SelectionArea parity and the registry reasoning were the right calls.

@itsezlife

itsezlife commented Sep 14, 2026 •

Copy link
Copy Markdown
Author

clampMarkdownToLines I've added for my domain application to implement feature expand/collapse markdown, so that's why I've put it here

mike-doctorina added a commit that referenced this pull request Sep 14, 2026
* 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 #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>
Describe surface-bounds gesture starts and empty-click dismiss in plain
terms so the package history stays host-agnostic.
@itsezlife

Copy link
Copy Markdown
Author

Closing in favor of #30.

Same selection-chrome work lives there, on a branch under this repo so review isn't blocked on the fork. Mike's review follow-ups are on top (the eight defects, the autoscroll target protocol), and line clamp / monospace moved to #28 and #29.

Credit and the original commits are preserved on that branch. Thanks for carrying it the rest of the way.

@itsezlife itsezlife closed this Sep 17, 2026
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants