Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | 0 |
TIP This summary will be updated as you push new changes. Give us feedback
More templates
algoliasearch-helper
instantsearch-ui-components
instantsearch.css
instantsearch.js
react-instantsearch
react-instantsearch-core
react-instantsearch-nextjs
react-instantsearch-router-nextjs
vue-instantsearch
commit: |
|
instead, should clear maybe stop the streaming too? good catch though |
2ebac09 to
15b4a5d
Compare
that could be possible yes, but would it not be a bit unclear to the user that pressing clear somehow also stops it? Maybe we change the text to 'Stop'? Seems redundant due to a stop button already being present. |
There was a problem hiding this comment.
Pull request overview
Prevents users from clearing chat messages while the chat is generating a response, to avoid inconsistent UI/message states during streaming.
Changes:
- Disable the Chat “Clear” button when chat status is not
ready(React InstantSearch + InstantSearch.js widget wrapper). - Add a widget options test asserting the Clear button becomes disabled while the chat is streaming.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/common/widgets/chat/options.tsx | Adds regression test for Clear button disabling during in-progress chat. |
| packages/react-instantsearch/src/widgets/Chat.tsx | Gates canClear on status === 'ready' in React widget header props. |
| packages/instantsearch.js/src/widgets/chat/chat.tsx | Gates canClear on chatStatus === 'ready' in InstantSearch.js widget wrapper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I think if you want to clear the input while it's busy rendering, it's likely because you want to send a new request, no? so stopping makes sense to me with the same clear text already there @shaejaz |
Haroenv
left a comment
There was a problem hiding this comment.
I think instead, we should make clear stop the current request too.
Haroenv
left a comment
There was a problem hiding this comment.
might also be interesting to set the messages to empty array already before the transition? not sure if that matters or why the connector knows about the transition. Good to go though
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Currently, the user is able to click on the clear button while messages are streaming, resulting in weird message states. This prevents the chat to be cleared until the chat is back in a ready state.