Use full height of the glyph for caret height on Android#30991
Merged
GaryQian merged 5 commits intoflutter:masterfrom Apr 16, 2019
Merged
Use full height of the glyph for caret height on Android#30991GaryQian merged 5 commits intoflutter:masterfrom
GaryQian merged 5 commits intoflutter:masterfrom
Conversation
GaryQian
added
a: text input
GaryQian
added a commit
to flutter/goldens
that referenced
this pull request
Apr 12, 2019
justinmc
approved these changes
Apr 15, 2019
Contributor
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
LGTM, just a question about the tests.
Contributor
Author
|
Something else we can do here is to also center the caret vertically instead of scaling off of the top left corner. This allows for adjustment of the caret height, which is not currently possible due to the origin of the caret. |
GaryQian
added a commit
that referenced
this pull request
Apr 16, 2019
)" This reverts commit 96e1fc9.
GaryQian
added a commit
that referenced
this pull request
Apr 16, 2019
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The caret on Android has since drifted from canonical carets on Android (comparing against stock Google apps, and blank Android textfields). Flutter's carets are significantly shorter and are not centered. This problem becomes worse when mixing multiple scripts together such as english+chinese.
This PR changes the non-iOS caret (iOS caret has special handling) to instead be the full height of the glyph at the current caret position. This causes the caret to always fully cover the glyph as well as look much closer to the stock Android behavior.
To achieve this, we add
getFullHeightForCaret()to TextPainter, which returns the height the caret should be if it were to fully cover the height of the glyph. To prevent multiple calculations of the boxes around the glyphs, the metrics are cached and only recalculated if the TextPosition and caret proto passed in differ.This fixes #23934 (as well as numerous duplicates of it)
Since this changes the caret behavior, it will likely break some golden/scuba tests, although this should be a strict improvement in fidelity compared to the current behavior. I will still label it breaking change, although there are no API breakages.