From 55540f311cbbd024fb8302bf169ea4b8ff530049 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Chalella?= Date: Sun, 22 Dec 2024 21:19:40 -0300 Subject: [PATCH 1/5] PR 7327 tests can be integrated with preexisting tests --- packages/flutter_markdown/test/table_test.dart | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/packages/flutter_markdown/test/table_test.dart b/packages/flutter_markdown/test/table_test.dart index 9a7a4daf87e..abbc46fb29a 100644 --- a/packages/flutter_markdown/test/table_test.dart +++ b/packages/flutter_markdown/test/table_test.dart @@ -48,7 +48,7 @@ void defineTests() { 'should work with alignments', (WidgetTester tester) async { const String data = - '|Header 1|Header 2|\n|:----:|----:|\n|Col 1|Col 2|'; + '|Header 1|Header 2|Header 3|\n|:----|:----:|----:|\n|Col 1|Col 2|Col 3|'; await tester.pumpWidget( boilerplate( const MarkdownBody(data: data), @@ -58,21 +58,9 @@ void defineTests() { final Iterable styles = tester.widgetList(find.byType(DefaultTextStyle)); - expect(styles.first.textAlign, TextAlign.center); + expect(styles.first.textAlign, TextAlign.left); + expect(styles.elementAt(1).textAlign, TextAlign.center); expect(styles.last.textAlign, TextAlign.right); - }, - ); - - testWidgets( - 'should work with table alignments', - (WidgetTester tester) async { - const String data = - '|Header 1|Header 2|Header 3|\n|:----|:----:|----:|\n|Col 1|Col 2|Col 3|'; - await tester.pumpWidget( - boilerplate( - const MarkdownBody(data: data), - ), - ); final Iterable wraps = tester.widgetList(find.byType(Wrap)); From 1c939dd4ea82dfcfbd4273210041fa643238b032 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Chalella?= Date: Sun, 22 Dec 2024 21:20:28 -0300 Subject: [PATCH 2/5] new tests --- packages/flutter_markdown/test/table_test.dart | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/flutter_markdown/test/table_test.dart b/packages/flutter_markdown/test/table_test.dart index abbc46fb29a..dbd88d15956 100644 --- a/packages/flutter_markdown/test/table_test.dart +++ b/packages/flutter_markdown/test/table_test.dart @@ -67,6 +67,12 @@ void defineTests() { expect(wraps.first.alignment, WrapAlignment.start); expect(wraps.elementAt(1).alignment, WrapAlignment.center); expect(wraps.last.alignment, WrapAlignment.end); + + final Iterable texts = tester.widgetList(find.byType(Text)); + + expect(texts.first.textAlign, TextAlign.left); + expect(texts.elementAt(1).textAlign, TextAlign.center); + expect(texts.last.textAlign, TextAlign.right); }, ); From 0d27289cce611c7ceb6789dd7d10900c97170eb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Chalella?= Date: Sun, 22 Dec 2024 21:21:25 -0300 Subject: [PATCH 3/5] correct textAlign in text widgets --- .../flutter_markdown/lib/src/builder.dart | 123 ++++++++---------- 1 file changed, 51 insertions(+), 72 deletions(-) diff --git a/packages/flutter_markdown/lib/src/builder.dart b/packages/flutter_markdown/lib/src/builder.dart index 50bdd11591f..be4f8fe1850 100644 --- a/packages/flutter_markdown/lib/src/builder.dart +++ b/packages/flutter_markdown/lib/src/builder.dart @@ -766,7 +766,7 @@ class MarkdownBuilder implements md.NodeVisitor { } /// Extracts all spans from an inline element and merges them into a single list - Iterable _getInlineSpans(InlineSpan span) { + Iterable _getInlineSpansFromSpan(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 [span]; @@ -790,95 +790,74 @@ class MarkdownBuilder implements md.NodeVisitor { return spans; } - /// Merges adjacent [TextSpan] children + // Accesses the TextSpan property correctly depending on the widget type. + // Returns null if not a valid (text) widget. + InlineSpan? _getInlineSpanFromText(Widget widget) => + switch (widget) { + SelectableText() => widget.textSpan, + Text() => widget.textSpan, + RichText() => widget.text, + _ => null + }; + + /// Merges adjacent [TextSpan] children. + /// Also forces a specific [TextAlign] regardless of merging. + /// This is essential for table column alignment, since desired column alignment + /// is discovered after the text widgets have been created. This function is the + /// last chance to enforce the desired column alignment in the texts. List _mergeInlineChildren( List children, TextAlign? textAlign, ) { - // List of merged text spans and widgets - final List mergedTexts = []; + // List of text widgets (merged) and non-text widgets (non-merged) + final List mergedWidgets = []; + bool lastIsText = false; for (final Widget child in children) { - // If the list is empty, add the current widget to the list - if (mergedTexts.isEmpty) { - mergedTexts.add(child); + + final InlineSpan? currentSpan = _getInlineSpanFromText(child); + final bool currentIsText = currentSpan != null; + + if (!currentIsText) { + // There is no merging to do, so just add and continue + mergedWidgets.add(child); + lastIsText = false; 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 spans = []; - // 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([last, child]); - continue; + if (lastIsText) { + // Removes last widget from the list for merging and extracts its spans + spans.addAll( + _getInlineSpansFromSpan( + _getInlineSpanFromText( + mergedWidgets.removeLast())!)); } - // 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([last, child]); - continue; - } + spans.addAll(_getInlineSpansFromSpan(currentSpan)); + spans = _mergeSimilarTextSpans(spans); - if (spans.isNotEmpty) { - // Merge similar text spans - spans = _mergeSimilarTextSpans(spans); + final Widget mergedWidget; - // Create a new text widget with the merged text spans - InlineSpan child; - if (spans.length == 1) { - child = spans.first; - } else { - child = TextSpan(children: spans); - } + if (spans.isEmpty) { + // no spans found, just insert the current widget + mergedWidget = child; - // 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, - )); - } } else { - // If no text spans were found, add the current widget to the list - mergedTexts.add(child); + final InlineSpan first = spans.first; + final TextSpan textSpan = + (spans.length == 1 && first is TextSpan) + ? first : TextSpan(children: spans); + mergedWidget = _buildRichText(textSpan, textAlign: textAlign); } + + mergedWidgets.add(mergedWidget); + lastIsText = true; } - return mergedTexts; + return mergedWidgets; } TextAlign _textAlignForBlockTag(String? blockTag) { @@ -992,12 +971,12 @@ class MarkdownBuilder implements md.NodeVisitor { return mergedSpans; } - Widget _buildRichText(TextSpan? text, {TextAlign? textAlign, String? key}) { + Widget _buildRichText(TextSpan text, {TextAlign? textAlign, String? key}) { //Adding a unique key prevents the problem of using the same link handler for text spans with the same text final Key k = key == null ? UniqueKey() : Key(key); if (selectable) { return SelectableText.rich( - text!, + text, textScaler: styleSheet.textScaler, textAlign: textAlign ?? TextAlign.start, onSelectionChanged: onSelectionChanged != null @@ -1009,7 +988,7 @@ class MarkdownBuilder implements md.NodeVisitor { ); } else { return Text.rich( - text!, + text, textScaler: styleSheet.textScaler, textAlign: textAlign ?? TextAlign.start, key: k, From eab07955002c6c654b5b0c05f2305062658b4d15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Chalella?= Date: Sun, 22 Dec 2024 23:31:26 -0300 Subject: [PATCH 4/5] bump up version --- packages/flutter_markdown/CHANGELOG.md | 4 ++++ packages/flutter_markdown/pubspec.yaml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/flutter_markdown/CHANGELOG.md b/packages/flutter_markdown/CHANGELOG.md index 30e4a380497..c911097d5f7 100644 --- a/packages/flutter_markdown/CHANGELOG.md +++ b/packages/flutter_markdown/CHANGELOG.md @@ -2,6 +2,10 @@ * Updates minimum supported SDK version to Flutter 3.22/Dart 3.4. +## 0.7.4+4 + +* Makes table column custom alignment work even when text wraps. + ## 0.7.4+3 * Passes a default error builder to image widgets. diff --git a/packages/flutter_markdown/pubspec.yaml b/packages/flutter_markdown/pubspec.yaml index 3c8f7b6ffff..68eab8c7ae4 100644 --- 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.7.4+3 +version: 0.7.4+4 environment: sdk: ^3.4.0 From 4d75ff17cd45a3e937f6bba0aa797c3231b17242 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Chalella?= Date: Mon, 23 Dec 2024 13:20:56 -0300 Subject: [PATCH 5/5] fixing Linux repo_checks --- packages/flutter_markdown/CHANGELOG.md | 5 +--- .../flutter_markdown/lib/src/builder.dart | 27 ++++++++----------- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/packages/flutter_markdown/CHANGELOG.md b/packages/flutter_markdown/CHANGELOG.md index c911097d5f7..9aac7c62f58 100644 --- a/packages/flutter_markdown/CHANGELOG.md +++ b/packages/flutter_markdown/CHANGELOG.md @@ -1,10 +1,7 @@ -## NEXT - -* Updates minimum supported SDK version to Flutter 3.22/Dart 3.4. - ## 0.7.4+4 * Makes table column custom alignment work even when text wraps. +* Updates minimum supported SDK version to Flutter 3.22/Dart 3.4. ## 0.7.4+3 diff --git a/packages/flutter_markdown/lib/src/builder.dart b/packages/flutter_markdown/lib/src/builder.dart index be4f8fe1850..fda25cf4e63 100644 --- a/packages/flutter_markdown/lib/src/builder.dart +++ b/packages/flutter_markdown/lib/src/builder.dart @@ -792,13 +792,12 @@ class MarkdownBuilder implements md.NodeVisitor { // Accesses the TextSpan property correctly depending on the widget type. // Returns null if not a valid (text) widget. - InlineSpan? _getInlineSpanFromText(Widget widget) => - switch (widget) { - SelectableText() => widget.textSpan, - Text() => widget.textSpan, - RichText() => widget.text, - _ => null - }; + InlineSpan? _getInlineSpanFromText(Widget widget) => switch (widget) { + SelectableText() => widget.textSpan, + Text() => widget.textSpan, + RichText() => widget.text, + _ => null + }; /// Merges adjacent [TextSpan] children. /// Also forces a specific [TextAlign] regardless of merging. @@ -814,7 +813,6 @@ class MarkdownBuilder implements md.NodeVisitor { bool lastIsText = false; for (final Widget child in children) { - final InlineSpan? currentSpan = _getInlineSpanFromText(child); final bool currentIsText = currentSpan != null; @@ -830,10 +828,8 @@ class MarkdownBuilder implements md.NodeVisitor { if (lastIsText) { // Removes last widget from the list for merging and extracts its spans - spans.addAll( - _getInlineSpansFromSpan( - _getInlineSpanFromText( - mergedWidgets.removeLast())!)); + spans.addAll(_getInlineSpansFromSpan( + _getInlineSpanFromText(mergedWidgets.removeLast())!)); } spans.addAll(_getInlineSpansFromSpan(currentSpan)); @@ -844,12 +840,11 @@ class MarkdownBuilder implements md.NodeVisitor { if (spans.isEmpty) { // no spans found, just insert the current widget mergedWidget = child; - } else { final InlineSpan first = spans.first; - final TextSpan textSpan = - (spans.length == 1 && first is TextSpan) - ? first : TextSpan(children: spans); + final TextSpan textSpan = (spans.length == 1 && first is TextSpan) + ? first + : TextSpan(children: spans); mergedWidget = _buildRichText(textSpan, textAlign: textAlign); }