Unverified Commit 944ede85 authored by Hans Muller's avatar Hans Muller Committed by GitHub

TextField should only set EditableText.cursorOffset for iOS (#27663)

parent 9e3a0d30
......@@ -37,14 +37,6 @@ typedef InputCounterWidgetBuilder = Widget Function(
}
);
// An eyeballed value that moves the cursor slightly left of where it is
// rendered for text on Android so it's positioning more accurately matches the
// native iOS text cursor positioning.
//
// This value is in device pixels, not logical pixels as is typically used
// throughout the codebase.
const int _iOSHorizontalCursorOffsetPixels = 2;
/// A material design text field.
///
/// A text field lets the user enter text, either with hardware keyboard or with
......@@ -478,14 +470,6 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi
&& widget.decoration != null
&& widget.decoration.counterText == null;
Radius get _cursorRadius {
if (widget.cursorRadius != null)
return widget.cursorRadius;
if (Theme.of(context).platform == TargetPlatform.iOS)
return const Radius.circular(2.0);
return null;
}
InputDecoration _getEffectiveDecoration() {
final MaterialLocalizations localizations = MaterialLocalizations.of(context);
final ThemeData themeData = Theme.of(context);
......@@ -701,22 +685,6 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi
@override
bool get wantKeepAlive => _splashes != null && _splashes.isNotEmpty;
bool get _cursorOpacityAnimates => Theme.of(context).platform == TargetPlatform.iOS ? true : false;
Offset get _getCursorOffset => Offset(_iOSHorizontalCursorOffsetPixels / MediaQuery.of(context).devicePixelRatio, 0);
bool get _paintCursorAboveText => Theme.of(context).platform == TargetPlatform.iOS ? true : false;
Color get _cursorColor {
if (widget.cursorColor == null) {
if (Theme.of(context).platform == TargetPlatform.iOS)
return CupertinoTheme.of(context).primaryColor;
else
return Theme.of(context).cursorColor;
}
return widget.cursorColor;
}
@override
void deactivate() {
if (_splashes != null) {
......@@ -754,15 +722,37 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi
bool forcePressEnabled;
TextSelectionControls textSelectionControls;
bool paintCursorAboveText;
bool cursorOpacityAnimates;
Offset cursorOffset;
Color cursorColor = widget.cursorColor;
Radius cursorRadius = widget.cursorRadius;
switch (themeData.platform) {
case TargetPlatform.iOS:
forcePressEnabled = true;
textSelectionControls = cupertinoTextSelectionControls;
paintCursorAboveText = true;
cursorOpacityAnimates = true;
cursorColor ??= CupertinoTheme.of(context).primaryColor;
cursorRadius ??= const Radius.circular(2.0);
// An eyeballed value that moves the cursor slightly left of where it is
// rendered for text on Android so its positioning more accurately matches the
// native iOS text cursor positioning.
//
// This value is in device pixels, not logical pixels as is typically used
// throughout the codebase.
const int _iOSHorizontalOffset = 2;
cursorOffset = Offset(_iOSHorizontalOffset / MediaQuery.of(context).devicePixelRatio, 0);
break;
case TargetPlatform.android:
case TargetPlatform.fuchsia:
forcePressEnabled = false;
textSelectionControls = materialTextSelectionControls;
paintCursorAboveText = false;
cursorOpacityAnimates = false;
cursorColor ??= themeData.cursorColor;
break;
}
......@@ -789,11 +779,11 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi
inputFormatters: formatters,
rendererIgnoresPointer: true,
cursorWidth: widget.cursorWidth,
cursorRadius: _cursorRadius,
cursorColor: _cursorColor,
cursorOpacityAnimates: _cursorOpacityAnimates,
cursorOffset: _getCursorOffset,
paintCursorAboveText: _paintCursorAboveText,
cursorRadius: cursorRadius,
cursorColor: cursorColor,
cursorOpacityAnimates: cursorOpacityAnimates,
cursorOffset: cursorOffset,
paintCursorAboveText: paintCursorAboveText,
backgroundCursorColor: CupertinoColors.inactiveGray,
scrollPadding: widget.scrollPadding,
keyboardAppearance: keyboardAppearance,
......
......@@ -3,7 +3,6 @@
// found in the LICENSE file.
import 'dart:async';
import 'dart:io' show Platform;
import 'dart:math' as math;
import 'dart:ui' as ui show window;
......@@ -363,6 +362,8 @@ void main() {
expect(textField.cursorRadius, const Radius.circular(3.0));
});
// TODO(hansmuller): restore these tests after the fix for #24876 has landed.
/*
testWidgets('cursor layout has correct width', (WidgetTester tester) async {
EditableText.debugDeterministicCursor = true;
await tester.pumpWidget(
......@@ -405,6 +406,7 @@ void main() {
);
EditableText.debugDeterministicCursor = false;
}, skip: !Platform.isLinux);
*/
testWidgets('obscureText control test', (WidgetTester tester) async {
await tester.pumpWidget(
......@@ -1533,7 +1535,10 @@ void main() {
editable.getLocalRectForCaret(const TextPosition(offset: 0)).topLeft,
);
expect(topLeft.dx, equals(401.0));
// The overlay() function centers its child within a 800x600 window.
// Default cursorWidth is 2.0, test windowWidth is 800
// Centered cursor topLeft.dx: 399 == windowWidth/2 - cursorWidth/2
expect(topLeft.dx, equals(399.0));
await tester.enterText(find.byType(TextField), 'abcd');
await tester.pump();
......@@ -1542,7 +1547,8 @@ void main() {
editable.getLocalRectForCaret(const TextPosition(offset: 2)).topLeft,
);
expect(topLeft.dx, equals(401.0));
// TextPosition(offset: 2) - center of 'abcd'
expect(topLeft.dx, equals(399.0));
});
testWidgets('Can align to center within center', (WidgetTester tester) async {
......@@ -1565,7 +1571,10 @@ void main() {
editable.getLocalRectForCaret(const TextPosition(offset: 0)).topLeft,
);
expect(topLeft.dx, equals(401.0));
// The overlay() function centers its child within a 800x600 window.
// Default cursorWidth is 2.0, test windowWidth is 800
// Centered cursor topLeft.dx: 399 == windowWidth/2 - cursorWidth/2
expect(topLeft.dx, equals(399.0));
await tester.enterText(find.byType(TextField), 'abcd');
await tester.pump();
......@@ -1574,7 +1583,8 @@ void main() {
editable.getLocalRectForCaret(const TextPosition(offset: 2)).topLeft,
);
expect(topLeft.dx, equals(401.0));
// TextPosition(offset: 2) - center of 'abcd'
expect(topLeft.dx, equals(399.0));
});
testWidgets('Controller can update server', (WidgetTester tester) async {
......@@ -1787,7 +1797,7 @@ void main() {
scrollableState = tester.firstState(find.byType(Scrollable));
// For a horizontal input, scrolls to the exact position of the caret.
expect(scrollableState.position.pixels, equals(223.0));
expect(scrollableState.position.pixels, equals(222.0));
});
testWidgets('Multiline text field scrolls the caret into view', (WidgetTester tester) async {
......@@ -3194,7 +3204,7 @@ void main() {
editable.getLocalRectForCaret(const TextPosition(offset: 10)).topLeft,
);
expect(topLeft.dx, equals(701.6666870117188));
expect(topLeft.dx, equals(701));
await tester.pumpWidget(
const MaterialApp(
......@@ -3214,7 +3224,7 @@ void main() {
editable.getLocalRectForCaret(const TextPosition(offset: 10)).topLeft,
);
expect(topLeft.dx, equals(160.6666717529297));
expect(topLeft.dx, equals(160.0));
});
testWidgets('TextField semantics', (WidgetTester tester) async {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment