From 944ede85ff8fa5800aaa8b5adf3f31e18ffb7dcc Mon Sep 17 00:00:00 2001 From: Hans Muller Date: Fri, 8 Feb 2019 14:31:35 -0800 Subject: [PATCH] TextField should only set EditableText.cursorOffset for iOS (#27663) --- .../flutter/lib/src/material/text_field.dart | 64 ++++++++----------- .../test/material/text_field_test.dart | 26 +++++--- 2 files changed, 45 insertions(+), 45 deletions(-) diff --git a/packages/flutter/lib/src/material/text_field.dart b/packages/flutter/lib/src/material/text_field.dart index e0e0e969d7..a13dcd7f6c 100644 --- a/packages/flutter/lib/src/material/text_field.dart +++ b/packages/flutter/lib/src/material/text_field.dart @@ -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 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 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 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 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, diff --git a/packages/flutter/test/material/text_field_test.dart b/packages/flutter/test/material/text_field_test.dart index c762974020..caff3de270 100644 --- a/packages/flutter/test/material/text_field_test.dart +++ b/packages/flutter/test/material/text_field_test.dart @@ -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 {