Skip to content

Commit f9b3b84

Browse files
Reverts "Changing TextPainter.getOffsetForCaret implementation to remove the logarithmic search (#143281)" (#143801)
Reverts flutter/flutter#143281 Initiated by: LongCatIsLooong Reason for reverting: flutter/flutter#143797 Original PR Author: LongCatIsLooong Reviewed By: {justinmc, jason-simmons} This change reverts the following previous change: Original Description: The behavior largely remains the same, except: 1. The EOT cursor `(textLength, downstream)` for text ending in the opposite writing direction as the paragraph is now placed at the visual end of the last line. For example, in a LTR paragraph, the EOT cursor for `aA` (lowercase for LTR and uppercase for RTL) is placed to the right of the line: `aA|` (it was `a|A` before). This matches the behavior of most applications that do logical order arrow key navigation instead of visual order navigation. And it makes the navigation order consistent for `aA\naA`: ``` |aA => aA| => aA| => aA => aA => aA aA aA aA |aA aA| aA| (1) (2) (3) (4) (5) (6) ``` This is indeed still pretty confusing as (2) and (3), as well as (5) and (6) are hard to distinguish (when the I beam has a large width they are actually visually distinguishable -- they use the same anchor but one gets painted to the left and the other to the right. I noticed that emacs does the same). But logical order navigation will always be confusing in bidi text, in one way or another. Interestingly there are 3 different behaviors I've observed in chrome: - the chrome download dialog (which I think uses GTK text widgets but not sure which version) gives me 2 cursors when navigating bidi text, and - its HTML fields only show one, and presumably they place the I beam at the **trailing edge** of the character (which makes more sense for backspacing I guess). - On the other hand, its (new) omnibar seems to use visual order arrow navigation Side note: we may need to update the "tap to place the caret here" logic to handle the case where the tap lands outside of the text and the text ends in the opposite writing direction. 2. Removed the logarithmic search. The same could be done using the characters package but when glyphInfo tells you about the baseline location in the future we probably don't need the `getBoxesForRange` call. This should fix flutter/flutter#123424. ## Internal Tests This is going to change the image output of some internal golden tests. I'm planning to merge flutter/flutter#143281 before this to avoid updating the same golden files twice for invalid selections.
1 parent 84b5e79 commit f9b3b84

File tree

7 files changed

+805
-902
lines changed

7 files changed

+805
-902
lines changed

examples/api/test/widgets/text_magnifier/text_magnifier.0_test.dart

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,8 @@ void main() {
4646
const Duration durationBetweenActions = Duration(milliseconds: 20);
4747
const String defaultText = 'I am a magnifier, fear me!';
4848

49-
Future<void> showMagnifier(WidgetTester tester, int textOffset) async {
50-
assert(textOffset >= 0);
51-
final Offset tapOffset = _textOffsetToPosition(tester, textOffset);
49+
Future<void> showMagnifier(WidgetTester tester, String characterToTapOn) async {
50+
final Offset tapOffset = _textOffsetToPosition(tester, defaultText.indexOf(characterToTapOn));
5251

5352
// Double tap 'Magnifier' word to show the selection handles.
5453
final TestGesture testGesture = await tester.startGesture(tapOffset);
@@ -60,11 +59,11 @@ void main() {
6059
await testGesture.up();
6160
await tester.pumpAndSettle();
6261

63-
final TextEditingController controller = tester
64-
.firstWidget<TextField>(find.byType(TextField))
65-
.controller!;
62+
final TextSelection selection = tester
63+
.firstWidget<TextField>(find.byType(TextField))
64+
.controller!
65+
.selection;
6666

67-
final TextSelection selection = controller.selection;
6867
final RenderEditable renderEditable = _findRenderEditable(tester);
6968
final List<TextSelectionPoint> endpoints = _globalize(
7069
renderEditable.getEndpointsForSelection(selection),
@@ -87,7 +86,7 @@ void main() {
8786
testWidgets('should show custom magnifier on drag', (WidgetTester tester) async {
8887
await tester.pumpWidget(const example.TextMagnifierExampleApp(text: defaultText));
8988

90-
await showMagnifier(tester, defaultText.indexOf('e'));
89+
await showMagnifier(tester, 'e');
9190
expect(find.byType(example.CustomMagnifier), findsOneWidget);
9291

9392
await expectLater(
@@ -97,15 +96,16 @@ void main() {
9796
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS, TargetPlatform.android }));
9897

9998

100-
testWidgets('should show custom magnifier in RTL', (WidgetTester tester) async {
101-
const String text = 'أثارت زر';
102-
const String textToTapOn = 'ت';
99+
for (final TextDirection textDirection in TextDirection.values) {
100+
testWidgets('should show custom magnifier in $textDirection', (WidgetTester tester) async {
101+
final String text = textDirection == TextDirection.rtl ? 'أثارت زر' : defaultText;
102+
final String textToTapOn = textDirection == TextDirection.rtl ? 'ت' : 'e';
103103

104-
await tester.pumpWidget(const example.TextMagnifierExampleApp(textDirection: TextDirection.rtl, text: text));
104+
await tester.pumpWidget(example.TextMagnifierExampleApp(textDirection: textDirection, text: text));
105105

106-
await showMagnifier(tester, text.indexOf(textToTapOn));
107-
108-
expect(find.byType(example.CustomMagnifier), findsOneWidget);
109-
});
106+
await showMagnifier(tester, textToTapOn);
110107

108+
expect(find.byType(example.CustomMagnifier), findsOneWidget);
109+
});
110+
}
111111
}

0 commit comments

Comments
 (0)