-
Notifications
You must be signed in to change notification settings - Fork 0
bubbletea
: adds bubbletea table component and prepares Update
method for supporting multi models.
#18
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
bubbletea
: adds bubbletea table component and prepares Update
method for supporting multi models.
#18
Changes from all commits
ecf71ff
68430ff
b4b8caa
be81b82
b27f0d1
c017986
2e5695b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -8,12 +8,12 @@ import ( | |||||
|
||||||
// `TableModel` represents the component that implements the `Model` interface. | ||||||
type TableModel struct { | ||||||
// baseStyle is the base styling for the `table` model. | ||||||
baseStyle lipgloss.Style | ||||||
|
||||||
// table is the `bubbletea` table model. | ||||||
// TODO(@chris-ramon): Wire to the `TableModel` component. | ||||||
table table.Model | ||||||
|
||||||
// baseStyle is the base styling for the `table` model. | ||||||
baseStyle lipgloss.Style | ||||||
} | ||||||
|
||||||
// `Init` is the `TableModel` method required for implementing the `Model` interface. | ||||||
|
@@ -84,7 +84,18 @@ type TableModelParams struct { | |||||
|
||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency with the other methods in TableModel and to ensure proper state management, it would be good to use a pointer receiver for the Run method as well.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MentatBot nice suggestion - I've added this to the follow-up PR issue: #19, I'll consider addressing this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for adding this to the follow-up issue as well. Both suggestions relate to the same architectural pattern of using pointer receivers consistently, so addressing them together makes sense. Since the TableModel isn't actively used yet, this approach maintains good code organization while planning for future improvements. |
||||||
// `NewTableModel` returns a pointer to a `TableModel`. | ||||||
func NewTableModel(p *TableModelParams) *TableModel { | ||||||
columns := []table.Column{} | ||||||
rows := []table.Row{} | ||||||
|
||||||
t := table.New( | ||||||
table.WithColumns(columns), | ||||||
table.WithRows(rows), | ||||||
table.WithFocused(true), | ||||||
table.WithHeight(7), | ||||||
) | ||||||
|
||||||
return &TableModel{ | ||||||
baseStyle: p.BaseStyle, | ||||||
table: t, | ||||||
} | ||||||
} |
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 TableModel.Update method should use a pointer receiver and return (Model, tea.Cmd) to match the Model interface signature and ensure state is properly maintained (like the changes made to ChoicesModel.Update).
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.
@MentatBot nice catch ! - since currently we are not using
TableModel
I'll go ahead and update it in a follow-up PR, I've created an issue for addressing this: #19.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.
That sounds like a reasonable approach! Thanks for creating an issue to track this. Addressing it in a follow-up PR makes sense, especially since the TableModel isn't currently in active use. The current PR already makes significant architectural improvements to the codebase.