feat: Add bidirectional communication to exchange texts#108
feat: Add bidirectional communication to exchange texts#108nidhal-labidi wants to merge 2 commits intomainfrom
Conversation
Signed-off-by: nidhal-labidi <nidhal.labidi@compose.us>
Narigo
left a comment
Narigo
left a comment
There was a problem hiding this comment.
Not sure whether it's better to expose APIs for using bi-directional AND allowing to send just in one direction instead of only bi-directional then...
| }; | ||
|
|
||
| private configureDataChannel = () => { | ||
| if (this.dataChannel == null) { |
There was a problem hiding this comment.
| if (this.dataChannel == null) { | |
| if (this.dataChannel === null) { |
|
|
||
| private configureDataChannel = () => { | ||
| if (this.dataChannel == null) { | ||
| this.changeState('error', 'dataChannel is null. Unable to setup the configure it!'); |
There was a problem hiding this comment.
| this.changeState('error', 'dataChannel is null. Unable to setup the configure it!'); | |
| this.changeState('error', 'dataChannel is null. Unable to configure it!'); |
| if (this.dataChannel == null) { | ||
| this.changeState( | ||
| 'error', | ||
| 'dataChannel is null. Unable to setup the listeners for the data channel' | ||
| ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Either === null or something like this:
| if (this.dataChannel == null) { | |
| this.changeState( | |
| 'error', | |
| 'dataChannel is null. Unable to setup the listeners for the data channel' | |
| ); | |
| return; | |
| } | |
| if (!this.dataChannel) { | |
| this.changeState( | |
| 'error', | |
| 'dataChannel is not defined. Unable to setup the listeners for the data channel' | |
| ); | |
| return; | |
| } |
|
|
||
| this.dataChannel = this.createDataChannel(); | ||
| if (this.dataChannel) { | ||
| //this.dataChannel.bufferedAmountLowThreshold = this.BUFFER_THRESHOLD; |
There was a problem hiding this comment.
dead code
| //this.dataChannel.bufferedAmountLowThreshold = this.BUFFER_THRESHOLD; |
|
|
||
| let currentState = $state< | ||
| 'init' | 'connected' | 'sending' | 'done' | 'error-user-denied' | 'error' | ||
| 'init' | 'connected' | 'text-transfered' | 'sending' | 'error-user-denied' | 'error' |
There was a problem hiding this comment.
typo
| 'init' | 'connected' | 'text-transfered' | 'sending' | 'error-user-denied' | 'error' | |
| 'init' | 'connected' | 'text-transferred' | 'sending' | 'error-user-denied' | 'error' |
There was a problem hiding this comment.
Shouldn't the host page have some change as well (looking for text-received event?)
There was a problem hiding this comment.
The file src/routes/flottform-text-client/[endpointId]/+page.svelte was created to test sending texts using Flottform. However, since we didn’t find a practical use case for sending texts at the time, we didn’t add a host file in the demo folder.
If you'd like, we can remove this file, as this PR contains a new messaging app in the demo that uses FlottformTextInputHost and FlottformTextInputClient. (demo/src/routes/flottform-messaging and demo/src/routes/flottform-messaging-client/[endpointId])
…om of the messages Signed-off-by: nidhal-labidi <nidhal.labidi@compose.us>
|
Will be divided into 3 separate PRs:
|
|
Please refer to the previous comment and the following PRs to see how Flottform implemented the new API and dealt specifically with bidirectional text transfer: |
2 participants
Checklist
Reviewer
Changes
Check the issue #107