Skip to content

Add Copy Image option#79

Merged
sindresorhus merged 4 commits intosindresorhus:masterfrom
whitecrownclown:feat/save-image
Aug 30, 2019
Merged

Add Copy Image option#79
sindresorhus merged 4 commits intosindresorhus:masterfrom
whitecrownclown:feat/save-image

Conversation

@whitecrownclown
Copy link
Copy Markdown
Contributor

@sindresorhus

This PR follows up on sindresorhus/caprine#943.

A couple of mentions:

  • Electron can only copy png or jpg images. I tried with webp, for example, but electron.nativeImage has no support for it. I suppose we could convert to png or jpg.. thoughts?
  • I'm not sure if it should support file:// protocol or not
  • Used the request module for fetching the image buffer

Thanks!

@whitecrownclown whitecrownclown changed the title Add Save Image option [Feature] Add Save Image option Jun 25, 2019
@sindresorhus
Copy link
Copy Markdown
Owner

sindresorhus commented Jun 26, 2019

The implementation here is more complicated than I'd like. I think you instead should open an issue on https://github.com/electron/electron about adding an API to create a native image directly from an URL. They already have all the Chrome machinery to download images, so it would be better if they could handle it. Could be electron.nativeImage.createFromURL(url);. In your use-case and link to this PR. Maybe they can suggest even a better solution. Also mention the WebP problem.

Comment thread readme.md Outdated
@whitecrownclown whitecrownclown force-pushed the feat/save-image branch 7 times, most recently from 19186d0 to 75836cb Compare July 3, 2019 07:30
@whitecrownclown whitecrownclown changed the title [Feature] Add Save Image option [Feature] Add Copy Image option Jul 9, 2019
Comment thread readme.md Outdated
Comment thread readme.md Outdated
Comment thread index.js Outdated
Comment thread index.d.ts
Comment thread index.d.ts
Comment thread index.d.ts
Comment thread index.d.ts Outdated
Comment thread readme.md
Comment thread index.js Outdated
@sindresorhus
Copy link
Copy Markdown
Owner

I think you could have spent more time on perfecting this PR. At this point, it would have been much faster for me to just implement it myself.

@sindresorhus sindresorhus changed the title [Feature] Add Copy Image option Add Copy Image option Aug 8, 2019
@whitecrownclown
Copy link
Copy Markdown
Contributor Author

Sorry about that, I’ll do better on future PRs.

@sindresorhus
Copy link
Copy Markdown
Owner

@sindresorhus sindresorhus merged commit 79868f1 into sindresorhus:master Aug 30, 2019
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