Skip to content

msglist: Make links touchable, opening in browser #71

@gnprice

Description

@gnprice

Many Zulip messages have links in them. We should recognize when the user taps such a link, and follow the link for them.

For this issue, it's OK not to try to recognize links that are internal to the Zulip realm (that'll be #73); we can just open all links as generic URLs, in a browser.

The tricky parts here are about managing the GestureRecognizer objects for recognizing taps on the links.

First, there's a quirk in Flutter's API for using a gesture recognizer on text spans: see flutter/flutter#10623 and in particular flutter/flutter#10623 (comment) . I have a draft branch from back in December that deals with that; the key lines are this in _buildInlineNode:

   if (node is TextNode) {
-    return TextSpan(text: node.text);
+    return TextSpan(text: node.text, recognizer: recognizer);

and then recognizer is a parameter passed down recursively by _buildInlineNode and everything else that builds InlineSpans out of other InlineSpans.

Second, there's the question of where those recognizers should live as state. In the draft branch, we render a LinkNode with a StatefulWidget, which creates a recognizer in initState. That was good enough for a proof-of-concept of solving the first issue, but isn't the layout we want. Here's my notes from there on the way forward:

  } else if (node is LinkNode) {
    // TODO this shouldn't be a widget -- should participate in paragraph layout
    //   Likely this means: enclosing BlockContentNodeWidget becomes stateful;
    //   on initState, it walks the inline nodes to find links, and then creates
    //   appropriate recognizers; a data structure of those gets passed down
    //   through _buildInlineNode; when reaching a link here at this spot,
    //   we look up the appropriate recognizer.
    //
    //   We'd still have _buildInlineNode argument for the individual
    //   actually-applicable recognizer, in addition to the registry of them for
    //   the paragraph.
    //
    //   Perhaps instead of BlockContentNodeWidget being the home of the
    //   recognizers, it's its appropriate children/descendants which contain
    //   lists of inline nodes directly: so Paragraph, ListItem, Heading, etc.
    //   Then put the relevant logic in a mixin so those can all reuse it.
    return WidgetSpan(child: Link(node: node));

Better yet, walking the inline nodes to find links should happen within parseContent. From brief discussion later in chat:

We will later need to add some kind of context object that gets threaded through the recursion when parsing message content.

We'll […] need that in order to accumulate on a block node a list of the inline links inside it, in order to make links clickable (because the gesture handler needs to live on a widget).

Metadata

Metadata

Assignees

Labels

a-contentParsing and rendering Zulip HTML content, notably message contentsa-msglistThe message-list screen, except what's label:a-content

Type

No type

Projects

Status

Done

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions