feat(widgets::table): add option to always allocate the "selection" constraint

[hasezoey]

This PR adds a option for Table to always allocate the "selection column", regardless of if something is selected, this way there is less "moving around" of the layout.

The default is still false (only allocated if something is selected), but it can both be enabled or disabled in a builder function.

Note: also renamed some internal variables to better reflect on what they do / track

[hasezoey]

should i do a similar thing for List?

[codecov]

Codecov Report

Merging #375 (114d2b3) into main (3293c6b) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #375      +/-   ##
==========================================
+ Coverage   85.10%   85.11%   +0.01%     
==========================================
  Files          40       40              
  Lines        8727     8736       +9     
==========================================
+ Hits         7427     7436       +9     
  Misses       1300     1300              

Files Changed	Coverage Δ
src/widgets/mod.rs	64.70% <ø> (ø)
src/widgets/table.rs	92.15% <100.00%> (+0.20%)	⬆️

[joshka]

Also +1 to implementing this idea - this is otherwise pretty jarring.

[hasezoey]

i think i addressed all requests, is the current state OK?

PS: someone with write, could the "CI / coverage (pull_request)" workflow be re-run?

[joshka]

I'll wait on a second review on this one.

[a-kenji]

Thanks for implementing this, this has been a pet peeve of mine on other ratatui apps.

I am not sure if allocating is a fitting word here, it confused me initially.
I also think that this is a breaking change, since the interface of a public struct is changing.

Otherwise LGTM.

[joshka]

I also think that this is a breaking change, since the interface of a public struct is changing.

All the fields on Table are private, and the change just adds a new method, so I think this is probably in the mostly ok bucket (except for people doing "Strange Things"(TM)). What do you think?

[a-kenji]

You are right, I somehow thought the fields were public.
LGTM.

[hasezoey]

I am not sure if allocating is a fitting word here, it confused me initially.

any suggestions what to use instead?

[joshka]

any suggestions what to use instead?

How about something like:

feat(table): add option to reserve space for highlight symbol

with a commit body like:

Before this option was available, selecting an item when there was no item selected
previously made the table's layout change. This enables the table width to take up the
same amount of space whether there is a selection or not.

[hasezoey]

is the change in 146d791 enough or should something else be changed?

[joshka]

Sorry - I miscommunicated here a little. The suggested text in my previous comment was for the git commit history rather than the doc comments.

I suggest the docs should be written about the current implementation, and they don't need to cover history unless there's a strong likelihood of users needing to transition. Because this is new code, I'd avoid this.

The git commit is where we talk in terms of what the change was "This is how things changed".

Aside from that, the other comment changes are good.

If you can rebase and squash this with a new commit message that better explains the changes (rather than 8 commits), that would be great.

[hasezoey]

rebased onto latest master, squashing commits that could / should be squashed, i think any further squashing would reduce clarity of the commits

also changed the initial commits messaging based on the feedback

[kdheepak]

Thanks for the PR! This is a feature that I've wanted for a while as well!

[kdheepak]

I only have minor documentation related comments. The PR LGTM otherwise.

[hasezoey]

just as a note, i have explicitly mentioned "row selection symbol" instead of just "selection symbol" for things like #376

[hasezoey]

force-pushed to rebase onto latest master (which added Hash on Table) and to fix cargo fmt for the suggestions commit

[hasezoey]

i will ask again, just to be sure:

should i do a similar change for List?

[orhun]

All good, but let's use a commit message such as feat(table): ... before merge 🐻

[hasezoey]

@orhun are the commits correct now? i personally use that style for my projects, to quickly note which subject (file) is meant, because there could be multiple things that have the same name in different paths (example in this project table as in widgets or as in example)

[joshka]

@orhun are the commits correct now? i personally use that style for my projects, to quickly note which subject (file) is meant, because there could be multiple things that have the same name in different paths (example in this project table as in widgets or as in example)

Keeping it broad is fine for this - it's mostly for the Changelog

[joshka]

should i do a similar change for List?

Yes please. Thanks for this change.