fix(table): substract headers Y size from the total table size #434

prgres · 2023-11-13T13:39:49Z

I spotted a bug while creating a table and viewport side by side. Even though they have set the exact height value, both have different sizes. While debugging, it came out that we only passed it down to the nested viewport component by setting table height, but the render method looks like this.

// View renders the component.
func (m Model) View() string {
	return m.headersView() + "\n" + m.viewport.View()
}

So, the total height of the table is the sum of the viewport's height and the headers' height. I have simply subtracted headings Y size from the viewport height, and it looks like this fixed the issue.

prgres · 2023-11-13T14:34:30Z

Okay, it does not work as I expected. I will dig it up latter

prgres · 2023-11-13T14:53:42Z

Okay, first of all, I changed GetFrameSize to GetVerticalFrameSize since X calculations are not helpful here.

Secondly, I realized that GetVerticalFrameSize does not return the total height of the component - just the sum of margins, padding, and borders. So, besides subtracting that one bad boi, I have to get in content height. I have used lipgloss.Height(m.headersView()) in the SetHeight function. I am unsure if this approach is good since it renders it twice (in SetHeight and View functions).

What is worth mentioning is that the content height, with the current implementation, is always 0 or 1 due to truncating the header text. But I keep in mind that if #433 will merge, I would also like to make a similar one for that part. That's why I decided to calculate that value here.

prgres · 2023-11-13T17:44:00Z

Update once again:

During testing on a local setup, it came out that GetVerticalFrameSize is not necessary at all. But would great if somebody could confirm that

prgres · 2023-11-27T09:08:41Z

@meowgorithm @muesli i do not who to ping

maaslalani · 2024-02-28T17:23:18Z

Hey @prgres, thanks so much for the PR ❤️

maaslalani · 2024-03-13T18:33:05Z

Thanks for the PR @prgres, I merged this one since it's a simple bug fix but just to give you a heads up, we're going to swap out the entire implementation for the Lip Gloss table renderer:

https://github.com/charmbracelet/lipgloss/releases/tag/v0.9.0

…bracelet#434) * fix(table): substract headers Y size from the total table size * fix: change GetFrameSize to GetVerticalFrameSize and calc header height * fix: remove GetVerticalFrameSize at all * fix: account for headers in height WithHeight, SetHeight --------- Co-authored-by: Maas Lalani <[email protected]>

…(#33) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [github.com/charmbracelet/bubbles](https://github.com/charmbracelet/bubbles) | require | minor | `v0.18.0` -> `v0.19.0` | --- ### Release Notes <details> <summary>charmbracelet/bubbles (github.com/charmbracelet/bubbles)</summary> ### [`v0.19.0`](https://github.com/charmbracelet/bubbles/releases/tag/v0.19.0) [Compare Source](charmbracelet/bubbles@v0.18.0...v0.19.0) ### Bugs? Squashed (along with a few nice lil’ features). Community-Driven Development?! Yep, the majority of the changes in this release were done by the community. *Thank you* all for your contributions that made this release possible. #### Progress: custom chars You can now customize the filled and empty characters of the progress bar. ```go p := progress.New(progress.WithFillCharacters('>', '.')) ``` ![progress bar example](https://github.com/user-attachments/assets/ee1a1351-ebee-4f39-8543-af464e60b099) #### Table improvements ##### Help is on the way Table now includes a short and full help view so it's easier than ever to tell your users how to interact with the table. ```go // Render a table with its help. t := table.New() view := t.View() + "\n" + t.HelpView() ``` <img src="https://github.com/user-attachments/assets/22195043-5578-4201-982d-a8f9b7eefc5f" width="600"> ##### Accessing columns You can also now get the table's columns (this already existed for rows). ```go package table // Columns returns the current columns. func (m Model) Columns() []Column ``` #### List: page navigation is fixed! Previously, `list.NextPage()` and `list.PrevPage()` didn't work because the methods did not have pointer receivers. We've fixed this…by making them pointer receivers! ⚠️ Note that this is a minor API change and you *might* need to update your app to pass a pointer receiver to your model rather than a copy. Details in [#458](charmbracelet/bubbles#458). ```go package progress // NextPage moves to the next page, if available. func (m *Model) NextPage() // PrevPage moves to the previous page, if available. func (m *Model) PrevPage() ``` *** #### What’s Changed ##### Changed - Textarea: Improve setting width by [@mikelorant](https://github.com/mikelorant) in charmbracelet/bubbles#496 - Textinput: fix out of range panic if no matched suggestions by [@rdnt](https://github.com/rdnt) in charmbracelet/bubbles#473 - List: Fix no-op list pagination functions by [@nekopy](https://github.com/nekopy) in charmbracelet/bubbles#458 - Table: Clarify position constant in JoinHorizontal by [@aditipatelpro](https://github.com/aditipatelpro) in charmbracelet/bubbles#577 - Progress: make full/empty fill characters configurable by [@rwinkhart](https://github.com/rwinkhart) in charmbracelet/bubbles#409 - Dependencies: switch to x/ansi for text manipulation by [@aymanbagabas](https://github.com/aymanbagabas) in charmbracelet/bubbles#505 ##### Added - Textarea: add help to textarea key bindings by [@TravisYeah](https://github.com/TravisYeah) in charmbracelet/bubbles#418 - Textarea: Add multiline placeholder by [@mikelorant](https://github.com/mikelorant) in charmbracelet/bubbles#302 - Table: Add column return function by [@abeleinin](https://github.com/abeleinin) in charmbracelet/bubbles#369 - Table: Implement help.Keymap interface and add quit mapping by [@prgres](https://github.com/prgres) in charmbracelet/bubbles#440 - Ctrl+Left/Right for WordForward/Backward by [@maaslalani](https://github.com/maaslalani) in charmbracelet/bubbles#387 - Use goreleaser for releases by [@aymanbagabas](https://github.com/aymanbagabas) in charmbracelet/bubbles#526 ##### Fixed - Table: Render Row Tests by [@maaslalani](https://github.com/maaslalani) in charmbracelet/bubbles#487 - Table: Only render columns with a positive width by [@fabio42](https://github.com/fabio42) in charmbracelet/bubbles#465 - Table: Fix inheritence of SelectedStyle in StyleFunc by [@gabrielfu](https://github.com/gabrielfu) in charmbracelet/bubbles#539 - Table: Don't include header height in the total table size by [@prgres](https://github.com/prgres) in charmbracelet/bubbles#434 - Table: Fix premature viewport scroll by [@dzeleniak](https://github.com/dzeleniak) in charmbracelet/bubbles#429 - Textarea: Fix end of buffer character by [@mikelorant](https://github.com/mikelorant) in charmbracelet/bubbles#491 - Textarea: Set textarea default EndOfBufferCharacter to ' ' by [@blvrd](https://github.com/blvrd) in charmbracelet/bubbles#510 - Textarea: End of Buffer alignment by [@maaslalani](https://github.com/maaslalani) in charmbracelet/bubbles#486 - Textinput: don't block input on validation by [@GabrielNagy](https://github.com/GabrielNagy) in charmbracelet/bubbles#185 - Viewport: Fix division by zero in scrollpercentage by [@zMoooooritz](https://github.com/zMoooooritz) in charmbracelet/bubbles#494 - Help: Fix centering by [@gabe565](https://github.com/gabe565) in charmbracelet/bubbles#516 - Progress: Stop spring defaults from overriding WithStringOptions by [@nervo](https://github.com/nervo) in charmbracelet/bubbles#540 - Cursor: Make SetMode method in cursor library handle invalid mode values correctly by [@anirudhaCodes](https://github.com/anirudhaCodes) in charmbracelet/bubbles#477 ##### Test coverage ✅ - Add tests for textarea view by [@mikelorant](https://github.com/mikelorant) in charmbracelet/bubbles#485 - Add tests for paginator by [@anirudhaCodes](https://github.com/anirudhaCodes) in charmbracelet/bubbles#480 - Add tests for textInput Tests by [@KevM](https://github.com/KevM) in charmbracelet/bubbles#500 - Improve textarea tests by [@mikelorant](https://github.com/mikelorant) in charmbracelet/bubbles#490 #### New Contributors - [@rdnt](https://github.com/rdnt) made their first contribution in charmbracelet/bubbles#473 - [@rwinkhart](https://github.com/rwinkhart) made their first contribution in charmbracelet/bubbles#409 - [@mikelorant](https://github.com/mikelorant) made their first contribution in charmbracelet/bubbles#485 - [@anirudhaCodes](https://github.com/anirudhaCodes) made their first contribution in charmbracelet/bubbles#480 - [@nekopy](https://github.com/nekopy) made their first contribution in charmbracelet/bubbles#458 - [@TravisYeah](https://github.com/TravisYeah) made their first contribution in charmbracelet/bubbles#418 - [@abeleinin](https://github.com/abeleinin) made their first contribution in charmbracelet/bubbles#369 - [@fabio42](https://github.com/fabio42) made their first contribution in charmbracelet/bubbles#465 - [@prgres](https://github.com/prgres) made their first contribution in charmbracelet/bubbles#440 - [@zMoooooritz](https://github.com/zMoooooritz) made their first contribution in charmbracelet/bubbles#494 - [@dzeleniak](https://github.com/dzeleniak) made their first contribution in charmbracelet/bubbles#429 - [@KevM](https://github.com/KevM) made their first contribution in charmbracelet/bubbles#500 - [@gabe565](https://github.com/gabe565) made their first contribution in charmbracelet/bubbles#516 - [@blvrd](https://github.com/blvrd) made their first contribution in charmbracelet/bubbles#510 - [@nervo](https://github.com/nervo) made their first contribution in charmbracelet/bubbles#540 - [@gabrielfu](https://github.com/gabrielfu) made their first contribution in charmbracelet/bubbles#539 - [@aditipatelpro](https://github.com/aditipatelpro) made their first contribution in charmbracelet/bubbles#577 **Full Changelog**: charmbracelet/bubbles@v0.18.0...v0.19.0 *** <a href="https://charm.sh/"><img alt="The Charm logo" src="https://stuff.charm.sh/charm-badge.jpg?1" width="400"></a> Thoughts? Questions? We love hearing from you. Feel free to reach out on [Twitter](https://twitter.com/charmcli), [The Fediverse](https://mastodon.social/@charm), or on [Discord](https://charm.sh/chat). </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: Renovate Bot <[email protected]> Reviewed-on: https://git.asdf.cafe/abs3nt/gspot/pulls/33

fix(table): substract headers Y size from the total table size

8fdaca5

fix: change GetFrameSize to GetVerticalFrameSize and calc header height

26a544f

fix: remove GetVerticalFrameSize at all

d21a343

fix: account for headers in height WithHeight, SetHeight

c0461cd

maaslalani merged commit 6dbfc82 into charmbracelet:master Mar 13, 2024
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(table): substract headers Y size from the total table size #434

fix(table): substract headers Y size from the total table size #434

prgres commented Nov 13, 2023

prgres commented Nov 13, 2023

prgres commented Nov 13, 2023

prgres commented Nov 13, 2023

prgres commented Nov 27, 2023

maaslalani commented Feb 28, 2024

maaslalani commented Mar 13, 2024

fix(table): substract headers Y size from the total table size #434

fix(table): substract headers Y size from the total table size #434

Conversation

prgres commented Nov 13, 2023

prgres commented Nov 13, 2023

prgres commented Nov 13, 2023

prgres commented Nov 13, 2023

prgres commented Nov 27, 2023

maaslalani commented Feb 28, 2024

maaslalani commented Mar 13, 2024