Skip to content

Conversation

cylewitruk
Copy link
Contributor

@cylewitruk cylewitruk commented Aug 10, 2024

Offers a solution for #64 by allowing the user to specify a max window size -- works with filtering as well. See the select_window_size example.

An ultimate solution would probably use something like termsize and calculate based on the actual size of the viewport.

@fadeevab
Copy link
Owner

@cylewitruk It's better to move with the ultimate solution with automatic terminal size identification.

@cylewitruk
Copy link
Contributor Author

cylewitruk commented Aug 11, 2024

@fadeevab alright, will have a look at that then too. I still want the windowing function so I can control the display size of the list if I want to, but using terminal size as a default will be a huge improvement 👍

I think the termsize solution will simply set the window size to term height - header - [filter] - footer roughly, as a default anyway.

I'll probably expose some convenience function to get terminal size to the consumer as well, in-case they want to do their own window size caculation (could be embedded in a TUI or something).

@cylewitruk
Copy link
Contributor Author

@fadeevab pushed

@fadeevab
Copy link
Owner

@fadeevab pushed

@cylewitruk. Busy days. I hope I will review it soon.

@cylewitruk
Copy link
Contributor Author

@fadeevab pushed

@cylewitruk. Busy days. I hope I will review it soon.

@fadeevab no rush 😊

Comment on lines +37 to +39
window_size: usize,
window_pos: usize,
term_size: Option<Size>,
Copy link
Owner

Choose a reason for hiding this comment

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

@cylewitruk

Thanks for the patch! :) I believe this part should be used in the multiselect as well, thus I would ask to move it into a separate structure, e.g. "TermSize", something like I did with filter.rs for filtering in the both selection lists.

TermSize::default()
TermSize::get_size() -> usize
TermSize::set_size(usize)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fadeevab agreed, been a bit tight with time the last weeks but I'll get to this eventually 😄

Copy link
Owner

Choose a reason for hiding this comment

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

@cylewitruk hello, no rush :) meanwhile, we managed to release few improvements to the multiprogress bar (insert and println)

@fadeevab
Copy link
Owner

Closed in favor of PR #83

@fadeevab fadeevab closed this Jan 27, 2025
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.

2 participants