-
Notifications
You must be signed in to change notification settings - Fork 46
Chat room attachment wording #2957
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
base: main
Are you sure you want to change the base?
Conversation
|
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 ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
src/pages/docs/chat/rooms/index.mdx
Outdated
| ## Attach to a room <a id="attach"/> | ||
|
|
||
| As soon as a client is attached to a room, Ably will begin streaming messages and events to them, regardless of whether or not they have registered any listeners to receive those messages and events. | ||
| To start receiving messages and events from a room, you need to attach to it. Attaching to a room tells Ably to make sure the room is created server-side, starts streaming messages to the client, and ensures that events are not missed in case of temporary network interruptions. |
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.
Do we want to use the word created here? Might imply some explicit creation/deletion of rooms (which is quite standard in other providers).
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.
Rewording to avoid saying anything about server-side state
| By default the `ChatRoomProvider` will automatically call [`release()`](https://sdk.ably.com/builds/ably/ably-chat-js/main/typedoc/interfaces/chat-js.Room.html#release) on the room when it unmounts. Set the [`release`](https://sdk.ably.com/builds/ably/ably-chat-js/main/typedoc/interfaces/chat-react.ChatRoomProviderProps.html#release) property to `false` to change this behavior and have the room only [detach](#detach) when the component unmounts. You can manually control this attachment behavior using the [`useRoom`](https://sdk.ably.com/builds/ably/ably-chat-js/main/typedoc/functions/chat-react.useRoom.html) hook. | ||
| </If> | ||
|
|
||
| ## Attach to a room <a id="attach"/> |
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.
I think we should explicitly call out here that attachment does not confer membership of a room. That's the common misunderstanding from the LLM - that attachment is akin to membership and thus can be instigated on behalf of someone else from the server (it can't) and that its a persistent thing beyond the current connection (it isn't).
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.
I'll add a paragraph.
src/pages/docs/chat/rooms/index.mdx
Outdated
| ``` | ||
| </Code> | ||
|
|
||
| As soon as a client is attached to a room, Ably will begin streaming messages and events to them. To receive the messages and events in your application code, you need to add listeners to the events that you are interested in by subscribing, for example using the [`messages.subscribe()`](/docs/chat/rooms/messages#subscribe) method. |
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.
Perhaps worth suggesting that listeners be added before subscribe is called to avoid missing messages?
src/pages/docs/chat/rooms/index.mdx
Outdated
| The channel name is the same as the room name with an appended suffix of `::$chat` (e.g `some-room-id::$chat`). In most cases, you will not need to worry about this, as the Chat SDK handles the channel creation and management for you and capabilities can be configured at the room level. | ||
|
|
||
| ## Create or retrieve a room <a id="create"/> | ||
| ## Get a reference to a room <a id="create"/> |
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.
In Pub/Sub we use the phrase Use a channel - does that wording also work here?
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.
I was in-between "Use a room" or "Get a reference". I'll change to "Use a room"
|
|
||
| <If lang="javascript,swift,kotlin"> | ||
| A `room` is created, or an existing one is retrieved from the `rooms` collection using the <If lang="javascript">[`rooms.get()`](https://sdk.ably.com/builds/ably/ably-chat-js/main/typedoc/interfaces/chat-js.Rooms.html#get)</If><If lang="swift">[`rooms.get()`](https://sdk.ably.com/builds/ably/ably-chat-swift/main/AblyChat/documentation/ablychat/rooms/get%28named%3Aoptions%3A%29)</If><If lang="kotlin">[`rooms.get()`](https://sdk.ably.com/builds/ably/ably-chat-kotlin/main/dokka/chat/com.ably.chat/-rooms/get.html)</If> method: | ||
| To get a reference to a chat room, use the <If lang="javascript">[`rooms.get()`](https://sdk.ably.com/builds/ably/ably-chat-js/main/typedoc/interfaces/chat-js.Rooms.html#get)</If><If lang="swift">[`rooms.get()`](https://sdk.ably.com/builds/ably/ably-chat-swift/main/AblyChat/documentation/ablychat/rooms/get%28named%3Aoptions%3A%29)</If><If lang="kotlin">[`rooms.get()`](https://sdk.ably.com/builds/ably/ably-chat-kotlin/main/dokka/chat/com.ably.chat/-rooms/get.html)</If> method. It will create a new room reference if one doesn't already exist, or return the existing one if it does. |
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.
"Reference" implies something already exists - is that the right word here?
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.
I wanted to avoid wording that suggests a room is created (so avoid "a new room"). I find it more accurate that we create a reference to a room, or a room client (I wouldn't say that though) or similar.
Reference was the best I could think of?
If it creates more confusion than before maybe just go back to "a new room" type of wording?
WDYT?
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.
Perhaps get an instance of a chat room?
Co-authored-by: Andy Ford <[email protected]>
Description
CHA-1231
Tweak wording around presence, rooms.get and attachment to make these concepts clearer. LLMs get confused currently.
Checklist