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

Play/pause now properly return Promises #1229

Merged
merged 15 commits into from
Jan 18, 2018
Merged

Play/pause now properly return Promises #1229

merged 15 commits into from
Jan 18, 2018

Conversation

ffxsam
Copy link
Contributor

@ffxsam ffxsam commented Oct 13, 2017

Short description of changes:

This should fix errors such as "Uncaught (in promise) DOMException: The play() request was interrupted by a new load request", as developers can now use the returned Promise to wait for subsequent media actions.

Breaking in the external API:

N/A - though the documentation should be updated to let developers know that play, pause, and playPause now return a Promise as per the HTML5 audio spec (this PR should also work for WebAudio API as well, but please verify!)

Breaking changes in the internal API:

Not sure. Shouldn't be any.

Todos/Notes:

Might be a good idea to review WebAudio and HTMLAudio specs and check which methods return Promises, and return those from Wavesurfer's methods.

Related Issues and other PRs:

Issue #771 could be related.

@mspae
Copy link
Contributor

mspae commented Oct 22, 2017

Great idea!

The media element play method returns a promise, the pause method is currently not standardised and therefore we cannot rely on it returning a promise.

In general I think it is a good idea to return the play promise as you said. I think adjusting the JSdoc comments above the functions are still missing. (Maybe also with an @example block with an example of how to use the promise)

Regarding the implementation for WebAudio the specs should be useful: AudioContext.resume and AudioBufferSourceNode.start

@ffxsam
Copy link
Contributor Author

ffxsam commented Oct 22, 2017

That's fine if pause doesn't return a promise. In the code I've written, it should just return undefined.

I can update the JSDoc blocks when I get some spare time again.

@thijstriemstra thijstriemstra merged commit cec39f9 into katspaugh:master Jan 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants