From cf4c465d462e1d89030dce700ae7fc709b562cd7 Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Wed, 24 Jul 2024 12:41:10 +0530 Subject: [PATCH 1/3] content: Parse original image dimensions --- lib/model/content.dart | 43 ++++++++++++++++++- test/model/content_test.dart | 81 +++++++++++++++++++++++++++--------- 2 files changed, 102 insertions(+), 22 deletions(-) diff --git a/lib/model/content.dart b/lib/model/content.dart index cf2ebd28b5..e9ee7875ef 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -356,6 +356,8 @@ class ImageNode extends BlockContentNode { required this.srcUrl, required this.thumbnailUrl, required this.loading, + required this.originalWidth, + required this.originalHeight, }); /// The canonical source URL of the image. @@ -379,16 +381,25 @@ class ImageNode extends BlockContentNode { /// Typically it will be `true` while Server is generating thumbnails. final bool loading; + /// The width of the canonical image. + final double? originalWidth; + + /// The height of the canonical image. + final double? originalHeight; + @override bool operator ==(Object other) { return other is ImageNode && other.srcUrl == srcUrl && other.thumbnailUrl == thumbnailUrl - && other.loading == loading; + && other.loading == loading + && other.originalWidth == originalWidth + && other.originalHeight == originalHeight; } @override - int get hashCode => Object.hash('ImageNode', srcUrl, thumbnailUrl, loading); + int get hashCode => Object.hash('ImageNode', + srcUrl, thumbnailUrl, loading, originalWidth, originalHeight); @override void debugFillProperties(DiagnosticPropertiesBuilder properties) { @@ -396,6 +407,8 @@ class ImageNode extends BlockContentNode { properties.add(StringProperty('srcUrl', srcUrl)); properties.add(StringProperty('thumbnailUrl', thumbnailUrl)); properties.add(FlagProperty('loading', value: loading, ifTrue: "is loading")); + properties.add(DoubleProperty('originalWidth', originalWidth)); + properties.add(DoubleProperty('originalHeight', originalHeight)); } } @@ -1037,6 +1050,8 @@ class _ZulipContentParser { return CodeBlockNode(spans, debugHtmlNode: debugHtmlNode); } + static final _imageDimensionsRegExp = RegExp(r'^(\d+)x(\d+)$'); + BlockContentNode parseImageNode(dom.Element divElement) { assert(_debugParserContext == _ParserContext.block); final elements = () { @@ -1071,6 +1086,8 @@ class _ZulipContentParser { srcUrl: href, thumbnailUrl: null, loading: true, + originalWidth: null, + originalHeight: null, debugHtmlNode: debugHtmlNode); } final src = imgElement.attributes['src']; @@ -1093,10 +1110,32 @@ class _ZulipContentParser { } else { return UnimplementedBlockContentNode(htmlNode: divElement); } + + double? originalWidth, originalHeight; + final originalDimensions = imgElement.attributes['data-original-dimensions']; + if (originalDimensions != null) { + // Server encodes this string as "{width}x{height}" (eg. "300x400") + final match = _imageDimensionsRegExp.firstMatch(originalDimensions); + if (match != null) { + final width = int.tryParse(match.group(1)!, radix: 10); + final height = int.tryParse(match.group(2)!, radix: 10); + if (width != null && height != null) { + originalWidth = width.toDouble(); + originalHeight = height.toDouble(); + } + } + + if (originalWidth == null || originalHeight == null) { + return UnimplementedBlockContentNode(htmlNode: divElement); + } + } + return ImageNode( srcUrl: srcUrl, thumbnailUrl: thumbnailUrl, loading: false, + originalWidth: originalWidth, + originalHeight: originalHeight, debugHtmlNode: debugHtmlNode); } diff --git a/test/model/content_test.dart b/test/model/content_test.dart index 60a5698c99..0846273795 100644 --- a/test/model/content_test.dart +++ b/test/model/content_test.dart @@ -263,7 +263,8 @@ class ContentExample { ]), ImageNodeList([ ImageNode(srcUrl: 'https://chat.zulip.org/user_avatars/2/realm/icon.png?version=3', - thumbnailUrl: null, loading: false), + thumbnailUrl: null, loading: false, + originalWidth: null, originalHeight: null), ]), ], content: [ParagraphNode(links: null, nodes: [TextNode('hello world')])], @@ -421,6 +422,22 @@ class ContentExample { static const imageSingle = ContentExample( 'single image', + // https://chat.zulip.org/#narrow/stream/7-test-here/topic/Thumbnails/near/1900103 + "[image.jpg](/user_uploads/2/ce/nvoNL2LaZOciwGZ-FYagddtK/image.jpg)", + '
' + '' + '
', [ + ImageNodeList([ + ImageNode(srcUrl: '/user_uploads/2/ce/nvoNL2LaZOciwGZ-FYagddtK/image.jpg', + thumbnailUrl: '/user_uploads/thumbnail/2/ce/nvoNL2LaZOciwGZ-FYagddtK/image.jpg/840x560.webp', + loading: false, + originalWidth: 6000, + originalHeight: 4000), + ]), + ]); + + static const imageSingleNoDimensions = ContentExample( + 'single image no dimensions', // https://chat.zulip.org/#narrow/stream/7-test-here/topic/Thumbnails/near/1893590 "[image.jpg](/user_uploads/2/c3/wb9FXk8Ej6qIc28aWKcqUogD/image.jpg)", '
' @@ -429,7 +446,9 @@ class ContentExample { ImageNodeList([ ImageNode(srcUrl: '/user_uploads/2/c3/wb9FXk8Ej6qIc28aWKcqUogD/image.jpg', thumbnailUrl: '/user_uploads/thumbnail/2/c3/wb9FXk8Ej6qIc28aWKcqUogD/image.jpg/840x560.webp', - loading: false), + loading: false, + originalWidth: null, + originalHeight: null), ]), ]); @@ -441,7 +460,8 @@ class ContentExample { '
', [ ImageNodeList([ ImageNode(srcUrl: 'https://chat.zulip.org/user_avatars/2/realm/icon.png?version=3', - thumbnailUrl: null, loading: false), + thumbnailUrl: null, loading: false, + originalWidth: null, originalHeight: null), ]), ]); @@ -454,7 +474,8 @@ class ContentExample { '', [ ImageNodeList([ ImageNode(srcUrl: '/user_uploads/2/c3/wb9FXk8Ej6qIc28aWKcqUogD/image.jpg', - thumbnailUrl: null, loading: true), + thumbnailUrl: null, loading: true, + originalWidth: null, originalHeight: null), ]), ]); @@ -467,7 +488,8 @@ class ContentExample { '', [ ImageNodeList([ ImageNode(srcUrl: '/external_content/de28eb3abf4b7786de4545023dc42d434a2ea0c2/68747470733a2f2f75706c6f61642e77696b696d656469612e6f72672f77696b6970656469612f636f6d6d6f6e732f372f37382f566572726567656e64655f626c6f656d5f76616e5f65656e5f48656c656e69756d5f253237456c5f446f7261646f2532372e5f32322d30372d323032332e5f253238642e6a2e622532392e6a7067', - thumbnailUrl: null, loading: false), + thumbnailUrl: null, loading: false, + originalWidth: null, originalHeight: null), ]), ]); @@ -478,7 +500,9 @@ class ContentExample { '' '', [ ImageNodeList([ - ImageNode(srcUrl: '::not a URL::', thumbnailUrl: null, loading: false), + ImageNode(srcUrl: '::not a URL::', + thumbnailUrl: null, loading: false, + originalWidth: null, originalHeight: null), ]), ]); @@ -504,10 +528,14 @@ class ContentExample { ImageNodeList([ ImageNode(srcUrl: '/user_uploads/2/9b/WkDt2Qsy79iwf3sM9EMp9fYL/image.jpg', thumbnailUrl: '/user_uploads/thumbnail/2/9b/WkDt2Qsy79iwf3sM9EMp9fYL/image.jpg/840x560.webp', - loading: false), + loading: false, + originalWidth: null, + originalHeight: null), ImageNode(srcUrl: '/user_uploads/2/70/pVeI52TwFUEoFE2qT_u9AMCO/image2.jpg', thumbnailUrl: '/user_uploads/thumbnail/2/70/pVeI52TwFUEoFE2qT_u9AMCO/image2.jpg/840x560.webp', - loading: false), + loading: false, + originalWidth: null, + originalHeight: null), ]), ]); @@ -531,9 +559,11 @@ class ContentExample { ]), ImageNodeList([ ImageNode(srcUrl: 'https://uploads.zulipusercontent.net/f535ba07f95b99a83aa48e44fd62bbb6c6cf6615/68747470733a2f2f636861742e7a756c69702e6f72672f757365725f617661746172732f322f7265616c6d2f69636f6e2e706e673f76657273696f6e3d33', - thumbnailUrl: null, loading: false), + thumbnailUrl: null, loading: false, + originalWidth: null, originalHeight: null), ImageNode(srcUrl: 'https://uploads.zulipusercontent.net/8f63bc2632a0e41be3f457d86c077e61b4a03e7e/68747470733a2f2f636861742e7a756c69702e6f72672f757365725f617661746172732f322f7265616c6d2f69636f6e2e706e673f76657273696f6e3d34', - thumbnailUrl: null, loading: false), + thumbnailUrl: null, loading: false, + originalWidth: null, originalHeight: null), ]), ]); @@ -558,9 +588,11 @@ class ContentExample { ]), ImageNodeList([ ImageNode(srcUrl: 'https://chat.zulip.org/user_avatars/2/realm/icon.png', - thumbnailUrl: null, loading: false), + thumbnailUrl: null, loading: false, + originalWidth: null, originalHeight: null), ImageNode(srcUrl: 'https://chat.zulip.org/user_avatars/2/realm/icon.png?version=2', - thumbnailUrl: null, loading: false), + thumbnailUrl: null, loading: false, + originalWidth: null, originalHeight: null), ]), ParagraphNode(links: null, nodes: [ TextNode('more content'), @@ -596,9 +628,11 @@ class ContentExample { ]), ImageNodeList([ ImageNode(srcUrl: 'https://uploads.zulipusercontent.net/34b2695ca83af76204b0b25a8f2019ee35ec38fa/68747470733a2f2f656e2e77696b6970656469612e6f72672f7374617469632f696d616765732f69636f6e732f77696b6970656469612e706e67', - thumbnailUrl: null, loading: false), + thumbnailUrl: null, loading: false, + originalWidth: null, originalHeight: null), ImageNode(srcUrl: 'https://uploads.zulipusercontent.net/d200fb112aaccbff9df767373a201fa59601f362/68747470733a2f2f656e2e77696b6970656469612e6f72672f7374617469632f696d616765732f69636f6e732f77696b6970656469612e706e673f763d31', - thumbnailUrl: null, loading: false), + thumbnailUrl: null, loading: false, + originalWidth: null, originalHeight: null), ]), ParagraphNode(links: null, nodes: [ TextNode('Test'), @@ -611,9 +645,11 @@ class ContentExample { ]), ImageNodeList([ ImageNode(srcUrl: 'https://uploads.zulipusercontent.net/c4db87e81348dac94eacaa966b46d968b34029cc/68747470733a2f2f656e2e77696b6970656469612e6f72672f7374617469632f696d616765732f69636f6e732f77696b6970656469612e706e673f763d32', - thumbnailUrl: null, loading: false), + thumbnailUrl: null, loading: false, + originalWidth: null, originalHeight: null), ImageNode(srcUrl: 'https://uploads.zulipusercontent.net/51b70540cf6a5b3c8a0b919c893b8abddd447e88/68747470733a2f2f656e2e77696b6970656469612e6f72672f7374617469632f696d616765732f69636f6e732f77696b6970656469612e706e673f763d33', - thumbnailUrl: null, loading: false), + thumbnailUrl: null, loading: false, + originalWidth: null, originalHeight: null), ]), ]); @@ -628,7 +664,8 @@ class ContentExample { ListNode(ListStyle.unordered, [[ ImageNodeList([ ImageNode(srcUrl: 'https://chat.zulip.org/user_avatars/2/realm/icon.png', - thumbnailUrl: null, loading: false), + thumbnailUrl: null, loading: false, + originalWidth: null, originalHeight: null), ]), ]]), ]); @@ -654,9 +691,11 @@ class ContentExample { ]), ImageNodeList([ ImageNode(srcUrl: 'https://chat.zulip.org/user_avatars/2/realm/icon.png', - thumbnailUrl: null, loading: false), + thumbnailUrl: null, loading: false, + originalWidth: null, originalHeight: null), ImageNode(srcUrl: 'https://chat.zulip.org/user_avatars/2/realm/icon.png?version=2', - thumbnailUrl: null, loading: false), + thumbnailUrl: null, loading: false, + originalWidth: null, originalHeight: null), ]), ]]), ]); @@ -680,7 +719,8 @@ class ContentExample { ]), const ImageNodeList([ ImageNode(srcUrl: 'https://chat.zulip.org/user_avatars/2/realm/icon.png', - thumbnailUrl: null, loading: false), + thumbnailUrl: null, loading: false, + originalWidth: null, originalHeight: null), ]), blockUnimplemented('more text'), ]]), @@ -1117,6 +1157,7 @@ void main() { testParseExample(ContentExample.mathBlockInQuote); testParseExample(ContentExample.imageSingle); + testParseExample(ContentExample.imageSingleNoDimensions); testParseExample(ContentExample.imageSingleNoThumbnail); testParseExample(ContentExample.imageSingleLoadingPlaceholder); testParseExample(ContentExample.imageSingleExternal); From 4ef597389aa20ca5046e3f98e77008f8252b63ff Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Wed, 24 Jul 2024 12:43:53 +0530 Subject: [PATCH 2/3] lightbox [nfc]: Split getLightboxRoute for images vs. videos --- lib/widgets/content.dart | 11 +++--- lib/widgets/lightbox.dart | 64 +++++++++++++++++++++------------ test/widgets/lightbox_test.dart | 3 +- 3 files changed, 46 insertions(+), 32 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 795e1389e2..ac3b188911 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -572,12 +572,11 @@ class MessageImage extends StatelessWidget { return MessageMediaContainer( onTap: resolvedSrcUrl == null ? null : () { // TODO(log) - Navigator.of(context).push(getLightboxRoute( + Navigator.of(context).push(getImageLightboxRoute( context: context, message: message, src: resolvedSrcUrl, - thumbnailUrl: resolvedThumbnailUrl, - mediaType: MediaType.image)); + thumbnailUrl: resolvedThumbnailUrl)); }, child: node.loading ? const CupertinoActivityIndicator() @@ -603,12 +602,10 @@ class MessageInlineVideo extends StatelessWidget { return MessageMediaContainer( onTap: resolvedSrc == null ? null : () { // TODO(log) - Navigator.of(context).push(getLightboxRoute( + Navigator.of(context).push(getVideoLightboxRoute( context: context, message: message, - src: resolvedSrc, - thumbnailUrl: null, - mediaType: MediaType.video)); + src: resolvedSrc)); }, child: Container( color: Colors.black, // Web has the same color in light and dark mode. diff --git a/lib/widgets/lightbox.dart b/lib/widgets/lightbox.dart index 33ed3f68de..418cd16037 100644 --- a/lib/widgets/lightbox.dart +++ b/lib/widgets/lightbox.dart @@ -536,18 +536,10 @@ class _VideoLightboxPageState extends State with PerAccountSt } } -enum MediaType { - video, - image -} - -Route getLightboxRoute({ - int? accountId, - BuildContext? context, - required Message message, - required Uri src, - required Uri? thumbnailUrl, - required MediaType mediaType, +Route _getLightboxRoute({ + required int? accountId, + required BuildContext? context, + required RoutePageBuilder pageBuilder, }) { return AccountPageRouteBuilder( accountId: accountId, @@ -559,17 +551,7 @@ Route getLightboxRoute({ Animation secondaryAnimation, ) { // TODO(#40): Drag down to close? - return switch (mediaType) { - MediaType.image => _ImageLightboxPage( - routeEntranceAnimation: animation, - message: message, - src: src, - thumbnailUrl: thumbnailUrl), - MediaType.video => VideoLightboxPage( - routeEntranceAnimation: animation, - message: message, - src: src), - }; + return pageBuilder(context, animation, secondaryAnimation); }, transitionsBuilder: ( BuildContext context, @@ -583,3 +565,39 @@ Route getLightboxRoute({ }, ); } + +Route getImageLightboxRoute({ + int? accountId, + BuildContext? context, + required Message message, + required Uri src, + required Uri? thumbnailUrl, +}) { + return _getLightboxRoute( + accountId: accountId, + context: context, + pageBuilder: (context, animation, secondaryAnimation) { + return _ImageLightboxPage( + routeEntranceAnimation: animation, + message: message, + src: src, + thumbnailUrl: thumbnailUrl); + }); +} + +Route getVideoLightboxRoute({ + int? accountId, + BuildContext? context, + required Message message, + required Uri src, +}) { + return _getLightboxRoute( + accountId: accountId, + context: context, + pageBuilder: (context, animation, secondaryAnimation) { + return VideoLightboxPage( + routeEntranceAnimation: animation, + message: message, + src: src); + }); +} diff --git a/test/widgets/lightbox_test.dart b/test/widgets/lightbox_test.dart index 23bb15b6b9..6d653a9740 100644 --- a/test/widgets/lightbox_test.dart +++ b/test/widgets/lightbox_test.dart @@ -211,12 +211,11 @@ void main() { await tester.pumpWidget(const ZulipApp()); await tester.pump(); final navigator = await ZulipApp.navigator; - navigator.push(getLightboxRoute( + navigator.push(getImageLightboxRoute( accountId: eg.selfAccount.id, message: message ?? eg.streamMessage(), src: src, thumbnailUrl: thumbnailUrl, - mediaType: MediaType.image, )); await tester.pump(); // per-account store await tester.pump(const Duration(milliseconds: 301)); // nav transition From 4efcaa7c7094ae64adb536f8790b663515395d9a Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Wed, 24 Jul 2024 13:10:32 +0530 Subject: [PATCH 3/3] lightbox: Display thumbnail with original image sizing Fixes: #830 --- lib/widgets/content.dart | 4 +++- lib/widgets/lightbox.dart | 26 ++++++++++++++++++-------- test/widgets/lightbox_test.dart | 2 ++ 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index ac3b188911..7b567b74fb 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -576,7 +576,9 @@ class MessageImage extends StatelessWidget { context: context, message: message, src: resolvedSrcUrl, - thumbnailUrl: resolvedThumbnailUrl)); + thumbnailUrl: resolvedThumbnailUrl, + originalWidth: node.originalWidth, + originalHeight: node.originalHeight)); }, child: node.loading ? const CupertinoActivityIndicator() diff --git a/lib/widgets/lightbox.dart b/lib/widgets/lightbox.dart index 418cd16037..dffe16dfbb 100644 --- a/lib/widgets/lightbox.dart +++ b/lib/widgets/lightbox.dart @@ -217,12 +217,16 @@ class _ImageLightboxPage extends StatefulWidget { required this.message, required this.src, required this.thumbnailUrl, + required this.originalWidth, + required this.originalHeight, }); final Animation routeEntranceAnimation; final Message message; final Uri src; final Uri? thumbnailUrl; + final double? originalWidth; + final double? originalHeight; @override State<_ImageLightboxPage> createState() => _ImageLightboxPageState(); @@ -259,8 +263,14 @@ class _ImageLightboxPageState extends State<_ImageLightboxPage> { if (frame != null) return child; // Display the thumbnail image while original image is downloading. - return RealmContentNetworkImage(widget.thumbnailUrl!, - filterQuality: FilterQuality.medium); + return FittedBox( + fit: BoxFit.scaleDown, + child: SizedBox( + width: widget.originalWidth, + height: widget.originalHeight, + child: RealmContentNetworkImage(widget.thumbnailUrl!, + filterQuality: FilterQuality.medium, + fit: BoxFit.contain))); } Widget _loadingBuilder(BuildContext context, Widget child, ImageChunkEvent? loadingProgress) { @@ -301,11 +311,7 @@ class _ImageLightboxPageState extends State<_ImageLightboxPage> { child: RealmContentNetworkImage(widget.src, filterQuality: FilterQuality.medium, frameBuilder: _frameBuilder, - loadingBuilder: _loadingBuilder), - ), - ), - ), - )); + loadingBuilder: _loadingBuilder)))))); } } @@ -572,6 +578,8 @@ Route getImageLightboxRoute({ required Message message, required Uri src, required Uri? thumbnailUrl, + required double? originalWidth, + required double? originalHeight, }) { return _getLightboxRoute( accountId: accountId, @@ -581,7 +589,9 @@ Route getImageLightboxRoute({ routeEntranceAnimation: animation, message: message, src: src, - thumbnailUrl: thumbnailUrl); + thumbnailUrl: thumbnailUrl, + originalWidth: originalWidth, + originalHeight: originalHeight); }); } diff --git a/test/widgets/lightbox_test.dart b/test/widgets/lightbox_test.dart index 6d653a9740..419690f788 100644 --- a/test/widgets/lightbox_test.dart +++ b/test/widgets/lightbox_test.dart @@ -216,6 +216,8 @@ void main() { message: message ?? eg.streamMessage(), src: src, thumbnailUrl: thumbnailUrl, + originalHeight: null, + originalWidth: null, )); await tester.pump(); // per-account store await tester.pump(const Duration(milliseconds: 301)); // nav transition