Skip to content
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.

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

Fixes for edge cases with markdown link parsing #898

Merged
merged 4 commits into from
Nov 17, 2023

Conversation

hjiangsu
Copy link
Member

@hjiangsu hjiangsu commented Nov 16, 2023

Pull Request Description

This PR fixes the issues mentioned in #896 and changes the underlying logic that enables long-press actions for markdown links. The fix for this ended up being more complicated than initially thought due to some of the behaviours of custom markdown elements.

Duplicate markdown text
I was not able to fully narrow down the cause of this issue. However, my assumption revolves around how MarkdownElementBuilder handles links and text. This could be due to the case where [some text](somelink) gets split up into multiple elements, and causes some weird behaviours when attempting to render the widget.

Smaller font sizes for links
While MarkdownElementBuilder does pass down the text styles down, RichText requires TextStyles to be explicitly set. Furthermore, surrounding the text with an InkWell causes the text to shrink slightly.

Markdown links are sometimes missing
This is also a weird issue that I could not fully track down. It seems like its a combination of how RichText, TextSpan, InkWell, and WidgetSpan interact with one another. It seems like some combinations

Resolution
To resolve these issues, and some other issues (WidgetSpans not allowing long links to be soft-wrapped, causing them to show up on a new line), I've created some new classes which extend the behaviour of MarkdownBody and MarkdownWidget.

I've extended their behaviours to add a onLongPressLink callback function which acts similar to onTapLink. The one caveat so far is that the InkWell is no longer present on links, but that can be addressed in the future as that feature is less critical than the link parsing.

Additionally, I've fixed #897 and I've added an InlineExtension to properly highlight Lemmy links: LemmyLinkSyntax. This portion of the code is originally from Liftoff, so I'll give credits to that!

@micahmo - I'm just cc-ing you in this PR so that you're aware of the changes to the logic! No need to do any reviews or testing 😁

Issue Being Fixed

Issue Number: #896, #897

Screenshots / Recordings

image image

Checklist

  • Did you update CHANGELOG.md?
  • Did you use localized strings where applicable?
  • Did you add semanticLabels where applicable for accessibility?

@hjiangsu hjiangsu merged commit 0bc0623 into develop Nov 17, 2023
1 check passed
@hjiangsu hjiangsu deleted the fix/markdown-link-parsing branch November 17, 2023 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Comment Content Font Scale setting non-functional
1 participant