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

Improved url selection #64

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tbrams
Copy link

@tbrams tbrams commented Sep 27, 2024

Thank you for providing this very useful plugin ^^

When I started using it though, I wondered why it never gave me the option to activate it using the right click menu, and even after defining a hotkey, I had to manually select the entire url to make it do something. For me, that seemed like an unnecessary effort...

For that reason, I have made the small improvement that if the cursor is placed in, or next to, a valid youtube url, the urlwill automatically be selected and replaced by the iframe generated by your plugin.

const cursor = editor.getCursor();
const line = editor.getLine(cursor.line);

const youtubeRegex = /https?:\/\/(?:www\.)?(?:youtube\.com\/watch\?v=|youtu\.be\/)([a-zA-Z0-9_-]+)(?:&\S*)?/;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make sure that we detect any url? The goal is to support other urls than youtube, for example twitter etc.

@FHachez
Copy link
Owner

FHachez commented Oct 22, 2024

Thank you @tbrams for this PR and updating the dependencies.

It's a good point that it's too difficult to activate, since you need to select precisely the URL. It's a nice idea to use match and then select the relevant part.

Can you make sure that this works with other URLs than YouTube? For example, we could rework the isUrl into a matchUrl. You can see the tests of the regex here https://github.com/FHachez/obsidian-convert-url-to-iframe/blob/master/src/utils/__tests__/url.utils.spec.ts

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.

2 participants