diff --git a/lib/model/content.dart b/lib/model/content.dart index 022e333097..dce6e45207 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -251,21 +251,11 @@ class HeadingNode extends BlockInlineContainerNode { } } -enum ListStyle { ordered, unordered } +sealed class ListNode extends BlockContentNode { + const ListNode(this.items, {super.debugHtmlNode}); -class ListNode extends BlockContentNode { - const ListNode(this.style, this.items, {super.debugHtmlNode}); - - final ListStyle style; final List> items; - @override - void debugFillProperties(DiagnosticPropertiesBuilder properties) { - super.debugFillProperties(properties); - properties.add(FlagProperty('ordered', value: style == ListStyle.ordered, - ifTrue: 'ordered', ifFalse: 'unordered')); - } - @override List debugDescribeChildren() { return items @@ -275,6 +265,22 @@ class ListNode extends BlockContentNode { } } +class UnorderedListNode extends ListNode { + const UnorderedListNode(super.items, {super.debugHtmlNode}); +} + +class OrderedListNode extends ListNode { + const OrderedListNode(super.items, {required this.start, super.debugHtmlNode}); + + final int start; + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(IntProperty('start', start)); + } +} + class QuotationNode extends BlockContentNode { const QuotationNode(this.nodes, {super.debugHtmlNode}); @@ -1108,12 +1114,7 @@ class _ZulipContentParser { } BlockContentNode parseListNode(dom.Element element) { - ListStyle? listStyle; - switch (element.localName) { - case 'ol': listStyle = ListStyle.ordered; break; - case 'ul': listStyle = ListStyle.unordered; break; - } - assert(listStyle != null); + assert(element.localName == 'ol' || element.localName == 'ul'); assert(element.className.isEmpty); final debugHtmlNode = kDebugMode ? element : null; @@ -1126,7 +1127,15 @@ class _ZulipContentParser { items.add(parseImplicitParagraphBlockContentList(item.nodes)); } - return ListNode(listStyle!, items, debugHtmlNode: debugHtmlNode); + if (element.localName == 'ol') { + final startAttr = element.attributes['start']; + final start = startAttr == null ? 1 + : int.tryParse(startAttr, radix: 10); + if (start == null) return UnimplementedBlockContentNode(htmlNode: element); + return OrderedListNode(items, start: start, debugHtmlNode: debugHtmlNode); + } else { + return UnorderedListNode(items, debugHtmlNode: debugHtmlNode); + } } BlockContentNode parseSpoilerNode(dom.Element divElement) { diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 5306d74a35..ed7613e3f6 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -492,7 +492,7 @@ class ListNodeWidget extends StatelessWidget { final items = List.generate(node.items.length, (index) { final item = node.items[index]; String marker; - switch (node.style) { + switch (node) { // TODO(#161): different unordered marker styles at different levels of nesting // see: // https://html.spec.whatwg.org/multipage/rendering.html#lists @@ -500,37 +500,27 @@ class ListNodeWidget extends StatelessWidget { // TODO proper alignment of unordered marker; should be "• ", one space, // but that comes out too close to item; not sure what's fixing that // in a browser - case ListStyle.unordered: marker = "• "; break; - // TODO(#59) ordered lists starting not at 1 - case ListStyle.ordered: marker = "${index+1}. "; break; + case UnorderedListNode(): marker = "• "; break; + case OrderedListNode(:final start): marker = "${start + index}. "; break; } - return ListItemWidget(marker: marker, nodes: item); + return TableRow(children: [ + Align( + alignment: AlignmentDirectional.topEnd, + child: Text(marker)), + BlockContentList(nodes: item), + ]); }); + return Padding( padding: const EdgeInsets.only(top: 2, bottom: 5), - child: Column(children: items)); - } -} - -class ListItemWidget extends StatelessWidget { - const ListItemWidget({super.key, required this.marker, required this.nodes}); - - final String marker; - final List nodes; - - @override - Widget build(BuildContext context) { - return Row( - mainAxisAlignment: MainAxisAlignment.start, - crossAxisAlignment: CrossAxisAlignment.baseline, - textBaseline: localizedTextBaseline(context), - children: [ - SizedBox( - width: 20, // TODO handle long numbers in
    , like https://github.com/zulip/zulip/pull/25063 - child: Align( - alignment: AlignmentDirectional.topEnd, child: Text(marker))), - Expanded(child: BlockContentList(nodes: nodes)), - ]); + child: Table( + defaultVerticalAlignment: TableCellVerticalAlignment.baseline, + textBaseline: localizedTextBaseline(context), + columnWidths: const { + 0: IntrinsicColumnWidth(), + 1: FlexColumnWidth(), + }, + children: items)); } } diff --git a/test/flutter_checks.dart b/test/flutter_checks.dart index 505f5189f2..08dd99f630 100644 --- a/test/flutter_checks.dart +++ b/test/flutter_checks.dart @@ -4,6 +4,7 @@ library; import 'package:checks/checks.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; +import 'package:flutter/rendering.dart'; import 'package:flutter/services.dart'; extension PaintChecks on Subject { @@ -11,9 +12,12 @@ extension PaintChecks on Subject { } extension RectChecks on Subject { + Subject get left => has((d) => d.left, 'left'); Subject get top => has((d) => d.top, 'top'); + Subject get right => has((d) => d.right, 'right'); Subject get bottom => has((d) => d.bottom, 'bottom'); - + Subject get width => has((d) => d.width, 'width'); + Subject get height => has((d) => d.height, 'height'); // TODO others } @@ -36,6 +40,10 @@ extension GlobalKeyChecks> on Subject get currentState => has((k) => k.currentState, 'currentState'); } +extension RenderBoxChecks on Subject { + Subject get size => has((x) => x.size, 'size'); +} + extension IconChecks on Subject { Subject get icon => has((i) => i.icon, 'icon'); Subject get color => has((i) => i.color, 'color'); @@ -130,6 +138,11 @@ extension InlineSpanChecks on Subject { Subject get style => has((x) => x.style, 'style'); } +extension RenderParagraphChecks on Subject { + Subject get text => has((x) => x.text, 'text'); + Subject get didExceedMaxLines => has((x) => x.didExceedMaxLines, 'didExceedMaxLines'); +} + extension SizeChecks on Subject { Subject get width => has((x) => x.width, 'width'); Subject get height => has((x) => x.height, 'height'); diff --git a/test/model/content_test.dart b/test/model/content_test.dart index 24d60fb2af..5a6a55698e 100644 --- a/test/model/content_test.dart +++ b/test/model/content_test.dart @@ -269,6 +269,26 @@ class ContentExample { url: '/#narrow/channel/378-api-design/topic/notation.20for.20near.20links/near/1972281', nodes: [TextNode('#api design > notation for near links @ 💬')])); + static const orderedListCustomStart = ContentExample( + 'ordered list with custom start', + '5. fifth\n6. sixth', + '
      \n
    1. fifth
      2. \n
    2. sixth
      3. \n
    ', + [OrderedListNode(start: 5, [ + [ParagraphNode(wasImplicit: true, links: null, nodes: [TextNode('fifth')])], + [ParagraphNode(wasImplicit: true, links: null, nodes: [TextNode('sixth')])], + ])], + ); + + static const orderedListLargeStart = ContentExample( + 'ordered list with large start number', + '9999. first\n10000. second', + '
      \n
    1. first
      2. \n
    2. second
      3. \n
    ', + [OrderedListNode(start: 9999, [ + [ParagraphNode(wasImplicit: true, links: null, nodes: [TextNode('first')])], + [ParagraphNode(wasImplicit: true, links: null, nodes: [TextNode('second')])], + ])], + ); + static const spoilerDefaultHeader = ContentExample( 'spoiler with default header', '```spoiler\nhello world\n```', @@ -306,8 +326,8 @@ class ContentExample { '

    italic zulip

    \n' '', [SpoilerNode( - header: [ListNode(ListStyle.ordered, [ - [ListNode(ListStyle.unordered, [ + header: [OrderedListNode(start: 1, [ + [UnorderedListNode([ [HeadingNode(level: HeadingLevel.h2, links: null, nodes: [ TextNode('hello'), ])] @@ -836,7 +856,7 @@ class ContentExample { '
    ' '' '
    \n', [ - ListNode(ListStyle.unordered, [[ + UnorderedListNode([[ ImageNodeList([ ImageNode(srcUrl: 'https://chat.zulip.org/user_avatars/2/realm/icon.png', thumbnailUrl: null, loading: false, @@ -858,7 +878,7 @@ class ContentExample { '
    ' '' '
    \n', [ - ListNode(ListStyle.unordered, [[ + UnorderedListNode([[ ParagraphNode(wasImplicit: true, links: null, nodes: [ LinkNode(url: 'https://chat.zulip.org/user_avatars/2/realm/icon.png', nodes: [TextNode('icon.png')]), TextNode(' '), @@ -887,7 +907,7 @@ class ContentExample { '' '' 'more text\n', [ - ListNode(ListStyle.unordered, [[ + UnorderedListNode([[ const ParagraphNode(wasImplicit: true, links: null, nodes: [ LinkNode(url: 'https://chat.zulip.org/user_avatars/2/realm/icon.png', nodes: [TextNode('icon.png')]), TextNode(' '), @@ -1559,7 +1579,7 @@ void main() { testParse('
      ', // "1. first\n2. then" '
        \n
      1. first
        2. \n
      2. then
        3. \n
      ', const [ - ListNode(ListStyle.ordered, [ + OrderedListNode(start: 1, [ [ParagraphNode(wasImplicit: true, links: null, nodes: [TextNode('first')])], [ParagraphNode(wasImplicit: true, links: null, nodes: [TextNode('then')])], ]), @@ -1568,7 +1588,7 @@ void main() { testParse('
        ', // "* something\n* another" '
          \n
        • something
          • \n
        • another
          • \n
        ', const [ - ListNode(ListStyle.unordered, [ + UnorderedListNode([ [ParagraphNode(wasImplicit: true, links: null, nodes: [TextNode('something')])], [ParagraphNode(wasImplicit: true, links: null, nodes: [TextNode('another')])], ]), @@ -1577,7 +1597,7 @@ void main() { testParse('implicit paragraph with internal
        ', // "* a\n b" '
          \n
        • a
          \n b
          • \n
        ', const [ - ListNode(ListStyle.unordered, [ + UnorderedListNode([ [ParagraphNode(wasImplicit: true, links: null, nodes: [ TextNode('a'), LineBreakInlineNode(), @@ -1589,13 +1609,16 @@ void main() { testParse('explicit paragraphs', // "* a\n\n b" '
          \n
        • \n

          a

          \n

          b

          \n
          • \n
        ', const [ - ListNode(ListStyle.unordered, [ + UnorderedListNode([ [ ParagraphNode(links: null, nodes: [TextNode('a')]), ParagraphNode(links: null, nodes: [TextNode('b')]), ], ]), ]); + + testParseExample(ContentExample.orderedListCustomStart); + testParseExample(ContentExample.orderedListLargeStart); }); testParseExample(ContentExample.spoilerDefaultHeader); @@ -1628,7 +1651,7 @@ void main() { testParse('link in list item', // "* [t](/u)" '
          \n
        • t
          • \n
        ', const [ - ListNode(ListStyle.unordered, [ + UnorderedListNode([ [ParagraphNode(links: null, wasImplicit: true, nodes: [ LinkNode(url: '/u', nodes: [TextNode('t')]), ])], @@ -1695,10 +1718,10 @@ void main() { '
          \n
        1. \n
          \n
          two
          \n
            \n
          • three
            • \n' '
          \n
          \n
          '
         'four\n
          \n\n
          2. \n
        ', const [ - ListNode(ListStyle.ordered, [[ + OrderedListNode(start: 1, [[ QuotationNode([ HeadingNode(level: HeadingLevel.h6, links: null, nodes: [TextNode('two')]), - ListNode(ListStyle.unordered, [[ + UnorderedListNode([[ ParagraphNode(wasImplicit: true, links: null, nodes: [TextNode('three')]), ]]), ]), diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 0b81365ea8..88f94a002e 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -230,6 +230,45 @@ void main() { }); }); + group('ListNodeWidget', () { + testWidgets('ordered list with custom start', (tester) async { + await prepareContent(tester, plainContent('
          \n
        1. third
          2. \n
        2. fourth
          3. \n
        ')); + expect(find.text('3. '), findsOneWidget); + expect(find.text('4. '), findsOneWidget); + expect(find.text('third'), findsOneWidget); + expect(find.text('fourth'), findsOneWidget); + }); + + testWidgets('list uses correct text baseline alignment', (tester) async { + await prepareContent(tester, plainContent(ContentExample.orderedListLargeStart.html)); + final table = tester.widget(find.byType(Table)); + check(table.defaultVerticalAlignment).equals(TableCellVerticalAlignment.baseline); + check(table.textBaseline).equals(localizedTextBaseline(tester.element(find.byType(Table)))); + }); + + testWidgets('ordered list markers have enough space to render completely', (tester) async { + await prepareContent(tester, plainContent(ContentExample.orderedListLargeStart.html)); + final marker = tester.renderObject(find.textContaining('9999.')) as RenderParagraph; + // The marker has the height of just one line of text, not more. + final textHeight = marker.size.height; + final lineHeight = marker.text.style!.height! * marker.text.style!.fontSize!; + check(textHeight).equals(lineHeight); + // The marker's text didn't overflow to more lines + // (and get cut off by a `maxLines: 1`). + check(marker).didExceedMaxLines.isFalse(); + }); + + testWidgets('ordered list markers are end-aligned', (tester) async { + await prepareContent(tester, plainContent(ContentExample.orderedListLargeStart.html)); + final marker9999 = tester.getRect(find.textContaining('9999.')); + final marker10000 = tester.getRect(find.textContaining('10000.')); + // The markers are aligned at their right edge... + check(marker9999).right.equals(marker10000.right); + // ... and not because they somehow happen to have the same width. + check(marker9999).width.isLessThan(marker10000.width); + }); + }); + group('Spoiler', () { testContentSmoke(ContentExample.spoilerDefaultHeader); testContentSmoke(ContentExample.spoilerPlainCustomHeader);