-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Obs AI Assistant] Show Conversations list in flyout #175226
[Obs AI Assistant] Show Conversations list in flyout #175226
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
hasOtherConversations | ||
? conversations.value!.conversations[0].conversation.id | ||
: undefined | ||
hasOtherConversations ? hasOtherConversations.conversation.id : undefined |
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.
If you deleted the first conversation in the list, you'd get a "conversation not found" error.
@@ -161,13 +163,13 @@ export function ChatBody({ | |||
} | |||
}; | |||
|
|||
const handleChangeHeight = (editorHeight: number) => { | |||
const handleChangeHeight = useCallback((editorHeight: number) => { |
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.
This caused the render to loop which made focus be put onto the PromptEditor over and over.
@@ -142,6 +142,8 @@ export function PromptEditor({ | |||
useEffect(() => { | |||
if (hidden) { | |||
onChangeHeight(0); | |||
} else if (containerRef.current) { | |||
onChangeHeight(containerRef.current.clientHeight); |
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.
Because I memoized the callback being passed in, this wasn't being called as often due to less renders so had to make sure it's called at least when this flag switches.
The ResizeablePanel causes more renders to happen which caused the focus bug due to missing memoization
…rsations-in-flyout
…rsations-in-flyout
It wasn't straightforward to make the conversations list maintain a minimum width due to how the ResizeablePanels work, and the size changing wasn't smooth. We'll need to follow up with better solutions in this area (in collaboration with Design and the EUI team). |
…rsations-in-flyout
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
As far as I know, the work attempted in this PR was included in #174677 |
Summary
Changes the
ChatFlyout
to useEuiResizeablePanel
to split the flyout content in two.On the left we show the Conversations list and this panel can be collapsed to free up space.
On the right we show the
ChatBody
like before.Screenshots
Default:
Conversation list made larger:
Conversation list collapsed:
Conversations link moved to three-dot menu: