From 1f37e1904064dbaeec71e2640ea76d0c2449d3a9 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 13 Jun 2023 19:19:28 -0700 Subject: [PATCH 1/5] content [nfc]: Factor out Heading widget --- lib/widgets/content.dart | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 24c9bff7ba..2f573ed3fc 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -80,13 +80,7 @@ class BlockContentNodeWidget extends StatelessWidget { } else if (node is ParagraphNode) { return Paragraph(node: node); } else if (node is HeadingNode) { - // TODO(#192) h1, h2, h3, h4, h5 -- same as h6 except font size - assert(node.level == HeadingLevel.h6); - return Padding( - padding: const EdgeInsets.only(top: 15, bottom: 5), - child: Text.rich(TextSpan( - style: const TextStyle(fontWeight: FontWeight.w600, height: 1.4), - children: _buildInlineList(node.nodes)))); + return Heading(node: node); } else if (node is ListNode) { return ListNodeWidget(node: node); } else if (node is QuotationNode) { @@ -138,6 +132,23 @@ class Paragraph extends StatelessWidget { } } +class Heading extends StatelessWidget { + const Heading({super.key, required this.node}); + + final HeadingNode node; + + @override + Widget build(BuildContext context) { + // TODO(#192) h1, h2, h3, h4, h5 -- same as h6 except font size + assert(node.level == HeadingLevel.h6); + return Padding( + padding: const EdgeInsets.only(top: 15, bottom: 5), + child: Text.rich(TextSpan( + style: const TextStyle(fontWeight: FontWeight.w600, height: 1.4), + children: _buildInlineList(node.nodes)))); + } +} + class ListNodeWidget extends StatelessWidget { const ListNodeWidget({super.key, required this.node}); From a75b3845aa4ac309f824a3763c8fa54271bcabbc Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 13 Jun 2023 19:21:36 -0700 Subject: [PATCH 2/5] content [nfc]: Factor out Quotation widget --- lib/widgets/content.dart | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 2f573ed3fc..5a19fd82de 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -81,19 +81,10 @@ class BlockContentNodeWidget extends StatelessWidget { return Paragraph(node: node); } else if (node is HeadingNode) { return Heading(node: node); + } else if (node is QuotationNode) { + return Quotation(node: node); } else if (node is ListNode) { return ListNodeWidget(node: node); - } else if (node is QuotationNode) { - return Padding( - padding: const EdgeInsets.only(left: 10), - child: Container( - padding: const EdgeInsets.only(left: 5), - decoration: BoxDecoration( - border: Border( - left: BorderSide( - width: 5, - color: const HSLColor.fromAHSL(1, 0, 0, 0.87).toColor()))), - child: BlockContentList(nodes: node.nodes))); } else if (node is CodeBlockNode) { return CodeBlock(node: node); } else if (node is ImageNode) { @@ -149,6 +140,26 @@ class Heading extends StatelessWidget { } } +class Quotation extends StatelessWidget { + const Quotation({super.key, required this.node}); + + final QuotationNode node; + + @override + Widget build(BuildContext context) { + return Padding( + padding: const EdgeInsets.only(left: 10), + child: Container( + padding: const EdgeInsets.only(left: 5), + decoration: BoxDecoration( + border: Border( + left: BorderSide( + width: 5, + color: const HSLColor.fromAHSL(1, 0, 0, 0.87).toColor()))), + child: BlockContentList(nodes: node.nodes))); + } +} + class ListNodeWidget extends StatelessWidget { const ListNodeWidget({super.key, required this.node}); From afe831feb77ac27e996ef42c2b85c05a0fc7e65e Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 13 Jun 2023 19:28:54 -0700 Subject: [PATCH 3/5] content [nfc]: Inline away BlockContentNodeWidget --- lib/widgets/content.dart | 60 ++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 5a19fd82de..6fed938d50 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -58,46 +58,34 @@ class BlockContentList extends StatelessWidget { @override Widget build(BuildContext context) { return Column(crossAxisAlignment: CrossAxisAlignment.stretch, children: [ - ...nodes.map((node) => BlockContentNodeWidget(node: node)), - // Text(nodes.map((n) => n.debugHtmlText ?? "").join()) + ...nodes.map((node) { + if (node is LineBreakNode) { + // This goes in a Column. So to get the effect of a newline, + // just use an empty Text. + return const Text(''); + } else if (node is ParagraphNode) { + return Paragraph(node: node); + } else if (node is HeadingNode) { + return Heading(node: node); + } else if (node is QuotationNode) { + return Quotation(node: node); + } else if (node is ListNode) { + return ListNodeWidget(node: node); + } else if (node is CodeBlockNode) { + return CodeBlock(node: node); + } else if (node is ImageNode) { + return MessageImage(node: node); + } else if (node is UnimplementedBlockContentNode) { + return Text.rich(_errorUnimplemented(node)); + } else { + // TODO(dart-3): Use a sealed class / pattern-matching to exclude this. + throw Exception("impossible BlockContentNode: ${node.debugHtmlText}"); + } + }), ]); } } -/// A single DOM node to display in block layout. -class BlockContentNodeWidget extends StatelessWidget { - const BlockContentNodeWidget({super.key, required this.node}); - - final BlockContentNode node; - - @override - Widget build(BuildContext context) { - final node = this.node; - if (node is LineBreakNode) { - // In block context, the widget we return is going into a Column. - // So to get the effect of a newline, just use an empty Text. - return const Text(''); - } else if (node is ParagraphNode) { - return Paragraph(node: node); - } else if (node is HeadingNode) { - return Heading(node: node); - } else if (node is QuotationNode) { - return Quotation(node: node); - } else if (node is ListNode) { - return ListNodeWidget(node: node); - } else if (node is CodeBlockNode) { - return CodeBlock(node: node); - } else if (node is ImageNode) { - return MessageImage(node: node); - } else if (node is UnimplementedBlockContentNode) { - return Text.rich(_errorUnimplemented(node)); - } else { - // TODO(dart-3): Use a sealed class / pattern-matching to exclude this. - throw Exception("impossible BlockContentNode: ${node.debugHtmlText}"); - } - } -} - class Paragraph extends StatelessWidget { const Paragraph({super.key, required this.node}); From 8a07af3083969c9b642e9c0b7def4f2716265d95 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 13 Jun 2023 19:38:31 -0700 Subject: [PATCH 4/5] content: Use "growable: false" optimization in _buildInlineList Making lists non-growable is a small optimization that's particularly helpful when creating a large number of short lists. That describes what we do here, mapping inline content nodes to corresponding inline spans: there will be tons of these nodes in a message list; tons of them will be in lists of one element (like any paragraph of plain text, or any link or strong/bold or emphasis/italic span with no further markup inside it); and probably the bulk of the rest will be in lists of two or three. So this is a spot where the optimization may well have a material performance impact. In any case it's essentially free; since we didn't want to add or remove elements anyway, the only cost is the extra few tokens in the source code. --- lib/widgets/content.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 6fed938d50..74832a446e 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -298,7 +298,7 @@ class _SingleChildScrollViewWithScrollbarState // List _buildInlineList(List nodes) => - List.of(nodes.map(_buildInlineNode)); + nodes.map(_buildInlineNode).toList(growable: false); InlineSpan _buildInlineNode(InlineContentNode node) { InlineSpan styled(List nodes, TextStyle style) => From a6dc643bf7b1db27d3c2c28343276bcf3fbe2242 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 13 Jun 2023 19:49:24 -0700 Subject: [PATCH 5/5] content [nfc]: Push TextSpan calls down into _buildInlineSpan This was basically the only appropriate way to use the return value of _buildInlineList, so it might as well get pushed inside the function itself. --- lib/widgets/content.dart | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 74832a446e..acbd83d868 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -97,7 +97,7 @@ class Paragraph extends StatelessWidget { // The paragraph has vertical CSS margins, but those have no effect. if (node.nodes.isEmpty) return const SizedBox(); - final text = Text.rich(TextSpan(children: _buildInlineList(node.nodes))); + final text = Text.rich(_buildInlineSpan(node.nodes, style: null)); // If the paragraph didn't actually have a `p` element in the HTML, // then apply no margins. (For example, these are seen in list items.) @@ -122,9 +122,9 @@ class Heading extends StatelessWidget { assert(node.level == HeadingLevel.h6); return Padding( padding: const EdgeInsets.only(top: 15, bottom: 5), - child: Text.rich(TextSpan( + child: Text.rich(_buildInlineSpan( style: const TextStyle(fontWeight: FontWeight.w600, height: 1.4), - children: _buildInlineList(node.nodes)))); + node.nodes))); } } @@ -297,12 +297,15 @@ class _SingleChildScrollViewWithScrollbarState // Inline layout. // -List _buildInlineList(List nodes) => - nodes.map(_buildInlineNode).toList(growable: false); +InlineSpan _buildInlineSpan(List nodes, {required TextStyle? style}) { + return TextSpan( + style: style, + children: nodes.map(_buildInlineNode).toList(growable: false)); +} InlineSpan _buildInlineNode(InlineContentNode node) { InlineSpan styled(List nodes, TextStyle style) => - TextSpan(children: _buildInlineList(nodes), style: style); + _buildInlineSpan(nodes, style: style); if (node is TextNode) { return TextSpan(text: node.text); @@ -362,8 +365,7 @@ InlineSpan inlineCode(InlineCodeNode node) { // TODO `code`: find equivalent of web's `unicode-bidi: embed; direction: ltr` // Use a light gray background, instead of a border. - return TextSpan(style: _kInlineCodeStyle, - children: _buildInlineList(node.nodes)); + return _buildInlineSpan(style: _kInlineCodeStyle, node.nodes); // Another fun solution -- we can in fact have a border! Like so: // TextStyle( @@ -428,7 +430,7 @@ class UserMention extends StatelessWidget { return Container( decoration: _kDecoration, padding: const EdgeInsets.symmetric(horizontal: 0.2 * kBaseFontSize), - child: Text.rich(TextSpan(children: _buildInlineList(node.nodes)))); + child: Text.rich(_buildInlineSpan(node.nodes, style: null))); } static get _kDecoration => BoxDecoration(