Skip to content

Commit 5018fd3

Browse files
[flutter_markdown] Make custom table column alignments work when text wraps (#8340)
This PR complements PR #7327 (Fix table column alignment), making the table column custom alignment keep working even when text wraps. Ironically, setting `Wrap.alignment` wasn't enough, we needed to correct each `Text.textAlign` in the function `_mergeInlineChildren()`. Basically, this PR is a major refactor of said function, ensuring that the loop runs to completion even when there is a single widget in the children (thus remaking the widget with the correct `textAlign`). This fixes flutter/flutter#160746.
1 parent e47b3d8 commit 5018fd3

File tree

4 files changed

+62
-90
lines changed

4 files changed

+62
-90
lines changed

packages/flutter_markdown/CHANGELOG.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
1-
## 0.7.6
1+
## 0.7.6+1
22

33
* Adds horizontal scrolling for table when using `tableColumnWidth: IntrinsicColumnWidth()`.
4+
5+
## 0.7.6
6+
47
* Adds styleSheet option `tableScrollbarThumbVisibility` for setting the `thumbVisibility` on tables' `ScrollBar`.
58

69
## 0.7.5
710

11+
* Makes table column custom alignment work even when text wraps.
812
* Updates minimum supported SDK version to Flutter 3.22/Dart 3.4.
913
* Fixes some memory leaks.
1014

packages/flutter_markdown/lib/src/builder.dart

Lines changed: 47 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -777,7 +777,7 @@ class MarkdownBuilder implements md.NodeVisitor {
777777
}
778778

779779
/// Extracts all spans from an inline element and merges them into a single list
780-
Iterable<InlineSpan> _getInlineSpans(InlineSpan span) {
780+
Iterable<InlineSpan> _getInlineSpansFromSpan(InlineSpan span) {
781781
// If the span is not a TextSpan or it has no children, return the span
782782
if (span is! TextSpan || span.children == null) {
783783
return <InlineSpan>[span];
@@ -801,95 +801,69 @@ class MarkdownBuilder implements md.NodeVisitor {
801801
return spans;
802802
}
803803

804-
/// Merges adjacent [TextSpan] children
804+
// Accesses the TextSpan property correctly depending on the widget type.
805+
// Returns null if not a valid (text) widget.
806+
InlineSpan? _getInlineSpanFromText(Widget widget) => switch (widget) {
807+
SelectableText() => widget.textSpan,
808+
Text() => widget.textSpan,
809+
RichText() => widget.text,
810+
_ => null
811+
};
812+
813+
/// Merges adjacent [TextSpan] children.
814+
/// Also forces a specific [TextAlign] regardless of merging.
815+
/// This is essential for table column alignment, since desired column alignment
816+
/// is discovered after the text widgets have been created. This function is the
817+
/// last chance to enforce the desired column alignment in the texts.
805818
List<Widget> _mergeInlineChildren(
806819
List<Widget> children,
807820
TextAlign? textAlign,
808821
) {
809-
// List of merged text spans and widgets
810-
final List<Widget> mergedTexts = <Widget>[];
822+
// List of text widgets (merged) and non-text widgets (non-merged)
823+
final List<Widget> mergedWidgets = <Widget>[];
811824

825+
bool lastIsText = false;
812826
for (final Widget child in children) {
813-
// If the list is empty, add the current widget to the list
814-
if (mergedTexts.isEmpty) {
815-
mergedTexts.add(child);
827+
final InlineSpan? currentSpan = _getInlineSpanFromText(child);
828+
final bool currentIsText = currentSpan != null;
829+
830+
if (!currentIsText) {
831+
// There is no merging to do, so just add and continue
832+
mergedWidgets.add(child);
833+
lastIsText = false;
816834
continue;
817835
}
818836

819-
// Remove last widget from the list to merge it with the current widget
820-
final Widget last = mergedTexts.removeLast();
821-
822837
// Extracted spans from the last and the current widget
823838
List<InlineSpan> spans = <InlineSpan>[];
824839

825-
// Extract the text spans from the last widget
826-
if (last is SelectableText) {
827-
final TextSpan span = last.textSpan!;
828-
spans.addAll(_getInlineSpans(span));
829-
} else if (last is Text) {
830-
final InlineSpan span = last.textSpan!;
831-
spans.addAll(_getInlineSpans(span));
832-
} else if (last is RichText) {
833-
final InlineSpan span = last.text;
834-
spans.addAll(_getInlineSpans(span));
835-
} else {
836-
// If the last widget is not a text widget,
837-
// add both the last and the current widget to the list
838-
mergedTexts.addAll(<Widget>[last, child]);
839-
continue;
840+
if (lastIsText) {
841+
// Removes last widget from the list for merging and extracts its spans
842+
spans.addAll(_getInlineSpansFromSpan(
843+
_getInlineSpanFromText(mergedWidgets.removeLast())!));
840844
}
841845

842-
// Extract the text spans from the current widget
843-
if (child is Text) {
844-
final InlineSpan span = child.textSpan!;
845-
spans.addAll(_getInlineSpans(span));
846-
} else if (child is SelectableText) {
847-
final TextSpan span = child.textSpan!;
848-
spans.addAll(_getInlineSpans(span));
849-
} else if (child is RichText) {
850-
final InlineSpan span = child.text;
851-
spans.addAll(_getInlineSpans(span));
852-
} else {
853-
// If the current widget is not a text widget,
854-
// add both the last and the current widget to the list
855-
mergedTexts.addAll(<Widget>[last, child]);
856-
continue;
857-
}
846+
spans.addAll(_getInlineSpansFromSpan(currentSpan));
847+
spans = _mergeSimilarTextSpans(spans);
858848

859-
if (spans.isNotEmpty) {
860-
// Merge similar text spans
861-
spans = _mergeSimilarTextSpans(spans);
849+
final Widget mergedWidget;
862850

863-
// Create a new text widget with the merged text spans
864-
InlineSpan child;
865-
if (spans.length == 1) {
866-
child = spans.first;
867-
} else {
868-
child = TextSpan(children: spans);
869-
}
870-
871-
// Add the new text widget to the list
872-
if (selectable) {
873-
mergedTexts.add(SelectableText.rich(
874-
TextSpan(children: spans),
875-
textScaler: styleSheet.textScaler,
876-
textAlign: textAlign ?? TextAlign.start,
877-
onTap: onTapText,
878-
));
879-
} else {
880-
mergedTexts.add(Text.rich(
881-
child,
882-
textScaler: styleSheet.textScaler,
883-
textAlign: textAlign ?? TextAlign.start,
884-
));
885-
}
851+
if (spans.isEmpty) {
852+
// no spans found, just insert the current widget
853+
mergedWidget = child;
886854
} else {
887-
// If no text spans were found, add the current widget to the list
888-
mergedTexts.add(child);
855+
final InlineSpan first = spans.first;
856+
final TextSpan textSpan = (spans.length == 1 && first is TextSpan)
857+
? first
858+
: TextSpan(children: spans);
859+
mergedWidget = _buildRichText(textSpan, textAlign: textAlign);
889860
}
861+
862+
mergedWidgets.add(mergedWidget);
863+
lastIsText = true;
890864
}
891865

892-
return mergedTexts;
866+
return mergedWidgets;
893867
}
894868

895869
TextAlign _textAlignForBlockTag(String? blockTag) {
@@ -1003,12 +977,12 @@ class MarkdownBuilder implements md.NodeVisitor {
1003977
return mergedSpans;
1004978
}
1005979

1006-
Widget _buildRichText(TextSpan? text, {TextAlign? textAlign, String? key}) {
980+
Widget _buildRichText(TextSpan text, {TextAlign? textAlign, String? key}) {
1007981
//Adding a unique key prevents the problem of using the same link handler for text spans with the same text
1008982
final Key k = key == null ? UniqueKey() : Key(key);
1009983
if (selectable) {
1010984
return SelectableText.rich(
1011-
text!,
985+
text,
1012986
textScaler: styleSheet.textScaler,
1013987
textAlign: textAlign ?? TextAlign.start,
1014988
onSelectionChanged: onSelectionChanged != null
@@ -1020,7 +994,7 @@ class MarkdownBuilder implements md.NodeVisitor {
1020994
);
1021995
} else {
1022996
return Text.rich(
1023-
text!,
997+
text,
1024998
textScaler: styleSheet.textScaler,
1025999
textAlign: textAlign ?? TextAlign.start,
10261000
key: k,

packages/flutter_markdown/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ description: A Markdown renderer for Flutter. Create rich text output,
44
formatted with simple Markdown tags.
55
repository: https://github.com/flutter/packages/tree/main/packages/flutter_markdown
66
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+flutter_markdown%22
7-
version: 0.7.6
7+
version: 0.7.6+1
88

99
environment:
1010
sdk: ^3.4.0

packages/flutter_markdown/test/table_test.dart

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ void defineTests() {
4848
'should work with alignments',
4949
(WidgetTester tester) async {
5050
const String data =
51-
'|Header 1|Header 2|\n|:----:|----:|\n|Col 1|Col 2|';
51+
'|Header 1|Header 2|Header 3|\n|:----|:----:|----:|\n|Col 1|Col 2|Col 3|';
5252
await tester.pumpWidget(
5353
boilerplate(
5454
const MarkdownBody(data: data),
@@ -58,27 +58,21 @@ void defineTests() {
5858
final Iterable<DefaultTextStyle> styles =
5959
tester.widgetList(find.byType(DefaultTextStyle));
6060

61-
expect(styles.first.textAlign, TextAlign.center);
61+
expect(styles.first.textAlign, TextAlign.left);
62+
expect(styles.elementAt(1).textAlign, TextAlign.center);
6263
expect(styles.last.textAlign, TextAlign.right);
63-
},
64-
);
65-
66-
testWidgets(
67-
'should work with table alignments',
68-
(WidgetTester tester) async {
69-
const String data =
70-
'|Header 1|Header 2|Header 3|\n|:----|:----:|----:|\n|Col 1|Col 2|Col 3|';
71-
await tester.pumpWidget(
72-
boilerplate(
73-
const MarkdownBody(data: data),
74-
),
75-
);
7664

7765
final Iterable<Wrap> wraps = tester.widgetList(find.byType(Wrap));
7866

7967
expect(wraps.first.alignment, WrapAlignment.start);
8068
expect(wraps.elementAt(1).alignment, WrapAlignment.center);
8169
expect(wraps.last.alignment, WrapAlignment.end);
70+
71+
final Iterable<Text> texts = tester.widgetList(find.byType(Text));
72+
73+
expect(texts.first.textAlign, TextAlign.left);
74+
expect(texts.elementAt(1).textAlign, TextAlign.center);
75+
expect(texts.last.textAlign, TextAlign.right);
8276
},
8377
);
8478

0 commit comments

Comments
 (0)