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

Signaling "Typing indicators" #9272

Closed
2 tasks
nickvergessen opened this issue Apr 6, 2023 · 7 comments · Fixed by #9431
Closed
2 tasks

Signaling "Typing indicators" #9272

nickvergessen opened this issue Apr 6, 2023 · 7 comments · Fixed by #9431
Assignees
Labels
1. to develop enhancement feature: chat 💬 Chat and system messages feature: signaling 📶 Internal and external signaling backends low

Comments

@nickvergessen
Copy link
Member


  • When the chat input is non empty a signaling message is sent in periodical interval
  • When the signaling messages is received a small max contrast line is shown above the chat input
@nickvergessen nickvergessen added 1. to develop enhancement medium feature: chat 💬 Chat and system messages feature: signaling 📶 Internal and external signaling backends labels Apr 6, 2023
@nickvergessen nickvergessen added this to the 💙 Next Major (27) milestone Apr 6, 2023
@github-project-automation github-project-automation bot moved this to 🧭 Planning evaluation (don't pick) in 💬 Talk team Apr 6, 2023
@nickvergessen nickvergessen moved this from 🧭 Planning evaluation (don't pick) to 📄 To do (~10 entries) in 💬 Talk team Apr 6, 2023
@nickvergessen nickvergessen added low and removed medium labels Apr 6, 2023
@fancycode
Copy link
Member

Maybe the transient data objects could be re-used for this:
https://nextcloud-spreed-signaling.readthedocs.io/en/latest/standalone-signaling-api-v1/#transient-data

A user could set a property when typing starts and remove it when it ended, so no need for periodic messages. Only thing that needs to be added is to remove the property when a session disconnects while typing.

@danxuliu
Copy link
Member

When the chat input is non empty a signaling message is sent in periodical interval

What is the reason to send a signaling message in periodic intervals? Would it not be enough just to send startedTyping and stoppedTyping, similar to how startedSpeaking and stoppedSpeaking are sent during calls?

Only thing that needs to be added is to remove the property when a session disconnects while typing.

Do you mean in the clients or in the signaling server itself? Besides that, the transient data would be something like typing-{SESSION_ID}: true, right? Is there any limit in the key length that could be reached by adding the session id?

Nevertheless, although the transient data looks like a good fit if typing indicators are also added to the internal signaling server maybe it would be better to use signaling messages in both cases for simplicity/consistency. After all, if started/stoppedTyping messages are used as far as I understand the benefit provided by the transient data object would be that they are automatically sent to participants joining the room, and with plain signaling messages it would need to be explicitly done by the clients, but they would still need to do it anyway to support the internal signaling server.

@fancycode
Copy link
Member

When the chat input is non empty a signaling message is sent in periodical interval

What is the reason to send a signaling message in periodic intervals? Would it not be enough just to send startedTyping and stoppedTyping, similar to how startedSpeaking and stoppedSpeaking are sent during calls?

Periodic messages should be avoided, that's why I suggested the transient data.

Only thing that needs to be added is to remove the property when a session disconnects while typing.

Do you mean in the clients or in the signaling server itself? Besides that, the transient data would be something like typing-{SESSION_ID}: true, right? Is there any limit in the key length that could be reached by adding the session id?

The key length is currently not limited. I was thinking that every session can always set a key OWN_SESSION_ID (with an arbitrary object as value) which automatically gets removed by the server when the session is deleted. This is not available yet though - but should be easy to implement if necessary.

Nevertheless, although the transient data looks like a good fit if typing indicators are also added to the internal signaling server maybe it would be better to use signaling messages in both cases for simplicity/consistency. After all, if started/stoppedTyping messages are used as far as I understand the benefit provided by the transient data object would be that they are automatically sent to participants joining the room, and with plain signaling messages it would need to be explicitly done by the clients, but they would still need to do it anyway to support the internal signaling server.

Right, the clients don't have to make sure that any current state has to be sent to newly connected clients with the transient data. Keep in mind that this would have to be implemented for web and the mobile clients independently. With the transient data you get it for free. But sure, if you want to support the internal signaling for typing indicators, it must be done that way.

@nickvergessen
Copy link
Member Author

What is the reason to send a signaling message in periodic intervals? Would it not be enough just to send startedTyping and stoppedTyping, similar to how startedSpeaking and stoppedSpeaking are sent during calls?

That was my "dumb" assumption of being less work :P
I was not exactly aware how the speaking indicator works, so yeah totally fine to follow that.

@SystemKeeper
Copy link
Contributor

What is the reason to send a signaling message in periodic intervals? Would it not be enough just to send startedTyping and stoppedTyping, similar to how startedSpeaking and stoppedSpeaking are sent during calls?

The advantage of using transient data here would be some kind of "initial state". When we only send started/stopped events once, you'll never see a typing indicator when you enter a conversation. Theoretically if you write a text and never reach the "stopped" threshold, newly joined clients would never see that you're typing right now.
Is there any other way to provide a initial state for the typing indicator when using just started/stopped messages?

@SystemKeeper
Copy link
Contributor

After discussing with @danxuliu we noted that we had were different assumptions about how typing indicators would work with signaling messages, so some notes:

  • startedTyping messages will be send
    • To all clients currently active in the room
    • To all clients joining the room while we are still typing
  • stoppedTyping messages will be send to all clients currently active in the room
  • Messages will not be send periodically

@SystemKeeper
Copy link
Contributor

SystemKeeper commented Apr 29, 2023

Currently the signaling message would look like this for me:
image

Questions:

  • Do we need any kind of payload here? (RoomToken and Displayname came to my mind, but both would not be necessary I think).
  • Do we need a roomType here? It's default in the current signaling messages on talk-ios, but I think it can be omitted / left empty in this case
  • How are we handling multi session here? My suggestion would be
    • Each userId should be shown at max 1 time, even when using multiple sessions (which raises the question if you can type on multiple devices at the same time, but still).
    • Ignore typing events that come from our own userId (so if I start typing on web, I would not see myself as typing on mobile)
  • How is leaving a conversation handled? Should the client send „stoppedTyping“ when we leave a conversation and we were typing before or is it expected from the clients to remove any typing indicator when it received a leave message?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop enhancement feature: chat 💬 Chat and system messages feature: signaling 📶 Internal and external signaling backends low
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants