Skip to content
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

create debounce method in util.js #964

Merged
merged 4 commits into from
Feb 7, 2017
Merged

Conversation

entonbiba
Copy link
Contributor

@entonbiba entonbiba commented Feb 4, 2017

Can be used with a resize event to prevent drawBuffer() from running concurrently when resizing the browser window. @mspae @thijstriemstra #963

preview:
http://codepen.io/entonbiba/pen/jyKrEz

var responsiveWave = WaveSurfer.util.debounce(function() {
  wavesurfer.empty();
  wavesurfer.drawBuffer();
}, 150);

window.addEventListener(‘resize’, responsiveWave);

Can be used as follows with a resize event to prevent drawBuffer() from running concurrently when resizing the browser window.
preview:
http://codepen.io/entonbiba/pen/jyKrEz

var responsiveWave = debounce(function() {
  wavesurfer.empty();
  wavesurfer.drawBuffer();
}, 150);

window.addEventListener(‘resize’, responsiveWave);
@mspae
Copy link
Contributor

mspae commented Feb 4, 2017

Hey awesome! Maybe you might want to checkout the next branch (thats where my previous PR was directed) - as far as I'm concerned we can do the same for master for this feature though.

@katspaugh
Copy link
Owner

I'm a bit reluctant to add code that is not used inside wavesurfer.js itself, or inside any plugins. Why can't the app authors define or import the debounce function themselves?

@mspae
Copy link
Contributor

mspae commented Feb 7, 2017

The original idea came up in the context of this PR #963 (responsive option)

I would like to add debounce to the next version to enable debouncing of event listeners which are called constantly (mostly resize and zoom, but maybe also scroll and audioprocess in some cases) – Off the top of my head I can think of two use cases where I would like to use debounce: the new responsive feature and the resizing of the minimap plugin. Not adding debounce would mean we can't implement the responsive feature event because it would crash or cause immense lags for large files and waveforms.

I have also though about how this could be kept seperate from the wavesurfer lib so we don't have to maintain it. But apart from providing a debounce parameter where the function can be specified I couldn't think of a good way of doing it. – and IMHO that would be even more complicated than just exposing the well-tested debounce package. (documenting, answering questions, maintaining extra code for parameter checking etc.)

Note that this is my opinion regarding the next branch/version.

@katspaugh
Copy link
Owner

OK, if it's going to be used in the code, let's add it. Thanks, @entonbiba!

@katspaugh katspaugh merged commit f177220 into katspaugh:master Feb 7, 2017
@katspaugh
Copy link
Owner

katspaugh commented Feb 7, 2017

I've published version 1.3.1 on npm.

Also, @entonbiba I've invited you as a collaborator, I hope you don't mind.

@entonbiba
Copy link
Contributor Author

@katspaugh thanks! Yes, I'm going to update a couple of the plugins to include the debounce feature on the windows resize event. This feature also comes in handy when having more than one wavesurfer initialized on the page and in case the user keeps resizing the window.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants