Update wavesurfer.minimap.js #955

entonbiba · 2017-02-02T03:11:45Z

renders the minimap without the need to resize the window. #954 #942 #951

Preview:
http://codepen.io/entonbiba/pen/ggeGyG

Changed

    this.wavesurfer.on('ready', this.render.bind(this));

to

    this.wavesurfer.on('ready', my.render.bind(this));

renders the minimap without the need to resize the window. Changed this.wavesurfer.on('ready', this.render.bind(this)); to this.wavesurfer.on('ready', my.render.bind(this));

added the fixes for the drawPeaks()

piem · 2017-02-02T08:28:42Z

plugin/wavesurfer.minimap.js

@@ -94,7 +94,7 @@ WaveSurfer.Minimap = WaveSurfer.util.extend({}, WaveSurfer.Drawer, WaveSurfer.Dr

    bindWaveSurferEvents: function () {
        var my = this;
-        this.wavesurfer.on('ready', this.render.bind(this));
+        this.wavesurfer.on('ready', my.render());


This is causing errors. I guess you meant my.render.bind(this)?

the error i get with my.render() is:

Uncaught TypeError: Cannot read property 'apply' of undefined at [...]/wavesurfer.js/dist/wavesurfer.js:736:15

This change is not needed at all.

@piem what code are you using that it gives the error?

@katspaugh without the change it wouldn't show the minimap on on ready event and the user is required to resize the window for it to work properly or call minimap.render(); right after.

var minimap1 = wavesurfer.initMinimap({ container: document.querySelector('#minimap'), height: 30, waveColor: '#ddd', progressColor: '#999', cursorColor: '#68A93D' }); //without calling this it won't render on ready minimap1.render();

In the resize event it's calling my.render(); which makes it work when resizing the window. On the ready event it's not calling it so it won't render the minimap until you resize the window or call minimap.render() yourself after wavesurfer.initMinimap().

var onResize = function () { if (prevWidth != my.wrapper.clientWidth) { prevWidth = my.wrapper.clientWidth; my.render(); my.progress(my.wavesurfer.backend.getPlayedPercents()); } };

@piem @katspaugh happy with @entonbiba's feedback? thoughts?

this.wavesurfer.on('ready', my.render()); is certainly wrong, since it's calling the render function immediately.

You need to either subscribe like the original code:

this.wavesurfer.on('ready', this.render.bind(this))):

Or simply call this.render().

@katspaugh yes this.render(); will work. I had already updated it to that in the preview code but didn't in this pull. I'll do that now.

@entonbiba you are missing the point.

this.wavesurfer.on('ready', someCallback) – that's a subscription. The first parameter is the name of an event, the second parameter is a function.

What you are doing is calling the callback immediately, instead of passing it as a parameter:
this.wavesurfer.on('ready', someCallback())

This is equivalent to

someCallback(); this.wavesurfer.on('ready', undefined)

So if you think the render method should be called immediately, just call it without the on-ready call.

check to see if a parameter called "miniRenderOnLoad" is set to true, if it is render the minimap on load.

entonbiba · 2017-02-06T16:38:43Z

@katspaugh what about adding a check to see if a parameter called "miniRenderOnLoad" is set to true? It would make the rending on load optional instead of default. This way this.wavesurfer.on('ready', this.render.bind(this))): remains as default.

Something like this ok to add?

katspaugh · 2017-02-06T16:44:45Z

@entonbiba I don't really get why it's needed. How can it render the minimap (which is just a small copy of the waveform) before the main waveform is ready?

thijstriemstra · 2017-02-06T16:54:27Z

plugin/wavesurfer.minimap.js

@@ -94,7 +94,17 @@ WaveSurfer.Minimap = WaveSurfer.util.extend({}, WaveSurfer.Drawer, WaveSurfer.Dr

    bindWaveSurferEvents: function () {
        var my = this;
-        this.wavesurfer.on('ready', this.render());
+        // check if parameter renderOnLoad is definied and set to true to render minimap on load


typo 'definied'

thijstriemstra · 2017-02-06T16:54:41Z

plugin/wavesurfer.minimap.js

@@ -94,7 +94,17 @@ WaveSurfer.Minimap = WaveSurfer.util.extend({}, WaveSurfer.Drawer, WaveSurfer.Dr

    bindWaveSurferEvents: function () {
        var my = this;
-        this.wavesurfer.on('ready', this.render());
+        // check if parameter renderOnLoad is definied and set to true to render minimap on load
+        var miniRenderOnLoad = (this.params.miniRenderOnLoad||false);


spaces around || missing

thijstriemstra · 2017-02-06T16:54:56Z

plugin/wavesurfer.minimap.js

+        if (typeof this.params.miniRenderOnLoad !== "undefined") {
+            if (miniRenderOnLoad) {
+                this.render();
+            } else{


space after else

thijstriemstra · 2017-02-06T16:55:01Z

plugin/wavesurfer.minimap.js

+            } else{
+                this.wavesurfer.on('ready', this.render.bind(this))):
+            }
+        } else{


space after else

entonbiba · 2017-02-06T17:21:55Z

@katspaugh It will render after the initMinimap is called and if the parameter miniRenderOnLoad is set to true. Which will be inside the wavesurfer ready event. Therefore it will render after the main waveform is rendered, fixing the onload issue with the minimap.

I added the parameter as an option, in case someone doesn't want to render it right away. They will have to resize the window like that, the way it currently works. I still think it should render right away.

katspaugh · 2017-02-06T22:00:11Z

Let's just render it right away unconditionally, maybe?

will update docs and create example for minimap plugin - minimap should be initialized within the main wavesurfer ready event.

entonbiba · 2017-02-06T22:07:36Z

@katspaugh ok updated. I'll update the example and add it to the plugins section.

Once the debounce function is added to master, I'll update the resizing event for minimap. debounce pull request #964

katspaugh · 2017-02-07T09:27:34Z

Merged, thank you!

entonbiba added 2 commits February 1, 2017 22:11

Update wavesurfer.minimap.js

2560011

renders the minimap without the need to resize the window. Changed this.wavesurfer.on('ready', this.render.bind(this)); to this.wavesurfer.on('ready', my.render.bind(this));

Update wavesurfer.minimap.js

19c9392

added the fixes for the drawPeaks()

piem mentioned this pull request Feb 2, 2017

minimap waveform only appears after resizing the window #954

Closed

piem reviewed Feb 2, 2017

View reviewed changes

entonbiba added 3 commits February 5, 2017 10:51

update my.render to this.render

0036b7b

add parameter check renderOnLoad

004564d

check to see if a parameter called "miniRenderOnLoad" is set to true, if it is render the minimap on load.

Update wavesurfer.minimap.js

b297ef5

thijstriemstra reviewed Feb 6, 2017

View reviewed changes

add space after else

11704bd

render minimap on load

3536d75

will update docs and create example for minimap plugin - minimap should be initialized within the main wavesurfer ready event.

katspaugh merged commit e633171 into katspaugh:master Feb 7, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update wavesurfer.minimap.js #955

Update wavesurfer.minimap.js #955

entonbiba commented Feb 2, 2017 •

edited

Loading

piem Feb 2, 2017

piem Feb 2, 2017

katspaugh Feb 2, 2017

entonbiba Feb 2, 2017

thijstriemstra Feb 4, 2017

katspaugh Feb 5, 2017

entonbiba Feb 5, 2017

katspaugh Feb 5, 2017

entonbiba commented Feb 6, 2017

katspaugh commented Feb 6, 2017

thijstriemstra Feb 6, 2017

thijstriemstra Feb 6, 2017

thijstriemstra Feb 6, 2017

thijstriemstra Feb 6, 2017

entonbiba commented Feb 6, 2017 •

edited

Loading

katspaugh commented Feb 6, 2017

entonbiba commented Feb 6, 2017

katspaugh commented Feb 7, 2017

Update wavesurfer.minimap.js #955

Update wavesurfer.minimap.js #955

Conversation

entonbiba commented Feb 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

entonbiba commented Feb 6, 2017

katspaugh commented Feb 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

entonbiba commented Feb 6, 2017 • edited Loading

katspaugh commented Feb 6, 2017

entonbiba commented Feb 6, 2017

katspaugh commented Feb 7, 2017

entonbiba commented Feb 2, 2017 •

edited

Loading

entonbiba commented Feb 6, 2017 •

edited

Loading