-
Notifications
You must be signed in to change notification settings - Fork 114
feat(Chat, MarkdownStream): Support Shiny UI inside of message content #1868
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
Conversation
4aee016
to
298accc
Compare
403317d
to
b59c931
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.
The examples in the PR description show inputs/outputs in initial messages, but it's very hard to see how this feature would be used mid-stream or in the regular course of action with the chat component.
The test_chat_shiny_output
test gets a little bit closer by showing that you can pass UI to chat.append_message()
or chat.append_message_stream()
. That's appropriate for a test where we're proving that the feature works, but it's still not approximating how someone using the chat component would interact with these features.
I wouldn't say the above is a blocker for this PR -- it's something that can be addressed with the documentation you're working on -- but I do think it limits my ability to review the PR.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as off-topic.
This comment was marked as off-topic.
d9cdeee
to
2f46b2c
Compare
@gadenbuie, just curious, were you wanting more time to review before merging this one? |
shiny/ui/_markdown_stream.py
Outdated
content | ||
Content to display when the UI element is first rendered. | ||
A string of content to display before any streaming occurs. When | ||
`content_type` isn't `"text"`, it may also be UI element(s) such as input |
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.
`content_type` isn't `"text"`, it may also be UI element(s) such as input | |
`content_type` is Markdown or HTML, it may also be UI element(s) such as input |
It's a bit easier to parse this sentence if we say when UI elements are allowed.
I think we probably should update the content_type="semi-markdown"
description that currently say "with HTML tags escaped" to clarify that Shiny UI tags aren't escaped. (If I understand correctly and that's true.)
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.
to clarify that Shiny UI tags aren't escaped. (If I understand correctly and that's true.)
Shiny UI tags get escaped as well:
from shiny import reactive
from shiny.express import ui
md = ui.MarkdownStream("stream")
md.ui(content_type="semi-markdown")
@reactive.effect
async def _():
await md.stream([ui.div("Hello"), ui.tags.b("world")])
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.
Looks great! I had just a few small comments, all minor
Closes #1813
This PR adds support for the content of a message provided to
ui.Chat()
orui.MarkdownStream()
to include Shiny UI, and moreover, if that UI includes output or input bindings, they are bound to the server session.This means, among other things, you can now display things like data grids or Jupyter Widgets as well as collect user input from things like a select input. Here are a couple basic examples:
Output example
Input example
NOTE
chat_append()
andmarkdown_stream()
shinychat#29