Skip to content

Commit

Permalink
Remove Editable.onCaretChanged callback (#109114)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tgucio
tgucio committed Jun 27, 2023
1 parent 5ea2be6 commit ca6f23e
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 57 deletions.
45 changes: 10 additions & 35 deletions packages/flutter/lib/src/rendering/editable.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,10 @@ const double _kCaretHeightOffset = 2.0; // pixels

// The additional size on the x and y axis with which to expand the prototype
// cursor to render the floating cursor in pixels.
const EdgeInsets _kFloatingCaretSizeIncrease = EdgeInsets.symmetric(horizontal: 0.5, vertical: 1.0);
const EdgeInsets _kFloatingCursorSizeIncrease = EdgeInsets.symmetric(horizontal: 0.5, vertical: 1.0);

// The corner radius of the floating cursor in pixels.
const Radius _kFloatingCaretRadius = Radius.circular(1.0);

/// Signature for the callback that reports when the caret location changes.
///
/// Used by [RenderEditable.onCaretChanged].
typedef CaretChangedHandler = void Function(Rect caretRect);
const Radius _kFloatingCursorRadius = Radius.circular(1.0);

/// Represents the coordinates of the point in a selection, and the text
/// direction at that point, relative to top left of the [RenderEditable] that
Expand Down Expand Up @@ -260,9 +255,6 @@ class VerticalCaretMovementRun implements Iterator<TextPosition> {
/// position. The cursor is shown while [showCursor] is true. It is painted in
/// the [cursorColor].
///
/// If, when the render object paints, the caret is found to have changed
/// location, [onCaretChanged] is called.
///
/// Keyboard handling, IME handling, scrolling, toggling the [showCursor] value
/// to actually blink the cursor, and other features not mentioned above are the
/// responsibility of higher layers and not handled by this object.
Expand Down Expand Up @@ -299,7 +291,6 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
double textScaleFactor = 1.0,
TextSelection? selection,
required ViewportOffset offset,
this.onCaretChanged,
this.ignorePointer = false,
bool readOnly = false,
bool forceLine = true,
Expand Down Expand Up @@ -474,8 +465,8 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
}

// Caret Painters:
// The floating painter. This painter paints the regular caret as well.
late final _FloatingCursorPainter _caretPainter = _FloatingCursorPainter(_onCaretChanged);
// A single painter for both the regular caret and the floating cursor.
late final _CaretPainter _caretPainter = _CaretPainter();

// Text Highlight painters:
final _TextHighlightPainter _selectionPainter = _TextHighlightPainter();
Expand Down Expand Up @@ -515,19 +506,6 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
);
}

Rect? _lastCaretRect;
// TODO(LongCatIsLooong): currently EditableText uses this callback to keep
// the text field visible. But we don't always paint the caret, for example
// when the selection is not collapsed.
/// Called during the paint phase when the caret location changes.
CaretChangedHandler? onCaretChanged;
void _onCaretChanged(Rect caretRect) {
if (_lastCaretRect != caretRect) {
onCaretChanged?.call(caretRect);
}
_lastCaretRect = onCaretChanged == null ? null : caretRect;
}

/// Whether the [handleEvent] will propagate pointer events to selection
/// handlers.
///
Expand Down Expand Up @@ -2396,8 +2374,8 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
_floatingCursorTextPosition = lastTextPosition;
final double? animationValue = _resetFloatingCursorAnimationValue;
final EdgeInsets sizeAdjustment = animationValue != null
? EdgeInsets.lerp(_kFloatingCaretSizeIncrease, EdgeInsets.zero, animationValue)!
: _kFloatingCaretSizeIncrease;
? EdgeInsets.lerp(_kFloatingCursorSizeIncrease, EdgeInsets.zero, animationValue)!
: _kFloatingCursorSizeIncrease;
_caretPainter.floatingCursorRect = sizeAdjustment.inflateRect(_caretPrototype).shift(boundedOffset);
} else {
_caretPainter.floatingCursorRect = null;
Expand Down Expand Up @@ -2777,8 +2755,8 @@ class _TextHighlightPainter extends RenderEditablePainter {
}
}

class _FloatingCursorPainter extends RenderEditablePainter {
_FloatingCursorPainter(this.caretPaintCallback);
class _CaretPainter extends RenderEditablePainter {
_CaretPainter();

bool get shouldPaint => _shouldPaint;
bool _shouldPaint = true;
Expand All @@ -2790,8 +2768,6 @@ class _FloatingCursorPainter extends RenderEditablePainter {
notifyListeners();
}

CaretChangedHandler caretPaintCallback;

bool showRegularCaret = false;

final Paint caretPaint = Paint();
Expand Down Expand Up @@ -2863,7 +2839,6 @@ class _FloatingCursorPainter extends RenderEditablePainter {
canvas.drawRRect(caretRRect, caretPaint);
}
}
caretPaintCallback(integralRect);
}

@override
Expand Down Expand Up @@ -2897,7 +2872,7 @@ class _FloatingCursorPainter extends RenderEditablePainter {
}

canvas.drawRRect(
RRect.fromRectAndRadius(floatingCursorRect, _kFloatingCaretRadius),
RRect.fromRectAndRadius(floatingCursorRect, _kFloatingCursorRadius),
floatingCursorPaint..color = floatingCursorColor,
);
}
Expand All @@ -2911,7 +2886,7 @@ class _FloatingCursorPainter extends RenderEditablePainter {
if (oldDelegate == null) {
return shouldPaint;
}
return oldDelegate is! _FloatingCursorPainter
return oldDelegate is! _CaretPainter
|| oldDelegate.shouldPaint != shouldPaint
|| oldDelegate.showRegularCaret != showRegularCaret
|| oldDelegate.caretColor != caretColor
Expand Down
36 changes: 16 additions & 20 deletions packages/flutter/lib/src/widgets/editable_text.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2819,17 +2819,17 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
_formatAndSetValue(value, SelectionChangedCause.keyboard);
}

if (_showBlinkingCursor && _cursorTimer != null) {
// To keep the cursor from blinking while typing, restart the timer here.
_stopCursorBlink(resetCharTicks: false);
_startCursorBlink();
}

// Wherever the value is changed by the user, schedule a showCaretOnScreen
// to make sure the user can see the changes they just made. Programmatic
// changes to `textEditingValue` do not trigger the behavior even if the
// text field is focused.
_scheduleShowCaretOnScreen(withAnimation: true);
if (_hasInputConnection) {
// To keep the cursor from blinking while typing, we want to restart the
// cursor timer every time a new character is typed.
_stopCursorBlink(resetCharTicks: false);
_startCursorBlink();
}
}

bool _checkNeedsAdjustAffinity(TextEditingValue value) {
Expand Down Expand Up @@ -3415,18 +3415,12 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
}

// To keep the cursor from blinking while it moves, restart the timer here.
if (_cursorTimer != null) {
if (_showBlinkingCursor && _cursorTimer != null) {
_stopCursorBlink(resetCharTicks: false);
_startCursorBlink();
}
}

Rect? _currentCaretRect;
// ignore: use_setters_to_change_properties, (this is used as a callback, can't be a setter)
void _handleCaretChanged(Rect caretRect) {
_currentCaretRect = caretRect;
}

// Animation configuration for scrolling the caret back on screen.
static const Duration _caretAnimationDuration = Duration(milliseconds: 100);
static const Curve _caretAnimationCurve = Curves.fastOutSlowIn;
Expand All @@ -3440,7 +3434,13 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
_showCaretOnScreenScheduled = true;
SchedulerBinding.instance.addPostFrameCallback((Duration _) {
_showCaretOnScreenScheduled = false;
if (_currentCaretRect == null || !_scrollController.hasClients) {
// Since we are in a post frame callback, check currentContext in case
// RenderEditable has been disposed (in which case it will be null).
final RenderEditable? renderEditable =
_editableKey.currentContext?.findRenderObject() as RenderEditable?;
if (renderEditable == null
|| !(renderEditable.selection?.isValid ?? false)
|| !_scrollController.hasClients) {
return;
}

Expand Down Expand Up @@ -3471,7 +3471,8 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
final EdgeInsets caretPadding = widget.scrollPadding
.copyWith(bottom: bottomSpacing);

final RevealedOffset targetOffset = _getOffsetToRevealCaret(_currentCaretRect!);
final Rect caretRect = renderEditable.getLocalRectForCaret(renderEditable.selection!.extent);
final RevealedOffset targetOffset = _getOffsetToRevealCaret(caretRect);

final Rect rectToReveal;
final TextSelection selection = textEditingValue.selection;
Expand Down Expand Up @@ -4644,7 +4645,6 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
obscuringCharacter: widget.obscuringCharacter,
obscureText: widget.obscureText,
offset: offset,
onCaretChanged: _handleCaretChanged,
rendererIgnoresPointer: widget.rendererIgnoresPointer,
cursorWidth: widget.cursorWidth,
cursorHeight: widget.cursorHeight,
Expand Down Expand Up @@ -4769,7 +4769,6 @@ class _Editable extends MultiChildRenderObjectWidget {
required this.obscuringCharacter,
required this.obscureText,
required this.offset,
this.onCaretChanged,
this.rendererIgnoresPointer = false,
required this.cursorWidth,
this.cursorHeight,
Expand Down Expand Up @@ -4810,7 +4809,6 @@ class _Editable extends MultiChildRenderObjectWidget {
final TextHeightBehavior? textHeightBehavior;
final TextWidthBasis textWidthBasis;
final ViewportOffset offset;
final CaretChangedHandler? onCaretChanged;
final bool rendererIgnoresPointer;
final double cursorWidth;
final double? cursorHeight;
Expand Down Expand Up @@ -4849,7 +4847,6 @@ class _Editable extends MultiChildRenderObjectWidget {
locale: locale ?? Localizations.maybeLocaleOf(context),
selection: value.selection,
offset: offset,
onCaretChanged: onCaretChanged,
ignorePointer: rendererIgnoresPointer,
obscuringCharacter: obscuringCharacter,
obscureText: obscureText,
Expand Down Expand Up @@ -4894,7 +4891,6 @@ class _Editable extends MultiChildRenderObjectWidget {
..locale = locale ?? Localizations.maybeLocaleOf(context)
..selection = value.selection
..offset = offset
..onCaretChanged = onCaretChanged
..ignorePointer = rendererIgnoresPointer
..textHeightBehavior = textHeightBehavior
..textWidthBasis = textWidthBasis
Expand Down
54 changes: 53 additions & 1 deletion packages/flutter/test/widgets/editable_text_show_on_screen_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ void main() {
final ScrollController scrollController = ScrollController();
final TextEditingController controller = TextEditingController();
final FocusNode focusNode = FocusNode();
controller.text = 'Start\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\nEnd';
controller.text = "Start${'\n' * 39}End";

await tester.pumpWidget(MaterialApp(
home: Center(
Expand Down Expand Up @@ -322,6 +322,58 @@ void main() {
expect(scrollController.offset, greaterThan(0.0));
});

testWidgets('focused multi-line editable does not scroll to old position when non-collapsed selection set', (WidgetTester tester) async {
final ScrollController scrollController = ScrollController();
final TextEditingController controller = TextEditingController();
final FocusNode focusNode = FocusNode();
final String text = "Start${'\n' * 39}End";
controller.value = TextEditingValue(text: text, selection: TextSelection.collapsed(offset: text.length - 3));

await tester.pumpWidget(MaterialApp(
home: Center(
child: SizedBox(
height: 300.0,
child: ListView(
controller: scrollController,
children: <Widget>[
EditableText(
backgroundCursorColor: Colors.grey,
maxLines: null, // multiline
controller: controller,
focusNode: focusNode,
style: textStyle,
cursorColor: cursorColor,
),
],
),
),
),
));

// Bring keyboard up and verify that end of EditableText is not on screen.
await tester.showKeyboard(find.byType(EditableText));
await tester.pumpAndSettle();

scrollController.jumpTo(0.0);
await tester.pumpAndSettle();
final RenderBox render = tester.renderObject(find.byType(EditableText));
expect(render.size.height, greaterThan(500.0));
expect(scrollController.offset, 0.0);

// Change selection to non-collapased so that cursor isn't shown
// and the location requires a bit of scroll.
tester.testTextInput.updateEditingValue(TextEditingValue(
text: text,
selection: const TextSelection(baseOffset: 26, extentOffset: 27),
));
await tester.pumpAndSettle();

// Selection extent scrolls into view.
expect(find.byType(EditableText), findsOneWidget);
expect(render.size.height, greaterThan(500.0));
expect(scrollController.offset, 28.0);
});

testWidgets('scrolls into view with scrollInserts after the keyboard pops up', (WidgetTester tester) async {
final ScrollController scrollController = ScrollController();
final TextEditingController controller = TextEditingController();
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/widgets/editable_text_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9652,7 +9652,7 @@ void main() {
expect(tester.takeException(), isNull);

await tester.pumpWidget(Container());
expect(tester.takeException(), isNotNull);
expect(tester.takeException(), isAssertionError);
});
});

Expand Down

0 comments on commit ca6f23e

Please sign in to comment.