Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix(table): use table height#358

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

Broderick-Westrope
Copy link
Contributor

@Broderick-WestropeBroderick-Westrope commentedAug 29, 2024
edited
Loading

Fixes#356

Changes

  • table/table.go: Modified theconstructRow method to accept anisOverflow parameter, allowing rows to be rendered as overflow rows with ellipsis when necessary.
  • table/table.go: Updated theString method to calculate the available height for rendering rows and handle the rendering of overflow rows correctly. Most of this lives in the newconstructRows method.
  • table/table_test.go: Added unit tests for theHeight method.
  • table/table.go: Added a booleanuseManualHeight to theTable 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. TheuseManualHeight boolean tracks whether the user has manually set the height of the table. Whenfalse, the new height functionality will not be used. It isfalse by default and only set totrue 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 everyNew call to have a correspondingHeight call. With this feature, height is flexible and calculated for you by default.
    • We could replace this bool with an understanding thatheight == -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:

  • CallingNew without callingHeight will render the full table (despite theTable.height value being 0). Example: the following tabletechnically 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 thetable.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 isalways 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.

ajpotts01 reacted with thumbs up emojibashbunni reacted with heart emoji
@bashbunnibashbunni self-assigned thisAug 30, 2024
@Broderick-Westrope
Copy link
ContributorAuthor

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

Copy link
Member

@bashbunnibashbunni 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 🙏

Broderick-Westrope reacted with heart emoji
@bashbunnibashbunni added this to thev0.14.0 milestoneSep 14, 2024
@bashbunnibashbunni merged commita5492bc intocharmbracelet:masterSep 14, 2024
7 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@bashbunnibashbunnibashbunni approved these changes

@meowgorithmmeowgorithmAwaiting requested review from meowgorithmmeowgorithm is a code owner

@aymanbagabasaymanbagabasAwaiting requested review from aymanbagabasaymanbagabas is a code owner

Assignees

@bashbunnibashbunni

Labels
None yet
Projects
None yet
Milestone
v0.14.0
Development

Successfully merging this pull request may close these issues.

setting table height doesn't do anything
2 participants
@Broderick-Westrope@bashbunni

[8]ページ先頭

©2009-2025 Movatter.jp