From cb11c59c6f0086f612e9c9dbab2d17073a67e5ff Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 12 Feb 2024 20:51:12 +0800 Subject: [PATCH 1/3] content [nfc]: Use toDiagnosticsNode(name: ...) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This produces slightly nicer output, by indicating the index for each item. For example, after introducing an error in one test, the expected and actual values it prints look like this: Actual: instead of like this: Actual: This also means less code as we start adding more nodes that have this kind of structure; it avoids having to make a new subclass of DiagnosticableTree for each one. --- lib/model/content.dart | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/lib/model/content.dart b/lib/model/content.dart index 19317c2db9..1aba6f895d 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -1,3 +1,4 @@ +import 'package:collection/collection.dart'; import 'package:flutter/foundation.dart'; import 'package:html/dom.dart' as dom; import 'package:html/parser.dart'; @@ -113,6 +114,20 @@ class UnimplementedBlockContentNode extends BlockContentNode // No ==/hashCode, because htmlNode is a whole subtree. } +class _BlockContentListNode extends DiagnosticableTree { + const _BlockContentListNode(this.nodes); + + final List nodes; + + @override + String toStringShort() => 'BlockContentNode list'; + + @override + List debugDescribeChildren() { + return nodes.map((node) => node.toDiagnosticsNode()).toList(); + } +} + /// A block content node whose children are inline content nodes. /// /// A node of this type expects a block layout context from its parent, @@ -226,25 +241,12 @@ class ListNode extends BlockContentNode { @override List debugDescribeChildren() { return items - .map((nodes) => _ListItemDiagnosticableNode(nodes).toDiagnosticsNode()) + .mapIndexed((i, nodes) => + _BlockContentListNode(nodes).toDiagnosticsNode(name: 'item $i')) .toList(); } } -class _ListItemDiagnosticableNode extends DiagnosticableTree { - _ListItemDiagnosticableNode(this.nodes); - - final List nodes; - - @override - String toStringShort() => 'list item'; - - @override - List debugDescribeChildren() { - return nodes.map((node) => node.toDiagnosticsNode()).toList(); - } -} - class QuotationNode extends BlockContentNode { const QuotationNode(this.nodes, {super.debugHtmlNode}); From 466311804fbfd55eba31e4d66ad4250186fcae35 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 12 Feb 2024 21:56:18 +0800 Subject: [PATCH 2/3] content test: Support localizable UI text in smoke tests When we support spoilers, soon, we'll want to translate the default header text ("Spoiler") into the user's language. It seems safe to assume that Zulip content will always have `ZulipLocalizations.of` support wherever it appears in the widget tree. The language will be a client setting with a default value. There are certain other assumptions that `prepareContentBare` will probably want to avoid making, though, including: - that the content is rendered in a particular message - that the content is rendered in a message at all (#488) - that the content is rendered in a per-account context (#488) But we'll think more about that later, when we start using testContentSmoke in more places. --- test/widgets/content_test.dart | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index f408854442..7653dcb55f 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -40,7 +40,11 @@ void main() { TestZulipBinding.ensureInitialized(); Future prepareContentBare(WidgetTester tester, String html) async { - await tester.pumpWidget(MaterialApp(home: BlockContentList(nodes: parseContent(html).nodes))); + await tester.pumpWidget(MaterialApp( + localizationsDelegates: ZulipLocalizations.localizationsDelegates, + supportedLocales: ZulipLocalizations.supportedLocales, + home: BlockContentList(nodes: parseContent(html).nodes), + )); } /// Test that the given content example renders without throwing an exception. From 0be474a5652c651b7e8a96ae02c059ceb42c8be8 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 29 Jan 2024 17:00:37 -0800 Subject: [PATCH 3/3] content: Handle spoilers Fixes: #358 --- assets/l10n/app_en.arb | 4 ++ lib/model/content.dart | 39 ++++++++++++++ lib/widgets/content.dart | 90 +++++++++++++++++++++++++++++++++ test/flutter_checks.dart | 4 ++ test/model/content_test.dart | 78 ++++++++++++++++++++++++++++ test/widgets/content_test.dart | 92 ++++++++++++++++++++++++++++++++++ 6 files changed, 307 insertions(+) diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index edb01af23d..95b924fa18 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -366,6 +366,10 @@ "@serverUrlValidationErrorUnsupportedScheme": { "description": "Error message when URL has an unsupported scheme." }, + "spoilerDefaultHeaderText": "Spoiler", + "@spoilerDefaultHeaderText": { + "description": "The default header text in a spoiler block ( https://zulip.com/help/spoilers )." + }, "markAllAsReadLabel": "Mark all messages as read", "@markAllAsReadLabel": { "description": "Button text to mark messages as read." diff --git a/lib/model/content.dart b/lib/model/content.dart index 1aba6f895d..05067c1d58 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -258,6 +258,21 @@ class QuotationNode extends BlockContentNode { } } +class SpoilerNode extends BlockContentNode { + const SpoilerNode({super.debugHtmlNode, required this.header, required this.content}); + + final List header; + final List content; + + @override + List debugDescribeChildren() { + return [ + _BlockContentListNode(header).toDiagnosticsNode(name: 'header'), + _BlockContentListNode(content).toDiagnosticsNode(name: 'content'), + ]; + } +} + class CodeBlockNode extends BlockContentNode { const CodeBlockNode(this.spans, {super.debugHtmlNode}); @@ -809,6 +824,26 @@ class _ZulipContentParser { return ListNode(listStyle!, items, debugHtmlNode: debugHtmlNode); } + BlockContentNode parseSpoilerNode(dom.Element divElement) { + assert(_debugParserContext == _ParserContext.block); + assert(divElement.localName == 'div' + && divElement.className == 'spoiler-block'); + + if (divElement.nodes case [ + dom.Element( + localName: 'div', className: 'spoiler-header', nodes: var headerNodes), + dom.Element( + localName: 'div', className: 'spoiler-content', nodes: var contentNodes), + ]) { + return SpoilerNode( + header: parseBlockContentList(headerNodes), + content: parseBlockContentList(contentNodes), + ); + } else { + return UnimplementedBlockContentNode(htmlNode: divElement); + } + } + BlockContentNode parseCodeBlock(dom.Element divElement) { assert(_debugParserContext == _ParserContext.block); final mainElement = () { @@ -977,6 +1012,10 @@ class _ZulipContentParser { parseBlockContentList(element.nodes)); } + if (localName == 'div' && className == 'spoiler-block') { + return parseSpoilerNode(element); + } + if (localName == 'div' && className == 'codehilite') { return parseCodeBlock(element); } diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 1f70556f86..1c5d23de87 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -3,6 +3,7 @@ import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; import 'package:html/dom.dart' as dom; import 'package:intl/intl.dart'; +import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import '../api/core.dart'; import '../api/model/model.dart'; @@ -79,6 +80,8 @@ class BlockContentList extends StatelessWidget { return Quotation(node: node); } else if (node is ListNode) { return ListNodeWidget(node: node); + } else if (node is SpoilerNode) { + return Spoiler(node: node); } else if (node is CodeBlockNode) { return CodeBlock(node: node); } else if (node is MathBlockNode) { @@ -235,6 +238,93 @@ class ListItemWidget extends StatelessWidget { } } +class Spoiler extends StatefulWidget { + const Spoiler({super.key, required this.node}); + + final SpoilerNode node; + + @override + State createState() => _SpoilerState(); +} + +class _SpoilerState extends State with TickerProviderStateMixin { + bool expanded = false; + + late final AnimationController _controller = AnimationController( + duration: const Duration(milliseconds: 400), vsync: this); + late final Animation _animation = CurvedAnimation( + parent: _controller, curve: Curves.easeInOut); + + @override + void dispose() { + _controller.dispose(); + super.dispose(); + } + + void _handleTap() { + setState(() { + if (!expanded) { + _controller.forward(); + expanded = true; + } else { + _controller.reverse(); + expanded = false; + } + }); + } + + @override + Widget build(BuildContext context) { + final zulipLocalizations = ZulipLocalizations.of(context); + final header = widget.node.header; + final effectiveHeader = header.isNotEmpty + ? header + : [ParagraphNode(links: null, + nodes: [TextNode(zulipLocalizations.spoilerDefaultHeaderText)])]; + return Padding( + padding: const EdgeInsets.fromLTRB(0, 5, 0, 15), + child: DecoratedBox( + decoration: BoxDecoration( + border: Border.all(color: const Color(0xff808080)), + borderRadius: BorderRadius.circular(10), + ), + child: Padding(padding: const EdgeInsetsDirectional.fromSTEB(10, 2, 8, 2), + child: Column( + children: [ + GestureDetector( + behavior: HitTestBehavior.translucent, + onTap: _handleTap, + child: Padding( + padding: const EdgeInsets.all(5), + child: Row(crossAxisAlignment: CrossAxisAlignment.end, children: [ + Expanded( + child: DefaultTextStyle.merge( + style: weightVariableTextStyle(context, wght: 700), + child: BlockContentList( + nodes: effectiveHeader))), + RotationTransition( + turns: _animation.drive(Tween(begin: 0, end: 0.5)), + child: const Icon(color: Color(0xffd4d4d4), size: 25, + Icons.expand_more)), + ]))), + FadeTransition( + opacity: _animation, + child: const SizedBox(height: 0, width: double.infinity, + child: DecoratedBox( + decoration: BoxDecoration( + border: Border( + bottom: BorderSide(width: 1, color: Color(0xff808080))))))), + SizeTransition( + sizeFactor: _animation, + axis: Axis.vertical, + axisAlignment: -1, + child: Padding( + padding: const EdgeInsets.all(5), + child: BlockContentList(nodes: widget.node.content))), + ])))); + } +} + class MessageImageList extends StatelessWidget { const MessageImageList({super.key, required this.node}); diff --git a/test/flutter_checks.dart b/test/flutter_checks.dart index 81977e9933..4344cc7ad1 100644 --- a/test/flutter_checks.dart +++ b/test/flutter_checks.dart @@ -35,6 +35,10 @@ extension RouteChecks on Subject> { Subject get settings => has((r) => r.settings, 'settings'); } +extension PageRouteChecks on Subject { + Subject get fullscreenDialog => has((x) => x.fullscreenDialog, 'fullscreenDialog'); +} + extension RouteSettingsChecks on Subject { Subject get name => has((s) => s.name, 'name'); Subject get arguments => has((s) => s.arguments, 'arguments'); diff --git a/test/model/content_test.dart b/test/model/content_test.dart index 572e176157..a89277daf0 100644 --- a/test/model/content_test.dart +++ b/test/model/content_test.dart @@ -125,6 +125,79 @@ class ContentExample { const ImageEmojiNode( src: '/static/generated/emoji/images/emoji/unicode/zulip.png', alt: ':zulip:')); + static const spoilerDefaultHeader = ContentExample( + 'spoiler with default header', + '```spoiler\nhello world\n```', + expectedText: 'Spoiler', // or a translation + '
\n' + '
', + [SpoilerNode( + header: [], + content: [ParagraphNode(links: null, nodes: [TextNode('hello world')])], + )]); + + static const spoilerPlainCustomHeader = ContentExample( + 'spoiler with plain custom header', + '```spoiler hello\nworld\n```', + expectedText: 'hello', + '
\n' + '

hello

\n' + '
', + [SpoilerNode( + header: [ParagraphNode(links: null, nodes: [TextNode('hello')])], + content: [ParagraphNode(links: null, nodes: [TextNode('world')])], + )]); + + static const spoilerRichHeaderAndContent = ContentExample( + 'spoiler with rich header and content', + '```spoiler 1. * ## hello\n*italic* [zulip](https://zulip.com/)\n```', + expectedText: 'hello', + '
\n' + '
    \n
  1. \n
      \n
    • \n

      hello

      \n
      • \n
    \n
    2. \n
\n
' + '
', + [SpoilerNode( + header: [ListNode(ListStyle.ordered, [ + [ListNode(ListStyle.unordered, [ + [HeadingNode(level: HeadingLevel.h2, links: null, nodes: [ + TextNode('hello'), + ])] + ])], + ])], + content: [ParagraphNode(links: null, nodes: [ + EmphasisNode(nodes: [TextNode('italic')]), + TextNode(' '), + LinkNode(url: 'https://zulip.com/', nodes: [TextNode('zulip')]) + ])], + )]); + + static const spoilerHeaderHasImage = ContentExample( + 'spoiler a header that has an image in it', + '```spoiler [image](https://chat.zulip.org/user_avatars/2/realm/icon.png?version=3)\nhello world\n```', + '
\n' + '

image

\n' + '
' + '
\n', + [SpoilerNode( + header: [ + ParagraphNode(links: null, nodes: [ + LinkNode(url: 'https://chat.zulip.org/user_avatars/2/realm/icon.png?version=3', + nodes: [TextNode('image')]), + ]), + ImageNodeList([ + ImageNode(srcUrl: 'https://chat.zulip.org/user_avatars/2/realm/icon.png?version=3'), + ]), + ], + content: [ParagraphNode(links: null, nodes: [TextNode('hello world')])], + )]); + static const quotation = ContentExample( 'quotation', "```quote\nwords\n```", @@ -726,6 +799,11 @@ void main() { ]); }); + testParseExample(ContentExample.spoilerDefaultHeader); + testParseExample(ContentExample.spoilerPlainCustomHeader); + testParseExample(ContentExample.spoilerRichHeaderAndContent); + testParseExample(ContentExample.spoilerHeaderHasImage); + group('track links inside block-inline containers', () { testParse('multiple links in paragraph', // "before[text](/there)mid[other](/else)after" diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 7653dcb55f..0df06c8929 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -15,6 +15,7 @@ import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/store.dart'; import '../example_data.dart' as eg; +import '../flutter_checks.dart'; import '../model/binding.dart'; import '../model/content_test.dart'; import '../model/test_store.dart'; @@ -93,6 +94,97 @@ void main() { }); }); + group('Spoiler', () { + testContentSmoke(ContentExample.spoilerDefaultHeader); + testContentSmoke(ContentExample.spoilerPlainCustomHeader); + testContentSmoke(ContentExample.spoilerRichHeaderAndContent); + + group('interactions: spoiler with tappable content (an image) in the header', () { + Future>> prepareContent(WidgetTester tester, String html) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + prepareBoringImageHttpClient(); + + final pushedRoutes = >[]; + final testNavObserver = TestNavigatorObserver() + ..onPushed = (route, prevRoute) => pushedRoutes.add(route); + + await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp( + localizationsDelegates: ZulipLocalizations.localizationsDelegates, + supportedLocales: ZulipLocalizations.supportedLocales, + navigatorObservers: [testNavObserver], + home: PerAccountStoreWidget(accountId: eg.selfAccount.id, + child: MessageContent( + message: eg.streamMessage(content: html), + content: parseContent(html)))))); + await tester.pump(); // global store + await tester.pump(); // per-account store + debugNetworkImageHttpClientProvider = null; + + // `tester.pumpWidget` introduces an initial route; + // remove it so consumers only have newly pushed routes. + assert(pushedRoutes.length == 1); + pushedRoutes.removeLast(); + return pushedRoutes; + } + + void checkIsExpanded(WidgetTester tester, + bool expected, { + Finder? contentFinder, + }) { + final sizeTransition = tester.widget(find.ancestor( + of: contentFinder ?? find.text('hello world'), + matching: find.byType(SizeTransition), + )); + check(sizeTransition.sizeFactor) + ..value.equals(expected ? 1 : 0) + ..status.equals(expected ? AnimationStatus.completed : AnimationStatus.dismissed); + } + + const example = ContentExample.spoilerHeaderHasImage; + + testWidgets('tap image', (tester) async { + final pushedRoutes = await prepareContent(tester, example.html); + + await tester.tap(find.byType(RealmContentNetworkImage)); + check(pushedRoutes).single.isA() + .fullscreenDialog.isTrue(); // recognize the lightbox + }); + + testWidgets('tap header on expand/collapse icon', (tester) async { + final pushedRoutes = await prepareContent(tester, example.html); + checkIsExpanded(tester, false); + + await tester.tap(find.byIcon(Icons.expand_more)); + await tester.pumpAndSettle(); + check(pushedRoutes).isEmpty(); // no lightbox + checkIsExpanded(tester, true); + + await tester.tap(find.byIcon(Icons.expand_more)); + await tester.pumpAndSettle(); + check(pushedRoutes).isEmpty(); // no lightbox + checkIsExpanded(tester, false); + }); + + testWidgets('tap header away from expand/collapse icon (and image)', (tester) async { + final pushedRoutes = await prepareContent(tester, example.html); + checkIsExpanded(tester, false); + + await tester.tapAt( + tester.getTopRight(find.byType(RealmContentNetworkImage)) + const Offset(10, 0)); + await tester.pumpAndSettle(); + check(pushedRoutes).isEmpty(); // no lightbox + checkIsExpanded(tester, true); + + await tester.tapAt( + tester.getTopRight(find.byType(RealmContentNetworkImage)) + const Offset(10, 0)); + await tester.pumpAndSettle(); + check(pushedRoutes).isEmpty(); // no lightbox + checkIsExpanded(tester, false); + }); + }); + }); + testContentSmoke(ContentExample.quotation); group('MessageImage, MessageImageList', () {