diff --git a/lib/flutter_math.dart b/lib/flutter_math.dart index cc757a85..0fce1855 100644 --- a/lib/flutter_math.dart +++ b/lib/flutter_math.dart @@ -11,4 +11,5 @@ export 'src/parser/tex/parse_error.dart'; export 'src/parser/tex/settings.dart'; export 'src/widgets/exception.dart'; export 'src/widgets/math.dart'; +export 'src/widgets/controller.dart' show MathController; export 'src/widgets/selectable.dart' show SelectableMath; diff --git a/lib/src/ast/syntax_tree.dart b/lib/src/ast/syntax_tree.dart index 3491b637..b5b91496 100644 --- a/lib/src/ast/syntax_tree.dart +++ b/lib/src/ast/syntax_tree.dart @@ -53,10 +53,54 @@ class SyntaxTree { 'The replaced node is not the root of this tree but has no parent'); } return replaceNode( - posParent, - posParent.value.updateChildren(posParent.children - .map((child) => identical(child, pos) ? newNode : child?.value) - .toList(growable: false))); + posParent, _rebuildParent(posParent, pos, newNode)); + } + + /// Build a new green node equivalent to [posParent].value but with [pos]'s + /// green node replaced by [newNode]. Dispatches on the concrete parent type + /// because every subclass of [GreenNode] narrows the type of the list + /// argument passed to [updateChildren]: + /// - [EquationRowNode] → List (non-null) + /// - [FracNode] & friends → List (typed, non-null) + /// - [MultiscriptsNode] & friends → List (nullable) + /// + /// A naive `posParent.value.updateChildren(children.map(...).toList())` + /// builds a `List` which fails every covariant check at runtime. + /// We materialise the correctly-typed list here instead. + static GreenNode _rebuildParent( + SyntaxNode posParent, SyntaxNode pos, GreenNode newNode) { + final parentValue = posParent.value; + final rawChildren = posParent.children; + + if (parentValue is EquationRowNode) { + final newChildren = [ + for (final child in rawChildren) + identical(child, pos) ? newNode : child!.value, + ]; + return parentValue.updateChildren(newChildren); + } + + // Every other non-leaf parent type expects EquationRowNode children + // (either nullable or non-nullable). Build the correctly-typed list. + final nullableChildren = []; + var hasNulls = false; + for (final child in rawChildren) { + if (child == null) { + nullableChildren.add(null); + hasNulls = true; + } else if (identical(child, pos)) { + // newNode has static type GreenNode (non-null), so no need to set + // hasNulls here; the cast validates the element type. + nullableChildren.add(newNode as EquationRowNode?); + } else { + nullableChildren.add(child.value as EquationRowNode); + } + } + if (!hasNulls) { + return parentValue + .updateChildren(nullableChildren.cast()); + } + return parentValue.updateChildren(nullableChildren); } List findNodesAtPosition(int position) { diff --git a/lib/src/render/layout/line_editable.dart b/lib/src/render/layout/line_editable.dart index c9b1a3f4..4a593eec 100644 --- a/lib/src/render/layout/line_editable.dart +++ b/lib/src/render/layout/line_editable.dart @@ -308,6 +308,23 @@ class RenderEditableLine extends RenderLine { double preferredLineHeight; + /// Effective baseline used for cursor positioning and for reporting + /// this row's distance-to-baseline to ancestors (e.g. [Baseline] + /// widgets). Empty rows have no natural baseline (no children → + /// `maxHeightAboveBaseline` is 0), so we fall back to ~80% of + /// [preferredLineHeight] — roughly where a typical font's alphabetic + /// baseline sits. This keeps the caret stable across empty↔non-empty + /// transitions when the row is inside a [Baseline] widget. + double get _effectiveBaselineY => maxHeightAboveBaseline > 0 + ? maxHeightAboveBaseline + : preferredLineHeight * 0.8; + + @override + double computeDistanceToActualBaseline(TextBaseline baseline) { + assert(!debugNeedsLayout); + return _effectiveBaselineY; + } + TextSelection get selection => _selection; TextSelection _selection; set selection(TextSelection value) { @@ -447,7 +464,13 @@ class RenderEditableLine extends RenderLine { if (showCursor && _selection.isCollapsed && isSelectionInRange) { final cursorOffset = caretOffsets[selection.baseOffset]; - _paintCaret(context.canvas, Offset(cursorOffset, size.height) + offset); + // Anchor the caret at the alphabetic baseline (reported via + // [computeDistanceToActualBaseline]), not the bottom of the render + // box. For rows containing superscripts / fractions, size.height is + // much greater than the baseline, and using it pushes the caret + // visually below the main text. + _paintCaret( + context.canvas, Offset(cursorOffset, _effectiveBaselineY) + offset); } if (!_paintCursorAboveText) { diff --git a/lib/src/widgets/controller.dart b/lib/src/widgets/controller.dart index 6bff90c4..76c37c75 100644 --- a/lib/src/widgets/controller.dart +++ b/lib/src/widgets/controller.dart @@ -14,7 +14,10 @@ class MathController extends ChangeNotifier { set ast(SyntaxTree value) { if (_ast != value) { _ast = value; - _selection = const TextSelection.collapsed(offset: -1); + // Preserve the selection across tree mutations, clamped to the new + // tree's valid range. Required for editable-math widgets that need + // the caret to survive insertions/deletions. + _selection = sanitizeSelection(_ast, _selection); notifyListeners(); } } diff --git a/lib/src/widgets/selectable.dart b/lib/src/widgets/selectable.dart index a23f5e94..9087391a 100644 --- a/lib/src/widgets/selectable.dart +++ b/lib/src/widgets/selectable.dart @@ -41,6 +41,7 @@ class SelectableMath extends StatelessWidget { const SelectableMath({ Key? key, this.ast, + this.controller, this.autofocus = false, this.cursorColor, this.cursorRadius, @@ -72,6 +73,12 @@ class SelectableMath extends StatelessWidget { /// It can be null only when [parseException] is not null. final SyntaxTree? ast; + /// Optional externally-owned [MathController]. When non-null, the widget + /// uses this controller instead of creating its own. Lets callers drive + /// the selection (cursor position) from outside — required for editable + /// math widgets that mutate the tree at the cursor. + final MathController? controller; + /// {@macro flutter.widgets.editableText.autofocus} final bool autofocus; @@ -308,6 +315,7 @@ class SelectableMath extends StatelessWidget { return RepaintBoundary( child: InternalSelectableMath( ast: ast!, + controller: controller, autofocus: autofocus, cursorColor: cursorColor, cursorOffset: cursorOffset, @@ -339,6 +347,7 @@ class InternalSelectableMath extends StatefulWidget { const InternalSelectableMath({ Key? key, required this.ast, + this.controller, this.autofocus = false, required this.cursorColor, this.cursorOffset, @@ -361,6 +370,9 @@ class InternalSelectableMath extends StatefulWidget { final SyntaxTree ast; + /// Optional externally-owned [MathController]. See [SelectableMath.controller]. + final MathController? controller; + final bool autofocus; final Color cursorColor; @@ -424,20 +436,44 @@ class InternalSelectableMathState extends State DragStartBehavior get dragStartBehavior => widget.dragStartBehavior; late MathController controller; + bool _ownsController = true; late FocusNode _oldFocusNode; @override void initState() { - controller = MathController(ast: widget.ast); + if (widget.controller != null) { + controller = widget.controller!; + _ownsController = false; + } else { + controller = MathController(ast: widget.ast); + _ownsController = true; + } _oldFocusNode = focusNode..addListener(updateKeepAlive); super.initState(); } @override void didUpdateWidget(InternalSelectableMath oldWidget) { - if (widget.ast != controller.ast) { - controller = MathController(ast: widget.ast); + if (widget.controller != oldWidget.controller) { + // Dispose the previously-owned controller before swapping, otherwise + // the instance created in initState (or the last didUpdateWidget) + // leaks its listeners and internal state. + if (_ownsController) { + controller.dispose(); + } + if (widget.controller != null) { + controller = widget.controller!; + _ownsController = false; + } else { + controller = MathController(ast: widget.ast); + _ownsController = true; + } + } else if (_ownsController && widget.ast != controller.ast) { + // We own the controller — update the ast on the existing instance so + // the selection is preserved (via MathController.ast setter, which + // now sanitizes the selection against the new tree instead of resetting). + controller.ast = widget.ast; } if (_oldFocusNode != focusNode) { _oldFocusNode.removeListener(updateKeepAlive); @@ -465,7 +501,9 @@ class InternalSelectableMathState extends State void dispose() { _oldFocusNode.removeListener(updateKeepAlive); super.dispose(); - controller.dispose(); + if (_ownsController) { + controller.dispose(); + } } void onSelectionChanged( @@ -512,6 +550,7 @@ class InternalSelectableMathState extends State cursorHeight: widget.cursorHeight, selectionColor: widget.selectionColor, paintCursorAboveText: widget.paintCursorAboveText, + showCursor: widget.showCursor, ), ), Provider.value( diff --git a/test/replace_node_test.dart b/test/replace_node_test.dart new file mode 100644 index 00000000..1213a8e3 --- /dev/null +++ b/test/replace_node_test.dart @@ -0,0 +1,103 @@ +// Regression tests for SyntaxTree.replaceNode. +// +// Before the fix, replaceNode built a `List` and passed it to +// the concrete overrides of `updateChildren`, which narrow the parameter +// type in every subclass (EquationRowNode wants List, FracNode +// wants List, etc.). This produced a runtime _TypeError +// on virtually any in-tree mutation. These tests cover the three parent +// shapes exhaustively: an EquationRowNode parent, a FracNode parent, and a +// MultiscriptsNode parent (nullable-slot case). + +import 'package:flutter_math_fork/ast.dart'; +import 'package:flutter_math_fork/tex.dart'; +import 'package:flutter_test/flutter_test.dart'; + +SyntaxTree _parse(String tex) => + SyntaxTree(greenRoot: TexParser(tex, const TexParserSettings()).parse()); + +void main() { + group('SyntaxTree.replaceNode', () { + test('replaces a direct child of EquationRowNode', () { + final tree = _parse(r'x + y'); + final root = tree.root; + // Find the first non-null red child — on `x + y` it will be the + // SymbolNode for `x`. + final firstChild = root.children.firstWhere((c) => c != null)!; + expect(firstChild.value, isA()); + + final replacement = SymbolNode(symbol: 'z'); + final updated = tree.replaceNode(firstChild, replacement); + + // The root row should still have the same number of children, + // with the first one replaced. + expect(updated.greenRoot, isA()); + final newFirst = updated.greenRoot.children.first; + expect(newFirst, isA()); + expect((newFirst as SymbolNode).symbol, 'z'); + expect(updated.greenRoot.children.length, + tree.greenRoot.children.length); + }); + + test('replaces the numerator of a FracNode', () { + final tree = _parse(r'\frac{1}{2}'); + // Walk the red tree: root -> FracNode -> numerator (EquationRowNode) + final fracRed = tree.root.children.firstWhere((c) => c != null)!; + expect(fracRed.value, isA()); + final numeratorRed = fracRed.children.firstWhere((c) => c != null)!; + expect(numeratorRed.value, isA()); + + final replacement = EquationRowNode( + children: [SymbolNode(symbol: '9')], + ); + final updated = tree.replaceNode(numeratorRed, replacement); + + final newFrac = updated.greenRoot.children.first as FracNode; + final newNumLeaf = newFrac.numerator.children.first as SymbolNode; + expect(newNumLeaf.symbol, '9'); + // Denominator must be untouched. + final denomLeaf = newFrac.denominator.children.first as SymbolNode; + expect(denomLeaf.symbol, '2'); + }); + + test('replaces the superscript slot of a MultiscriptsNode (nullable)', () { + final tree = _parse(r'x^{2}'); + final multiRed = tree.root.children.firstWhere((c) => c != null)!; + expect(multiRed.value, isA()); + // Children of MultiscriptsNode are [base, sub, sup, presub, presup] — + // many of which are null. The non-null ones here should be base & sup. + final nonNullChildren = + multiRed.children.whereType().toList(); + expect(nonNullChildren.length, greaterThanOrEqualTo(2)); + + // Locate the sup slot — it's the one whose green value contains the `2`. + final supRed = nonNullChildren.firstWhere((c) { + final row = c.value as EquationRowNode; + return row.children.any( + (x) => x is SymbolNode && x.symbol == '2'); + }); + + final replacement = EquationRowNode( + children: [SymbolNode(symbol: '7')], + ); + final updated = tree.replaceNode(supRed, replacement); + + final newMulti = updated.greenRoot.children.first as MultiscriptsNode; + expect(newMulti.sup, isNotNull); + final newSupLeaf = newMulti.sup!.children.first as SymbolNode; + expect(newSupLeaf.symbol, '7'); + // Base is untouched. + final newBaseLeaf = newMulti.base.children.first as SymbolNode; + expect(newBaseLeaf.symbol, 'x'); + }); + + test('replacing the root wraps in an EquationRowNode', () { + final tree = _parse(r'\frac{1}{2}'); + final replacement = SymbolNode(symbol: 'q'); + final updated = tree.replaceNode(tree.root, replacement); + // A LeafNode replacement at the root must be wrapped. + expect(updated.greenRoot, isA()); + expect(updated.greenRoot.children.length, 1); + expect((updated.greenRoot.children.first as SymbolNode).symbol, 'q'); + }); + }); +}