-
Notifications
You must be signed in to change notification settings - Fork 81
Add error handling to sound object #50
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! In fact this is indeed necessary, glad you took the work to add it. There is a comment however that we need to address before merging.
src/index.js
Outdated
@@ -154,6 +156,9 @@ export default class Sound extends React.Component { | |||
whileplaying() { | |||
props.onPlaying(this); | |||
}, | |||
onerror() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This event receives some useful parameters, such as the error number
and description (http://www.schillmania.com/projects/soundmanager2/doc/#smsound-onerror). Maybe we could add them as parameters in the callback prop? we could do:
props.onError(errorCode, description, this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet. Sounds good. I'll get around to this later on today, then resubmit 👌
README.md
Outdated
@@ -62,6 +62,7 @@ class MyComponentWithSound extends React.Component { | |||
* *volume (number)*: The current sound's volume. A value between 0 and 100. | |||
* *autoLoad (boolean)*: If the sound should start loading automatically (defaults to `false`). | |||
* *loop (boolean)*: If the sound should continue playing in a loop (defaults to `false`). | |||
* *onError (function)*: Function that gets called when the sound fails to load, or fails during load or playback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should explain that is received in this function
@leoasis Ok I made the updates! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution! this looks great
Some error handling would be nice :)