-
Notifications
You must be signed in to change notification settings - Fork 209
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
Add minify-image module #1149
Add minify-image module #1149
Conversation
@jywarren please have a look, rebased changes here. |
Codecov Report
@@ Coverage Diff @@
## main #1149 +/- ##
==========================================
+ Coverage 55.39% 55.66% +0.27%
==========================================
Files 113 114 +1
Lines 2356 2357 +1
Branches 364 364
==========================================
+ Hits 1305 1312 +7
+ Misses 1051 1045 -6
|
@jywarren please have a look. |
@jywarren please review this. Ready for final review. |
} | ||
|
||
// write the ArrayBuffer to a blob, and you're done | ||
var blob = new Blob([ab], { |
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.
This could be written just like return new Blob([ia], { type: mimeString });
if you want.
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.
Thanks @IgorWilbert
Done that for better readability.
function dataURItoBlob(dataURI) { | ||
// convert base64 to raw binary data held in a string | ||
// doesn't handle URLEncoded DataURIs - see SO answer #6850276 for code that does this | ||
var byteString = atob(dataURI.split(',')[1]); |
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.
No need for this?
var byteString;
if (dataURI.split(',')[0].indexOf('base64') >= 0)
byteString = atob(dataURI.split(',')[1]);
else
byteString = unescape(dataURI.split(',')[1]);
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.
as that completely works!!
Awesome job @Divy123 ! Just left some comments that are more like doubts, I hope they happen to be useful. Thank you! |
@jywarren please review!! |
Great, good to merge once conflicts are resolved, thanks! |
🎉 !!!!! Great work! |
* Add minify-image module * Add node modules * corrected path * Add browser func * Add test and documentation
Fixes #1142
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
npm test
@publiclab/is-reviewers
for help, in a comment belowIf tests do fail, click on the red
X
to learn why by reading the logs.Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Thanks!