[SuperEditor][SuperTextField] Fix text layout invalidating when selection changes (Resolves #2611) (#2614)

angelosilvestre · web-flow · commit 652f8ee3d8e1 · 2025-03-30T15:11:00.000-07:00
diff --git a/super_editor/lib/src/infrastructure/attributed_text_styles.dart b/super_editor/lib/src/infrastructure/attributed_text_styles.dart
@@ -55,7 +55,7 @@ extension ComputeTextSpan on AttributedText {
 
         if (inlineWidget != null) {
           inlineSpans.add(
-            WidgetSpan(
+            _LayoutOptimizedWidgetSpan(
               alignment: PlaceholderAlignment.middle,
               child: inlineWidget,
             ),
@@ -154,3 +154,46 @@ typedef InlineWidgetBuilder = Widget? Function(
   TextStyle textStyle,
   Object placeholder,
 );
+
+/// A [WidgetSpan] that does not re-layout its child changed.
+///
+/// The [WidgetSpan] class always invalidates its layout when the child
+/// widget changes. However, this shouldn't happen, since invalidating
+/// the layout should happen at `RenderObject` level.
+///
+/// When the child widget do change its layout, i.e., by changing its size,
+/// the build pipeline will already mark the layout as dirty.
+class _LayoutOptimizedWidgetSpan extends WidgetSpan {
+  const _LayoutOptimizedWidgetSpan({
+    required Widget child,
+    required PlaceholderAlignment alignment,
+  }) : super(child: child, alignment: alignment);
+
+  @override
+  RenderComparison compareTo(InlineSpan other) {
+    if (identical(this, other)) {
+      return RenderComparison.identical;
+    }
+    if (other.runtimeType != runtimeType) {
+      return RenderComparison.layout;
+    }
+    if ((style == null) != (other.style == null)) {
+      return RenderComparison.layout;
+    }
+    final typedOther = other as WidgetSpan;
+    if (alignment != typedOther.alignment) {
+      return RenderComparison.layout;
+    }
+    RenderComparison result = RenderComparison.identical;
+    if (style != null) {
+      final candidate = style!.compareTo(other.style!);
+      if (candidate.index > result.index) {
+        result = candidate;
+      }
+      if (result == RenderComparison.layout) {
+        return result;
+      }
+    }
+    return result;
+  }
+}
diff --git a/super_editor/lib/src/super_textfield/android/android_textfield.dart b/super_editor/lib/src/super_textfield/android/android_textfield.dart
@@ -1,4 +1,3 @@
-import 'package:attributed_text/attributed_text.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/services.dart';
 import 'package:follow_the_leader/follow_the_leader.dart';
@@ -573,14 +572,6 @@ class SuperAndroidTextFieldState extends State<SuperAndroidTextField>
 
   @override
   Widget build(BuildContext context) {
-    return OverlayPortal(
-      controller: _popoverController,
-      overlayChildBuilder: _buildPopoverToolbar,
-      child: _buildTextField(),
-    );
-  }
-
-  Widget _buildTextField() {
     return TapRegion(
       groupId: widget.tapRegionGroupId,
       child: Focus(
@@ -688,13 +679,17 @@ class SuperAndroidTextFieldState extends State<SuperAndroidTextField>
             return const SizedBox();
           }
 
-          return TextLayoutCaret(
-            textLayout: textLayout,
-            style: widget.caretStyle,
-            position: _textEditingController.selection.isCollapsed //
-                ? _textEditingController.selection.extent
-                : null,
-            blinkController: _caretBlinkController,
+          return OverlayPortal(
+            controller: _popoverController,
+            overlayChildBuilder: _buildPopoverToolbar,
+            child: TextLayoutCaret(
+              textLayout: textLayout,
+              style: widget.caretStyle,
+              position: _textEditingController.selection.isCollapsed //
+                  ? _textEditingController.selection.extent
+                  : null,
+              blinkController: _caretBlinkController,
+            ),
           );
         },
       ),
diff --git a/super_editor/lib/src/super_textfield/ios/ios_textfield.dart b/super_editor/lib/src/super_textfield/ios/ios_textfield.dart
@@ -573,14 +573,6 @@ class SuperIOSTextFieldState extends State<SuperIOSTextField>
 
   @override
   Widget build(BuildContext context) {
-    return OverlayPortal(
-      controller: _popoverController,
-      overlayChildBuilder: _buildOverlayIosControls,
-      child: _buildTextField(),
-    );
-  }
-
-  Widget _buildTextField() {
     return TapRegion(
       groupId: widget.tapRegionGroupId,
       child: Focus(
@@ -694,21 +686,25 @@ class SuperIOSTextFieldState extends State<SuperIOSTextField>
             return const SizedBox();
           }
 
-          return Stack(
-            clipBehavior: Clip.none,
-            children: [
-              TextLayoutCaret(
-                textLayout: textLayout,
-                style: widget.caretStyle,
-                position: _textEditingController.selection.isCollapsed //
-                    ? _textEditingController.selection.extent
-                    : null,
-                blinkController: _caretBlinkController,
-              ),
-              IOSFloatingCursor(
-                controller: _floatingCursorController,
-              ),
-            ],
+          return OverlayPortal(
+            controller: _popoverController,
+            overlayChildBuilder: _buildOverlayIosControls,
+            child: Stack(
+              clipBehavior: Clip.none,
+              children: [
+                TextLayoutCaret(
+                  textLayout: textLayout,
+                  style: widget.caretStyle,
+                  position: _textEditingController.selection.isCollapsed //
+                      ? _textEditingController.selection.extent
+                      : null,
+                  blinkController: _caretBlinkController,
+                ),
+                IOSFloatingCursor(
+                  controller: _floatingCursorController,
+                ),
+              ],
+            ),
           );
         },
       ),
diff --git a/super_editor/test/super_editor/supereditor_inline_widgets_test.dart b/super_editor/test/super_editor/supereditor_inline_widgets_test.dart
@@ -0,0 +1,90 @@
+import 'package:flutter/material.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:flutter_test_runners/flutter_test_runners.dart';
+import 'package:super_editor/super_editor.dart';
+import 'package:super_editor/super_editor_test.dart';
+import 'package:super_text_layout/super_text_layout.dart';
+
+import 'supereditor_test_tools.dart';
+
+void main() {
+  group('SuperEditor > inline widgets >', () {
+    testWidgetsOnAllPlatforms('does not invalidate layout when selection changes', (tester) async {
+      await tester
+          .createDocument()
+          .withCustomContent(
+            MutableDocument(
+              nodes: [
+                ParagraphNode(
+                  id: '1',
+                  text: AttributedText('Hello, world!', null, {
+                    7: const _NamedPlaceHolder('world'),
+                  }),
+                ),
+              ],
+            ),
+          )
+          .useStylesheet(
+            defaultStylesheet.copyWith(
+              inlineWidgetBuilders: [_boxPlaceHolderBuilder],
+            ),
+          )
+          .pump();
+
+      // Place the caret at the beginning of the paragraph.
+      await tester.placeCaretInParagraph('1', 0);
+
+      // Keep track of whether of not the layout was invalidated.
+      bool wasLayoutInvalidated = false;
+
+      final renderParagraph = find
+          .byType(LayoutAwareRichText) //
+          .evaluate()
+          .first
+          .findRenderObject() as RenderLayoutAwareParagraph;
+      renderParagraph.onMarkNeedsLayout = () {
+        wasLayoutInvalidated = true;
+      };
+
+      // Place the selection somewhere else.
+      await tester.placeCaretInParagraph('1', 2);
+
+      // Ensure the layout was not invalidated.
+      expect(wasLayoutInvalidated, isFalse);
+    });
+  });
+}
+
+/// A builder that renders a [ColoredBox] for a [_NamedPlaceHolder].
+Widget? _boxPlaceHolderBuilder(BuildContext context, TextStyle textStyle, Object placeholder) {
+  if (placeholder is! _NamedPlaceHolder) {
+    return null;
+  }
+
+  return KeyedSubtree(
+    key: ValueKey('placeholder-${placeholder.name}'),
+    child: LineHeight(
+      style: textStyle,
+      child: const SizedBox(
+        width: 24,
+        child: ColoredBox(
+          color: Colors.yellow,
+        ),
+      ),
+    ),
+  );
+}
+
+/// A placeholder that is identified by a name.
+class _NamedPlaceHolder {
+  const _NamedPlaceHolder(this.name);
+
+  final String name;
+
+  @override
+  bool operator ==(Object other) =>
+      identical(this, other) || other is _NamedPlaceHolder && runtimeType == other.runtimeType && name == other.name;
+
+  @override
+  int get hashCode => name.hashCode;
+}
diff --git a/super_editor/test/super_textfield/super_textfield_inline_widgets_test.dart b/super_editor/test/super_textfield/super_textfield_inline_widgets_test.dart
@@ -4,6 +4,7 @@ import 'package:flutter_test/flutter_test.dart';
 import 'package:flutter_test_robots/flutter_test_robots.dart';
 import 'package:flutter_test_runners/flutter_test_runners.dart';
 import 'package:super_editor/super_editor.dart';
+import 'package:super_text_layout/super_text_layout.dart';
 
 import 'super_textfield_inspector.dart';
 import 'super_textfield_robot.dart';
@@ -318,6 +319,73 @@ void main() {
         'before  after',
       );
     });
+
+    testWidgetsOnAllPlatforms('selects inline widget upon double tap', (tester) async {
+      // This test ensures that SuperTextField does not crash upon double tap
+      // when there is an inline widget in the text.
+      // See https://github.com/superlistapp/super_editor/issues/2611 for more details.
+
+      final controller = AttributedTextEditingController(
+        text: AttributedText(
+          '< inline',
+          null,
+          {
+            0: const _NamedPlaceHolder('1'),
+          },
+        ),
+      );
+
+      await _pumpTestApp(tester, controller: controller);
+
+      // Double tap at the inline widget.
+      final inlineWidgetCenter = tester.getCenter(find.byPlaceholderName('1'));
+      await tester.tapAt(inlineWidgetCenter);
+      await tester.pump(kDoubleTapMinTime);
+      await tester.tapAt(inlineWidgetCenter);
+      // Wait for the double tap to be recognized.
+      await tester.pump(kTapMinTime);
+
+      // Ensure the inline widget was selected.
+      expect(
+        SuperTextFieldInspector.findSelection(),
+        const TextSelection(baseOffset: 0, extentOffset: 1),
+      );
+    });
+
+    testWidgetsOnAllPlatforms('does not invalidate layout when selection changes', (tester) async {
+      final controller = AttributedTextEditingController(
+        text: AttributedText(
+          'Hello',
+          null,
+          {
+            5: const _NamedPlaceHolder('1'),
+          },
+        ),
+      );
+
+      await _pumpTestApp(tester, controller: controller);
+
+      // Place the caret at the beginning of the textfield.
+      await tester.placeCaretInSuperTextField(0);
+
+      // Keep track of whether of not the layout was invalidated.
+      bool wasLayoutInvalidated = false;
+
+      final renderParagraph = find
+          .byType(LayoutAwareRichText) //
+          .evaluate()
+          .first
+          .findRenderObject() as RenderLayoutAwareParagraph;
+      renderParagraph.onMarkNeedsLayout = () {
+        wasLayoutInvalidated = true;
+      };
+
+      // Place the selection somewhere else.
+      await tester.placeCaretInSuperTextField(1);
+
+      // Ensure the layout was not invalidated.
+      expect(wasLayoutInvalidated, isFalse);
+    });
   });
 }
 
