Skip to content
This repository was archived by the owner on Jul 22, 2024. It is now read-only.

ping on peer join#48

Open
cdeng001 wants to merge 4 commits intoIBM:masterfrom
cdeng001:client-ping
Open

ping on peer join#48
cdeng001 wants to merge 4 commits intoIBM:masterfrom
cdeng001:client-ping

Conversation

@cdeng001
Copy link
Contributor

@cdeng001 cdeng001 commented Nov 5, 2018

Adds feature for keeping sounds and playing them.
Adds a sound .wav for when peers join.


addSound (name, src) {
const newSound = {}
const soundElement = document.createElement('audio')
Copy link
Contributor

@daviddahl daviddahl Nov 5, 2018

Choose a reason for hiding this comment

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

One thing to note - what if this is a node.js app consuming the API library? What does this look like where there is no document object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me do some digging

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Im thinking the expected behavior for now is to throw an error and let the user know that node env are not supported.

Copy link
Contributor

@daviddahl daviddahl Nov 6, 2018

Choose a reason for hiding this comment

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

or... find something like https://www.npmjs.com/package/play - which is pretty old but still has some 270 weekly downloads
edit
See: https://www.npmjs.com/package/play-sound

@@ -0,0 +1,30 @@
module.exports = class AudioMachine {
Copy link
Contributor

Choose a reason for hiding this comment

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

filename should be audio-machine.js

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants