emeraldpay / hashicon

Generates a beautiful representation of any hash.
Apache License 2.0
191 stars 22 forks source link

Update add plugin node-canvas , return base64 format url #24

Closed kiwilili closed 5 years ago

kiwilili commented 5 years ago

The default browser, passing the {type: 'node'} parameter, will base64 format url

CLAassistant commented 5 years ago

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


kiwi seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

oori commented 5 years ago

Thanks a lot for the PR, but right now I'd opt not to pull it as is. Please see my comment at https://github.com/ETCDEVTeam/hashicon/issues/23

  1. Setting node-canvas as dependency will bloat this library for browser-only users, that's why I suggested we allow an alternative createCanvas function rather then type as input. This would also allow further flexibility for people with other custom canvas requirements.

  2. hashicon returns a canvas, that should be consistent. so I wouldn't return base64 for node and canvas for browser. If one wants base64, they should toDataURL(), but maybe they'd want to hashicon(hash).createPNGStream() or any other output node-canvas provides.

So, I think it's better changing the renderer function to something like: function renderer(hashValues, params, createCanvasFn) (and the relevant changes for the wrapping main.js). I can do it in the next week (lack of time now).

Thanks again for your PR!

kiwilili commented 5 years ago

Thank you for your reply, I think your suggestion will be more suitable for the purpose of using this project.

justinmchase commented 5 years ago

Hey, I really need to generate these images on the server also. Any word on this?

oori commented 5 years ago

Closing PR. has been resolve in v0.3.0