-
Notifications
You must be signed in to change notification settings - Fork 42
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
Chat: Update typing indicator API documentation #2473
base: integration/chat-single-channel
Are you sure you want to change the base?
Chat: Update typing indicator API documentation #2473
Conversation
Typing is no longer presence-based, the docs have been updated to reflect changes in logic to support this. - Also added details about the new `change` property in typing events and updated the timeout behavior with `start()` calls and grace periods. - `typing.get()` is also now synchronous.
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
… `heartbeatThrottleMs`
04fbca8
to
1eaf703
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you create an integration branch and point this branch at that, thanks :)
content/chat/rooms/typing.textile
Outdated
@@ -100,7 +107,7 @@ blang[javascript,kotlin]. | |||
```[javascript] | |||
// Initial subscription | |||
const { unsubscribe } = room.typing.subscribe((event) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we pop a type hint in here, for explicitness?
content/chat/rooms/typing.textile
Outdated
``` | ||
|
||
```[swift] | ||
let currentlyTypingClientIds = try await room.typing.get() | ||
let currentlyTypingClientIds = try room.typing.get() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we leave this one as-is for now, the Swift team can change as they see fit :)
content/chat/rooms/typing.textile
Outdated
|
||
Consequently, if you want to keep the typing indicator active, you should call `start()` with each key press to ensure a new typing event is emitted after the timeout has expired. | ||
|
||
For a client receiving typing events, the typing indicator they have received will remain active for the duration of the timeout, plus a predefined grace period of 2000ms. If a new typing event is received for the same user before the grace period has expired, the typing indicator will be reset to the full duration of the timeout. For example, if the timeout is 15000ms, the typing indicator will remain active for 12000ms. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this say "if the timeout is 10000ms"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth a comment re giving developers the flexibility to balance the number of messages and the responsiveness?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, will add that in!
content/chat/rooms/typing.textile
Outdated
There is a timeout associated with start events. A stop event will be automatically emitted after it expires if one isn't received before the timeout. The length of this timeout is customizable using the @timeoutMs@ parameter that can be configured in the @RoomOptions@ that you set when you "create a room":/docs/chat/rooms#create. The default is 10000ms. | ||
There is a timeout associated with start events. On first calling `start()`, this timer is set, subsequent calls to `start()` before the timer expires will result in a no-op. The length of this timeout is customizable using the @heartbeatThrottleMs@ parameter that can be configured in the @RoomOptions@ that you set when you "create a room":/docs/chat/rooms#create. The default is 10000ms. | ||
|
||
Consequently, if you want to keep the typing indicator active, you should call `start()` with each key press to ensure a new typing event is emitted after the timeout has expired. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could add a line here about how you might achieve it in AI/bot settings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only a small recommendation. But other than that all good! 👍
Description
Updating the typing portion of the docs to reflect the move from presence to ephemeral messaging.
For more context, please see [CHADR-093]
Checklist