Skip to content
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

bug(table): computeHeight doesn't honor all rows when hasHeaders is false #468

Open
wI2L opened this issue Feb 1, 2025 · 5 comments
Open
Labels
bug Something isn't working

Comments

@wI2L
Copy link

wI2L commented Feb 1, 2025

Hello,

The computeHeight function doesn't seem to honor the number of rows returned by the Data.Rows() interface method.

I have implemented the table.Data interface to return the number of rows my table bubble contains using the Rows() method.

I have noticed that when I "hide" the headers by using table.Headers(), then the computeHeight function returns a value that doesn't represent the correct number of rows.

I believe this is due to the fact that the function substracts 1 from the returned height, which I cannot understand why: return sum(t.heights) - 1 + btoi(hasHeaders) +.

When hasHeaders is true, it essentially "cancels out" the previous substraction, and thus the number of rows that will be displayed is correct, since the returned value is used to define the MaxHeight.

I have confirmed that if I "artificially increase" the number of rows returned by my Data.Rows() method implem, then the height of the table is correct and all rows are displayed, however this implies that the x index passed to the Data.At method is wrong and leads to out of bounds panics.

@meowgorithm meowgorithm added the bug Something isn't working label Feb 3, 2025
@meowgorithm
Copy link
Member

Hi! Thanks for the detailed report. Do you have any code to reproduce the issue? If so, it will help accelerate a fix.

@wI2L
Copy link
Author

wI2L commented Feb 3, 2025

@meowgorithm Still working on my pet project, with which I found this. I'll write a simpler reproducible example to demonstrate the behavior ASAP.

@meowgorithm
Copy link
Member

Thank you!

@wI2L
Copy link
Author

wI2L commented Feb 3, 2025

@meowgorithm I think the following example shows the behavior quite right.

The content of the table is generated from random strings, but what matters if the "rows height" of the table.

You can see that by default I return 10 as the number of rows that the table should display. When the program starts, the headers are visible. If you press the key h, it will toggle hide the headers using table.Headers().

When the headers are hidden, the number of visible rows is not 10 (as it should), but 9.

IMO this is 100% the responsibility of the style which uses the return of the computeHeight method to set the MaxHeight() of the table's style.

With headers
Image

Without headers
Image

package main

import (
	"fmt"
	"math/rand"

	"github.com/charmbracelet/bubbletea/v2"
	"github.com/charmbracelet/lipgloss/v2/table"
)

func main() {
	m := Model{
		Table:       &table.Table{},
		HideHeaders: false,
	}
	m.Table.Headers("Foo", "Bar", "Baz")
	m.Table.Data(data{RowsCount: 10})

	p := tea.NewProgram(m)
	p.Run()
}

type Model struct {
	Table       *table.Table
	HideHeaders bool
}

func (m Model) Init() (tea.Model, tea.Cmd) {
	return m, nil
}

func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
	switch msg := msg.(type) {
	case tea.KeyMsg:
		switch msg.String() {
		case "ctrl+c", "q":
			return m, tea.Quit
		case "h":
			m.HideHeaders = !m.HideHeaders
			if m.HideHeaders {
				m.Table.Headers()
			} else {
				m.Table.Headers("Foo", "Bar", "Baz")
			}
		}
	}
	return m, nil
}

func (m Model) View() string {
	return fmt.Sprintf("%s\n", m.Table.Render())
}

type data struct {
	RowsCount int
}

func (d data) Rows() int {
	return d.RowsCount
}

func (d data) Columns() int {
	return 3
}

func (d data) At(x, y int) string {
	return randSeq(10)
}

var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ")

func randSeq(n int) string {
	b := make([]rune, n)
	for i := range b {
		b[i] = letters[rand.Intn(len(letters))]
	}
	return string(b)
}

@wI2L
Copy link
Author

wI2L commented Feb 3, 2025

In the end, I still cannot understand why 1 is substracted from the computed height on this line, but to me that's the culprit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants