search

charmbracelet / lipgloss

Style definitions for nice terminal layouts 👄
MIT License
8.22k stars 231 forks source link

fix(table): do not shrink table with offset #373

Closed bashbunni closed 1 month ago

bashbunni commented 2 months ago

We should fix this before merging. This failing test is to provide context on this issue and how to reproduce it. I believe in this case we should honour the height rather than the offset since we are more likely to see dependents on the height.

bashbunni commented 2 months ago

Fixed the issue in the second commit :)

meowgorithm commented 1 month ago

Hey Bash! So what's the bug you're solving here exactly?

bashbunni commented 1 month ago

Hey, sorry I thought I has included screenshots or something. When you get to a certain offset, it would shrink the table instead of just moving the cursor, causing the height of the table to get smaller than the height specified by the user. If you run the test on master vs this branch you'll be able to see what I mean. Sorry I'll include images/recordings next time :)

meowgorithm commented 1 month ago

Wow, what a bug! Anyway, the fix looks good so far. To state the issue simply:

When the table is rendered with an offset the height of the table doesn't always match the actual height that was set.

Does that sound correct?

Here is some code to reproduce the issue: https://gist.github.com/meowgorithm/99a20cd66739a3d0550213c06da0f9a1

Let's make sure we test this on some real-world examples, including with the Table Bubble.

bashbunni commented 1 month ago

@meowgorithm yeah that's right! The table bubble doesn't currently use lipgloss table, that's how I discovered this bug was through porting the table bubble to use lipgloss' table renderer. As soon as there was the interactivity element, it broke.

This is the table bubble wip - https://github.com/charmbracelet/bubbles/pull/617 that you can use with this branch. I've actually got a working demo too on a drafted Bubble Tea PR that you can check out. Included instructions there for what to change in your go.work file to test locally.

https://github.com/charmbracelet/bubbletea/pull/1168

meowgorithm commented 1 month ago

Aha! That's right, table in Bubbles is totally its own thing. Okay cool, I'll test a bit more but generally this looks good.