-
-
Notifications
You must be signed in to change notification settings - Fork 10
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
Implement message attachments #148
Conversation
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.
@brichet Thank you for working on this while I was away! Left some feedback below.
d3cfaae
to
a96b7e5
Compare
Thanks for the review @dlqqq, I addressed some of your comments. I'll take a look at an alternative to |
2658548
to
f9f5a6b
Compare
The recurrent failing test do not fails locally. And strangely, it constantly fails at opening the chat, which is done in other tests with the same chat file... |
The failing test is the first one of the notification tests. After reordering the tests, the 'new' first test is now failing. |
6402bb1
to
217ed22
Compare
I finally skipped the notification test for now, I'll open a issue when this PR is merged to restore it in a follow up PR. |
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.
Thank you @brichet, the new changes look great! Left some additional minor feedback.
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 approve this PR because my latest feedback is minor & non-blocking.
If you do merge, please open follow-up issues for my latest feedback, thank you!
Co-authored-by: David L. Qiu <[email protected]>
Co-authored-by: David L. Qiu <[email protected]>
217ed22
to
cc6634f
Compare
Description
Demo
record-2025-02-13_11.04.21.webm
There is now a lite deployment in the built doc including a simple extension with a chat panel. I updated it to allow attachments https://jupyter-chat--148.org.readthedocs.build/en/148/lite/lab/index.html