Skip to content

use self instead of this in sound.onEnded callback #4061

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 7, 2019
Merged

use self instead of this in sound.onEnded callback #4061

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5e587d5
use self instead of this in sound.onEnded callback
lazychip Mar 6, 2019
ed30417
sound component onEnded method
ngokevin Mar 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/sound.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ module.exports.Component = registerComponent('sound', {
for (i = 0; i < this.pool.children.length; i++) {
sound = this.pool.children[i];
sound.onEnded = function () {
this.isPlaying = false;
self.isPlaying = false;
Copy link
Contributor

@DigiTec DigiTec Mar 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question for whomever does review this from the AFrame team.

Are we explicitly avoiding arrow functions to capture this?

If we are, should we instead be using the bind() helper instead of capturing a "self"? This would make things more consistent. We tend to use bind() for simple functions and any complex function wrappers we instead use self.

Copy link
Member

@dmarcos dmarcos Mar 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, no arrow functions: Not supported in IE 11. As @DigiTec said probably bind a bit cleaner than self.

el.emit('sound-ended', self.evtDetail, false);
};
}
Expand Down