Skip to content

Conversation

@cybex-dev
Copy link

@cybex-dev cybex-dev commented Apr 20, 2023

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

Description

Added (missing?) Call event 'connected' when call is connected to 'other party'. This assists in UI/state management for browser implementation to be aware of the actual call state (event).

Twilio Android/iOS provide this connected functionality, (for usage across platform libraries e.g. Flutter package), one expects this functionality (event) to be present.

@cybex-dev cybex-dev marked this pull request as ready for review April 20, 2023 00:17
@cybex-dev cybex-dev changed the title Add Call emit 'connected' status [WIP] Add Call emit 'connected' status Apr 20, 2023
@charliesantos
Copy link
Collaborator

Hey @cybex-dev , thanks for submitting. This is a good idea. However, as you already noticed while going through the code, there are already equivalent events that are emitted to achieve the same functionality. For example, accept event translate to connected event, etc. I understand we want to be on par with the mobile SDKs. We need to think through this more and submit an internal feature request to consider. Feel free to use this space to continue the discussion.

@cybex-dev
Copy link
Author

m libraries e.g. Flutter package), one expects this

Hi @charliesantos - since this is still a work-in-progress along with the implementation in Twilio Voice for Flutter, I expect to update this PR including as suggested a discussion for feature request (which I'd happily assist in collaborating on).

@charliesantos
Copy link
Collaborator

Hi @cybex-dev . Sounds good. To start, feel free to define the public facing SDK APIs needed before doing any implementation. That way, we don't have to spend multiple implementation cycles.

@cybex-dev
Copy link
Author

cybex-dev commented May 17, 2023

@charliesantos

Hmm - a good starting point would be replicating (expected) functionality from mobile platforms into web?

Feature proposal

As a feature I'm working on and as a possible first step, a core piece of functionality I've found missing on web is registering fcm tokens for incoming call notifications (when the browser is closed, etc via e.g. firebase-messaging-sw.js).

Problem/Issue

Currently this is currently handled by (a very responsive) WebSockets implementation (via pstream.js & wstransport.ts), however it does not cater for needs of incoming calls via e.g. PushAPI (browser closed, inactive, etc).

Solution / Considerations

To Address the issue above, an ideal implementation would require one to register the browser's FCM token with Twilio. Since functionality for registering FCM tokens aren't present (non that I could see, nor is the native Android Voice.register(token, fcm) function accessible/readable), I've resorted to a (temporary) deattachment incoming transport message(s) from Device.ts and pushing this into a service worker with PushAPI integration. This however is proving quite challenging, due to limitations of 'Manifest v3' & background service workers and persistent websocket connections (for receiving incoming calls), see for more information.


Are there any plans to incorporate registering FCM tokens for twilio-voice.js in the near future, do you have a proposed alternative, or have (you) not added this for any specific reason (yet)?

@charliesantos
Copy link
Collaborator

Hey @cybex-dev , there is currently no plan to replicate mobile features into JS SDK. Similarly, there is no plan to incorporate registering FCM tokens for JS SDK. We always recommend to use our Voice SDKs for Android, iOS, and React Native instead when developing for mobile. However, it is still something we can consider if we get enough customer interest for this feature.

@cybex-dev
Copy link
Author

Hey @cybex-dev , there is currently no plan to replicate mobile features into JS SDK. Similarly, there is no plan to incorporate registering FCM tokens for JS SDK. We always recommend to use our Voice SDKs for Android, iOS, and React Native instead when developing for mobile. However, it is still something we can consider if we get enough customer interest for this feature.

Sure - makes sense. For my needs, this would be essential. I could make a PR for this in the twilio-voice.js however I don't think there is support for this in the Twilio backend.

Android & iOS have been integrated into the Flutter twilio_voice package, however web implementation lacks this functionality which is required for my client (and I'd assume others might find it useful).

@cybex-dev
Copy link
Author

Hi @cybex-dev . Sounds good. To start, feel free to define the public facing SDK APIs needed before doing any implementation. That way, we don't have to spend multiple implementation cycles.

I intend to revisit this in the near future, currently we have functional support for Flutter on platforms:

Once this implementation has stabilised and public API is satisfied, I'll open a separate feature request to continue this discussion in more detail.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants