Stuk / jszip-utils

Other
228 stars 193 forks source link

Support for download progress callback, added promise syntax (non-breaking) #19

Closed victornpb closed 5 years ago

victornpb commented 7 years ago

Added support for download progress. For that I implemented a extensible callback style with object props, inspired by jQuery.

JSZipUtils.getBinaryContent("ref/amount.txt", {
    done: function(data) {
        console.debug(data);
    },
    fail: function(err) {
        console.error(err);
    },
    progress: function(p) { /* new */
        console.log(p);
    }
});

NOTE: Old callback still supported. But, if you want to use the progress status you'll need to switch to the new syntax.

I updated the QUnit to v2, as it is easier to test async code, they deprecated start/stop and global functions.

I also returned the xhr object, so one can still attach other events to it if they want to implement things like progress on their own.


Edit: After the PR Review, we settled at a new API favoring Promise support.

Old syntax still supported, but you don't get any new functionality.

JSZipUtils.getBinaryContent("ref/amount.txt", callback)

Callback style

JSZipUtils.getBinaryContent("ref/amount.txt", {
    callback(e) {
        // done
    },
    progress(e) { // optional
        // progress
    }
})

Promise style

JSZipUtils.getBinaryContent("ref/amount.txt", {
    progress(e) { // optional
        // progress
    }
}).then((e)=>{
    //done
})

async await

const result = await JSZipUtils.getBinaryContent("ref/amount.txt", {
    progress(e){ // optional
        // progress
    }
})
pikilipita commented 6 years ago

on this line: if(typeof cbo!=="object" || typeof cbo.done==="function" || typeof cbo.fail==="function") throw new Error("You must specify a callback.done and callback.fail");

Are you sure you meant to throw an error if done and fail ARE functions?

also it seems that on line 121, it should be 'cbo.progress.call(xhr, {' ( not onprogress.call )

victornpb commented 5 years ago

I'm making these adjustments, I have to rebase first I'm 2 behind. Sorry about the late reply, this notification got lost in the middle of all the other ones, I was looking at old stuff and stumbled here.

victornpb commented 5 years ago

Can you review my latest changes?