fix(table): fix some table rendering bugs #526
Conversation
6a1fbe8 to
70aaeac
Compare
table/resizing.go
Outdated
| if printedRows+rowHeight+btoi(isHeader && r.borderHeader)+btoi(r.borderBottom)+btoi(!isHeader && !isLastRow)+btoi(!isLastRow && r.borderRow) > r.tableHeight { | ||
| return i - btoi(hasHeaders) | ||
| } |
There was a problem hiding this comment.
I know this line looks complex, but it's a result of extensive testing and debugging, so we're covered (even by unit tests).
I can consider separating it into variables if you think it'd help.
There was a problem hiding this comment.
I improved this slightly since, but still open to opinions if you have.
There was a problem hiding this comment.
This one ended up being a small regression due to being tricky.
In theory the intended behavior is debatable, but I'm open to work on restoring the previous behavior.
In short, the previous behavior was forcing the table to show more records than asked if yOffset is too high. (For example, if the user asked to offset 90 out of 100 total rows, we'll still show more then 10 records if we're space "height" available for it).
There was a problem hiding this comment.
OK, I was actually able to restore the previous behavior and also fix some edge cases (but not all).
31f4ea2 to
b0c2052
Compare
7f01702 to
8471504
Compare
|
Hey @Broderick-Westrope, Just an update from charmbracelet/bubbles#601 (review) to let you know that your tests helped us to identify many bugs on table rendering and I have fixed them here. This will be ported over to the Bubbles' Table component because it'll soon use this under the hood. |
b369203 to
13b0c4e
Compare
fe54c28 to
1c5efe7
Compare
There was a problem hiding this comment.
Incredible work, @andreynering. This LGTM. Just to have it documented, how is the table API changing in this PR?
These functions were exposed as public because they're needed on my Bubbles PR:
|
|
We should probably also document that headers don't wrap in the README(?) |
|
Good call. I’d document it in the GoDocs. |
YOffsetsetting to offset terminal rows instead of table cells/rows #532fix(table): do not print final empty row when overflow
Before
After
fix(table): fix headers with custom vertical padding
Note that we're still always truncating headers to 1 line, but now we're respecting any custom vertical padding.
Before
After
Long Headers
fix(table): fix rendering for bordered cells
Before
After
fix(table): fix horizontal shrink with outline borders only
Before
After
fix(table): fix layout for tables with inner borders only
Before
After
feat(table): rework height rendering logic to fix related bugs
I suggest to checkout and take a look at the golden files at
table/testdata/TestTableHeightShrinkto understand the fixes done here.In short, the behavior was unreliable before, in particular when
BorderRow(true)was set or when cell had custom margin / padding.