Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements conversation search functionality for the thread list using the built-in bubbles list filter component. It addresses issue #202 by enabling users to press / to search thread titles.
Changes:
- Added width and height tracking to ThreadListModel to support fixed-size rendering with lipgloss.Place
- Refactored message handling in ChatModel to forward non-KeyMsg messages to ThreadListModel, enabling interactive features like filtering
- Added test coverage for non-KeyMsg message forwarding
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/ui/thread_list.go | Added width/height fields to ThreadListModel and wrapped View output with lipgloss.Place for fixed-size rendering |
| internal/ui/chat.go | Refactored if statements to switch statement and added ModeThreadList case to forward non-KeyMsg messages to the thread list component |
| internal/ui/chat_thread_list_test.go | Added test to verify non-KeyMsg messages are properly forwarded to ThreadListModel |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| logger *log.Logger | ||
| list list.Model | ||
| store *storage.Store | ||
| width, height int |
There was a problem hiding this comment.
The width and height fields stored in ThreadListModel are not updated when the window is resized. When handleWindowSize is called in ChatModel, it updates model.width and model.height but doesn't update model.threadList.width and model.threadList.height. This means lipgloss.Place will use stale dimensions if the user resizes the terminal window while in thread list mode.
Consider updating ThreadListModel's dimensions in handleWindowSize when in ModeThreadList, or forward WindowSizeMsg to the threadList.Update method to allow it to update its dimensions.
| func (model ThreadListModel) View() tea.View { | ||
| view := tea.NewView(model.list.View()) | ||
| view := tea.NewView( | ||
| lipgloss.Place(model.width, model.height, lipgloss.Left, lipgloss.Center, model.list.View()), |
There was a problem hiding this comment.
Using lipgloss.Center for vertical alignment might not be the best choice for a thread list UI. Lists are typically top-aligned so users can see the beginning of the list immediately. With center alignment, if the list has fewer items than can fit in the height, it will be vertically centered, which may look odd. Consider using lipgloss.Top instead of lipgloss.Center for the vertical alignment.
| lipgloss.Place(model.width, model.height, lipgloss.Left, lipgloss.Center, model.list.View()), | |
| lipgloss.Place(model.width, model.height, lipgloss.Left, lipgloss.Top, model.list.View()), |
Closes #202