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

[FEATURE/BUG] Using option "closeProtection" as TRUE when registering Twilio Device, doesn't trigger a warning dialog for incoming calls only for active calls #299

Open
jhonland opened this issue Nov 7, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@jhonland
Copy link

jhonland commented Nov 7, 2024

Is your feature request related to a problem? Please describe.
When using the closeProtection option when registering the Twilio Device, the behavior is to prevent the user from dropping the active call. This behavior is backed up by the documentation here Twilio Documentation For DeviceOptions. Ideally, this should also be activated for incoming calls.

Motivations for proposed change
Consider the following scenario:
A user who registers a new Device to listen for incoming calls to provide support for customers opens multiple browser tabs, initializing an instance of Twilio Device per opened tab that listens for incoming calls.
Then an incoming call is received but before the user accepts the call, the user closes by mistake one of the tabs where the Device has an incoming call pending.
At the time of writing, Twilio rejects any incoming calls and disconnects the active call as part of the Device's destroy cycle.
references: code extract

    ...
    const calls = this._calls.slice(0);
    calls.forEach((call: Call) => call.reject());

    this.disconnectAll();
    this._stopRegistrationTimer();
    ...

which is bound to the unload and pagehide events
references: code extract

    if (typeof window !== 'undefined' && window.addEventListener) {
      window.addEventListener('unload', this._boundDestroy);
      window.addEventListener('pagehide', this._boundDestroy);
    }

NOTE: This could probably be considered bad UX but users are users and it's hard to explain that you should only initialize a call support system (registering a Device to listen for calls) on one Tab only 🫠.

Describe the solution you'd like
Going through the Twilio Voice SDK source code, on file twilio-voice.js/blob/master/lib/twilio/device.ts#L886 the code indicates to not show the browser's confirmation dialog only if there is an active call:

private _confirmClose(event: any): string {
    if (!this._activeCall) { return ''; }
    ...

Propose solutions:

  • [SOLUTION 1]: Change the behavior of closeProtection flag to show the confirmation dialog also for incoming calls.
  • [SOLUTION 2]: Add an additional flag, something like closeProtectionIncomingCalls to also apply the behavior for incoming calls.
  • [SOLUTION 3]: Update the closeProtection flag that currently only accepts boolean | string to accept an object with extra parameters to also opt-in for incoming calls.

Example of solution:
NOTE: Currently there is no way to determine if the device has at least 1 incoming call that hasn't been accepted yet, for that METHOD_THAT_CALCS_IF_THERE_IS_AN_INCOMING_CALL assumes that that method exists

private _confirmClose(event: any): string {
    const closeProtection: boolean | string = this._options.closeProtection || false;
    const confirmationMsg: string = typeof closeProtection !== 'string'
      ? 'A call is currently in-progress. Leaving or reloading this page will end the call.'
      : closeProtection;

    if (this._activeCall || this.[METHOD_THAT_CALCS_IF_THERE_IS_AN_INCOMING_CALL]) {
        (event || window.event).returnValue = confirmationMsg;
        return confirmationMsg;
    }
    
    (event || window.event).returnValue = '';
    return '';
}

Example of implementation to know if there is an incoming call (METHOD_THAT_CALCS_IF_THERE_IS_AN_INCOMING_CALL) method:

function deviceHasIncomingCallsPending(): boolean {
    return this._calls.some((call) => call.status() === Call.State.Pending)
}

Describe alternatives you've considered
Implement the same solutions as proposed above from outside the SDK by listening directly to the beforeunload event.

Additional context
Do you need a new Dev? let's schedule an interview 😉.

@jhonland jhonland added the enhancement New feature or request label Nov 7, 2024
@jhonland
Copy link
Author

jhonland commented Nov 7, 2024

Forgot to mention that having multiple users using the same Device token identity, and one of the users closes the tab or window where the app is running, the same reject call behavior for the destroy flow will affect other users who would like to pickup the call.

@charliesantos
Copy link
Collaborator

@jhonland thanks for a very detailed ticket! I understand the need for this. I'll submit an internal ticket to discuss this further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants