Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(table): use table height #358

Merged

Conversation

Broderick-Westrope
Copy link
Contributor

@Broderick-Westrope Broderick-Westrope commented Aug 29, 2024

Fixes #356

Changes

  • table/table.go: Modified the constructRow method to accept an isOverflow parameter, allowing rows to be rendered as overflow rows with ellipsis when necessary.
  • table/table.go: Updated the String method to calculate the available height for rendering rows and handle the rendering of overflow rows correctly. Most of this lives in the new constructRows method.
  • table/table_test.go: Added unit tests for the Height method.
  • table/table.go: Added a boolean useManualHeight to the Table struct. I thought it would be good to maintain the existing functionality (auto table height) by default. I've put this in one commit so it can be easily removed if needed. The useManualHeight boolean tracks whether the user has manually set the height of the table. When false, the new height functionality will not be used. It is false by default and only set to true when the height is set (table.Height(x)).
    • Without this the existing table unit tests (and likely other users' code) would would have to change every New call to have a corresponding Height call. With this feature, height is flexible and calculated for you by default.
    • We could replace this bool with an understanding that height == -1 represents auto height. Not sure what is preferred.

How Height is Used

The default behaviour is the same as before, now there's just a few more features:

  • Calling New without calling Height will render the full table (despite the Table.height value being 0). Example: the following table technically has a height of 0, but the user did not set that so we assume they want to show the whole table:
╭──┬───────────────┬───────────────────╮
│ID│title          │description        │
├──┼───────────────┼───────────────────┤
│1 │Lost in Time   │A thrilling advent…│
│2 │Whispering Sha…│Secrets unravel in…│
│3 │Stolen Identity│An innocent man's …│
│4 │Into the Abyss │Exposing deep-root…│
╰──┴───────────────┴───────────────────╯

NOTE: When the following examples say "set a height" they mean make a call to the table.Height method

  • If you set a height larger than the tables total height, it won't look any different. No extra lines are added. This is the same if you choose the exact height needed. Example, we could set the table height to 8 or 800 and we would still get this:
╭──┬───────────────┬───────────────────╮
│ID│title          │description        │
├──┼───────────────┼───────────────────┤
│1 │Lost in Time   │A thrilling advent…│
│2 │Whispering Sha…│Secrets unravel in…│
│3 │Stolen Identity│An innocent man's …│
│4 │Into the Abyss │Exposing deep-root…│
╰──┴───────────────┴───────────────────╯
  • If you set a height so small that no rows can be rendered, and overflow row will still be rendered. Example, we could set the table height to 5 or anything less (including negatives) and we would get:
╭──┬───────────────┬───────────────────╮
│ID│title          │description        │
├──┼───────────────┼───────────────────┤
│… │...            │...                │
╰──┴───────────────┴───────────────────╯
  • And finally, if you set a height between the last two examples (ie. enough to show a row but not enough to show all rows) then we render as many as possible finishing with an overflow row. Example, the following table has a height of 6:
╭──┬───────────────┬───────────────────╮
│ID│title          │description        │
├──┼───────────────┼───────────────────┤
│1 │Lost in Time   │A thrilling advent…│
│… │...            │...                │
╰──┴───────────────┴───────────────────╯

Notes

  • The height of a table is defined as the number of lines (ie. vertical cells) that the table takes up. This includes headers, data rows, and any styling.
  • Since at least one data row is always displayed (even if it is just an overflow row), if the user sets the height to anything less than 5 but there is only one row of data to show, the data row will be shown instead of an overflow row. This stems from an assumption that if we are going to "force print" 5 lines (aka. 5 vertical cells) of text and there is only one data row anyway, we might as well show the user some useful information rather than an empty or overflow row.
  • If the only row printed is an overflow row, the column widths are calculated for the original data and yet we are only printing ellipsis. Let me know if this is something that needs to be altered.

@bashbunni bashbunni self-assigned this Aug 30, 2024
@Broderick-Westrope
Copy link
Contributor Author

@bashbunni hoping to close this one out. Any feedback or blockers?

Copy link
Member

@bashbunni bashbunni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey this looks great. Thank you so much for the very thorough work and tests! This was really quick to review as a result 🙏

@bashbunni bashbunni added this to the v0.14.0 milestone Sep 14, 2024
@bashbunni bashbunni merged commit a5492bc into charmbracelet:master Sep 14, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

setting table height doesn't do anything
2 participants