Skip to content

Conversation

@gadenbuie
Copy link
Collaborator

@gadenbuie gadenbuie commented Jul 30, 2025

Replaces chat_enable_bookmarking() with chat_restore(), which now also restores the message history either from the bookmark or from the messages on client when passed in.

@schloerke and I paired through getting chat_enable_bookmarking() to work inside modules. Along the way, we realized that it'd be best for this function to also handle setting UI state from the message history attached to the chat client, in particular because we only want to perform that restore if we're not restoring from a bookmark.

This change meant that chat_enable_bookmarking() was not quite accurately named. Also, because chat_enable_bookmarking() still requires enableBookmarking to be turned on in the Shiny app, chat_restore() feels a bit more generally appropriate.

We also now turn on bookmarking in chat_app() and we also avoid the issues described in #71.

Finally, chat_mod_server() gains the bookmark_on_* arguments of chat_restore() and does the restoration automatically. And we deprecate client in chat_mod_ui() because this is now handled entirely on the server.

Implementation note: this PR includes a contents_shinychat() placeholder that will be replaced with contents_shinychat() in #52.

@gadenbuie gadenbuie marked this pull request as ready for review July 30, 2025 20:52
@gadenbuie gadenbuie requested review from cpsievert and schloerke July 30, 2025 20:53
@gadenbuie gadenbuie changed the title Fix/tool-bookmarking feat: chat_restore() replaces chat_enable_bookmarking() Jul 30, 2025
chat_clear(id)
client_set_ui(client, id = id)
shiny::withReactiveDomain(session, {
chat_clear(id)
Copy link
Collaborator

@cpsievert cpsievert Jul 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this also clear starting messages (i.e., chat_ui(messages = list(...)))?

Copy link
Collaborator Author

@gadenbuie gadenbuie Jul 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point and yeah, that's true. I've updated the docs of chat_mod_ui() to clarify that messages only applies when the client doesn't have any history. And left a similar note in the chat_restore() docs.

@gadenbuie gadenbuie merged commit 1e5b7cc into main Jul 31, 2025
10 checks passed
@gadenbuie gadenbuie deleted the fix/tool-bookmarking branch July 31, 2025 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants