return array when using getImage() #1027

entonbiba · 2017-03-04T20:26:55Z

thijstriemstra

I would much rather have a data uri with a single canvas and an array with multiple canvasses. That way existing code doesn't break.

entonbiba · 2017-03-04T20:51:38Z

@thijstriemstra it will return the following now, what will break with this?

["base64","base64"]

mspae · 2017-03-04T20:58:13Z

@entonbiba He means only export an array if there are multiple canvases. Code which relied on the function returning a string and which worked before will still work. (Assuming there weren't multiple canvases)

thijstriemstra · 2017-03-04T20:58:21Z

Come on @entonbiba, it returned a string before, and now it returns an array.

entonbiba · 2017-03-05T00:22:55Z

@thijstriemstra ok lol, updated :)

thijstriemstra · 2017-03-05T03:00:59Z

src/drawer.multicanvas.js

@@ -337,6 +337,10 @@ WaveSurfer.util.extend(WaveSurfer.Drawer.MultiCanvas, {
            var getEntry = this.canvases[i].wave.getContext('2d');
            availableCanvas.push(getEntry.canvas.toDataURL(type, quality));
        }
-        return availableCanvas;
+        if(this.canvases.length == 1){


spaces around parentheses.

mspae · 2017-03-05T15:05:58Z

src/drawer.multicanvas.js

        for (var i in this.canvases) {
            var getEntry = this.canvases[i].wave.getContext('2d');
-            availableCanvas += getEntry.canvas.toDataURL(type, quality);
+            availableCanvas.push(getEntry.canvas.toDataURL(type, quality));


The entry object already includes a cached version of the 2d context.

The canvas property of the 2d context is just a reference to the underlying canvas HTMLElement. So at the moment this code is unnecessarily complex. Please do instead: availableCanvas.push(entry.wave.toDataURL(type, quality)); – No need for getEntry

mspae · 2017-03-05T15:06:21Z

src/drawer.multicanvas.js

@@ -332,11 +332,15 @@ WaveSurfer.util.extend(WaveSurfer.Drawer.MultiCanvas, {

    getImage: function(type, quality) {
        // combine all available canvasses together
-        var availableCanvas = '';
+        var availableCanvas = [];
        for (var i in this.canvases) {


this.canvases is an array, please use this.canvases.forEach(function (entry) {– This is a pattern which we shouldn't use in this context. see #961

mspae · 2017-03-05T15:08:11Z

src/drawer.multicanvas.js

+        if (this.canvases.length == 1) {
+            return availableCanvas[0];
+        } else {
+            return availableCanvas;


It's a matter of taste, but I'd prefer if you did return availableCanvas.length > 1 ? availableCanvas : availableCanvas[0];

mspae · 2017-03-05T15:08:48Z

src/drawer.multicanvas.js

@@ -332,11 +332,15 @@ WaveSurfer.util.extend(WaveSurfer.Drawer.MultiCanvas, {

    getImage: function(type, quality) {


Can you add Jsdoc comments?

entonbiba · 2017-03-06T13:29:30Z

@mspae updated

mspae · 2017-03-06T14:08:48Z

Thanks!

* return array when using getImage() * return one canvas image data if not multiple canvases * Update drawer.multicanvas.js * updated getImage() * add comments fir getImage() * updated comments

* added contributing info (#1006) * Readded global audio context singleton, functionally equal to PR #1021 * Cleanup npm package (#995) * ignore eclipse project files * exclude everything except dist, plugin and src directories in npm package * only include minified plugins in npm package * updated .npmignore to exclude everything except dist and src directories and subdirectories * Add support for forceDecode (#1009) * Add support for forceDecode Adds the `forceDecode` option to force client side decoding of the audio after download regardless if pre-decoded peaks are passed to the `.load/2` method. This is primarily for use with `.zoom/1` to enable high fidelity rendering on zoom. Without local decoding, the zoom on pre-decoded peaks is very low fidelity. * Update conditional brackets for failing test * nullify this.arraybuffer when destroying to prevent memory leak * removed <<<< HEAD * return array when using getImage() (#1027) * return array when using getImage() * return one canvas image data if not multiple canvases * Update drawer.multicanvas.js * updated getImage() * add comments fir getImage() * updated comments * updated getImage to es6 syntax and applied jsdoc formatting * create new labels feature for spectrogram plugin (#984) * create new labels feature for spectrogram plugin - create labels parameter (true|false) - create labels element and canvas - create labels frequency method loadLabels(); - update createWrapper() method and apply labels feature parameter. * create new labels feature for spectrogram plugin create new labels feature for spectrogram plugin - create labels parameter (true|false) - create labels element and canvas - create labels frequency method loadLabels(); - update createWrapper() method and apply labels feature parameter. * remove extra spaces * removed one more extra white space :) * update css formatting * remove extra space * remove extra space * Updated spectrogram labels to es6 * add getFilters method (#982) * add getFilters method * add getFilters() method add 'Filters must be set with setFilters method first' note. * return empty array if no filters are set * change to `return this.backend.filters || [];` * add debounce to resize/orientationchange event for minimap plugin (#987) * add debounce to resize event * add orientationchange event * getVolume method (#978) * getVolume method returns the current gainNode volume. * added getVolume method * added getVolume method * updated getVolume to es6 syntax * avoid error when calling pause before play (#977) This prevents a `Failed to execute 'stop' on 'AudioBufferSourceNode'` when stop() is called before play(). * fix getDuration() return value when using MediaElement (#941) * fix getDuration() return value for MediaElement change getDuration function in mediaelement.js to return the correct duration length same as WebAudio. from var duration = this.media.duration; // incorrect duration value returned console.log preview: https://cloud.githubusercontent.com/assets/5193884/22399178/6d26c1fe-e565-11e6-9b13-e6107641666a.png to var duration = this.buffer.duration; // correct duration value returned same as WebAudio console.log preview: https://cloud.githubusercontent.com/assets/5193884/22393549/7b130096-e4d6-11e6-83ff-4ebb78b9e42f.png * Update mediaelement.js * add wavesurfer.getPlaybackRate() (#939) add getPlaybackRate method and return playback rate.

return array when using getImage()

0acad4c

thijstriemstra suggested changes Mar 4, 2017

View reviewed changes

entonbiba mentioned this pull request Mar 4, 2017

add getImage method for MultiCanvas #1012

Merged

thijstriemstra and others added 2 commits March 4, 2017 22:02

Merge branch 'master' into entonbiba-patch-multicanvas-getImage

4e0a776

return one canvas image data if not multiple canvases

a028ee5

thijstriemstra reviewed Mar 5, 2017

View reviewed changes

Update drawer.multicanvas.js

d7d6177

thijstriemstra approved these changes Mar 5, 2017

View reviewed changes

Merge branch 'master' into entonbiba-patch-multicanvas-getImage

5634dbe

mspae added the bug label Mar 5, 2017

mspae suggested changes Mar 5, 2017

View reviewed changes

entonbiba added 4 commits March 5, 2017 10:11

Merge branch 'master' into entonbiba-patch-multicanvas-getImage

2e3ed5e

Merge branch 'master' into entonbiba-patch-multicanvas-getImage

c88fd86

updated getImage()

8e9e2c1

add comments fir getImage()

068479d

updated comments

253a7c3

mspae approved these changes Mar 6, 2017

View reviewed changes

mspae merged commit 17ecdcb into master Mar 6, 2017

mspae mentioned this pull request Mar 6, 2017

export image broken since MultiCanvas default #1011

Closed

entonbiba deleted the entonbiba-patch-multicanvas-getImage branch March 6, 2017 19:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

return array when using getImage() #1027

return array when using getImage() #1027

entonbiba commented Mar 4, 2017 •

edited

Loading

thijstriemstra left a comment

entonbiba commented Mar 4, 2017

mspae commented Mar 4, 2017

thijstriemstra commented Mar 4, 2017 •

edited

Loading

entonbiba commented Mar 5, 2017

thijstriemstra Mar 5, 2017

mspae Mar 5, 2017

mspae Mar 5, 2017

mspae Mar 5, 2017

mspae Mar 5, 2017

entonbiba commented Mar 6, 2017

mspae commented Mar 6, 2017

		@@ -332,11 +332,15 @@ WaveSurfer.util.extend(WaveSurfer.Drawer.MultiCanvas, {

		getImage: function(type, quality) {

return array when using getImage() #1027

return array when using getImage() #1027

Conversation

entonbiba commented Mar 4, 2017 • edited Loading

thijstriemstra left a comment

Choose a reason for hiding this comment

entonbiba commented Mar 4, 2017

mspae commented Mar 4, 2017

thijstriemstra commented Mar 4, 2017 • edited Loading

entonbiba commented Mar 5, 2017

thijstriemstra Mar 5, 2017

Choose a reason for hiding this comment

mspae Mar 5, 2017

Choose a reason for hiding this comment

mspae Mar 5, 2017

Choose a reason for hiding this comment

mspae Mar 5, 2017

Choose a reason for hiding this comment

mspae Mar 5, 2017

Choose a reason for hiding this comment

entonbiba commented Mar 6, 2017

mspae commented Mar 6, 2017

entonbiba commented Mar 4, 2017 •

edited

Loading

thijstriemstra commented Mar 4, 2017 •

edited

Loading