-
Notifications
You must be signed in to change notification settings - Fork 255
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
[Super Editor] - Make emails clickable with "mailto:", and linkify app URLs like "obsidian://" (Resolves #2426, Resolves #2427) #2493
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@miguelcmedeiros can you please give this PR a try for your relevant use-cases? |
angelosilvestre
approved these changes
Jan 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with some comments.
super_editor/lib/src/default_editor/default_document_editor_reactions.dart
Outdated
Show resolved
Hide resolved
miguelcmedeiros
approved these changes
Jan 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[Super Editor] - Make emails clickable with "mailto:", and linkify app URLs like "obsidian://" (Resolves #2426, Resolves #2427)
I added some nuance to how we store info in
LinkAttribution
. We now store a plain text version of a URI and (if provided) a structuredUri
object.The reason we need to support plain-text URIs is because content might come from existing documents, with invalid or incomplete URIs. Even a value like "google.com" can't be given a
Uri
with confidence. We'd have to assume the scheme. So sometimes the best we can do is plain text.However, it's often possible to provide the scheme, e.g., "https://", "mailto:", "obsidian://". When possible, a structured
Uri
object should be provided to aLinkAttribution
.When we have a scheme, we have a launchable URI. When we don't have a scheme, we're forced to guess what the scheme should be. So by holding a
Uri
with a scheme, we know we have a launchable URI.I added a property on
LinkAttribution
calledlaunchableUri
. This provides a "best guess" URI that can hopefully be launched. That property is now used by the tap handlers.Tests
I added some relevant tests.
I also altered a bunch of existing tests so that they try to match an attribution instead of trying to match
true
when looking for an attribution. When those tests fail, the message is just "Expected 'true' but got 'false'" which is very unhelpful when diagnosing what went wrong with the given attribution value.I added a test variant to switch between URLs with a scheme vs no scheme, e.g., "https://google.com" vs "google.com". This distinction is now relevant, especially when altering an existing link.
Demo
I added a demo with a variety of URIs, which are inserted both as Markdown and as a paste operation.