From 9d02ab8ad774ffa0818ed68beabaf15120c264dc Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Tue, 5 Nov 2024 00:36:02 +0530 Subject: [PATCH 1/2] content: Handle `` elements Fixes: #360 --- lib/model/content.dart | 145 +++++++++++++++++++++++++++++ lib/widgets/content.dart | 85 +++++++++++++++++ test/flutter_checks.dart | 12 +++ test/model/content_test.dart | 162 +++++++++++++++++++++++++++++++++ test/widgets/content_test.dart | 46 ++++++++++ 5 files changed, 450 insertions(+) diff --git a/lib/model/content.dart b/lib/model/content.dart index 0c797795ac..4386693ff9 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -504,6 +504,53 @@ class EmbedVideoNode extends BlockContentNode { } } +class TableNode extends BlockContentNode { + const TableNode({super.debugHtmlNode, required this.rows}); + + final List rows; + + @override + List debugDescribeChildren() { + return rows + .mapIndexed((i, row) => row.toDiagnosticsNode(name: 'row $i')) + .toList(); + } +} + +class TableRowNode extends BlockContentNode { + const TableRowNode({ + super.debugHtmlNode, + required this.cells, + required this.isHeader, + }); + + final List cells; + + /// Indicates whether this row is the header row. + final bool isHeader; + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(FlagProperty('isHeader', value: isHeader, ifTrue: "is header")); + } + + @override + List debugDescribeChildren() { + return cells + .mapIndexed((i, cell) => cell.toDiagnosticsNode(name: 'cell $i')) + .toList(); + } +} + +class TableCellNode extends BlockInlineContainerNode { + const TableCellNode({ + super.debugHtmlNode, + required super.nodes, + required super.links, + }); +} + /// A content node that expects an inline layout context from its parent. /// /// When rendered into a Flutter widget tree, an inline content node @@ -1222,6 +1269,99 @@ class _ZulipContentParser { return EmbedVideoNode(hrefUrl: href, previewImageSrcUrl: imgSrc, debugHtmlNode: debugHtmlNode); } + BlockContentNode parseTableContent(dom.Element tableElement) { + assert(_debugParserContext == _ParserContext.block); + assert(tableElement.localName == 'table' + && tableElement.className.isEmpty); + + TableCellNode? parseTableCell(dom.Element node, bool isHeader) { + assert(node.localName == (isHeader ? 'th' : 'td')); + assert(node.className.isEmpty); + + final parsed = parseBlockInline(node.nodes); + return TableCellNode( + nodes: parsed.nodes, + links: parsed.links); + } + + List? parseTableCells(dom.NodeList cellNodes, bool isHeader) { + final cells = []; + for (final node in cellNodes) { + if (node is dom.Text && node.text == '\n') continue; + + if (node is! dom.Element) return null; + if (node.localName != (isHeader ? 'th' : 'td')) return null; + if (node.className.isNotEmpty) return null; + + final cell = parseTableCell(node, isHeader); + if (cell == null) return null; + cells.add(cell); + } + return cells; + } + + final TableNode? tableNode = (() { + if (tableElement.nodes case [ + dom.Text(data: '\n'), + dom.Element(localName: 'thead') && final theadElement, + dom.Text(data: '\n'), + dom.Element(localName: 'tbody') && final tbodyElement, + dom.Text(data: '\n'), + ]) { + if (theadElement.className.isNotEmpty) return null; + if (theadElement.nodes.isEmpty) return null; + if (tbodyElement.className.isNotEmpty) return null; + if (tbodyElement.nodes.isEmpty) return null; + + final int headerColumnCount; + final parsedRows = []; + + // Parse header row element. + if (theadElement.nodes case [ + dom.Text(data: '\n'), + dom.Element(localName: 'tr') && final rowElement, + dom.Text(data: '\n'), + ]) { + if (rowElement.className.isNotEmpty) return null; + if (rowElement.nodes.isEmpty) return null; + + final cells = parseTableCells(rowElement.nodes, true); + if (cells == null) return null; + headerColumnCount = cells.length; + parsedRows.add(TableRowNode(cells: cells, isHeader: true)); + } else { + // TODO(dart): simplify after https://github.com/dart-lang/language/issues/2537 + return null; + } + + // Parse body row elements. + for (final node in tbodyElement.nodes) { + if (node is dom.Text && node.text == '\n') continue; + + if (node is! dom.Element) return null; + if (node.localName != 'tr') return null; + if (node.className.isNotEmpty) return null; + if (node.nodes.isEmpty) return null; + + final cells = parseTableCells(node.nodes, false); + if (cells == null) return null; + + // Ensure that the number of columns in this row matches + // the header row. + if (cells.length != headerColumnCount) return null; + parsedRows.add(TableRowNode(cells: cells, isHeader: false)); + } + + return TableNode(rows: parsedRows); + } else { + // TODO(dart): simplify after https://github.com/dart-lang/language/issues/2537 + return null; + } + })(); + + return tableNode ?? UnimplementedBlockContentNode(htmlNode: tableElement); + } + BlockContentNode parseBlockContent(dom.Node node) { assert(_debugParserContext == _ParserContext.block); final debugHtmlNode = kDebugMode ? node : null; @@ -1290,6 +1430,10 @@ class _ZulipContentParser { parseBlockContentList(element.nodes)); } + if (localName == 'table' && className.isEmpty) { + return parseTableContent(element); + } + if (localName == 'div' && className == 'spoiler-block') { return parseSpoilerNode(element); } @@ -1334,6 +1478,7 @@ class _ZulipContentParser { case 'h5': case 'h6': case 'blockquote': + case 'table': case 'div': return false; default: diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index d3a6ebb849..f201f4fec3 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -48,6 +48,8 @@ class ContentTheme extends ThemeExtension { colorPollVoteCountBackground: const HSLColor.fromAHSL(1, 0, 0, 1).toColor(), colorPollVoteCountBorder: const HSLColor.fromAHSL(1, 156, 0.28, 0.7).toColor(), colorPollVoteCountText: const HSLColor.fromAHSL(1, 156, 0.41, 0.4).toColor(), + colorTableCellBorder: const HSLColor.fromAHSL(1, 0, 0, 0.80).toColor(), + colorTableHeaderBackground: const HSLColor.fromAHSL(1, 0, 0, 0.93).toColor(), colorThematicBreak: const HSLColor.fromAHSL(1, 0, 0, .87).toColor(), textStylePlainParagraph: _plainParagraphCommon(context).copyWith( color: const HSLColor.fromAHSL(1, 0, 0, 0.15).toColor(), @@ -77,6 +79,8 @@ class ContentTheme extends ThemeExtension { colorPollVoteCountBackground: const HSLColor.fromAHSL(0.2, 0, 0, 0).toColor(), colorPollVoteCountBorder: const HSLColor.fromAHSL(1, 185, 0.35, 0.35).toColor(), colorPollVoteCountText: const HSLColor.fromAHSL(1, 185, 0.35, 0.65).toColor(), + colorTableCellBorder: const HSLColor.fromAHSL(1, 0, 0, 0.33).toColor(), + colorTableHeaderBackground: const HSLColor.fromAHSL(0.5, 0, 0, 0).toColor(), colorThematicBreak: const HSLColor.fromAHSL(1, 0, 0, .87).toColor().withValues(alpha: 0.2), textStylePlainParagraph: _plainParagraphCommon(context).copyWith( color: const HSLColor.fromAHSL(1, 0, 0, 0.85).toColor(), @@ -105,6 +109,8 @@ class ContentTheme extends ThemeExtension { required this.colorPollVoteCountBackground, required this.colorPollVoteCountBorder, required this.colorPollVoteCountText, + required this.colorTableCellBorder, + required this.colorTableHeaderBackground, required this.colorThematicBreak, required this.textStylePlainParagraph, required this.codeBlockTextStyles, @@ -134,6 +140,8 @@ class ContentTheme extends ThemeExtension { final Color colorPollVoteCountBackground; final Color colorPollVoteCountBorder; final Color colorPollVoteCountText; + final Color colorTableCellBorder; + final Color colorTableHeaderBackground; final Color colorThematicBreak; /// The complete [TextStyle] we use for plain, unstyled paragraphs. @@ -189,6 +197,8 @@ class ContentTheme extends ThemeExtension { Color? colorPollVoteCountBackground, Color? colorPollVoteCountBorder, Color? colorPollVoteCountText, + Color? colorTableCellBorder, + Color? colorTableHeaderBackground, Color? colorThematicBreak, TextStyle? textStylePlainParagraph, CodeBlockTextStyles? codeBlockTextStyles, @@ -208,6 +218,8 @@ class ContentTheme extends ThemeExtension { colorPollVoteCountBackground: colorPollVoteCountBackground ?? this.colorPollVoteCountBackground, colorPollVoteCountBorder: colorPollVoteCountBorder ?? this.colorPollVoteCountBorder, colorPollVoteCountText: colorPollVoteCountText ?? this.colorPollVoteCountText, + colorTableCellBorder: colorTableCellBorder ?? this.colorTableCellBorder, + colorTableHeaderBackground: colorTableHeaderBackground ?? this.colorTableHeaderBackground, colorThematicBreak: colorThematicBreak ?? this.colorThematicBreak, textStylePlainParagraph: textStylePlainParagraph ?? this.textStylePlainParagraph, codeBlockTextStyles: codeBlockTextStyles ?? this.codeBlockTextStyles, @@ -234,6 +246,8 @@ class ContentTheme extends ThemeExtension { colorPollVoteCountBackground: Color.lerp(colorPollVoteCountBackground, other.colorPollVoteCountBackground, t)!, colorPollVoteCountBorder: Color.lerp(colorPollVoteCountBorder, other.colorPollVoteCountBorder, t)!, colorPollVoteCountText: Color.lerp(colorPollVoteCountText, other.colorPollVoteCountText, t)!, + colorTableCellBorder: Color.lerp(colorTableCellBorder, other.colorTableCellBorder, t)!, + colorTableHeaderBackground: Color.lerp(colorTableHeaderBackground, other.colorTableHeaderBackground, t)!, colorThematicBreak: Color.lerp(colorThematicBreak, other.colorThematicBreak, t)!, textStylePlainParagraph: TextStyle.lerp(textStylePlainParagraph, other.textStylePlainParagraph, t)!, codeBlockTextStyles: CodeBlockTextStyles.lerp(codeBlockTextStyles, other.codeBlockTextStyles, t), @@ -324,6 +338,21 @@ class BlockContentList extends StatelessWidget { }(), InlineVideoNode() => MessageInlineVideo(node: node), EmbedVideoNode() => MessageEmbedVideo(node: node), + TableNode() => MessageTable(node: node), + TableRowNode() => () { + assert(false, + "[TableRowNode] not allowed in [BlockContentList]. " + "It should be wrapped in [TableNode]." + ); + return const SizedBox.shrink(); + }(), + TableCellNode() => () { + assert(false, + "[TableCellNode] not allowed in [BlockContentList]. " + "It should be wrapped in [TableRowNode]." + ); + return const SizedBox.shrink(); + }(), UnimplementedBlockContentNode() => Text.rich(_errorUnimplemented(node, context: context)), }; @@ -1196,6 +1225,62 @@ class GlobalTime extends StatelessWidget { } } +class MessageTable extends StatelessWidget { + const MessageTable({super.key, required this.node}); + + final TableNode node; + + @override + Widget build(BuildContext context) { + final contentTheme = ContentTheme.of(context); + return SingleChildScrollViewWithScrollbar( + scrollDirection: Axis.horizontal, + child: Padding( + padding: const EdgeInsets.symmetric(horizontal: 5), + child: Table( + border: TableBorder.all( + width: 1, + style: BorderStyle.solid, + color: contentTheme.colorTableCellBorder), + defaultColumnWidth: const IntrinsicColumnWidth(), + children: List.unmodifiable(node.rows.map((row) => TableRow( + decoration: row.isHeader + ? BoxDecoration(color: contentTheme.colorTableHeaderBackground) + : null, + children: List.unmodifiable(row.cells.map((cell) => + MessageTableCell(node: cell, isHeader: row.isHeader))))))))); + } +} + +class MessageTableCell extends StatelessWidget { + const MessageTableCell({super.key, required this.node, required this.isHeader}); + + final TableCellNode node; + final bool isHeader; + + @override + Widget build(BuildContext context) { + return TableCell( + verticalAlignment: TableCellVerticalAlignment.middle, + child: Padding( + // Web has 4px padding and 1px border on all sides. + // In web, the 1px border grows each cell by 0.5px in all directions. + // Our border doesn't affect the layout, it's just painted on, + // so we add 0.5px on all sides to match web. + // Ref: https://github.com/flutter/flutter/issues/78691 + padding: const EdgeInsets.all(4 + 0.5), + child: node.nodes.isEmpty + ? const SizedBox.shrink() + : _buildBlockInlineContainer( + node: node, + style: !isHeader + ? DefaultTextStyle.of(context).style + : DefaultTextStyle.of(context).style + .merge(weightVariableTextStyle(context, wght: 700))), + )); + } +} + void _launchUrl(BuildContext context, String urlString) async { DialogStatus showError(BuildContext context, String? message) { return showErrorDialog(context: context, diff --git a/test/flutter_checks.dart b/test/flutter_checks.dart index a7c9f1c185..9d81e8ea20 100644 --- a/test/flutter_checks.dart +++ b/test/flutter_checks.dart @@ -146,3 +146,15 @@ extension MaterialChecks on Subject { extension InputDecorationChecks on Subject { Subject get hintText => has((x) => x.hintText, 'hintText'); } + +extension BoxDecorationChecks on Subject { + Subject get color => has((x) => x.color, 'color'); +} + +extension TableRowChecks on Subject { + Subject get decoration => has((x) => x.decoration, 'decoration'); +} + +extension TableChecks on Subject
{ + Subject> get children => has((x) => x.children, 'children'); +} diff --git a/test/model/content_test.dart b/test/model/content_test.dart index 3bcdaf8a5d..d8491f941a 100644 --- a/test/model/content_test.dart +++ b/test/model/content_test.dart @@ -892,6 +892,160 @@ class ContentExample { ]), InlineVideoNode(srcUrl: '/user_uploads/2/78/_KoRecCHZTFrVtyTKCkIh5Hq/Big-Buck-Bunny.webm'), ]); + + static const tableWithSingleRow = ContentExample( + 'table with single row', + // https://chat.zulip.org/#narrow/channel/7-test-here/topic/Rajesh/near/1971202 + '| a | b | c | d |\n| - | - | - | - |\n| 1 | 2 | 3 | 4 |', + '
\n\n\n\n\n\n\n\n\n' + '\n\n\n\n\n\n\n\n
abcd
1234
', [ + TableNode(rows: [ + TableRowNode(cells: [ + TableCellNode(nodes: [TextNode('a')], links: []), + TableCellNode(nodes: [TextNode('b')], links: []), + TableCellNode(nodes: [TextNode('c')], links: []), + TableCellNode(nodes: [TextNode('d')], links: []), + ], isHeader: true), + TableRowNode(cells: [ + TableCellNode(nodes: [TextNode('1')], links: []), + TableCellNode(nodes: [TextNode('2')], links: []), + TableCellNode(nodes: [TextNode('3')], links: []), + TableCellNode(nodes: [TextNode('4')], links: []), + ], isHeader: false), + ]), + ]); + + static const tableWithMultipleRows = ContentExample( + 'table with multiple rows', + // https://chat.zulip.org/#narrow/channel/7-test-here/topic/Rajesh/near/1971203 + '| heading 1 | heading 2 | heading 3 |\n| - | - | - |\n| body11 | body12 | body13 |\n| body21 | body22 | body23 |\n| body31 | body32 | body33 |', + '\n\n\n\n\n\n\n\n' + '\n\n\n\n\n\n' + '\n\n\n\n\n' + '\n\n\n\n\n\n
heading 1heading 2heading 3
body11body12body13
body21body22body23
body31body32body33
', [ + TableNode(rows: [ + TableRowNode(cells: [ + TableCellNode(nodes: [TextNode('heading 1')], links: []), + TableCellNode(nodes: [TextNode('heading 2')], links: []), + TableCellNode(nodes: [TextNode('heading 3')], links: []), + ], isHeader: true), + TableRowNode(cells: [ + TableCellNode(nodes: [TextNode('body11')], links: []), + TableCellNode(nodes: [TextNode('body12')], links: []), + TableCellNode(nodes: [TextNode('body13')], links: []), + ], isHeader: false), + TableRowNode(cells: [ + TableCellNode(nodes: [TextNode('body21')], links: []), + TableCellNode(nodes: [TextNode('body22')], links: []), + TableCellNode(nodes: [TextNode('body23')], links: []), + ], isHeader: false), + TableRowNode(cells: [ + TableCellNode(nodes: [TextNode('body31')], links: []), + TableCellNode(nodes: [TextNode('body32')], links: []), + TableCellNode(nodes: [TextNode('body33')], links: []), + ], isHeader: false), + ]), + ]); + + static const tableWithBoldAndItalicHeaders = ContentExample( + 'table with bold and italic headers', + // https://chat.zulip.org/#narrow/channel/7-test-here/topic/Rajesh/near/1971911 + '| normal heading | *italic heading* | **bold heading** | ***italic bold heading*** |\n| - | - | - | - |\n| text | text | text | text |', + '\n\n\n\n\n\n\n\n\n' + '\n\n\n\n\n\n\n\n
normal headingitalic headingbold headingitalic bold heading
texttexttexttext
', [ + TableNode(rows: [ + TableRowNode(cells: [ + TableCellNode(nodes: [TextNode('normal heading')], links: []), + TableCellNode(nodes: [EmphasisNode(nodes: [TextNode('italic heading')])], links: []), + TableCellNode(nodes: [StrongNode(nodes: [TextNode('bold heading')])], links: []), + TableCellNode(nodes: [StrongNode(nodes: [EmphasisNode(nodes: [TextNode('italic bold heading')])])], links: []), + ], isHeader: true), + TableRowNode(cells: [ + TableCellNode(nodes: [TextNode('text')], links: []), + TableCellNode(nodes: [TextNode('text')], links: []), + TableCellNode(nodes: [TextNode('text')], links: []), + TableCellNode(nodes: [TextNode('text')], links: []), + ], isHeader: false), + ]), + ]); + + static const tableWithLinksInCells = ContentExample( + 'table with links in cells', + // https://chat.zulip.org/#narrow/channel/7-test-here/topic/.E2.9C.94.20Rajesh/near/1987662 + '| https://zulip.com |\n| - |\n| https://zulip.com |', + '\n\n\n\n\n\n' + '\n\n\n\n\n
https://zulip.com
https://zulip.com
', [ + TableNode(rows: [ + TableRowNode(cells: [ + TableCellNode(nodes: [LinkNode(nodes: [TextNode('https://zulip.com')], url: 'https://zulip.com')], links: []), + ], isHeader: true), + TableRowNode(cells: [ + TableCellNode(nodes: [LinkNode(nodes: [TextNode('https://zulip.com')], url: 'https://zulip.com')], links: []), + ], isHeader: false), + ]), + ]); + + static const tableWithImage = ContentExample( + 'table with image', + // https://chat.zulip.org/#narrow/channel/7-test-here/topic/.E2.9C.94.20Rajesh/near/1987666 + '| a |\n| - |\n| [image2.jpg](/user_uploads/2/6f/KS3vNT9c2tbMfMBkSbQF_Jlj/image2.jpg) |', + '\n\n\n\n\n\n' + '\n\n\n\n\n
a
image2.jpg
\n' + '
', [ + TableNode(rows: [ + TableRowNode(cells: [ + TableCellNode(nodes: [TextNode('a')], links: []), + ], isHeader: true), + TableRowNode(cells: [ + TableCellNode(nodes: [LinkNode(nodes: [TextNode('image2.jpg')], url: '/user_uploads/2/6f/KS3vNT9c2tbMfMBkSbQF_Jlj/image2.jpg')], links: []), + ], isHeader: false), + ]), + ImageNodeList([ + ImageNode(srcUrl: '/user_uploads/2/6f/KS3vNT9c2tbMfMBkSbQF_Jlj/image2.jpg', + thumbnailUrl: '/user_uploads/thumbnail/2/6f/KS3vNT9c2tbMfMBkSbQF_Jlj/image2.jpg/840x560.webp', + loading: false, + originalWidth: 2760, + originalHeight: 4912), + ]), + ]); + + // As is, this HTML doesn't look particularly different to our parser. + // But if Zulip's table support followed GFM, this would have no : + // https://github.github.com/gfm/#example-205 + // https://github.com/zulip/zulip-flutter/pull/1031#discussion_r1855931989 + static const tableWithoutAnyBodyCellsInMarkdown = ContentExample( + 'table without any body cells in markdown', + // https://chat.zulip.org/#narrow/channel/7-test-here/topic/.E2.9C.94.20Rajesh/near/1987687 + '| table |\n| - |', + '\n\n\n\n\n\n' + '\n\n\n\n\n
table
', [ + TableNode(rows: [ + TableRowNode(cells: [ + TableCellNode(nodes: [TextNode('table')], links: []), + ], isHeader: true), + TableRowNode(cells: [ + TableCellNode(nodes: [], links: []), + ], isHeader: false), + ]), + ]); + + static const tableMissingOneBodyColumnInMarkdown = ContentExample( + 'table missing one body column in markdown', + // https://chat.zulip.org/#narrow/channel/7-test-here/topic/.E2.9C.94.20Rajesh/near/1987693 + '| a | b |\n| - | - |\n| text |', + '\n\n\n\n\n\n\n' + '\n\n\n\n\n\n
ab
text
', [ + TableNode(rows: [ + TableRowNode(cells: [ + TableCellNode(nodes: [TextNode('a')], links: []), + TableCellNode(nodes: [TextNode('b')], links: []), + ], isHeader: true), + TableRowNode(cells: [ + TableCellNode(nodes: [TextNode('text')], links: []), + TableCellNode(nodes: [], links: []), + ], isHeader: false), + ]), + ]); } UnimplementedBlockContentNode blockUnimplemented(String html) { @@ -1221,6 +1375,14 @@ void main() { testParseExample(ContentExample.videoInline); testParseExample(ContentExample.videoInlineClassesFlipped); + testParseExample(ContentExample.tableWithSingleRow); + testParseExample(ContentExample.tableWithMultipleRows); + testParseExample(ContentExample.tableWithBoldAndItalicHeaders); + testParseExample(ContentExample.tableWithLinksInCells); + testParseExample(ContentExample.tableWithImage); + testParseExample(ContentExample.tableWithoutAnyBodyCellsInMarkdown); + testParseExample(ContentExample.tableMissingOneBodyColumnInMarkdown); + testParse('parse nested lists, quotes, headings, code blocks', // "1. > ###### two\n > * three\n\n four" '
    \n
  1. \n
    \n
    two
    \n
      \n
    • three
      • \n' diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index df40fa47d2..dbd8d7d660 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -607,6 +607,30 @@ void main() { ), styleFinder: findWordBold, ); + + testFontWeight('in table column header', + expectedWght: 900, + // | **bold** | + // | - | + // | text | + content: plainContent( + '\n' + '\n\n\n\n\n' + '\n\n\n\n\n' + '
      bold
      text
      '), + styleFinder: findWordBold); + + testFontWeight('in different kind of span in table column header', + expectedWght: 900, + // | *italic **bold*** | + // | - | + // | text | + content: plainContent( + '\n' + '\n\n\n\n\n' + '\n\n\n\n\n' + '
      italic bold
      text
      '), + styleFinder: findWordBold); }); testContentSmoke(ContentExample.emphasis); @@ -1057,4 +1081,26 @@ void main() { debugNetworkImageHttpClientProvider = null; }); }); + + group('MessageTable', () { + testFontWeight('bold column header label', + // | a | b | c | d | + // | - | - | - | - | + // | 1 | 2 | 3 | 4 | + content: plainContent(ContentExample.tableWithSingleRow.html), + expectedWght: 700, + styleFinder: (tester) { + final root = tester.renderObject(find.textContaining('a')).text; + return mergedStyleOfSubstring(root, 'a')!; + }); + + testWidgets('header row background color', (tester) async { + await prepareContent(tester, plainContent(ContentExample.tableWithSingleRow.html)); + final BuildContext context = tester.element(find.byType(Table)); + check(tester.widget(find.byType(Table))).children.first + .decoration + .isA() + .color.equals(ContentTheme.of(context).colorTableHeaderBackground); + }); + }); } From 15705ad9a913d015a760d2620bd2c3081aebe57b Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Mon, 25 Nov 2024 23:16:29 +0530 Subject: [PATCH 2/2] content: Handle column text-alignment in `
      ` --- lib/model/content.dart | 47 ++++++++++++- lib/widgets/content.dart | 35 ++++++++-- test/model/content_test.dart | 124 +++++++++++++++++++++++---------- test/widgets/content_test.dart | 32 +++++++++ 4 files changed, 193 insertions(+), 45 deletions(-) diff --git a/lib/model/content.dart b/lib/model/content.dart index 4386693ff9..7a16b9da2c 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -543,12 +543,42 @@ class TableRowNode extends BlockContentNode { } } +// The text-alignment setting that applies to a cell's column, from the delimiter row. +// +// See GitHub-flavored Markdown: +// https://github.github.com/gfm/#tables-extension- +enum TableColumnTextAlignment { + /// All cells' text left-aligned, represented in Markdown as `|: --- |`. + left, // TODO(i18n) RTL issues? https://github.com/zulip/zulip/issues/32265 + /// All cells' text center-aligned, represented in Markdown as `|: --- :|`. + center, + /// All cells' text right-aligned, represented in Markdown as `| --- :|`. + right, // TODO(i18n) RTL issues? https://github.com/zulip/zulip/issues/32265 + /// Cells' text aligned the default way, represented in Markdown as `| --- |`. + defaults +} + class TableCellNode extends BlockInlineContainerNode { const TableCellNode({ super.debugHtmlNode, required super.nodes, required super.links, + required this.textAlignment, }); + + /// The table column text-alignment to be used for this cell. + // In Markdown, alignment is defined per column using the delimiter row. + // However, the generated HTML specifies alignment for each cell in a row + // individually, that matches the UI widget implementation which is also + // row based and needs alignment information to be per cell. + final TableColumnTextAlignment textAlignment; + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(EnumProperty('textAlignment', textAlignment, + defaultValue: TableColumnTextAlignment.defaults)); + } } /// A content node that expects an inline layout context from its parent. @@ -1278,10 +1308,25 @@ class _ZulipContentParser { assert(node.localName == (isHeader ? 'th' : 'td')); assert(node.className.isEmpty); + final cellStyle = node.attributes['style']; + final TableColumnTextAlignment? textAlignment; + switch (cellStyle) { + case null: + textAlignment = TableColumnTextAlignment.defaults; + case 'text-align: left;': + textAlignment = TableColumnTextAlignment.left; + case 'text-align: center;': + textAlignment = TableColumnTextAlignment.center; + case 'text-align: right;': + textAlignment = TableColumnTextAlignment.right; + default: + return null; + } final parsed = parseBlockInline(node.nodes); return TableCellNode( nodes: parsed.nodes, - links: parsed.links); + links: parsed.links, + textAlignment: textAlignment); } List? parseTableCells(dom.NodeList cellNodes, bool isHeader) { diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index f201f4fec3..e401e5004e 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -835,22 +835,28 @@ class MathBlock extends StatelessWidget { Widget _buildBlockInlineContainer({ required TextStyle style, required BlockInlineContainerNode node, + TextAlign? textAlign, }) { if (node.links == null) { return InlineContent(recognizer: null, linkRecognizers: null, - style: style, nodes: node.nodes); + style: style, nodes: node.nodes, textAlign: textAlign); } return _BlockInlineContainer(links: node.links!, - style: style, nodes: node.nodes); + style: style, nodes: node.nodes, textAlign: textAlign); } class _BlockInlineContainer extends StatefulWidget { - const _BlockInlineContainer( - {required this.links, required this.style, required this.nodes}); + const _BlockInlineContainer({ + required this.links, + required this.style, + required this.nodes, + this.textAlign, + }); final List links; final TextStyle style; final List nodes; + final TextAlign? textAlign; @override State<_BlockInlineContainer> createState() => _BlockInlineContainerState(); @@ -895,7 +901,7 @@ class _BlockInlineContainerState extends State<_BlockInlineContainer> { @override Widget build(BuildContext context) { return InlineContent(recognizer: null, linkRecognizers: _recognizers, - style: widget.style, nodes: widget.nodes); + style: widget.style, nodes: widget.nodes, textAlign: widget.textAlign); } } @@ -906,6 +912,7 @@ class InlineContent extends StatelessWidget { required this.linkRecognizers, required this.style, required this.nodes, + this.textAlign, }) { assert(style.fontSize != null); assert( @@ -927,13 +934,16 @@ class InlineContent extends StatelessWidget { /// Similarly must set a font weight using [weightVariableTextStyle]. final TextStyle style; + /// A [TextAlign] applied to this content. + final TextAlign? textAlign; + final List nodes; late final _InlineContentBuilder _builder; @override Widget build(BuildContext context) { - return Text.rich(_builder.build(context)); + return Text.rich(_builder.build(context), textAlign: textAlign); } } @@ -1260,6 +1270,18 @@ class MessageTableCell extends StatelessWidget { @override Widget build(BuildContext context) { + final textAlign = switch (node.textAlignment) { + TableColumnTextAlignment.left => TextAlign.left, + TableColumnTextAlignment.center => TextAlign.center, + TableColumnTextAlignment.right => TextAlign.right, + // The web client sets `text-align: left;` for the header cells, + // overriding the default browser alignment (which is `center` for header + // and `start` for body). By default, the [Table] widget uses `start` for + // text alignment, a saner choice that supports RTL text. So, defer to that. + // See discussion: + // https://github.com/zulip/zulip-flutter/pull/1031#discussion_r1831950371 + TableColumnTextAlignment.defaults => null, + }; return TableCell( verticalAlignment: TableCellVerticalAlignment.middle, child: Padding( @@ -1273,6 +1295,7 @@ class MessageTableCell extends StatelessWidget { ? const SizedBox.shrink() : _buildBlockInlineContainer( node: node, + textAlign: textAlign, style: !isHeader ? DefaultTextStyle.of(context).style : DefaultTextStyle.of(context).style diff --git a/test/model/content_test.dart b/test/model/content_test.dart index d8491f941a..98cf4d28e6 100644 --- a/test/model/content_test.dart +++ b/test/model/content_test.dart @@ -901,16 +901,16 @@ class ContentExample { '\n\n\n\n\n\n\n\n
      1234
      ', [ TableNode(rows: [ TableRowNode(cells: [ - TableCellNode(nodes: [TextNode('a')], links: []), - TableCellNode(nodes: [TextNode('b')], links: []), - TableCellNode(nodes: [TextNode('c')], links: []), - TableCellNode(nodes: [TextNode('d')], links: []), + TableCellNode(nodes: [TextNode('a')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('b')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('c')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('d')], links: [], textAlignment: TableColumnTextAlignment.defaults), ], isHeader: true), TableRowNode(cells: [ - TableCellNode(nodes: [TextNode('1')], links: []), - TableCellNode(nodes: [TextNode('2')], links: []), - TableCellNode(nodes: [TextNode('3')], links: []), - TableCellNode(nodes: [TextNode('4')], links: []), + TableCellNode(nodes: [TextNode('1')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('2')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('3')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('4')], links: [], textAlignment: TableColumnTextAlignment.defaults), ], isHeader: false), ]), ]); @@ -925,24 +925,24 @@ class ContentExample { '\nbody31\nbody32\nbody33\n\n\n', [ TableNode(rows: [ TableRowNode(cells: [ - TableCellNode(nodes: [TextNode('heading 1')], links: []), - TableCellNode(nodes: [TextNode('heading 2')], links: []), - TableCellNode(nodes: [TextNode('heading 3')], links: []), + TableCellNode(nodes: [TextNode('heading 1')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('heading 2')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('heading 3')], links: [], textAlignment: TableColumnTextAlignment.defaults), ], isHeader: true), TableRowNode(cells: [ - TableCellNode(nodes: [TextNode('body11')], links: []), - TableCellNode(nodes: [TextNode('body12')], links: []), - TableCellNode(nodes: [TextNode('body13')], links: []), + TableCellNode(nodes: [TextNode('body11')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('body12')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('body13')], links: [], textAlignment: TableColumnTextAlignment.defaults), ], isHeader: false), TableRowNode(cells: [ - TableCellNode(nodes: [TextNode('body21')], links: []), - TableCellNode(nodes: [TextNode('body22')], links: []), - TableCellNode(nodes: [TextNode('body23')], links: []), + TableCellNode(nodes: [TextNode('body21')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('body22')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('body23')], links: [], textAlignment: TableColumnTextAlignment.defaults), ], isHeader: false), TableRowNode(cells: [ - TableCellNode(nodes: [TextNode('body31')], links: []), - TableCellNode(nodes: [TextNode('body32')], links: []), - TableCellNode(nodes: [TextNode('body33')], links: []), + TableCellNode(nodes: [TextNode('body31')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('body32')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('body33')], links: [], textAlignment: TableColumnTextAlignment.defaults), ], isHeader: false), ]), ]); @@ -955,16 +955,16 @@ class ContentExample { '\n\ntext\ntext\ntext\ntext\n\n\n', [ TableNode(rows: [ TableRowNode(cells: [ - TableCellNode(nodes: [TextNode('normal heading')], links: []), - TableCellNode(nodes: [EmphasisNode(nodes: [TextNode('italic heading')])], links: []), - TableCellNode(nodes: [StrongNode(nodes: [TextNode('bold heading')])], links: []), - TableCellNode(nodes: [StrongNode(nodes: [EmphasisNode(nodes: [TextNode('italic bold heading')])])], links: []), + TableCellNode(nodes: [TextNode('normal heading')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [EmphasisNode(nodes: [TextNode('italic heading')])], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [StrongNode(nodes: [TextNode('bold heading')])], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [StrongNode(nodes: [EmphasisNode(nodes: [TextNode('italic bold heading')])])], links: [], textAlignment: TableColumnTextAlignment.defaults), ], isHeader: true), TableRowNode(cells: [ - TableCellNode(nodes: [TextNode('text')], links: []), - TableCellNode(nodes: [TextNode('text')], links: []), - TableCellNode(nodes: [TextNode('text')], links: []), - TableCellNode(nodes: [TextNode('text')], links: []), + TableCellNode(nodes: [TextNode('text')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('text')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('text')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('text')], links: [], textAlignment: TableColumnTextAlignment.defaults), ], isHeader: false), ]), ]); @@ -977,10 +977,10 @@ class ContentExample { '\n\nhttps://zulip.com\n\n\n', [ TableNode(rows: [ TableRowNode(cells: [ - TableCellNode(nodes: [LinkNode(nodes: [TextNode('https://zulip.com')], url: 'https://zulip.com')], links: []), + TableCellNode(nodes: [LinkNode(nodes: [TextNode('https://zulip.com')], url: 'https://zulip.com')], links: [], textAlignment: TableColumnTextAlignment.defaults), ], isHeader: true), TableRowNode(cells: [ - TableCellNode(nodes: [LinkNode(nodes: [TextNode('https://zulip.com')], url: 'https://zulip.com')], links: []), + TableCellNode(nodes: [LinkNode(nodes: [TextNode('https://zulip.com')], url: 'https://zulip.com')], links: [], textAlignment: TableColumnTextAlignment.defaults), ], isHeader: false), ]), ]); @@ -994,10 +994,10 @@ class ContentExample { '
      ', [ TableNode(rows: [ TableRowNode(cells: [ - TableCellNode(nodes: [TextNode('a')], links: []), + TableCellNode(nodes: [TextNode('a')], links: [], textAlignment: TableColumnTextAlignment.defaults), ], isHeader: true), TableRowNode(cells: [ - TableCellNode(nodes: [LinkNode(nodes: [TextNode('image2.jpg')], url: '/user_uploads/2/6f/KS3vNT9c2tbMfMBkSbQF_Jlj/image2.jpg')], links: []), + TableCellNode(nodes: [LinkNode(nodes: [TextNode('image2.jpg')], url: '/user_uploads/2/6f/KS3vNT9c2tbMfMBkSbQF_Jlj/image2.jpg')], links: [], textAlignment: TableColumnTextAlignment.defaults), ], isHeader: false), ]), ImageNodeList([ @@ -1021,10 +1021,10 @@ class ContentExample { '\n\n\n\n\n', [ TableNode(rows: [ TableRowNode(cells: [ - TableCellNode(nodes: [TextNode('table')], links: []), + TableCellNode(nodes: [TextNode('table')], links: [], textAlignment: TableColumnTextAlignment.defaults), ], isHeader: true), TableRowNode(cells: [ - TableCellNode(nodes: [], links: []), + TableCellNode(nodes: [], links: [], textAlignment: TableColumnTextAlignment.defaults), ], isHeader: false), ]), ]); @@ -1037,12 +1037,58 @@ class ContentExample { '\n\ntext\n\n\n\n', [ TableNode(rows: [ TableRowNode(cells: [ - TableCellNode(nodes: [TextNode('a')], links: []), - TableCellNode(nodes: [TextNode('b')], links: []), + TableCellNode(nodes: [TextNode('a')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('b')], links: [], textAlignment: TableColumnTextAlignment.defaults), ], isHeader: true), TableRowNode(cells: [ - TableCellNode(nodes: [TextNode('text')], links: []), - TableCellNode(nodes: [], links: []), + TableCellNode(nodes: [TextNode('text')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [], links: [], textAlignment: TableColumnTextAlignment.defaults), + ], isHeader: false), + ]), + ]); + + static const tableWithDifferentTextAlignmentInColumns = ContentExample( + 'table with different text alignment in columns', + // https://chat.zulip.org/#narrow/channel/7-test-here/topic/Rajesh/near/1971201 + '| default-aligned | left-aligned | center-aligned | right-aligned |\n| - | :- | :-: | -: |\n| text | text | text | text |\n| long text long text long text | long text long text long text | long text long text long text | long text long text long text |', + '\n\n\n\n\n\n\n\n\n' + '\n\n\n\n\n\n\n' + '\n\n\n\n\n\n' + '\n
      default-alignedleft-alignedcenter-alignedright-aligned
      texttexttexttext
      long text long text long textlong text long text long textlong text long text long textlong text long text long text
      ', [ + TableNode(rows: [ + TableRowNode(cells: [ + TableCellNode(nodes: [TextNode('default-aligned')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('left-aligned')], links: [], textAlignment: TableColumnTextAlignment.left), + TableCellNode(nodes: [TextNode('center-aligned')], links: [], textAlignment: TableColumnTextAlignment.center), + TableCellNode(nodes: [TextNode('right-aligned')], links: [], textAlignment: TableColumnTextAlignment.right), + ], isHeader: true), + TableRowNode(cells: [ + TableCellNode(nodes: [TextNode('text')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('text')], links: [], textAlignment: TableColumnTextAlignment.left), + TableCellNode(nodes: [TextNode('text')], links: [], textAlignment: TableColumnTextAlignment.center), + TableCellNode(nodes: [TextNode('text')], links: [], textAlignment: TableColumnTextAlignment.right), + ], isHeader: false), + TableRowNode(cells: [ + TableCellNode(nodes: [TextNode('long text long text long text')], links: [], textAlignment: TableColumnTextAlignment.defaults), + TableCellNode(nodes: [TextNode('long text long text long text')], links: [], textAlignment: TableColumnTextAlignment.left), + TableCellNode(nodes: [TextNode('long text long text long text')], links: [], textAlignment: TableColumnTextAlignment.center), + TableCellNode(nodes: [TextNode('long text long text long text')], links: [], textAlignment: TableColumnTextAlignment.right), + ], isHeader: false), + ]), + ]); + + static const tableWithLinkCenterAligned = ContentExample( + 'table with link; center aligned', + // https://chat.zulip.org/#narrow/channel/7-test-here/topic/.E2.9C.94.20Rajesh/near/1987982 + '| header |\n| :-: |\n| https://zulip.com |', + '\n\n\n\n\n\n' + '\n\n\n\n\n
      header
      https://zulip.com
      ', [ + TableNode(rows: [ + TableRowNode(cells: [ + TableCellNode(nodes: [TextNode('header')], links: [], textAlignment: TableColumnTextAlignment.center), + ], isHeader: true), + TableRowNode(cells: [ + TableCellNode(nodes: [LinkNode(nodes: [TextNode('https://zulip.com')], url: 'https://zulip.com')], links: [], textAlignment: TableColumnTextAlignment.center), ], isHeader: false), ]), ]); @@ -1382,6 +1428,8 @@ void main() { testParseExample(ContentExample.tableWithImage); testParseExample(ContentExample.tableWithoutAnyBodyCellsInMarkdown); testParseExample(ContentExample.tableMissingOneBodyColumnInMarkdown); + testParseExample(ContentExample.tableWithDifferentTextAlignmentInColumns); + testParseExample(ContentExample.tableWithLinkCenterAligned); testParse('parse nested lists, quotes, headings, code blocks', // "1. > ###### two\n > * three\n\n four" diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index dbd8d7d660..fa7febaa17 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -1102,5 +1102,37 @@ void main() { .isA() .color.equals(ContentTheme.of(context).colorTableHeaderBackground); }); + + testWidgets('different text alignment in columns', (tester) async { + await prepareContent(tester, + // | default-aligned | left-aligned | center-aligned | right-aligned | + // | - | :- | :-: | -: | + // | text | text | text | text | + // | long text long text long text | long text long text long text | long text long text long text | long text long text long text | + plainContent(ContentExample.tableWithDifferentTextAlignmentInColumns.html)); + + final defaultAlignedText = tester.renderObject(find.textContaining('default-aligned')); + check(defaultAlignedText.textAlign).equals(TextAlign.start); + + final leftAlignedText = tester.renderObject(find.textContaining('left-aligned')); + check(leftAlignedText.textAlign).equals(TextAlign.left); + + final centerAlignedText = tester.renderObject(find.textContaining('center-aligned')); + check(centerAlignedText.textAlign).equals(TextAlign.center); + + final rightAlignedText = tester.renderObject(find.textContaining('right-aligned')); + check(rightAlignedText.textAlign).equals(TextAlign.right); + }); + + testWidgets('text alignment in column; with link', (tester) async { + await prepareContent(tester, + // | header | + // | :-: | + // | https://zulip.com | + plainContent(ContentExample.tableWithLinkCenterAligned.html)); + + final linkText = tester.renderObject(find.textContaining('https://zulip.com')); + check(linkText.textAlign).equals(TextAlign.center); + }); }); }