Pull Request Description

Hey! After over 9 months, I am back with another attempt to add some visual indication when links are being tapped in markdown. This was inspired by #1529, and although it doesn't necessarily fix that issue, it does at least provide some visual or psychological improvement.

Because these changes might seem very odd, I want to describe the process I went through to arrive where I did. Hopefully it will all make sense in the end!

Why the first attempt didn't work

My first attempt at doing this (#869) did not work because of the way that custom builders work in relation to RichText and TextSpans. If your builder returns any widget that cannot be placed into the root widget, then the markdown builder will create a Wrap, with RichTexts on either side, and your widget in the middle. As a result, widget wrapping will take precedence over text wrapping. So if you are dealing with a long link that should wrap lines, the custom widget will always be placed on the new line in its entirety, rather than filling the available space on the previous line and wrapping text, because it's not part of the same overall block of text that the rest of the paragraph is. In order to avoid this behavior and allow all text to be wrapped properly, the LinkElementBuilder's visitElementAfterWithContext method MUST return widgets in this hierarchy: Text.rich -> TextSpan -> <the text itself>. Any other widget in this tree will cause wrapping issues.

The other key here was to ensure that I assigned the same textScaleFactor to my TextSpan that was assigned to the original MarkdownStyleSheet.

How to show touch feedback without any other widget in the tree

Now I had figured out the formatting problems, so the text looked perfectly fine with my custom builder. But I was stuck with a conundrum. While I could control the widget for each link, I couldn't add an InkWell or any other similar widget to visually indicate touches. The best I could do was change the TextStyle to add a background. It seemed like a pretty good compromise. It wouldn't have a ripple animation, and I couldn't make the corners nice and rounded, but at least there would be some visual indication. However, while I could set the background statically, I couldn't change it dynamically in response to the tap because I was dealing with all stateless widgets!

How can you change the background dynamically for a stateless widget

The solution here ended up being twofold. First, I had to make the CommonMarkdownBody stateful. But that wasn't enough. Even forcing it to rebuild didn't end up invoking visitElementAfterWithContext to give me a chance to recreate the text with a background. I had to go one step further and expose a forceParseMarkdown method in ExtendedMarkdownBody. With these two things (I double-checked that either one on its own isn't enough), I was able to rebuild the widget and re-parse the markdown in response to the touch!

How to identify which link to highlight

Now that I had the ability to highlight links and the ability to force the widgets to be regenerated in response to a touch, how could I know which link was being tapped? I had a TapGestureRecognizer associated with the first instance of the text widget, but when everything rebuilt, that association was lost. The only information that is provided in visitElementAfterWithContext is the context, the element, and the text styles. The element alone is not unique enough to identify it across builds. I can access the text and the URL, but what if someone puts the same URL twice in the same body. I don't want them both to be highlighted.

The solution here was to take advantage of the fact that Elements have an arbitrary dictionary of attributes. With this, I was able to assign a UniqueKey to each Element the first time I saw it. Then in the markdown parser, I added recursive code to transfer the attributes from the previously built Nodes to the new ones, every time it's parsed. Now in visitElementAfterWithContext, I can check the attributes, see that I recognize the UniqueKey, and see that it's associated with the tap event that happened on the previous instance of the widget.

Whew, that was a lot, I know! Now whether this is the best approach is highly debatable! But it's what I could come up with! If there is any better approach, or if something is added upstream, I will gladly defer.

I am also going to leave this PR as a draft while I daily drive this. I'm hoping to really put these changes through their paces before opening this up for review to find any potential edge cases that I missed.

Demo