Add WidgetSpan support in TextFields and "Replacements" API for TextEditingController #80185

GaryQian · 2021-04-10T08:57:21Z

Since the addition of WidgetSpans as a feature of RichText widgets, there has been significant demand for addition of WidgetSpans to TextFields (#30688)

This PR is largely based off of the implementation in #30069

This PR adds the capability to add replacements for matched patterns/regexes with any InlineSpan (including WidgetSpan and TextSpan) as well as lower level support for PlaceholderSpans in Editable and TextPainter.

Adds ReplacementTextEditingController which holds a List of TextEditingInlineSpanReplacements, each of which define a Pattern to match against and a generator function that generates an InlineSpan from the matched string. On every change in the entered text, the replacements will be processed from first to last, replacing matches with the generated InlineSpan. By generating a WidgetSpan, widgets can be dynamically embedded. Since the matched string value is passed to the generator, this string can be parsed to dynamically generate any widget.

For example, this controller replaces {hello} with blue boxes:

ReplacementTextEditingController controller = ReplacementTextEditingController(
    replacements: <TextEditingInlineSpanReplacement>[
      TextEditingInlineSpanReplacement(
        RegExp(r'\{[\w]+\}'),
        (String value) => WidgetSpan(child: Container(height: 10, width: 10, color: Colors.blue)),
      ),
    ]);

Later, pass the controller to the TextField:

TextField(controller: controller)

In the background, this PR adds support for WidgetSpan rendering to RenderEditable and Editable, as well as handle caret positioning in TextPainter. WidgetSpans may be used in TextFields without using the replacement API by overriding buildTextSpan in TextEditingController

See #30069 and #33794 for the RichText implementation of this feature.

flutter-dashboard · 2021-04-10T08:57:24Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

HrX03 · 2021-04-14T06:37:24Z

packages/flutter/lib/src/widgets/editable_text.dart

-      const TextStyle(decoration: TextDecoration.underline),
-    );
-    return TextSpan(
+    for (final TextEditingInlineSpanReplacement replacement in textEditingInlineSpanReplacements) {


while testing out the pr, i noticed the entire app would freeze and crash if deleting too fast. im now wondering if these fors could be the culprit, as 3 nested fors could be heavy. for solving this, i was thinking of using the setter for textEditingInlineSpanReplacements for compiling the rangeSpanMapping instead of building these when building the text spans

This PR is now in a much more mature state. I'd be interested to see if you still see the freeze/crash. I am not seeing it on my test devices. The nested for loops are indeed inefficient for large amounts of text, but I would be very surprised to see it make a noticeable difference for the vast majority of use cases. We do similar/more work for the layout itself that this should be insignificant amount of work barring thousands of matched TextRanges.

Since the text can change at any point, the spans must be calculated on each text change event to ensure what we output at any given point is correct. Computing the replacements in the setter would not be able to capture all of the replacements as text is being typed.

Also, keep in mind, debug mode performance is much worse than release mode performance and is not indicative of the actual performance of the app.

Upon further testing, turns out the issue was on my app's code and not the pr itself. The pr seems to be working pretty good, gj!

LongCatIsLooong

Thanks for the PR! I have a few questions reguarding how text modification works when there's a PlaceholderSpan:

If I press backspace after placing the caret immediately after the PlaceholderSpan, it's supposed to delete the entire span, since it's just one code point?
If I press backspace after placing the caret at some other random places, how are we going to translate the modified value so it can be sent back to the ReplacementTextEditingController? The text editing controller has already replaced the matched patterns with placeholder spans, so the place that handles the keypress needs to know how to translate the placeholder spans back to the patterns right?

LongCatIsLooong · 2021-04-24T21:30:30Z

packages/flutter/lib/src/painting/text_painter.dart

+          // place offset at beginning or end of placeholder depending on
+          // which half it is in.
+          adjustment += offset - dims.range!.start;
+          if (offset > dims.range!.start + dims.range!.end / 2) {


Should this be decided by the placeholder, like TextAffinity, instead of depending on which half it is in?

I have added affinity support, but for caret positioning from tapping, I have still used the "which half it is in" since the affinity of the tap is ambiguous and we default to always picking one affinity first to try before the other, making it extremely difficult to select one side of the widget. The 'which half' method seems to be much more consistent and desirable UX.

For everything else, I pass the affinity in and we use that to determine positioning.

packages/flutter/lib/src/rendering/editable.dart

LongCatIsLooong · 2021-04-24T22:00:25Z

packages/flutter/lib/src/rendering/editable.dart

+        textParentData.offset.dx,
+        textParentData.offset.dy,
+        0.0,
+      )..scale(


doesn't the scale also scale the offset, if it's applied after the translation?

Yes, the scaling occurs with the entire paragraph and everything laid out in it.

packages/flutter/lib/src/rendering/editable.dart

LongCatIsLooong · 2021-04-28T05:33:09Z

packages/flutter/lib/src/widgets/editable_text.dart

+    // Add composing region as a replacement to a TextSpan with underline.
+    if (composingRegionPrioritized && value.isComposingRangeValid && withComposing) {
+      _addToMappingWithoutOverlap((String value, TextRange range) {
+          final TextStyle composingStyle = style!.merge(


style can be null from the signature of this method?

This improper non-null cast seems to also be in the current master implementation. I'll change it for both.

packages/flutter/lib/src/rendering/editable.dart

LongCatIsLooong · 2021-04-28T07:19:39Z

packages/flutter/lib/src/rendering/editable.dart

@@ -2982,6 +3032,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
      visitor(foregroundChild);
    if (backgroundChild != null)
      visitor(backgroundChild);
+    super.visitChildren(visitor);
  }


RenderEditable also overrides redepthChildren. Should that method be updated as well?

GaryQian · 2021-04-28T11:18:33Z

To answer your questions:

When you press backspace, it only edits the originally input text. The placeholder-replaced text is only used internally and is not editable directly by the IME. If you wish to delete the entire placeholder with a single backspace, a formatter should be implemented to delete entire matched strings when one char of it is deleted. A more detailed explanation of the original vs internally replaced text is my answer to your second question.
This system, as implemented, handles two textposition-spaces: the original/raw text as input by the IME, and the placeholder replaced text as consumed by the text shaping libs. The IME and Editable classes only edits the raw text which does not have any replacements. When we replace the text with a WidgetSpan, the length of the replaced text is cached. This allows TextPainter to automatically convert a TextPosition from the raw input space to the fully replaced space by adding and subtracting the lengths of the replaced strings. In TextPainter, you can see all calls to the text shaper are guarded by _getPlaceholderAdjustedPosition calls. This ensures that the rest of the framework can safely use raw textpositions and offsets, as TextPainter will automatically convert the raw textpositions into the actual ones as consumed by the shaper/renderer.

LongCatIsLooong · 2021-04-28T18:24:50Z

So if you press ctrl+del, on macOS it deletes the current line. This is an example of layout-dependent text modifications, and this kind of modification can't be directly used to update a TextEditingController: we have the ability to translate an "engine space" TextRange that needs to be deleted into a "framework space" TextRange, but the problem is the text editing controller's text might have been updated already (in the current frame before the delete event), and the range can be OOB in the new text.

goderbauer

Overall this is pretty cool.

packages/flutter/lib/src/painting/text_painter.dart

goderbauer · 2021-04-29T17:10:49Z

packages/flutter/lib/src/painting/text_painter.dart

+  /// This value is only relevant when used in TextFields/Editable widgets
+  /// as replacements for strings of regular text.
+  final TextRange? range;


Should this just be TextRange.empty in the cases where it is not relevant?

(That may remove the need for some of the ! below)

packages/flutter/lib/src/rendering/editable.dart

goderbauer · 2021-04-29T17:24:26Z

packages/flutter/lib/src/rendering/editable.dart

+  // restored to the original values before final layout and painting.
+  List<PlaceholderDimensions>? _placeholderDimensions;
+
+  // Layout the child inline widgets. We then pass the dimensions of the


A substantial amount of (somewhat complex) logic here seems to be shared with RenderParagraph for similar purposes. I wonder if we can find a way to share it between the implementations.

(true for other methods in this file as well)

Some of the work in #79420 may make it easier to support a lot of this logic in a common place by removing many of the editing methods, but as of now, reuse of code would require a fairly complex redesign/refactor.

packages/flutter/lib/src/widgets/editable_text.dart

goderbauer · 2021-04-29T17:34:05Z

packages/flutter/lib/src/widgets/editable_text.dart

+
+  /// If composing regions should be matched against for replacements.
+  ///
+  /// When true, composing regions are added before any patterns are applied,


Not sure what's meant by "added" here and in the paragraph below.

goderbauer · 2021-04-29T17:35:13Z

packages/flutter/lib/src/widgets/editable_text.dart

+  /// and replacements made. This means that composing region may sometimes
+  /// fail to display if the composing region matches against of the the
+  /// replacement patterns.
+  final bool composingRegionPrioritized;


why "prioritized"? Isn't this basically saying whether the composingRegion should be ignored for replacements? E.g. when this is true, a long as a string is part of the composing region, it cannot be replaced with a replacement?

Renamed to composingRegionReplaceable

goderbauer · 2021-04-29T17:37:17Z

packages/flutter/lib/src/widgets/editable_text.dart

+  }
+}
+
+void _addToMappingWithoutOverlap(InlineSpanGenerator generator, TextRange matchedRange, Map<TextRange, InlineSpan> rangeSpanMapping, String text) {


Can this just be a private (static) method on ReplacementTextEditingController? Why top-level?

goderbauer · 2021-04-29T17:53:56Z

The backspace behavior described above sounds a little strange. I think the expectation is that if I have my cursor positioned after a widgetspan and I hit backspace, the widgetspan should not decompose into its internal reprsentation. That's similar to how deleting a multi-byte emoji works: If I have my cursor placed after 🇩🇪 and hit backspace, I expect the entire flag to disappear. It shouldn't decompositions into DE.

Does navigating with arrow keys still work as expected? E.g. if the cursor is placed after a WidgetSpan and I hit the left arrow, does the cursor directly jump before the span? Or do I have to press the arrow multiple times? (Would be good to have a test for this behavior).

Similar, what does a widget span mean for multi-line text? E.g. if I press the up/down arrow, does the cursor still end up at the right position in the previous/next line? Or do widget spans interfere with that? (A test for this case would be cool, too)

GaryQian · 2021-05-01T00:53:20Z

On further thought, I agree the backspace should just delete the whole widgetspan by default. My initial idea was to use a formatter for maximum flexibility, but in hindsight, deleting part of a replaced span is unlikely to be a use case and can still be implemented via formatter if really desired.

Yes, arrow keys should still function as expected. I can add tests for this.

GaryQian · 2021-05-12T09:42:03Z

@goderbauer I found that the delete key is actually typically handled by the IME, and for most IMEs, it never passes through the Framework's RenderEditable deletion and other editing methods. For example, on android, InputConnectionAdaptor exposes an Editable, which can be directly modified by the IME and the results are sent to the framework as an update. This makes it very difficult to implement deletion of the entire replaced text range without entering into formatter territory.

In the framework, since we are only receiving an update from the IME post-modification, we would have to rely on comparing it with the pre-update string, which is essentially a heuristic (that is frequently used in formatters).

Since this is a more advanced feature, I am leaning towards keeping the current behavior of deletions decomposing the replaced widget rather than implementing something that isn't quite robust. On the bright side, leaving it as is technically allows for more flexibility in how this is used.

swirlee · 2021-05-19T03:03:58Z

How's it going?

swirlee · 2021-05-19T03:04:09Z

How's it going?

goderbauer · 2021-05-20T23:12:22Z

Could we replace the string sequence with a single placeholder character to enable the IME to delete it with one backspace hit?

GaryQian · 2021-05-27T14:18:09Z

There are a few problems with replacing the matched substrings with placeholder characters in the actual TextEditingValue:

Once replaced, there is no way to robustly match the replacement WidgetSpans to the placeholder characters when the editing value changes. For example, deleting multiple placeholders or copy-pasting large chunks of text around are two common cases that could throw off a one-to-one placeholder/WidgetSpan mapping.
The current design of buildTextSpan isn't meant to be able to handle modification of the raw TextEditingValue within the build method. Since the controller is also a ValueNotifier, it could lead to risks of looping when the value is changed, triggering another call to buildTextSpan, which changes the value, and so on.

GaryQian · 2021-05-27T14:22:10Z

I am actually leaning towards landing this in two sections:

RenderEditable lower level support for rendering and basic handling WidgetSpans, which should look very similar to that of RenderParagraph
Framework API that includes the Replacements system and other higher level editing concepts.

The second part of this is far more controversial and may require further work to refine. However, the first part should be relatively straightforward. More advanced developers should be able to work with just the first part to integrate WidgetSpans into their fields, while the second part provides a more easily accessible API for it.

GaryQian · 2021-06-10T03:55:53Z

Core WidgetSpan support in RenderEditable should land in #83537

We will further discuss a wrapper API for making using WidgetSpans easier.

In the meantime, passing WidgetSpans into RenderEditable should work, though you should not expect editing or carets to work perfectly out of the box depending on your specific use cases without additional supporting code to handle it. WidgetSpans can be added by subclassing TextEditingController to output WidgetSpans.

I will close this PR for now and open a new one when the user-friendly API is more ready.

Nithsua · 2024-02-29T06:39:55Z

@GaryQian caret positions are a mess even by handling it through extended controller classes and formatters especially when the user changes the caret position explicitly. Are you still working on this issue?

justinmc · 2024-02-29T18:41:26Z

He's not, so anyone else is welcome to open a PR.

flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Apr 10, 2021

google-cla bot added the cla: yes label Apr 10, 2021

GaryQian mentioned this pull request Apr 13, 2021

Add WidgetSpan support for TextField/RenderEditable #30688

Closed

HrX03 reviewed Apr 14, 2021

View reviewed changes

GaryQian requested review from jason-simmons, justinmc and LongCatIsLooong April 22, 2021 11:05

GaryQian changed the title ~~[WIP] Add Pattern replacement with InlineSpan/WidgetSpan in EditableText~~ Add WidgetSpan support in TextFields and "Replacements" API for TextEditingController Apr 22, 2021

GaryQian force-pushed the widgetspan branch from 2cbc172 to 6a4c94a Compare April 22, 2021 22:16

GaryQian requested a review from goderbauer April 22, 2021 22:23

LongCatIsLooong reviewed Apr 28, 2021

View reviewed changes

goderbauer reviewed Apr 29, 2021

View reviewed changes

benkaday3218 approved these changes May 1, 2021

View reviewed changes

GaryQian force-pushed the widgetspan branch from a4891cc to 5116676 Compare May 4, 2021 09:44

LongCatIsLooong mentioned this pull request May 10, 2021

[Text Selection] TextSelectionDelegate should not expose direct access to TextEditingController's TextEditingValue #79420

Open

GaryQian added 9 commits May 12, 2021 01:18

Initial impl

9c0ce4a

Initial semi-working version

d35a8aa

Fix overlap conditions

dff78ab

Initial caret tracking system

1f165db

Improve reverse offset conversion

1136d51

Customizable composing region priority

d49baca

Functioning caret

5fec947

Cleanup, docs

0cee168

Hit testing, improve overlap handling, docs, paint offsets

5f2f9b1

GaryQian added 11 commits May 12, 2021 01:18

Docs

b678a5e

Semantics and intrinsics handling, tests

979506f

More tests

4688993

More tests

34d2cee

Add more tests

2f2871b

TextPainter tests

1f80ab2

Docs and analyzer

d6586bb

Fix docs, whitespace

421773b

Spelling

6da950f

Nits, docs, fixes

58ecc73

Compiling and working again

a7f6e59

GaryQian force-pushed the widgetspan branch from 5116676 to a7f6e59 Compare May 12, 2021 10:39

GaryQian added 2 commits May 13, 2021 03:34

Use affinity in _getPlaceholderAdjustedOffset

97de035

Address comments, fixes

92ebfa1

GaryQian mentioned this pull request May 28, 2021

Support WidgetSpan in RenderEditable #83537

Merged

GaryQian closed this Jun 10, 2021

jason-simmons mentioned this pull request Jul 15, 2022

WidgetSpan wrong caret position #107432

Open

Navigation Menu

Add WidgetSpan support in TextFields and "Replacements" API for TextEditingController #80185

Add WidgetSpan support in TextFields and "Replacements" API for TextEditingController #80185

Conversation

GaryQian commented Apr 10, 2021 • edited

flutter-dashboard bot commented Apr 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LongCatIsLooong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GaryQian commented Apr 28, 2021 • edited

LongCatIsLooong commented Apr 28, 2021

goderbauer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goderbauer commented Apr 29, 2021

GaryQian commented May 1, 2021

GaryQian commented May 12, 2021 • edited

swirlee commented May 19, 2021

swirlee commented May 19, 2021

goderbauer commented May 20, 2021

GaryQian commented May 27, 2021

GaryQian commented May 27, 2021 • edited

GaryQian commented Jun 10, 2021

Nithsua commented Feb 29, 2024

justinmc commented Feb 29, 2024

GaryQian commented Apr 10, 2021 •

edited

GaryQian commented Apr 28, 2021 •

edited

GaryQian commented May 12, 2021 •

edited

GaryQian commented May 27, 2021 •

edited