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

No mechanism for disabling sounds #14

Closed
doryphores opened this issue Jul 15, 2021 · 15 comments
Closed

No mechanism for disabling sounds #14

doryphores opened this issue Jul 15, 2021 · 15 comments
Labels
bug Something isn't working jira added

Comments

@doryphores
Copy link

v1 had an API to enable/disable sounds:

Device.audio.incoming(false)

I can see some code related to this in v2:

private _enabledSounds: Record<Device.ToggleableSound, boolean> = {
[Device.SoundName.Disconnect]: true,
[Device.SoundName.Incoming]: true,
[Device.SoundName.Outgoing]: true,
};

But there doesn't seem to be a public API to configure the settings.

Is this an oversight? It is currently blocking us from upgrading.

@doryphores doryphores added the enhancement New feature or request label Jul 15, 2021
@ryan-rowland ryan-rowland added bug Something isn't working and removed enhancement New feature or request labels Jul 19, 2021
@ryan-rowland
Copy link
Contributor

ryan-rowland commented Jul 19, 2021

Hi @doryphores,
Thanks for bringing this to our attention. It looks like when we deprecated the old Device.setup() interface, we missed a step in wiring up the Device to check the AudioHelper's settings rather than its internal (and unchangeable) default settings.

I'm creating an internal ticket for this and you should expect this to be fixed in an upcoming patch release. If you were using Device.audio.incoming(false) before, your code should not need to change as we plan to restore this functionality.

@cjstewart88
Copy link

@ryan-rowland is there an update on this?

@cjstewart88
Copy link

Any update on this?

@doryphores
Copy link
Author

@ryan-rowland Any update on this? We really really want to upgrade 😄

@kamalbennani
Copy link
Contributor

@ryan-rowland any news on this?

@jakswa
Copy link

jakswa commented Sep 28, 2022

Maybe impacted by the recent twilio layoffs?

@charliesantos
Copy link
Collaborator

Hello everyone, apologies for the delay. I updated our internal ticket and it will be considered in the next release.

@kamalbennani
Copy link
Contributor

I'm obliged to make the "_enabledSounds" public to disable the outgoing and incoming sounds:

device._enabledSounds[Device.SoundName.Outgoing] = false;

Not great, but at least it does the trick.

@charliesantos
Copy link
Collaborator

@kamalbennani , you can definitely do that as a temporary workaround. But please keep in mind that _enabledSounds is a private member and might cause breaking changes in the future. But as long as you do manual updates and testing your updates before deployment, you should be good.

@kamalbennani
Copy link
Contributor

@charliesantos by checking the code I see that everything is already implemented, the only thing missing is to type the Audio class correctly, if that's the case, I can push a PR to fix the typing issue, is that fine with you?

@charliesantos
Copy link
Collaborator

@kamalbennani we have not looked into it yet but please feel free to submit a PR :)
I believe the fix is to update the references of _enabledSounds in the device class to use the values set in the _audio object. For example, this line needs needs updating https://github.com/twilio/twilio-voice.js/blob/master/lib/twilio/device.ts#L995 to use something like this._audio.outgoing().

@charliesantos
Copy link
Collaborator

Hey everyone, the mechanism to enable and disable sounds is still available. Since there's no singleton behavior on the Device anymore, You need to use the device audio API on the device instance. For example, to disable incoming, outgoing, and disconnect sounds

const device = new Device(...);
device.audio.incoming(false);
device.audio.outgoing(false);
device.audio.disconnect(false);

@doryphores
Copy link
Author

Hey everyone, the mechanism to enable and disable sounds is still available. Since there's no singleton behavior on the Device anymore, You need to use the device audio API on the device instance. For example, to disable incoming, outgoing, and disconnect sounds

const device = new Device(...);
device.audio.incoming(false);
device.audio.outgoing(false);
device.audio.disconnect(false);

That's great to hear @charliesantos. I can confirm that it does work but it looks like the Typescript type definitions are missing for these functions.

@charliesantos
Copy link
Collaborator

Thanks for confirming @doryphores . Definitions will be added in a future release.

@charliesantos
Copy link
Collaborator

Should be fixed in 2.4.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working jira added
Projects
None yet
Development

No branches or pull requests

6 participants