setting table height doesn't do anything

[bashbunni]

We should cut off additional rows when a height is explicitly set. The last allowed row should probably just show '...' to show that there's more content than the height allows. Checked the source code and the resizing logic is only applied to width

code to reproduce:

package main

import (
    "fmt"

    "github.com/charmbracelet/lipgloss"

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

func main() {
    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)
}

[Broderick-Westrope]

@bashbunni can you clarify what is meant to impact the height of the table?

1. Just the number of lines?
2. Or the number of rows?
3. Or the rows + header?

To demonstrate, the following is a table of height 4 using each of these approaches:

1.
╭──┬───────────────┬───────────────────╮
│ID│title          │description        │
├──┼───────────────┼───────────────────┤
╰──┴───────────────┴───────────────────╯

2.
╭──┬───────────────┬───────────────────╮
│ID│title          │description        │
├──┼───────────────┼───────────────────┤
│1 │Lost in Time   │A thrilling advent…│
│2 │Whispering Sha…│Secrets unravel in…│
│3 │Stolen Identity│An innocent man's …│
│4 │Into the Abyss │Exposing deep-root…│
╰──┴───────────────┴───────────────────╯

3.
╭──┬───────────────┬───────────────────╮
│ID│title          │description        │
├──┼───────────────┼───────────────────┤
│1 │Lost in Time   │A thrilling advent…│
│2 │Whispering Sha…│Secrets unravel in…│
│3 │Stolen Identity│An innocent man's …│
╰──┴───────────────┴───────────────────╯

NOTE: This example isn't concerned with adding ellipsis as the bottom row ;)

Currently bubbles/table takes a hybrid approach where the height is the sum of two things:

• lipgloss.Height(headers) which takes into account styling on the headers (eg. borders).
• and the length of the rows slice without styling.

Meaning the bubbles/table equivalent of length 4 would be:

╭──┬───────────────┬───────────────────╮
│ID│title          │description        │
├──┼───────────────┼───────────────────┤
│1 │Lost in Time   │A thrilling advent…│
╰──┴───────────────┴───────────────────╯

3 lines for the header with styling + 1 line for the row content (no styling)

[Broderick-Westrope]

The PR I mentioned is currently taking approach 1 from my original message. I am happy to fix the unit tests and close this out once you confirm the desired approach for calculating table height :)

It's also worth clarifying whether height should be increased beyond what is needed. Since bubbles/table sets the height on a viewport you can make it as long as you want, but in my current lipgloss/table solution you cannot increase the height of a table beyond the total number of rows (ie. no inserting empty rows).

[bashbunni]

@Broderick-Westrope I think the height should be counted by cell (number of lines) for consistency's sake. The width is set based on the total width (number of cells) of the table, including borders, so I think height should be the same

[meowgorithm]

I agree: the first approach makes the most sense to me and will generally help when reasoning about layout.

[Broderick-Westrope]

Great, thank you both for clarifying. That's what's currently implemented. I'll just fix up the unit tests and mark it ready for review.

[Broderick-Westrope]

Done. I've written a detailed description in the PR demonstrating different cases :)