Skip to content

Commit

Permalink
[flutter_markdown] Make custom table column alignments work when text…
Browse files Browse the repository at this point in the history 
… 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.
  • Loading branch information
@andrechalella
andrechalella authored Feb 4, 2025
1 parent e47b3d8 commit 5018fd3
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 90 deletions.
6 changes: 5 additions & 1 deletion packages/flutter_markdown/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
## 0.7.6
## 0.7.6+1

* Adds horizontal scrolling for table when using `tableColumnWidth: IntrinsicColumnWidth()`.

## 0.7.6

* Adds styleSheet option `tableScrollbarThumbVisibility` for setting the `thumbVisibility` on tables' `ScrollBar`.

## 0.7.5

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

Expand Down
120 changes: 47 additions & 73 deletions packages/flutter_markdown/lib/src/builder.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ class MarkdownBuilder implements md.NodeVisitor {
}

/// Extracts all spans from an inline element and merges them into a single list
Iterable<InlineSpan> _getInlineSpans(InlineSpan span) {
Iterable<InlineSpan> _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 <InlineSpan>[span];
Expand All @@ -801,95 +801,69 @@ 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<Widget> _mergeInlineChildren(
List<Widget> children,
TextAlign? textAlign,
) {
// List of merged text spans and widgets
final List<Widget> mergedTexts = <Widget>[];
// List of text widgets (merged) and non-text widgets (non-merged)
final List<Widget> mergedWidgets = <Widget>[];

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<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;
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(<Widget>[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);
}

// 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,
));
}
if (spans.isEmpty) {
// no spans found, just insert the current widget
mergedWidget = child;
} 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) {
Expand Down Expand Up @@ -1003,12 +977,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
Expand All @@ -1020,7 +994,7 @@ class MarkdownBuilder implements md.NodeVisitor {
);
} else {
return Text.rich(
text!,
text,
textScaler: styleSheet.textScaler,
textAlign: textAlign ?? TextAlign.start,
key: k,
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_markdown/pubspec.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.6
version: 0.7.6+1

environment:
sdk: ^3.4.0
Expand Down
24 changes: 9 additions & 15 deletions packages/flutter_markdown/test/table_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -58,27 +58,21 @@ void defineTests() {
final Iterable<DefaultTextStyle> 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<Wrap> wraps = tester.widgetList(find.byType(Wrap));

expect(wraps.first.alignment, WrapAlignment.start);
expect(wraps.elementAt(1).alignment, WrapAlignment.center);
expect(wraps.last.alignment, WrapAlignment.end);

final Iterable<Text> 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);
},
);

Expand Down

0 comments on commit 5018fd3

Please sign in to comment.