Evertras / bubble-table

A customizable, interactive table component for the Bubble Tea framework
MIT License
455 stars 27 forks source link

Multiline cells #159

Closed prgres closed 1 year ago

prgres commented 1 year ago

@Evertras I did the same in the bubbletea.Table and it renders ok https://github.com/charmbracelet/bubbles/pull/433

image

It looks ok hence it fails on movement (probably because I do not know how to manipulate the yoffset)

prgres commented 1 year ago

I tried to re-render data after the loop in renderRowData but I need access to columns style

multiline := false
    for _, column := range columnStrings {
        if lipgloss.Height(column) > 1 {
            multiline = true
            break
        }
    }

    if multiline {
        for _, column := range columnStrings {
            cellStyle := rowStyle.Copy().Inherit(column.style).Inherit(m.baseStyle)

        }
    }

I changed the plan. Now, I will try to store that info in a ~column~ row struct

prgres commented 1 year ago

Okay, I cannot do it that way since RowData is unaware of the column width.

What I am trying to do is to fill other cells with empty lines to align heigh of multiline

prgres commented 1 year ago

Some progress:

image

All we have to do is set the borderStyle.Height in renderRowData to the size of the highest cell

prgres commented 1 year ago

OMG I think I got this:

image
prgres commented 1 year ago

3 tests fails but I hope it will be straightforward to fix that

prgres commented 1 year ago

To test it on your own use option WithMultiline(true) while creating a new table

prgres commented 1 year ago

@Evertras if you will have some free time to help, I would be very grateful. The current state fails only 3 tests, but I think that not every case - something is noyes with styles for specific cells. I assume it is connected to the fact that I did not cover all logic in my loop

prgres commented 1 year ago

Do not bother with the look of the code - it is just POC that will need a refactor

prgres commented 1 year ago

For some reason, this is needed 🤔

image
prgres commented 1 year ago

And has to be particular Top - Botttom, Left, Center does not work

prgres commented 1 year ago

In a very dirty way, I checked that is not (probably) connected to overflow

func (m Model) renderRowData(row Row, rowStyle lipgloss.Style, last bool) string {
    numColumns := len(m.columns)

    columnStrings := []string{}
    totalRenderedWidth := 0
    tmpTotalRenderedWidth := 0
    rawColumnsStrings := make([]string, 3*len(m.columns))

    stylesInner, stylesLast := m.styleRows()

    maxCellHeight := 0
    columnIndex := 0
    for rawColumnIndex := 1; rawColumnIndex/3 < len(m.columns); rawColumnIndex += 3 {
        if rawColumnIndex != 1 {
            columnIndex = (rawColumnIndex - 1) / 3
            // 0 1
            // 1 4
            // 2 7
        }
        column := m.columns[columnIndex]
        emptyStyle := lipgloss.NewStyle()

        if m.horizontalScrollOffsetCol > 0 && columnIndex == m.horizontalScrollFreezeColumnsCount {
            rendered := m.renderRowColumnData(row, genOverflowColumnLeft(1), rowStyle, emptyStyle)

            rawColumnsStrings[rawColumnIndex-1] = rendered
        }

        if columnIndex >= m.horizontalScrollFreezeColumnsCount &&
            columnIndex < m.horizontalScrollOffsetCol+m.horizontalScrollFreezeColumnsCount {
            continue
        }

        //--
        cellStr := m.renderRowColumnData(row, column, rowStyle, emptyStyle)
        maxCellHeight = max(maxCellHeight, lipgloss.Height(cellStr))
        rawColumnsStrings[rawColumnIndex] = cellStr
        //--
        if m.maxTotalWidth != 0 {
            renderedWidth := lipgloss.Width(cellStr)

            const (
                borderAdjustment = 1
                overflowColWidth = 2
            )

            targetWidth := m.maxTotalWidth - overflowColWidth

            if columnIndex == len(m.columns)-1 {
                // If this is the last header, we don't need to account for the
                // overflow arrow column
                targetWidth = m.maxTotalWidth
            }

            if tmpTotalRenderedWidth+renderedWidth > targetWidth {
                overflowWidth := m.maxTotalWidth - tmpTotalRenderedWidth - borderAdjustment
                overflowColumn := genOverflowColumnRight(overflowWidth)
                overflowStr := m.renderRowColumnData(row, overflowColumn, rowStyle, emptyStyle)

                rawColumnsStrings[rawColumnIndex+1] = overflowStr

                break
            }

            tmpTotalRenderedWidth += renderedWidth
        }
    }
    // for i, column := range rawColumnsStrings {
    //  fmt.Println(i, column)
    // }

    // fmt.Println("---")

    for columnIndex, column := range m.columns {
        var borderStyle lipgloss.Style
        var rowStyles borderStyleRow

        if !last {
            rowStyles = stylesInner
        } else {
            rowStyles = stylesLast
        }

        if m.horizontalScrollOffsetCol > 0 && columnIndex == m.horizontalScrollFreezeColumnsCount {
            var borderStyle lipgloss.Style

            if columnIndex == 0 {
                borderStyle = rowStyles.left.Copy()
            } else {
                borderStyle = rowStyles.inner.Copy()
            }

            cellStyle := rowStyle.Copy().Inherit(genOverflowColumnLeft(1).style).
                Inherit(m.baseStyle).Height(maxCellHeight)
            cellStyle = cellStyle.Inherit(borderStyle)
            idx := columnIndex*3 + 1 - 1
            if idx <= 0 {
                idx = 1
            }
            rendered := cellStyle.Render(rawColumnsStrings[idx])
            // rendered := cellStyle.Render(rawColumnsStrings[columnIndex*3-1])

            totalRenderedWidth += lipgloss.Width(rendered)

            columnStrings = append(columnStrings, rendered)
        }

        if columnIndex >= m.horizontalScrollFreezeColumnsCount &&
            columnIndex < m.horizontalScrollOffsetCol+m.horizontalScrollFreezeColumnsCount {
            continue
        }

        if len(columnStrings) == 0 {
            borderStyle = rowStyles.left
        } else if columnIndex < numColumns-1 {
            borderStyle = rowStyles.inner
        } else {
            borderStyle = rowStyles.right
        }

        cellStyle := rowStyle.Copy().Inherit(column.style).
            Inherit(m.baseStyle).Height(maxCellHeight)
        cellStyle = cellStyle.Inherit(borderStyle)
        idx := columnIndex*3 + 1
        // if idx <= 0 {
        //  idx = 1
        // }
        // fmt.Println(idx, rawColumnsStrings[idx])
        cellStr := cellStyle.Render(rawColumnsStrings[idx])
        // cellStr := cellStyle.Render(rawColumnsStrings[columnIndex*3])

        if m.maxTotalWidth != 0 {
            renderedWidth := lipgloss.Width(cellStr)

            const (
                borderAdjustment = 1
                overflowColWidth = 2
            )

            targetWidth := m.maxTotalWidth - overflowColWidth

            if columnIndex == len(m.columns)-1 {
                // If this is the last header, we don't need to account for the
                // overflow arrow column
                targetWidth = m.maxTotalWidth
            }

            if totalRenderedWidth+renderedWidth > targetWidth {
                overflowWidth := m.maxTotalWidth - totalRenderedWidth - borderAdjustment
                cellStyle := rowStyle.Copy().Inherit(genOverflowStyle(rowStyles.right, overflowWidth)).
                    Inherit(m.baseStyle).Height(maxCellHeight)
                cellStyle = cellStyle.Inherit(borderStyle)
                rendered := cellStyle.Render(rawColumnsStrings[columnIndex*3+1+1])

                totalRenderedWidth += lipgloss.Width(rendered)

                columnStrings = append(columnStrings, rendered)
                break
            }

            totalRenderedWidth += renderedWidth
        }

        columnStrings = append(columnStrings, cellStr)
    }

    return lipgloss.JoinHorizontal(lipgloss.Bottom, columnStrings...)
}
prgres commented 1 year ago

I managed to achieve multiline feature with clean tests

image

The only drawback is that it renders cell content twice if the multiline option is enabled. But I do not have any other idea to handle it without refactoring the current setup.

Evertras commented 1 year ago

I'm okay with offering an additional feature at a small expense to performance if that feature is entirely opt-in, which it seems to be here. We should add a small note about potential performance issues in the WithMultiline func comment for docs.

Also seems I need to update the linter settings, haven't looked at that in a while...

Evertras commented 1 year ago

Looking at this a bit more, I feel like the perf shouldn't really be an issue. So maybe we don't need the comment. 🤔

Evertras commented 1 year ago

We'll also definitely want to add some tests around what multiline tables should look like with expected inputs, which should also bump our coverage up to where it needs to be. Can you add some tests around expected outputs to https://github.com/Evertras/bubble-table/blob/main/table/view_test.go ?

Sorry for missing messages before, was away for a bit.

Evertras commented 1 year ago

And as a final note, feel free to make the features example permanently show multiline as well as in your screenshot.

prgres commented 1 year ago

@Evertras I have been waiting to add tests and examples for your approval. As you agreed on that solution, I will add them soon :)

prgres commented 1 year ago

@Evertras done

No worries about the missed messages. I treated this PR as a dev log to maintain a history and share hints about the implementation for future contributors