Skip to content
New issue

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

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

Already on GitHub? # to your account

MarkdownViewer link click behaviour #5223

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lowekoz
Copy link

@lowekoz lowekoz commented Nov 9, 2024

Running examples\markdown.py and clicking the example.md from the demo app will also open the relative file link ./example.md in the browser, which is a bit wonky behaviour. Relates to #5171 #5169, suggested fix by separating behaviour of external link protocols (http://,https://,ftp://...) and local resource path/anchor. With fix links won't trigger the browser open_url command, only external links will (if open_links=True), and external links won't trigger local navigation changes.

On approval TODOs:

  • Docstrings on all new or modified functions / classes
  • Updated documentation(x)
  • Updated CHANGELOG.md (where appropriate)
  • Tests (?)

@lowekoz
Copy link
Author

lowekoz commented Nov 29, 2024

cut.mp4

@lowekoz
Copy link
Author

lowekoz commented Nov 29, 2024

Our previous discussion on discord about the Markdown click behaviour revealed that there was a lot more detail to the matter than I expected. I am still determined to try contribute with a little something. I have therefore decided to limit the scope of this PR to primarily target the _markdown/MarkdownViewer class (not the Markdown class).

The Problem:

When a relative link is clicked in MarkdownViewer, the link also opens in the browser apart from in the viewer (see video above). One can fix this by setting open_links=False. However, doing so makes it no longer possible to open links that are clearly referencing an external resource (e.g. https://github.com). In addition, the MarkdownViewer does not seem to nativly support external links (crashes as it tries to read the link from disk)

The Fix:

I still think that the url.parse has potential use case here as default behaviour as per the previous commit but only for the MarkdownViewer. I fail to see where this might be problematic, what are your thoughts on this @willmcgugan ?

@lowekoz lowekoz changed the title DRAFT: fix Markdown link click behaviour MarkdownViewer link click behaviour Dec 2, 2024
@lowekoz
Copy link
Author

lowekoz commented Dec 4, 2024

A display of the problem without current fix (MarkdownViewer crashes on external link pressed)

wofix.mp4

no longer crashes on external link pressed
in the MarkdownViewer widget
@JonathanPlasse
Copy link

Is there anything blocking this?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants