charmbracelet / lipgloss

Style definitions for nice terminal layouts 👄
MIT License
8.03k stars 228 forks source link

bug(table): cutting right aligned text when margin is set #398

Open bashbunni opened 2 days ago

bashbunni commented 2 days ago

Describe the bug Rightmost characters cut off when cell content is right-aligned with margins.

image

To Reproduce Steps to reproduce the behavior: If you set margins on a cell style and set the text to be right-aligned, you will see the rightmost character truncated.

func TestStyleFunc(t *testing.T) {
    TestStyle := func(row, col int) lipgloss.Style {
        switch {
        // this is the header
        case row == HeaderRow:
            return lipgloss.NewStyle().Align(lipgloss.Center)
        // this is the first row of data
        case row == 0:
            return lipgloss.NewStyle().Margin(0, 1).Align(lipgloss.Right)
        default:
            return lipgloss.NewStyle().Margin(0, 1)
        }
    }

    table := New().
        Border(lipgloss.NormalBorder()).
        StyleFunc(TestStyle).
        Headers("LANGUAGE", "FORMAL", "INFORMAL").
        Row("Chinese", "Nǐn hǎo", "Nǐ hǎo").
        Row("French", "Bonjour", "Salut").
        Row("Japanese", "こんにちは", "やあ").
        Row("Russian", "Zdravstvuyte", "Privet").
        Row("Spanish", "Hola", "¿Qué tal?")

    t.Log(table.String())
}

Expected

image

bashbunni commented 2 days ago

After thinking about this a bit, the way margin is working does make sense. It shouldn't be considered part of the width of the element (same as in CSS). Table cells should only use padding to define extra spacing within a cell.

I see two options here: a) before rendering cells, unset margins + add a note in the godoc for tables to "use Margin not Padding for cell styles". (Margins don't work by default) b) get margin values and convert that to padding before rendering the cells. (Margins are applied to padding)

Note: in both cases this change will need to be both for headers & cells

meowgorithm commented 2 days ago

When I was learning CSS I found it so confusing that margins weren't factored into widths. I would personally err on including both the margins and padding in the calculation. Case-in-point, you were initially confused about this as well.

Beyond that, though, one should be able use to margins and padding together for, say, an effect like the following:

Anyway, this is a great candidate for the various queries that bundle margin, padding, and border like GetFrameSize.

bashbunni commented 1 day ago

@meowgorithm thank you! Great point, I'll work on a fix for this then