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

Copy images (v2) #3

Merged
merged 2 commits into from Sep 5, 2021
Merged

Copy images (v2) #3

merged 2 commits into from Sep 5, 2021

Conversation

luckman212
Copy link
Contributor

@luckman212 luckman212 commented Sep 5, 2021

Ok @NomarCub, this is my 2nd try.

I'm still a bit unsure how to properly test this on mobile, so for now it's desktop only (added Platform.isDesktop check to avoid registering the callback on non-desktop)

I tried to put this one through its paces by testing various different types of images (local, remote, broken links, base64-encoded inlines, CORS protected etc) and it's working well for me. Also does not add any external dependencies. I added a check for CORS errors and a fallback to use allOrigins for now (should be rare). I heard on Discord that there may be API changes to better handle this coming soon.

Hope this one is merge-able!

video:

copy_image_v2.mp4

if anyone who doesn't have a nodejs environment set up wants to test, here's a precompiled gist:
https://gist.github.com/luckman212/18126c82fc90b9a66b16483e25e13dc9

@NomarCub NomarCub merged commit 8d59500 into NomarCub:master Sep 5, 2021
@NomarCub
Copy link
Owner

@NomarCub NomarCub commented Sep 5, 2021

Great rewrite, thank you! Works well on my end. I'm grateful that this feature will be at all available in Obsidian, in a plugin I published no less.
Are you down to help with future issues related to this? I'm not that familiar with it's intricacies.
Can I rename the plugin to Copy Image and URL in Preview? I think the image copying is the main pull for most people.

@luckman212
Copy link
Contributor Author

@luckman212 luckman212 commented Sep 5, 2021

Excellent, thanks @NomarCub 👍

Sure I'm definitely up for helping out as much as I can with issues. And yes I was going to suggest the rename as well to better describe the new feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants