Skip to content

feat(selection): SelectionArea-parity chrome, host gates, and protocol-agnostic autoscroll - #30

Open
mike-doctorina wants to merge 19 commits into
masterfrom
feat/selection-chrome-parity
Open

mike-doctorina wants to merge 19 commits into
masterfrom
feat/selection-chrome-parity

Conversation

@mike-doctorina

Copy link
Copy Markdown
Collaborator

Supersedes #27 — same work, reviewed, with the defects fixed and the two unrelated features carved off into #28 and #29. Opened here so it is not blocked on a fork branch.

Credit: the feature work is @itsezlife's (9 commits, preserved verbatim at the base of this branch). Everything on top is review follow-up.

What it does

Brings the controller-driven selection path up to Flutter SelectionArea / SelectableRegion quality without wrapping markdown in a SelectionArea:

  • Chrome parity — soft-wrap affinity on handles, directed base/extent edges with reversed handle types, coincident-caret separation, handle proxies when an endpoint surface unmounts, native SelectionOverlay handles + magnifier on touch.
  • Desktop parity — mouse drags, double/triple clicks, Cmd/Ctrl+A and programmatic selection no longer pop the toolbar; right-click opens the menu without mutating the selection; the menu keeps its right-click anchor and dismisses on ancestor scroll.
  • Host gates for chat mounts — enableTouchGestures, enableTouchConsecutiveTaps, ownsSelectionChrome, canStartSelectionAt.
  • Glyph-tight hit testing — hover I-beam and link hits use rendered line boxes, not the full max-width layout; gesture starts stay surface-bounds.
  • Highlight paint — stays outside the glyph Picture cache, under the glyphs by default, with a second pass above opaque chrome (code fences, table zebra rows, inline monospace).
  • Registry — removeDocument defers while a surface is mounted; rangeFor and ordering ignore unregistered ids instead of treating them as index -1.
  • Fenced code inside quotes / alerts — re-parsed as nested MD$Code in MD$Quote.blocks / MD$Alert.blocks instead of being eaten as inline monospace by backtick pairing.
  • Edge autoscroll while dragging near the viewport edge.

What changed versus #27

Eight defects, each with a regression test that failed before the fix:

  1. Registry corruption on element recycling — updateRenderObject pushed the new model before rewiring the id, so a ListView.builder reusing a slot wrote message B's body under message A's id. Silent, and it hits exactly the virtualized-chat case this is built for.
  2. setState() called during build — onSelectionChanged fired synchronously from inside build/layout on two ordinary paths (a streaming putDocument from updateRenderObject, a deferred removeDocument flushing from RenderObject.detach). Delivery is now coalesced to the end of the frame in those phases.
  3. canStartSelectionAt refusal left the drag armed — a long press on refused chrome kept extending the active selection from that point.
  4. Heal-registration jumped to the front of the conversation — putDocument with no order used _docs.length, which sorts ahead of sparse explicit orders.
  5. Unstable order tie-break — List.sort is not stable, so documents sharing an order could swap on any re-sort and reverse extracted text.
  6. Glyph boxes recomputed on every hover event — now cached per TextPainter layout.
  7. maxVelocity: 0 spun the frame ticker forever.
  8. Nested quote/alert children bypassed MarkdownThemeData.builder — a host that replaces the code painter lost it inside > ….

Autoscroll no longer speaks the sliver protocol

The band / host-union / arming math ran against ScrollableState and ScrollPosition.jumpTo, so it could not drive our own chat viewport, which owns an anchor offset and has no Scrollable in the tree at all. It now runs against a MarkdownAutoscrollTarget:

Member Meaning
MarkdownAutoscrollViewport? get viewport visible global bounds + the inset where the bands start
bool canScroll({required bool forward}) can a positive / negative delta still move content
double applyScrollDelta(double delta) move content by screen-space pixels; return what was applied (0 hard-stops and disarms)

MarkdownScrollableAutoscrollTarget keeps today's behaviour and fixes reverse: true lists (AxisDirection.up), which most chats use and the old sign handling got wrong. MarkdownSelectionAutoscrollConfig.targetResolver + MarkdownCallbackAutoscrollTarget cover everything else, and useHostUnionGate: false exists because the union gate assumes a cache extent — a viewport that builds only what is visible would have its own paging vetoed.

The surface is also resolved once per drag now, instead of two full element-subtree walks per frame.

Coverage

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; incremental matches batch on every prefix across all six chunkings.

1080 → 1222 tests (after moving 27 of them to #28 / #29). dart analyze --fatal-infos --fatal-warnings lib/ test/ clean, dart format clean, flutter build web on the example passes.

Breaking vs 0.2.0

Written out in the changelog. Source-breaking only for hosts that implement the render / selection extension points:

  • removed BlockPainter$Quote.painter, BlockPainter$Alert.bodyPainter;
  • both painters now mix in MultiPainterSelectable instead of SelectableTextBlock;
  • SelectableBlockPainter gained 4 members, MarkdownSelectionSurface gained 7 (the mixins cover anyone using them; direct implementers must add them);
  • MarkdownThemeData.copyWith returns MarkdownThemeData instead of the base ThemeExtension.

Please eyeball on a device before merge

Genuinely unverifiable in widget tests:

  • Handle chrome — affinity, directed edges, coincident-caret separation, proxies, multi-scope LayerLink ownership. The most intricate code here, 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 and friends are pinned numerically but nothing checks the menu stays on screen.
  • Autoscroll feel — the quadratic ramp and the edgeZone / maxVelocity defaults.

Targets: iOS, Android, mobile web, desktop web.

itsezlife and others added 19 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>
…te fixes

Review follow-up on #27.

Autoscroll no longer speaks the sliver protocol. The band / host-union / arming
math runs against a `MarkdownAutoscrollTarget` (`viewport`, `canScroll`,
`applyScrollDelta`) in screen-space deltas; `MarkdownScrollableAutoscrollTarget`
is the built-in `Scrollable` adapter and
`MarkdownSelectionAutoscrollConfig.targetResolver` +
`MarkdownCallbackAutoscrollTarget` let a host with its own scroll
implementation (an anchored chat viewport, a RenderBox that positions children
itself) be driven without a `Scrollable` in the tree. The surface is now
resolved once per drag instead of walking the element subtree twice per frame.

Fixes found while reviewing:

- Recycling a `MarkdownWidget` element onto another `documentId` (a virtualized
  chat list reusing a slot) overwrote the outgoing document's registry model
  with the incoming body.
- `putDocument` without an explicit order sorted ahead of sparse explicit
  orders, so a heal-registered body jumped to the front of the conversation.
- Documents sharing an order could swap places on any re-sort (`List.sort` is
  not stable), silently reversing extracted text.
- `canStartSelectionAt` refusal left the drag armed: a long press / drag on
  refused chrome kept extending the active selection from that point.
- Glyph boxes are cached per `TextPainter` layout instead of recomputed on
  every hover event.
- `maxVelocity: 0` past the outer band reported `scrolled` forever, spinning the
  frame ticker on a drag that could never scroll.

Adds 69 tests across autoscroll targets, host gates, registry lifecycle,
selection chrome and nested quote/alert bodies, plus the missing breaking-change
notes vs 0.2.0 in the changelog.
…yout

The controller notifies from the build/layout phase on two ordinary chat paths:
a streaming `putDocument` runs from `MarkdownWidget.updateRenderObject` and can
reconcile the range away, and a deferred `removeDocument` flushes from
`RenderObject.detach`. Hosts routinely `setState` from `onSelectionChanged`, so
both threw "setState() or markNeedsBuild() called during build".

Delivery is coalesced to the end of the frame while in `persistentCallbacks` /
`midFrameMicrotasks`, and reports the settled selection.

Also renames the getter behind `alwaysNeedsCompositing` off the `debug` prefix —
it is production logic, not a debug aid.
The gate assumes the mounted bodies extend past the viewport whenever there is
more to reveal — true for a ListView with a cache extent, false for a chat
viewport that builds only what is visible. There the union is barely larger
than the viewport, so the gate vetoes a drag that should keep paging through
history.

`useHostUnionGate: false` hands the decision to the scroll target's `canScroll`
and the delta it reports applying. Default stays on.

Also documents the host gates and the autoscroll target in the README.
Table rows, a quote with a nested fence, a negative line budget and a zero
measuring width for clampMarkdownToLines; controller dispose after unmount,
group exclusivity, moveSelectionEdgeToGlobal with nothing mounted, and
selectAll over an empty leading document.
A host that replaces a block painter through MarkdownThemeData.builder was
silently getting the default painter for blocks nested in a quote or alert —
so a custom code fence (copy button, line numbers) disappeared inside `> …`.
PR #27 replaced the CSS generic 'monospace' with a hardcoded Menlo-first stack
for every target, inside a commit about hit-testing and with nothing in the PR
description or changelog. The premise is right on two platforms and wrong on
the rest: CoreText (iOS / macOS) and DirectWrite (Windows) do not know
'monospace', so inline code silently rendered in the proportional body face
there — but Android's font config and fontconfig on Linux resolve it natively,
and Menlo/Monaco/Consolas exist on none of them.

Substitute a real face on exactly the two platforms that need it (Menlo,
Consolas) and keep 'monospace' 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 byte-identical to 0.2.0.
kMonospaceFontFamily picked a sane per-platform default but there was no
supported way to override it. Inline code went through
MarkdownThemeData.textStyleFor, so subclassing the theme moved that half;
fenced blocks did not — BlockPainter$Code built its base style in a static
method reading the global directly, so the only ways out were abusing
SyntaxHighlighter.baseStyleFor to set a font family or reimplementing the whole
fence painter through theme.builder. The two halves could not even be changed
by the same mechanism.

MarkdownThemeData.monospaceFontFamily / monospaceFontFamilyFallback (with
effectiveMonospace… getters) now feed both paths. Null keeps the
platform-resolved default; an empty fallback list means "no fallbacks" rather
than "unset".
The corpus had 'quote', 'quote-multi' and two alerts but no body containing a
fence — the one construct that makes the decoder re-enter itself. The chat
streams straight into it, so every mid-fence prefix now has to match a batch
parse across all six chunkings (whole, per-line, per-word, fixed sizes,
char-by-char, seeded-random): closed, unclosed, tilde, empty body, a nested
quote beside the fence, and the alert form.
Neither has anything to do with selection chrome, and nothing inside the
package calls either of them.

- clampMarkdownToLines / MarkdownLineClamp / MarkdownPainter.textLineBottoms
  → its own branch. Zero callers in lib/ and example/, so the only thing that
  ever exercised the line-counting heuristic was its own unit tests.
- The monospace family → its own branch. It changes rendering for every user
  of the package, it arrived here inside a commit about hit-testing, and font
  resolution is not observable in flutter_test at all, so it needs a real
  device pass that this PR cannot give it.
@itsezlife

itsezlife commented Sep 16, 2026 •

Copy link
Copy Markdown

Two toolbar timing bugs from dogfooding this branch with several MarkdownSelectionScopes on one controller. Landed together as #33 against this tip.

Ownership loss. When selection moves from A to B, scope A cleared handles but left the adaptive toolbar up. _updateHandlesAndOverlay only called _clearHandles() when !_handlesEnabled. _onControllerChanged only refreshed or hid the menu for an owning scope with toolbarWanted, so a non-owner with an already-shown menu never called _contextMenuController.remove().

Fix: if !_ownsChromeForCurrentSelection, remove this scope's context menu without clearing toolbarWanted. Same when handles are disabled for that reason. Regression in selection_host_gates_test.

Mid-drag scroll restore. Expand drag + edge autoscroll could re-show the toolbar before release. Scroll notifications hit the geometry restore path, and toolbarWanted was enough to present the menu. A host that clamps a cross-document wander back onto the subject via the public selection setter re-arms toolbarWanted mid-gesture, so the menu came back early.

Fix: _onScrollAffectingChrome / _refreshToolbarForVisibleGeometry / _scheduleToolbarGeometryRefresh no-op while _dragGlobal is set. Regression in selection_widget_test (expand drag keeps toolbar hidden across scroll even if toolbarWanted is re-armed mid-drag).

I cannot push to DoctorinaAI/md from here. Verified in the chat viewport for both cases. The host also clears toolbarWanted on subject adopt and after a same-document clamp restore so the new scope does not restore mid long-press.

@itsezlife

itsezlife commented Sep 16, 2026 •

Copy link
Copy Markdown

I've verified the new autoscroll works perfectly in the chat environment. Would let you know whether it behaves identically in my domain application with regular text selection inside sliver protocol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants