Evertras
fix: column style respects paddings and margins
Address https://github.com/Evertras/bubble-table/issues/130
A little context (copied and pasted from the issue):
What I found so far is this line. cellStyle is built by coping rowStyle and inherit a column.style
func (m Model) renderRowColumnData(row Row, column Column, rowStyle lipgloss.Style, borderStyle lipgloss.Style) string {
cellStyle := rowStyle.Copy().Inherit(column.style).Inherit(m.baseStyle)
In its implementation in the lipgloss package, we can see that margins and padding properties are skipped
func (s Style) Inherit(i Style) Style {
s.init()
for k, v := range i.rules {
switch k {
case marginTopKey, marginRightKey, marginBottomKey, marginLeftKey:
// Margins are not inherited
continue
case paddingTopKey, paddingRightKey, paddingBottomKey, paddingLeftKey:
// Padding is not inherited
continue
case backgroundKey:
s.rules[k] = v
// The margins also inherit the background color
if !s.isSet(marginBackgroundKey) && !i.isSet(marginBackgroundKey) {
s.rules[marginBackgroundKey] = v
}
}
if _, exists := s.rules[k]; exists {
continue
}
s.rules[k] = v
}
return s
}
With that knowledge, I created a column with left padding
columns := []table.Column{
table.NewColumn(columnKeyName, "Name", 10).WithStyle(
lipgloss.NewStyle().
Foreground(lipgloss.Color("#88f")),
),
table.NewColumn(columnKeyCountry, "Country", 20).WithStyle(
lipgloss.NewStyle().
Foreground(lipgloss.Color("#f88")).
PaddingLeft(5),
),
table.NewColumn(columnKeyCurrency, "Currency", 10),
and change the renderRowColumnData a little bit
func (m Model) renderRowColumnData(row Row, column Column, rowStyle lipgloss.Style, borderStyle lipgloss.Style) string {
cellStyle := rowStyle.Copy().Inherit(column.style).Inherit(m.baseStyle)
_, _, _, colPadLeft := column.style.GetPadding()
if colPadLeft > 0 {
cellStyle = cellStyle.PaddingLeft(colPadLeft)
}
Which results in the desired state:
What we can do is:
- modify the Inherit method or create a custom one overriding all values from the incoming style - this will be impossible to make in a finite period since contributions to charm are hella slow (or they do not like me dunno)
- append that logic into renderRowColumnData (for paddings and margins)
Okay, it may be tricky because GetPadding and GetMargins return values or 0 if a property is not set. Creating a simple if statement as I did may result in a situation when a rowStyle sets padding to >0 value and the user wants to have a column with padding ==0 and it will not be overridden.
Without access lipgloss.Style.rules it may be hard to implement simply. We can cover the override margins/paddings with a next column flag but I do not if it is a best approach here
It came out that we cannot set paddings and margins in base style because it breaks the layout
PaddingRight(10)
MarginRight()
Taking a moment to play with this, want to see if there's some potential way to break it...
Unfortunately, this doesn't seem to work as expected for me, or for the #130 code sample. Using your branch I updated the feature example column:
table.NewColumn(columnKeyDescription, "Description", 30).WithStyle(
lipgloss.NewStyle().PaddingLeft(3),
),
The result is unchanged:
I would have expected the padding to be applied to the Description column. Additionally, running the sample given in #130 doesn't give the expected output.
Oddly, using your given example, I still don't see the padding change. I do see the foreground color update, but the columns are still strictly left aligned. I feel like something might be odd here, but I'm not sure what. I confirmed I have the actual code changes in my local copy with your branch checked out.
It may also be helpful to create a test with the code in #130 to see if we can use that as a specific target.
Are you still taking a look at this? If not I'd like to close for cleaning things up, but we can always reopen later.
Closing this for now for tidiness, but we can reopen again later if you dig back in.