Skip to content

Conversation

jason-simmons
Copy link
Member

The regional indicator symbol characters were not being merged into a
flag grapheme when Libtxt uses the fonts available in the unit test
environment.

The regional indicator symbol characters were not being merged into a
flag grapheme when Libtxt uses the fonts available in the unit test
environment.
@jason-simmons jason-simmons requested a review from GaryQian October 8, 2020 17:22
@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Oct 8, 2020
@google-cla google-cla bot added the cla: yes label Oct 8, 2020
@jason-simmons
Copy link
Member Author

@Rusino

caretOffset = painter.getOffsetForCaret(const ui.TextPosition(offset: 20), ui.Rect.zero);
expect(caretOffset.dx, 112); // 🇺
expect(caretOffset.dx, 98); // 👏
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to a different type of emoji lgtm, but shouldn't we move the caret once the offset position is half-way through the hand emoji? (which is what the flag was doing) This behavior seems like it is not moving the caret at all until the end of line is reached/surpassed

Copy link
Contributor

Choose a reason for hiding this comment

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

Though I am not totally clear what the expected behavior is with modifiers

Copy link
Member Author

Choose a reason for hiding this comment

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

The positioning of the caret halfway through the flag was a bug.

In the test environment that uses the Ahem font, the "U" and "S" characters that encode the emoji were not being combined into one US flag grapheme. So getOffsetForCaret(20) was yielding a position between the "U" and the "S".

The test environment does correctly handle the emoji with a modifier. The framework will position the caret to the left of the last grapheme until reaching the end of the line.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh ok, thanks for the explanation. LGTM then!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants