Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds a fade in and out, rounds corners, fixes offset and fixes height of cursor on iOS #24876

Merged
merged 63 commits into from Jan 29, 2019

Conversation

jslavitz
Copy link
Contributor

@jslavitz jslavitz commented Nov 30, 2018

Fixes #22879.

@zoechi zoechi added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. labels Nov 30, 2018
Copy link
Member

@xster xster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work. General feedback is move aesthetic things out of widgets and rendering layers.

packages/flutter/lib/src/rendering/editable.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/rendering/editable.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/widgets/editable_text.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/widgets/editable_text.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/widgets/editable_text.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/widgets/editable_text.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/widgets/editable_text.dart Outdated Show resolved Hide resolved
@jslavitz jslavitz changed the title Adds a fade in and out as well as rounds corners of cursor on iOS Adds a fade in and out, rounds corners, fixes offset and fixes height of cursor on iOS Dec 1, 2018
@zoechi zoechi added the f: cupertino flutter/packages/flutter/cupertino repository label Dec 5, 2018
@xster
Copy link
Member

xster commented Dec 7, 2018

Let me know if you're ready for another look

@jslavitz
Copy link
Contributor Author

jslavitz commented Dec 13, 2018

Waiting on #24265 to land before finishing this.

@zoechi zoechi added the f: material design flutter/packages/flutter/material repository. label Jan 20, 2019
@xster
Copy link
Member

xster commented Jan 22, 2019

LGTM. cc @HansMuller FYI

@@ -1283,15 +1351,30 @@ class RenderEditable extends RenderBox {
offset.applyContentDimensions(0.0, _maxScrollExtent);
}

Offset _getPixelPerfectCursorOffset(Rect caretRect) {
final Offset caretPosition = localToGlobal(caretRect.topLeft);
final double pixelMultiple = 1.0 / _devicePixelRatio;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these divisions are a bit hard to read. Why not just

final Offset logicalCaretPosition = localToGlobal(caretRect.topLeft);
final int physicalCaretX = (caretPosition.dx * _devicePixelRatio).round();
final int physicalCaretY = (caretPosition.dy * _devicePixelRatio).round();
final double pixelPerfectLogicalX = globalToLocal(physicalCaretX / _devicePixelRatio); // I'm not sure what the - caretPosition.dx was for
final double pixelPerfectLogicalY = globalToLocal(physicalCaretY / _devicePixelRatio);

Otherwise, aliasing around the edge seems like 'working as intended'. Just have your test not land on .499999999-.5.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm just going to leave this logic as is? It's supposed to give the delta logical pixels to move the rect to be physically pixel perfectly aligned. So that logic isn't really going to work.

@@ -682,6 +699,22 @@ 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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be used unconditionally. It should only be used if the platform is iOS.

///
/// Defaults to a super ellipse with
// final ShapeBorder shape;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jslavitz looks like this bit was checked in by mistake -- please take a moment to clean it. Thanks!

kangwang1988 pushed a commit to XianyuTech/flutter that referenced this pull request Feb 12, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: text input Entering text in a text field or keyboard related problems f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

iOS caret fidelity
6 participants