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

[web] Fix edge cases in Paragraph.getPositionForOffset to match Flutter #16557

Merged
merged 1 commit into from Feb 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions lib/web_ui/lib/src/engine/text/measurement.dart
Expand Up @@ -429,6 +429,8 @@ class DomTextMeasurementService extends TextMeasurementService {
text,
startIndex: 0,
endIndex: text.length,
endIndexWithoutNewlines:
_excludeTrailing(text, 0, text.length, _newlinePredicate),
hardBreak: true,
width: lineWidth,
left: alignOffset,
Expand Down Expand Up @@ -796,6 +798,8 @@ class LinesCalculator {
_text.substring(_lineStart, breakingPoint) + _style.ellipsis,
startIndex: _lineStart,
endIndex: chunkEnd,
endIndexWithoutNewlines:
_excludeTrailing(_text, _chunkStart, chunkEnd, _newlinePredicate),
hardBreak: false,
width: widthOfResultingLine,
left: alignOffset,
Expand Down Expand Up @@ -861,6 +865,7 @@ class LinesCalculator {
_text.substring(_lineStart, endWithoutNewlines),
startIndex: _lineStart,
endIndex: lineEnd,
endIndexWithoutNewlines: endWithoutNewlines,
hardBreak: isHardBreak,
width: lineWidth,
left: alignOffset,
Expand Down
15 changes: 12 additions & 3 deletions lib/web_ui/lib/src/engine/text/paragraph.dart
Expand Up @@ -17,12 +17,14 @@ class EngineLineMetrics implements ui.LineMetrics {
this.lineNumber,
}) : text = null,
startIndex = -1,
endIndex = -1;
endIndex = -1,
endIndexWithoutNewlines = -1;

EngineLineMetrics.withText(
this.text, {
@required this.startIndex,
@required this.endIndex,
@required this.endIndexWithoutNewlines,
@required this.hardBreak,
this.ascent,
this.descent,
Expand All @@ -33,6 +35,9 @@ class EngineLineMetrics implements ui.LineMetrics {
this.baseline,
@required this.lineNumber,
}) : assert(text != null),
assert(startIndex != null),
assert(endIndex != null),
assert(endIndexWithoutNewlines != null),
assert(hardBreak != null),
assert(width != null),
assert(left != null),
Expand All @@ -50,6 +55,10 @@ class EngineLineMetrics implements ui.LineMetrics {
/// the text and doesn't stop at the overflow cutoff.
final int endIndex;

/// The index (exclusive) in the text where this line ends, ignoring newline
/// characters.
final int endIndexWithoutNewlines;

@override
final bool hardBreak;

Expand Down Expand Up @@ -416,7 +425,7 @@ class EngineParagraph implements ui.Paragraph {
// [offset] is to the right of the line.
if (offset.dx >= lineRight) {
return ui.TextPosition(
offset: lineMetrics.endIndex,
offset: lineMetrics.endIndexWithoutNewlines,
affinity: ui.TextAffinity.upstream,
);
}
Expand All @@ -429,7 +438,7 @@ class EngineParagraph implements ui.Paragraph {
final TextMeasurementService instance = _measurementService;

int low = lineMetrics.startIndex;
int high = lineMetrics.endIndex;
int high = lineMetrics.endIndexWithoutNewlines;
do {
final int current = (low + high) ~/ 2;
final double width = instance.measureSubstringWidth(this, lineMetrics.startIndex, current);
Expand Down
8 changes: 4 additions & 4 deletions lib/web_ui/test/paragraph_test.dart
Expand Up @@ -191,7 +191,7 @@ void main() async {
builder.addText('abcdefg\n');
builder.addText('ab');
final Paragraph paragraph = builder.build();
paragraph.layout(const ParagraphConstraints(width: 1000));
paragraph.layout(const ParagraphConstraints(width: 100));

// First line: "abcd\n"

Expand All @@ -208,7 +208,7 @@ void main() async {
// At the end of the first line.
expect(
paragraph.getPositionForOffset(Offset(50, 5)),
TextPosition(offset: 5, affinity: TextAffinity.upstream),
TextPosition(offset: 4, affinity: TextAffinity.upstream),
);
// On the left side of "b" in the first line.
expect(
Expand All @@ -232,7 +232,7 @@ void main() async {
// At the end of the second line.
expect(
paragraph.getPositionForOffset(Offset(100, 15)),
TextPosition(offset: 13, affinity: TextAffinity.upstream),
TextPosition(offset: 12, affinity: TextAffinity.upstream),
);
// On the left side of "e" in the second line.
expect(
Expand All @@ -255,7 +255,7 @@ void main() async {
);
// At the end of the last line.
expect(
paragraph.getPositionForOffset(Offset(40, 25)),
paragraph.getPositionForOffset(Offset(100, 25)),
TextPosition(offset: 15, affinity: TextAffinity.upstream),
);
// Below the last line.
Expand Down