[flutter_markdown] Fix WidgetSpan Support in MarkdownElementBuilder (flutter#6225)

pzierahn · web-flow · commit 5fed04790302 · 2024-03-05T10:19:05.000Z
This pull request addresses a critical issue identified in the `flutter_markdown` package, specifically relating to the handling of `WidgetSpan` elements within `Text.rich` elements when utilized inside `MarkdownElementBuilder`s. Prior to this fix, the inclusion of `WidgetSpan` instances within markdown content led to casting issues, disrupting the normal rendering flow of the markdown content. Key Changes: - Resolved casting problems associated with using `WidgetSpan` inside `Text.rich` elements by refining the type handling and span merging logic within the affected functions. - Thoroughly commented and cleaned up the code within the error-prone functions to enhance readability and maintainability, ensuring that future modifications can be made more easily. - Adjusted existing tests to align with the newly introduced span merging logic, which is now more efficient and produces more predictable outcomes. - Introduced new tests specifically designed to cover scenarios where `WidgetSpan` elements are included within markdown content, ensuring that this issue does not resurface in future updates. The adjustments made in this pull request ensure that developers utilizing the `flutter_markdown` package can now seamlessly incorporate `WidgetSpan` elements within their markdown content without encountering the previously observed casting issues. This fix not only improves the package's robustness but also expands its flexibility, allowing for richer text compositions within markdown. This pull request closes issue [flutter#144383](flutter#144383), effectively addressing the reported bug and enhancing the overall functionality of the `flutter_markdown` package. I'm welcoming feedback and suggestions for improvement. Fixed issues: * [flutter#144383](flutter#144383)
diff --git a/packages/flutter_markdown/CHANGELOG.md b/packages/flutter_markdown/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.6.21
+
+* Fixes support for `WidgetSpan` in `Text.rich` elements inside `MarkdownElementBuilder`.
+
 ## 0.6.20+1
 
 * Updates minimum supported SDK version to Flutter 3.19.
diff --git a/packages/flutter_markdown/lib/src/builder.dart b/packages/flutter_markdown/lib/src/builder.dart
@@ -707,52 +707,119 @@ class MarkdownBuilder implements md.NodeVisitor {
     }
   }
 
+  /// Extracts all spans from an inline element and merges them into a single list
+  Iterable<InlineSpan> _getInlineSpans(InlineSpan span) {
+    // If the span is not a TextSpan or it has no children, return the span
+    if (span is! TextSpan || span.children == null) {
+      return <InlineSpan>[span];
+    }
+
+    // Merge the style of the parent with the style of the children
+    final Iterable<InlineSpan> spans =
+        span.children!.map((InlineSpan childSpan) {
+      if (childSpan is TextSpan) {
+        return TextSpan(
+          text: childSpan.text,
+          recognizer: childSpan.recognizer,
+          semanticsLabel: childSpan.semanticsLabel,
+          style: childSpan.style?.merge(span.style),
+        );
+      } else {
+        return childSpan;
+      }
+    });
+
+    return spans;
+  }
+
   /// Merges adjacent [TextSpan] children
   List<Widget> _mergeInlineChildren(
     List<Widget> children,
     TextAlign? textAlign,
   ) {
+    // List of merged text spans and widgets
     final List<Widget> mergedTexts = <Widget>[];
+
     for (final Widget child in children) {
-      if (mergedTexts.isNotEmpty && mergedTexts.last is Text && child is Text) {
-        final Text previous = mergedTexts.removeLast() as Text;
-        final TextSpan previousTextSpan = previous.textSpan! as TextSpan;
-        final List<TextSpan> children = previousTextSpan.children != null
-            ? previousTextSpan.children!
-                .map((InlineSpan span) => span is! TextSpan
-                    ? TextSpan(children: <InlineSpan>[span])
-                    : span)
-                .toList()
-            : <TextSpan>[previousTextSpan];
-        children.add(child.textSpan! as TextSpan);
-        final TextSpan? mergedSpan = _mergeSimilarTextSpans(children);
-        mergedTexts.add(_buildRichText(
-          mergedSpan,
-          textAlign: textAlign,
-        ));
-      } else if (mergedTexts.isNotEmpty &&
-          mergedTexts.last is SelectableText &&
-          child is SelectableText) {
-        final SelectableText previous =
-            mergedTexts.removeLast() as SelectableText;
-        final TextSpan previousTextSpan = previous.textSpan!;
-        final List<TextSpan> children = previousTextSpan.children != null
-            ? List<TextSpan>.from(previousTextSpan.children!)
-            : <TextSpan>[previousTextSpan];
-        if (child.textSpan != null) {
-          children.add(child.textSpan!);
+      // If the list is empty, add the current widget to the list
+      if (mergedTexts.isEmpty) {
+        mergedTexts.add(child);
+        continue;
+      }
+
+      // Remove last widget from the list to merge it with the current widget
+      final Widget last = mergedTexts.removeLast();
+
+      // Extracted spans from the last and the current widget
+      List<InlineSpan> spans = <InlineSpan>[];
+
+      // Extract the text spans from the last widget
+      if (last is SelectableText) {
+        final TextSpan span = last.textSpan!;
+        spans.addAll(_getInlineSpans(span));
+      } else if (last is Text) {
+        final InlineSpan span = last.textSpan!;
+        spans.addAll(_getInlineSpans(span));
+      } else if (last is RichText) {
+        final InlineSpan span = last.text;
+        spans.addAll(_getInlineSpans(span));
+      } else {
+        // If the last widget is not a text widget,
+        // add both the last and the current widget to the list
+        mergedTexts.addAll(<Widget>[last, child]);
+        continue;
+      }
+
+      // Extract the text spans from the current widget
+      if (child is Text) {
+        final InlineSpan span = child.textSpan!;
+        spans.addAll(_getInlineSpans(span));
+      } else if (child is SelectableText) {
+        final TextSpan span = child.textSpan!;
+        spans.addAll(_getInlineSpans(span));
+      } else if (child is RichText) {
+        final InlineSpan span = child.text;
+        spans.addAll(_getInlineSpans(span));
+      } else {
+        // If the current widget is not a text widget,
+        // add both the last and the current widget to the list
+        mergedTexts.addAll(<Widget>[last, child]);
+        continue;
+      }
+
+      if (spans.isNotEmpty) {
+        // Merge similar text spans
+        spans = _mergeSimilarTextSpans(spans);
+
+        // Create a new text widget with the merged text spans
+        InlineSpan child;
+        if (spans.length == 1) {
+          child = spans.first;
+        } else {
+          child = TextSpan(children: spans);
+        }
+
+        // Add the new text widget to the list
+        if (selectable) {
+          mergedTexts.add(SelectableText.rich(
+            TextSpan(children: spans),
+            textScaler: styleSheet.textScaler,
+            textAlign: textAlign ?? TextAlign.start,
+            onTap: onTapText,
+          ));
+        } else {
+          mergedTexts.add(Text.rich(
+            child,
+            textScaler: styleSheet.textScaler,
+            textAlign: textAlign ?? TextAlign.start,
+          ));
         }
-        final TextSpan? mergedSpan = _mergeSimilarTextSpans(children);
-        mergedTexts.add(
-          _buildRichText(
-            mergedSpan,
-            textAlign: textAlign,
-          ),
-        );
       } else {
+        // If no text spans were found, add the current widget to the list
         mergedTexts.add(child);
       }
     }
+
     return mergedTexts;
   }
 
@@ -827,35 +894,44 @@ class MarkdownBuilder implements md.NodeVisitor {
   }
 
   /// Combine text spans with equivalent properties into a single span.
-  TextSpan? _mergeSimilarTextSpans(List<TextSpan>? textSpans) {
-    if (textSpans == null || textSpans.length < 2) {
-      return TextSpan(children: textSpans);
+  List<InlineSpan> _mergeSimilarTextSpans(List<InlineSpan> textSpans) {
+    if (textSpans.length < 2) {
+      return textSpans;
     }
 
-    final List<TextSpan> mergedSpans = <TextSpan>[textSpans.first];
+    final List<InlineSpan> mergedSpans = <InlineSpan>[];
 
     for (int index = 1; index < textSpans.length; index++) {
-      final TextSpan nextChild = textSpans[index];
-      if (nextChild.recognizer == mergedSpans.last.recognizer &&
-          nextChild.semanticsLabel == mergedSpans.last.semanticsLabel &&
-          nextChild.style == mergedSpans.last.style) {
-        final TextSpan previous = mergedSpans.removeLast();
+      final InlineSpan previous =
+          mergedSpans.isEmpty ? textSpans.first : mergedSpans.removeLast();
+      final InlineSpan nextChild = textSpans[index];
+
+      final bool previousIsTextSpan = previous is TextSpan;
+      final bool nextIsTextSpan = nextChild is TextSpan;
+      if (!previousIsTextSpan || !nextIsTextSpan) {
+        mergedSpans.addAll(<InlineSpan>[previous, nextChild]);
+        continue;
+      }
+
+      final bool matchStyle = nextChild.recognizer == previous.recognizer &&
+          nextChild.semanticsLabel == previous.semanticsLabel &&
+          nextChild.style == previous.style;
+
+      if (matchStyle) {
         mergedSpans.add(TextSpan(
           text: previous.toPlainText() + nextChild.toPlainText(),
           recognizer: previous.recognizer,
           semanticsLabel: previous.semanticsLabel,
           style: previous.style,
         ));
       } else {
-        mergedSpans.add(nextChild);
+        mergedSpans.addAll(<InlineSpan>[previous, nextChild]);
       }
     }
 
     // When the mergered spans compress into a single TextSpan return just that
     // TextSpan, otherwise bundle the set of TextSpans under a single parent.
-    return mergedSpans.length == 1
-        ? mergedSpans.first
-        : TextSpan(children: mergedSpans);
+    return mergedSpans;
   }
 
   Widget _buildRichText(TextSpan? text, {TextAlign? textAlign, String? key}) {
diff --git a/packages/flutter_markdown/pubspec.yaml b/packages/flutter_markdown/pubspec.yaml
@@ -4,7 +4,7 @@ description: A Markdown renderer for Flutter. Create rich text output,
   formatted with simple Markdown tags.
 repository: https://github.com/flutter/packages/tree/main/packages/flutter_markdown
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+flutter_markdown%22
-version: 0.6.20+1
+version: 0.6.21
 
 environment:
   sdk: ^3.3.0
diff --git a/packages/flutter_markdown/test/custom_syntax_test.dart b/packages/flutter_markdown/test/custom_syntax_test.dart
@@ -77,9 +77,8 @@ void defineTests() {
         );
 
         final Text textWidget = tester.widget(find.byType(Text));
-        final TextSpan span =
-            (textWidget.textSpan! as TextSpan).children![0] as TextSpan;
-        final WidgetSpan widgetSpan = span.children![0] as WidgetSpan;
+        final TextSpan textSpan = textWidget.textSpan! as TextSpan;
+        final WidgetSpan widgetSpan = textSpan.children![0] as WidgetSpan;
         expect(widgetSpan.child, isInstanceOf<Container>());
       },
     );
@@ -133,10 +132,9 @@ void defineTests() {
       final TextSpan textSpan = textWidget.textSpan! as TextSpan;
       final TextSpan start = textSpan.children![0] as TextSpan;
       expect(start.text, 'this test replaces a string with a ');
-      final TextSpan end = textSpan.children![1] as TextSpan;
-      final TextSpan foo = end.children![0] as TextSpan;
+      final TextSpan foo = textSpan.children![1] as TextSpan;
       expect(foo.text, 'foo');
-      final WidgetSpan widgetSpan = end.children![1] as WidgetSpan;
+      final WidgetSpan widgetSpan = textSpan.children![2] as WidgetSpan;
       expect(widgetSpan.child, isInstanceOf<Container>());
     },
   );
diff --git a/packages/flutter_markdown/test/inline_widget_test.dart b/packages/flutter_markdown/test/inline_widget_test.dart
@@ -0,0 +1,78 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter/cupertino.dart';
+import 'package:flutter/material.dart';
+import 'package:flutter_markdown/flutter_markdown.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:markdown/markdown.dart' as md;
+
+import 'utils.dart';
+
+void main() => defineTests();
+
+void defineTests() {
+  group('InlineWidget', () {
+    testWidgets(
+      'Test inline widget',
+      (WidgetTester tester) async {
+        await tester.pumpWidget(
+          boilerplate(
+            MarkdownBody(
+              data: 'Hello, foo bar',
+              builders: <String, MarkdownElementBuilder>{
+                'sub': SubscriptBuilder(),
+              },
+              extensionSet: md.ExtensionSet(
+                <md.BlockSyntax>[],
+                <md.InlineSyntax>[SubscriptSyntax()],
+              ),
+            ),
+          ),
+        );
+
+        final Text textWidget = tester.firstWidget(find.byType(Text));
+        final TextSpan span = textWidget.textSpan! as TextSpan;
+
+        final TextSpan part1 = span.children![0] as TextSpan;
+        expect(part1.toPlainText(), 'Hello, ');
+
+        final WidgetSpan part2 = span.children![1] as WidgetSpan;
+        expect(part2.alignment, PlaceholderAlignment.middle);
+        expect(part2.child, isA<Text>());
+        expect((part2.child as Text).data, 'foo');
+
+        final TextSpan part3 = span.children![2] as TextSpan;
+        expect(part3.toPlainText(), ' bar');
+      },
+    );
+  });
+}
+
+class SubscriptBuilder extends MarkdownElementBuilder {
+  @override
+  Widget visitElementAfterWithContext(
+    BuildContext context,
+    md.Element element,
+    TextStyle? preferredStyle,
+    TextStyle? parentStyle,
+  ) {
+    return Text.rich(WidgetSpan(
+      alignment: PlaceholderAlignment.middle,
+      child: Text(element.textContent),
+    ));
+  }
+}
+
+class SubscriptSyntax extends md.InlineSyntax {
+  SubscriptSyntax() : super(_pattern);
+
+  static const String _pattern = r'(foo)';
+
+  @override
+  bool onMatch(md.InlineParser parser, Match match) {
+    parser.addNode(md.Element.text('sub', match[1]!));
+    return true;
+  }
+}
