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

test: improve table tests based on reported issues #593

Open
bashbunni opened this issue Aug 21, 2024 · 4 comments
Open

test: improve table tests based on reported issues #593

bashbunni opened this issue Aug 21, 2024 · 4 comments
Labels
help wanted Extra attention is needed
Milestone

Comments

@bashbunni
Copy link
Member

We want to replace the current table rendering with lipgloss' table in the future. It would be great to create some test cases given reported issues in table. Check out the v0.20.0 milestone for some examples.

You can use this test as a reference to get started
https://github.com/charmbracelet/bubbles/blob/master/table/table_test.go#L112

x/golden will create a new golden file for your test when you run the test with the -update flag. E.g. go test -run "TestTableAlignment/No border" -update. You can use this to create golden files from your got value.

Other examples testing interactivity:

Other examples with golden files:

If anything is unclear, don't hesitate to ping me 😄

@bashbunni bashbunni added this to the v0.20.0 milestone Aug 21, 2024
@bashbunni bashbunni added the help wanted Extra attention is needed label Aug 21, 2024
@Broderick-Westrope
Copy link

Hey @bashbunni, I've started work on this in #601. Will continue to go through the v0.20.0 milestone issues and also have a think about some others that would be worthwhile. A few questions for you:

  • I've seen mention of deprecating/replacing bubbles/table with lipgloss/table. What's the ETA on this change?
  • Currently I have added a test which checks for the occurrence of issue Table cell padding no longer works since 1ba1200 #472 rather than checking the desired output. This is because the thread provided no decision on what output is correct when the table width is smaller than the combined cell widths. Clarity on this would be appreciated.

Thanks :)

@bashbunni
Copy link
Member Author

Hey @Broderick-Westrope thank you for your help on this! Ideally the change to lipgloss table would be the next release (no set date, but I'd estimate a few months give or take). I'll take a look at that issue and clarify what the expected behaviour should be 🙏

@bashbunni
Copy link
Member Author

Hey it looks like lipgloss table is calculating this behaviour properly, so you could probably create a test and create the golden file from lipgloss's table output. Although, I don't think Height is working as expected in the lipgloss table 🤔 Will look into it

To create the golden file you can use https://github.com/charmbracelet/x/blob/main/exp/golden/golden.go, create the table we want with lipgloss in the test, then run that specific test with the -update flag to create the golden file, then change the test to reproduce the actual behaviour we're seeing in bubbles right now.

this gives the same table, but rendered with lipgloss

package main

import (
	"fmt"

	"github.com/charmbracelet/bubbles/table"
	tea "github.com/charmbracelet/bubbletea"
	"github.com/charmbracelet/lipgloss"

	glossytable "github.com/charmbracelet/lipgloss/table"
)

var style = lipgloss.NewStyle().BorderStyle(lipgloss.NormalBorder())

type model struct {
	table table.Model
}

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

func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
	var cmd tea.Cmd
	switch msg := msg.(type) {
	case tea.KeyMsg:
		switch msg.String() {
		case "q", "ctrl+c":
			return m, tea.Quit
		}
	}
	m.table, cmd = m.table.Update(msg)
	return m, cmd
}

func (m model) View() string {
	return style.Render(m.table.View())
}

func main() {
	columns := []table.Column{
		{Title: "ID", Width: 4},
		{Title: "Title", Width: 10},
		{Title: "Description", Width: 10},
	}

	rows := []table.Row{
		{"1", "Lost in Time", "A thrilling adventure through the ages."},
		{"2", "Whispering Shadows", "Secrets unravel in a haunted town."},
		{"3", "Stolen Identity", "An innocent man's life turned upside down."},
		{"4", "Into the Abyss", "Exposing deep-rooted conspiracies."},
	}

	t := table.New(
		table.WithColumns(columns),
		table.WithRows(rows),
		table.WithFocused(true),
		table.WithWidth(20),
		table.WithHeight(4),
	)
	fmt.Println(style.Render(t.View()))

	gloss := glossytable.New().
		Headers("ID", "title", "description").
		Row("1", "Lost in Time", "A thrilling adventure through the ages.").
		Row("2", "Whispering Shadows", "Secrets unravel in a haunted town.").
		Row("3", "Stolen Identity", "An innocent man's life turned upside down.").
		Row("4", "Into the Abyss", "Exposing deep-rooted conspiracies.").
		Border(lipgloss.NormalBorder()).
		Width(20).
		Height(4)

	fmt.Println(gloss)
}

@bashbunni
Copy link
Member Author

After checking lipgloss's source code, looks like the height isn't accounted for during resizing. Created an issue to track that charmbracelet/lipgloss#356 :)

@caarlos0 caarlos0 modified the milestones: v0.20.0, v0.21.0 Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants