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

Table line separators not showing for the first line when table headers are hidden #1391

Closed
MoaidHathot opened this issue Dec 1, 2023 · 3 comments · Fixed by #1513
Closed
Assignees
Labels
bug Something isn't working needs triage

Comments

@MoaidHathot
Copy link

Information

  • OS: Windows 11 23H2 (OS Build 22631.2506)
  • Version: Spectre.Console 0.48.0
  • Terminal: Windows Terminal 1.19.3172.0

Describe the bug
When table headers are hidden, the first row separator is not drawn.

To Reproduce

var table = new Table();

table.AddColumn("First");
table.AddColumn("Second");

table.AddRow("1", "2");
table.AddRow("3", "4");
table.AddRow("5", "6");

table.ShowRowSeparators = true;
table.HideHeaders();

AnsiConsole.Write(table);

Expected behavior
The first row separator should be visible even if table headers are hidden

Screenshots
image

@MoaidHathot MoaidHathot added bug Something isn't working needs triage labels Dec 1, 2023
@BlazeFace
Copy link
Contributor

BlazeFace commented Apr 7, 2024

Hello! I'm proposing a fix for this issue, [Commit c63b842 ](https://github.com/BlazeFace/spectre.console/commit/ . The edge case causing this is that the code assumes that headers are shown and does not render the first separator, the fix is to check if headers are hidden and if so render the separator! Let me know if I need to make any other changes as this is my first contribution @patriksvensson . Also can go ahead and make a PR but wanted to get confirmation first!

https://github.com/BlazeFace/spectre.console/blob/c63b842645f116a26bf6c45a69a94da15c0a866b/src/Spectre.Console/Widgets/Table/TableRenderer.cs#L154

@BlazeFace
Copy link
Contributor

@FrankRay78 I think I have a pretty complete solution for this, should I go ahead and make a PR?

@FrankRay78
Copy link
Contributor

By all means @BlazeFace, please do. I normally maintain the CLI subsystem, but I can review/merge all PR's. I'll use this as an opportunity to upskill myself on the table widget.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs triage
Projects
Status: Done 🚀
Development

Successfully merging a pull request may close this issue.

3 participants