Skip to content

content: Handle link previews #1049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 26, 2025
Merged

content: Handle link previews #1049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1929da2
content [nfc]: Move link color to `ContentTheme.colorLink`
rajveermalviya Jan 20, 2025
08dc485
content: Handle `message_embed` website previews
rajveermalviya Feb 19, 2025
e0df0ed
content: Handle legacy website previews
rajveermalviya Feb 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 163 additions & 0 deletions lib/model/content.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,58 @@ class EmbedVideoNode extends BlockContentNode {
}
}

// See:
// https://ogp.me/
// https://oembed.com/
// https://zulip.com/help/image-video-and-website-previews#configure-whether-website-previews-are-shown
class WebsitePreviewNode extends BlockContentNode {
const WebsitePreviewNode({
super.debugHtmlNode,
required this.hrefUrl,
required this.imageSrcUrl,
required this.title,
required this.description,
});

/// The URL from which this preview data was retrieved.
final String hrefUrl;

/// The image URL representing the webpage, content value
/// of `og:image` HTML meta property.
final String imageSrcUrl;

/// Represents the webpage title, derived from either
/// the content of the `og:title` HTML meta property or
/// the <title> HTML element.
final String? title;

/// Description about the webpage, content value of
/// `og:description` HTML meta property.
final String? description;

@override
bool operator ==(Object other) {
return other is WebsitePreviewNode
&& other.hrefUrl == hrefUrl
&& other.imageSrcUrl == imageSrcUrl
&& other.title == title
&& other.description == description;
}

@override
int get hashCode =>
Object.hash('WebsitePreviewNode', hrefUrl, imageSrcUrl, title, description);

@override
void debugFillProperties(DiagnosticPropertiesBuilder properties) {
super.debugFillProperties(properties);
properties.add(StringProperty('hrefUrl', hrefUrl));
properties.add(StringProperty('imageSrcUrl', imageSrcUrl));
properties.add(StringProperty('title', title));
properties.add(StringProperty('description', description));
}
}

class TableNode extends BlockContentNode {
const TableNode({super.debugHtmlNode, required this.rows});

Expand Down Expand Up @@ -1339,6 +1391,113 @@ class _ZulipContentParser {
return EmbedVideoNode(hrefUrl: href, previewImageSrcUrl: imgSrc, debugHtmlNode: debugHtmlNode);
}

static final _websitePreviewImageSrcRegexp = RegExp(r'background-image: url\(("?)(.+?)\1\)');

BlockContentNode parseWebsitePreviewNode(dom.Element divElement) {
assert(divElement.localName == 'div'
&& divElement.className == 'message_embed');

final debugHtmlNode = kDebugMode ? divElement : null;
final result = () {
if (divElement.nodes case [
dom.Element(
localName: 'a',
className: 'message_embed_image',
attributes: {
'href': final String imageHref,
'style': final String imageStyleAttr,
},
nodes: []),
dom.Element(
localName: 'div',
className: 'data-container',
nodes: [...]) && final dataContainer,
]) {
final match = _websitePreviewImageSrcRegexp.firstMatch(imageStyleAttr);
if (match == null) return null;
final imageSrcUrl = match.group(2);
if (imageSrcUrl == null) return null;

String? parseTitle(dom.Element element) {
assert(element.localName == 'div' &&
element.className == 'message_embed_title');
if (element.nodes case [
dom.Element(localName: 'a', className: '') && final child,
]) {
final titleHref = child.attributes['href'];
// Make sure both image hyperlink and title hyperlink are same.
if (imageHref != titleHref) return null;

if (child.nodes case [dom.Text(text: final title)]) {
return title;
}
}
return null;
}

String? parseDescription(dom.Element element) {
assert(element.localName == 'div' &&
element.className == 'message_embed_description');
if (element.nodes case [dom.Text(text: final description)]) {
return description;
}
return null;
}

String? title, description;
switch (dataContainer.nodes) {
case [
dom.Element(
localName: 'div',
className: 'message_embed_title') && final first,
dom.Element(
localName: 'div',
className: 'message_embed_description') && final second,
]:
title = parseTitle(first);
if (title == null) return null;
description = parseDescription(second);
if (description == null) return null;

case [dom.Element(localName: 'div') && final single]:
switch (single.className) {
case 'message_embed_title':
title = parseTitle(single);
if (title == null) return null;

case 'message_embed_description':
description = parseDescription(single);
if (description == null) return null;

default:
return null;
}

case []:
// Server generates an empty `<div class="data-container"></div>`
// if website HTML has neither title (derived from
// `og:title` or `<title>…</title>`) nor description (derived from
// `og:description`).
break;

default:
return null;
}

return WebsitePreviewNode(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to pass debugHtmlNode — I noticed that while doing some debugging to follow up on #1049 (comment) 🙂

(The issue turned out to be that I'd done only half of the edits I suggested in the backreference comment above. Should make sure we support debugging, though.)

hrefUrl: imageHref,
imageSrcUrl: imageSrcUrl,
title: title,
description: description,
debugHtmlNode: debugHtmlNode);
} else {
return null;
}
}();

return result ?? UnimplementedBlockContentNode(htmlNode: divElement);
}

BlockContentNode parseTableContent(dom.Element tableElement) {
assert(tableElement.localName == 'table'
&& tableElement.className.isEmpty);
Expand Down Expand Up @@ -1583,6 +1742,10 @@ class _ZulipContentParser {
}
}

if (localName == 'div' && className == 'message_embed') {
return parseWebsitePreviewNode(element);
}

// TODO more types of node
return UnimplementedBlockContentNode(htmlNode: node);
}
Expand Down
110 changes: 108 additions & 2 deletions lib/widgets/content.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import 'package:flutter/cupertino.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/gestures.dart';
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart';
import 'package:html/dom.dart' as dom;
import 'package:intl/intl.dart';
Expand All @@ -18,6 +19,7 @@ import '../model/internal_link.dart';
import 'code_block.dart';
import 'dialog.dart';
import 'icons.dart';
import 'inset_shadow.dart';
import 'lightbox.dart';
import 'message_list.dart';
import 'poll.dart';
Expand All @@ -42,6 +44,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> {
colorDirectMentionBackground: const HSLColor.fromAHSL(0.2, 240, 0.7, 0.7).toColor(),
colorGlobalTimeBackground: const HSLColor.fromAHSL(1, 0, 0, 0.93).toColor(),
colorGlobalTimeBorder: const HSLColor.fromAHSL(1, 0, 0, 0.8).toColor(),
colorLink: const HSLColor.fromAHSL(1, 200, 1, 0.4).toColor(),
colorMathBlockBorder: const HSLColor.fromAHSL(0.15, 240, 0.8, 0.5).toColor(),
colorMessageMediaContainerBackground: const Color.fromRGBO(0, 0, 0, 0.03),
colorPollNames: const HSLColor.fromAHSL(1, 0, 0, .45).toColor(),
Expand Down Expand Up @@ -75,6 +78,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> {
colorDirectMentionBackground: const HSLColor.fromAHSL(0.25, 240, 0.52, 0.6).toColor(),
colorGlobalTimeBackground: const HSLColor.fromAHSL(0.2, 0, 0, 0).toColor(),
colorGlobalTimeBorder: const HSLColor.fromAHSL(0.4, 0, 0, 0).toColor(),
colorLink: const HSLColor.fromAHSL(1, 200, 1, 0.4).toColor(), // the same as light in Web
colorMathBlockBorder: const HSLColor.fromAHSL(1, 240, 0.4, 0.4).toColor(),
colorMessageMediaContainerBackground: const HSLColor.fromAHSL(0.03, 0, 0, 1).toColor(),
colorPollNames: const HSLColor.fromAHSL(1, 236, .15, .7).toColor(),
Expand Down Expand Up @@ -107,6 +111,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> {
required this.colorDirectMentionBackground,
required this.colorGlobalTimeBackground,
required this.colorGlobalTimeBorder,
required this.colorLink,
required this.colorMathBlockBorder,
required this.colorMessageMediaContainerBackground,
required this.colorPollNames,
Expand Down Expand Up @@ -139,6 +144,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> {
final Color colorDirectMentionBackground;
final Color colorGlobalTimeBackground;
final Color colorGlobalTimeBorder;
final Color colorLink;
final Color colorMathBlockBorder; // TODO(#46) this won't be needed
final Color colorMessageMediaContainerBackground;
final Color colorPollNames;
Expand Down Expand Up @@ -199,6 +205,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> {
Color? colorDirectMentionBackground,
Color? colorGlobalTimeBackground,
Color? colorGlobalTimeBorder,
Color? colorLink,
Color? colorMathBlockBorder,
Color? colorMessageMediaContainerBackground,
Color? colorPollNames,
Expand All @@ -221,6 +228,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> {
colorDirectMentionBackground: colorDirectMentionBackground ?? this.colorDirectMentionBackground,
colorGlobalTimeBackground: colorGlobalTimeBackground ?? this.colorGlobalTimeBackground,
colorGlobalTimeBorder: colorGlobalTimeBorder ?? this.colorGlobalTimeBorder,
colorLink: colorLink ?? this.colorLink,
colorMathBlockBorder: colorMathBlockBorder ?? this.colorMathBlockBorder,
colorMessageMediaContainerBackground: colorMessageMediaContainerBackground ?? this.colorMessageMediaContainerBackground,
colorPollNames: colorPollNames ?? this.colorPollNames,
Expand Down Expand Up @@ -250,6 +258,7 @@ class ContentTheme extends ThemeExtension<ContentTheme> {
colorDirectMentionBackground: Color.lerp(colorDirectMentionBackground, other.colorDirectMentionBackground, t)!,
colorGlobalTimeBackground: Color.lerp(colorGlobalTimeBackground, other.colorGlobalTimeBackground, t)!,
colorGlobalTimeBorder: Color.lerp(colorGlobalTimeBorder, other.colorGlobalTimeBorder, t)!,
colorLink: Color.lerp(colorLink, other.colorLink, t)!,
colorMathBlockBorder: Color.lerp(colorMathBlockBorder, other.colorMathBlockBorder, t)!,
colorMessageMediaContainerBackground: Color.lerp(colorMessageMediaContainerBackground, other.colorMessageMediaContainerBackground, t)!,
colorPollNames: Color.lerp(colorPollNames, other.colorPollNames, t)!,
Expand Down Expand Up @@ -364,6 +373,7 @@ class BlockContentList extends StatelessWidget {
);
return const SizedBox.shrink();
}(),
WebsitePreviewNode() => WebsitePreview(node: node),
UnimplementedBlockContentNode() =>
Text.rich(_errorUnimplemented(node, context: context)),
};
Expand Down Expand Up @@ -839,6 +849,103 @@ class MathBlock extends StatelessWidget {
}
}

class WebsitePreview extends StatelessWidget {
const WebsitePreview({super.key, required this.node});

final WebsitePreviewNode node;

@override
Widget build(BuildContext context) {
final store = PerAccountStoreWidget.of(context);
final resolvedImageSrcUrl = store.tryResolveUrl(node.imageSrcUrl);
final isSmallWidth = MediaQuery.sizeOf(context).width <= 576;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

576 is from the web app, right? And some other explicit width and height values below: 500, 80, 115, etc.

We could comment on each one, saying they come from the web app. But actually, I could imagine future design work where we want to tune these numbers to be different from the web app. In that case such comments would become wrong/misleading if we forgot to update them. So maybe best not.

To memoize the fact that they match web, though (so a reader doesn't have to check each one), let's mention it in the commit message.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love using MediaQuery.sizeOf here — it feels like it's inevitably asking the wrong question. (Instead of the width of the entire app's viewport, it'd be much more to the point to specify this design in terms of this widget's own width, which will have been dictated by its parent.)

But this is fine, because I think implementing this design, with the way it flips between wide and tall forms, in that cleaner way would require either (a) LayoutBuilder, which is significantly less clean in other ways, or (b) significantly more work. And the exact design of this feature isn't something I'd want us to spend a lot of time on.


// On Web on larger width viewports, the title and description container's
// width is constrained using `max-width: calc(100% - 115px)`, we do not
// follow the same here for potential benefits listed here:
// https://github.com/zulip/zulip-flutter/pull/1049#discussion_r1915740997
final titleAndDescription = Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: [
if (node.title != null)
GestureDetector(
onTap: () => _launchUrl(context, node.hrefUrl),
child: Text(node.title!,
style: TextStyle(
fontSize: 1.2 * kBaseFontSize,
// Web uses `line-height: normal` for title. MDN docs for it:
// https://developer.mozilla.org/en-US/docs/Web/CSS/line-height#normal
// says actual value depends on user-agent, and default value
// can be roughly 1.2 (unitless). So, use the same here.
height: 1.2,
color: ContentTheme.of(context).colorLink))),
if (node.description != null)
Container(
padding: const EdgeInsets.only(top: 3),
constraints: const BoxConstraints(maxWidth: 500),
child: Text(node.description!)),
]);

final clippedTitleAndDescription = Padding(
padding: const EdgeInsets.symmetric(horizontal: 5),
child: InsetShadowBox(
bottom: 8,
// TODO(#488) use different color for non-message contexts
// TODO(#647) use different color for highlighted messages
// TODO(#681) use different color for DM messages
color: MessageListTheme.of(context).streamMessageBgDefault,
child: ClipRect(
child: ConstrainedBox(
constraints: BoxConstraints(maxHeight: 80),
child: OverflowBox(
maxHeight: double.infinity,
alignment: AlignmentDirectional.topStart,
fit: OverflowBoxFit.deferToChild,
child: Padding(
padding: const EdgeInsets.only(bottom: 8),
child: titleAndDescription))))));

final image = resolvedImageSrcUrl == null ? null
: GestureDetector(
onTap: () => _launchUrl(context, node.hrefUrl),
child: RealmContentNetworkImage(
resolvedImageSrcUrl,
fit: BoxFit.cover));

final result = isSmallWidth
? Column(
crossAxisAlignment: CrossAxisAlignment.start,
spacing: 15,
children: [
if (image != null)
SizedBox(height: 110, width: double.infinity, child: image),
clippedTitleAndDescription,
])
: Row(crossAxisAlignment: CrossAxisAlignment.start, children: [
if (image != null)
SizedBox.square(dimension: 80, child: image),
Flexible(child: clippedTitleAndDescription),
]);

return Padding(
// TODO(?) Web has a bottom margin `--markdown-interelement-space-px`
// around the `message_embed` container, which is calculated here:
// https://github.com/zulip/zulip/blob/d28f7d86223bab4f11629637d4237381943f6fc1/web/src/information_density.ts#L80-L102
// But for now we use a static value of 6.72px instead which is the
// default in the web client, see discussion:
// https://github.com/zulip/zulip-flutter/pull/1049#discussion_r1915747908
padding: const EdgeInsets.only(bottom: 6.72),
child: Container(
height: !isSmallWidth ? 90 : null,
decoration: const BoxDecoration(
border: BorderDirectional(start: BorderSide(
// Web has the same color in light and dark mode.
color: Color(0xffededed), width: 3))),
padding: const EdgeInsets.all(5),
child: result));
}
}

//
// Inline layout.
//
Expand Down Expand Up @@ -1029,8 +1136,7 @@ class _InlineContentBuilder {
assert(recognizer != null);
_pushRecognizer(recognizer);
final result = _buildNodes(node.nodes,
// Web has the same color in light and dark mode.
style: TextStyle(color: const HSLColor.fromAHSL(1, 200, 1, 0.4).toColor()));
style: TextStyle(color: ContentTheme.of(_context!).colorLink));
_popRecognizer();
return result;

Expand Down
Loading