-
-
Notifications
You must be signed in to change notification settings - Fork 331
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
RFC: Design of Scrollable Widgets #174
Comments
I'm not sure I quite understand the library well enough to meaningfully contribute to this feature, but I fully agree that there needs to be a more consistent scroll solution, so just wanted to +1 this. |
There were a couple pull requests (#119 and #121) that are worth mentioning here. They aren't exactly the same but they suggest an interesting approach. Instead of putting scroll logic in to each widget, we could create a
When scrolling, update (The above would need to be fleshed out a bit more, with scrollbars, and so on, but I think those are doable with an approach like this). I've never looked at textarea, but if they are using viewports to do scrolling, it's probably a similar idea. |
@Eyesonjune18 wrote:
No worries. I appreciate you taking a look regardless. @sophacles wrote:
I'll add these to the list and ping @gibbz00 for their input on this as it sounds like they probably given it some thought already.
I thought about that approach a little for similar reasons. I quickly rejected it as viable, mainly because it requires the contained widget to render the entire widget rather than just the visible portion. Giving it a bit more thought though, I think there's a few more problems with it:
Given that most of the internal widgets have features that would need to interact with the scrolling in some way or other (item scrolling, total height calculation, partial item rendering, line wrapping, headers / footers, etc.), I think it's pretty clear that this shouldn't be a wrapper. I'm going to add the following to the initial comment: Possible implementation approaches
And for scrollbar:
|
Re problems with the wrapper approach, generally I agree, although re point 1: I don't have a problem with drawing everything the user hands us, data management doesn't belong in drawing functionality - it should be done outside the widget IMHO - that's a different discussion though (see #164 ) Re trait vs For example something like: https://gist.github.com/sophacles/53490288c367f1300c08ad16d152bab2 This has the added benefit that if the widget author wants to implement some different behavor for |
For the comment on 1, one example that might help make it easier to think about the issue is how would you handle centered text in a scrollable widget? Alternatively, how would I handle having a list that has millions of items with only the current 40 or so showing being rendered? Or a very long document showing in a paragraph. I'd still want the scrollbar to be useful there. (I've been meaning to give #164 some more thought - in case you're concerned about screaming into the void on that ;)) The scrollable + scroll state approach makes sense. I wonder if AsRef could be worth implementing? I've spent a little bit of time playing with what ScrollState might look like in a branch at main...joshka:ratatui:feat-scroll - very wip right now just there to understand barriers to implementation, and to spark ideas around missing behavior. (I also have some not yet finished work around pulling out the scrollbar as a widget) |
personally i would like it if the final scroll implementation would allow for the possibility of "overscroll" (/ maybe also "underscroll"?), like some text editors allow. maybe this is solved via some kind of explicit padding or should it be handled separately? |
Quoting the downsides of the wrapping widget approach:
I assume you mean in cases where the size of items are known since variable sized items would make this difficult for both item and line scrolling in all approaches. An examination of the wrapping approach from an API standpointOriginally, I was thinking the wrapping approach would be the most natural and could be salvaged, but after digging into it more closely, it definitely seems broken. Examining the this approach from an API standpoint, I observe a few things: A) Wrapping a widget in a scrolling widget is natural and feels right: ScrollableWidget::default()
.inner(
SomeWidget::default()
.props(...))
)
.render(area, buf, self.scroll_state); B) But if C) It's not so nice from a storage of state standpoint since each widget ends up needing a separate state associated with it: struct MyUi {
some_widget_state: SomeWidgetState,
some_widget_scroll_state: ScrollState,
another_widget_state: SomeWidgetState,
another_widget_scroll_state: ScrollState,
} D) State makes nested scrollbars impossible. Although, why someone would want that is a valid question. If the API is done right, it shouldn't be a necessary use-case, or if someone really needs it, they can drop down to using the scrollbar widget manually. What I'm thinking scrollbar inception would look like: ScrollableWidget::default()
.inner(
ScrollableWidget::default()
.inner(
SomeWidget::default()
.props(...)
)
)
// Assuming render takes two states as mentioned in B, how do you get state to `SomeWidget`?
.render(area, buf, self.outer_scroll_state, self.inner_scroll_state); I think the only way to salvage it would be to make binding state to a widget possible, as in Rendering the inner widget remains a problem, but I think that can be solved by adding some kind of |
Yes - variable size items is the main part of that downside, but even fixed size items means that when you scroll down by one item, you have to scroll down a certain number of lines. This means that the widget and scroll container must have some sort of communication about how to interpret scroll locations. |
how about making the aside from maybe needing communication to agree what a scroll should be (as @joshka noted), it would also likely be necessary that the inner struct needs to communicate what the actual size / max size is, so that there can actually be a scrollbar (otherwise how would you know how far the scrollbar is?) |
I'd encourage writing some code to show this. No need for a full PR implementing it. Instead show what it looks like from the callers perspective, where they create a widget that supports scrolling and render it. This removes any ambiguity over which parts of the problem we're solving and which parts we're not. |
I think I have a good approach for the Scrollbar configuration: Just like the user configures a Border for a given widget the user should be able to supply a Scrollbar: const widget = SomeWidget::new()
.border(Border::bordered())
.scrollbar(Some(Scrollbar::new(ScrollbarOrientation::VerticalRight))); The widget itself then knows about its border and things like headers (like the table has). let scrollbar_area = Rect {
// Inner height to be exactly as the content
y: inner_border_area.y,
height: inner_border_area.height,
// Outer width to stay on the right border
x: full_area.x,
width: full_area.width,
}; The widget also knows every aspect of the ScrollbarState by itself: Each widget knows precisely the area where the scrollable content is. The user shouldn't need to configure some margin when the widget supports scrollbars by itself. The widget also knows the scrollbar state. The widget does not know how exactly the scrollbar should look, but that's given by the user set-able scrollbar (like Border is set). I implemented this approach in my Tree widget v0.18.0. I think this approach is useful for going forward. Open question: The widget should have some control over the scrollbar being vertical or horizontal. For some widgets its either not useful or both possible and the widget needs to check which scrollbar is which. Currently, the Orientation is part of the Scrollbar definition. Maybe there should be two widgets, |
Another question that comes up: scroll the offset or the selection? With mqttui (and therefore my Tree and Binary Data Widget) I went for scrolling the offset which is something that felt more natural coming from normal GUIs. Since then, I noticed other places which do have selections and how they behave. htop for example has a weird in between, it scrolls the offset when possible and keeps the selection relative to the offset. When scrolling the offset isn't possible anymore, the selection is moved. Widgets on ratatui allow an optional selection. When there is no selection the widget should still be scrollable in my opinion (which isn't the case for some widgets currently). I think because of the optional selection the scrolling for offset is the most natural as the behavior does not change between a selection or not. Are there other thoughts on this? |
Good points - I suspect that both should be available. It's reasonable to have mouse scroll that does the offset, but have keyboard shortcuts for selection (and perhaps they should be able to do the offset as well). In particular, a list item that is larger than the screen height requires both types of scroll to be able to be displayed in full. |
Scrollable widgets should include a common struct ScrollBehaviour {
scroll_ends_on: LastItemOnEnd | LastItemOnStart=Overscroll | Ignore,
selection_always_in_view: true | false,
mousewheel_scrolls: Selection | Offset,
keep_end_in_view_when_nothing_selected: true | false,
show_horizontal_scrollbar: Always | OnlyWhenScrollable | Never,
show_vertical_scrollbar: Always | OnlyWhenScrollable | Never,
horizontal_scrollbar_location: Top | Bottom | Never,
vertical_scrollbar_location: Left | Right | Never,
} That way widgets can have similar behavior across many widgets. When this is not shared between multiple widgets (like it's currently the case) this will continue to have different implementations across different widgets. With this information widgets themselves should be able to create a As this will have a default behavior I think this could even be achieved without a breaking change. The current |
Much agreement that this should be moved into the widgets like block currently is.
Yeah, ScrollbarState was my idea when the ScrollBar was originally implemented. On reflection, it was a mistake to make that instead of putting the properties on the Scrollbar.
I don't mind keeping these available to people (as building blocks for other widgets) |
My idea was not to completely remove these building parts. But they should not be grouped as "full widgets" like a List or Table. |
I would suggest using a container widget that manages the scrollbars and It would use the following two traits, one for the widget and one for the state. /// Trait for a widget that can scroll.
pub trait ScrolledWidget: StatefulWidget {
/// Get the scrolling behaviour of the widget.
///
/// The area is the area for the scroll widget minus any block set on the [Scrolled] widget.
/// It doesn't account for the scroll-bars.
fn need_scroll(&self, area: Rect, state: &mut Self::State) -> ScrollParam;
} This trait is called before rendering the widget itself. This allows the scrolling-container to calculate the exact layout. The next trait is for the state: pub trait HasScrolling {
/// Maximum offset that is accessible with scrolling.
///
/// This is shorter than the length of the content by whatever fills the last page.
/// This is the base for the scrollbar content_length.
fn max_v_offset(&self) -> usize;
/// Maximum offset that is accessible with scrolling.
///
/// This is shorter than the length of the content by whatever fills the last page.
/// This is the base for the scrollbar content_length.
fn max_h_offset(&self) -> usize;
/// Vertical page-size at the current offset.
fn v_page_len(&self) -> usize;
/// Horizontal page-size at the current offset.
fn h_page_len(&self) -> usize;
/// Current vertical offset.
fn v_offset(&self) -> usize;
/// Current horizontal offset.
fn h_offset(&self) -> usize;
/// Change the vertical offset.
///
/// Due to overscroll it's possible that this is an invalid offset for the widget.
/// The widget must deal with this situation.
fn set_v_offset(&mut self, offset: usize);
/// Change the horizontal offset.
///
/// Due to overscroll it's possible that this is an invalid offset for the widget.
/// The widget must deal with this situation.
fn set_h_offset(&mut self, offset: usize);
} The scrolling widget imposes no unit on the usize values used in this trait. But this means that the scrolling widget can't make any connection between Page_lenThat's why it gets the current page_len via the trait. ScrollbarThe values used for the scrollbar are offset + max_offset. Both page_size and max_offset are values that can easily be calculated while Letting the widget do all these calculations gives the widgets enough freedom Setting the offsetWith the current offset, page_len and max_offset known all the wanted DrawbackThe one drawback I saw, was that currently there is a strong link between This is a bit annoying and would require one more switch to turn this off. |
ExampleI tried this, and here are the docs: ScrolledWidget I wrote adapters for List, Table, Paragraph and just to see if it works If you want to run it there is examples/sample1.rs with the crate. // I only tried this on windows with windows terminal and with alacritty The crate contains more stuff, but it's late alpha/early beta now. |
This list was put up as requirements: Features of solution
Yes
Not as they are. But a viewport widget could implement these traits and render any I haven't tried it, but buffer seems good enough for this.
Yes I tried
Abstract notion of an offset. Can denote any of the above.
Yes. Rendering is the job of the widget. Scrolling only handles some offset.
No. This must be implemented by the widget.
Yes, but not at the same time. A widget could have a switch that changes its behaviour though.
Definitively.
Yes
Only if the widget supports it.
Needs some place to store state. But an adapter should always be possible. (See ParagraphExt).
There could be some struct to hold the data, but I doubt all widgets that could
Yes, both. On each side if wanted.
pub enum ScrollbarPolicy {
Always,
#[default]
AsNeeded,
Never,
} With AsNeeded the widget can control it too.
That's up to the widget.
That's up to the widget too.
Don't understand what's meant here?
That's out of scope. There's no way enough infrastructure to accomplish that. Could add something like: fn ticked_scroll_to(tick_state: &mut TickState); with
which is driven by some external timer.
Adds two traits. One for the widget and one for the widget-state.
The current behaviour can stay. Maybe needs a flag to switch off the 'scroll-selected-to-visible'
Mouse interactions can be centralized on the Scrolled widget. List/Table may share some behaviour, but something like tree-widget will hardly conform.
Yes
Yes Questions / things to look into / choices
Wrapping text should probably not scroll at all, or maybe it needs
Letting the widget correct invalid offsets should be fine.
TODO: Not covered yet.
ViewPort seems only useful for a line/column based approach.
If a widget wants this differentation it can.
Trait
I don't think you can for the whole set of widgets that could scroll.
Maybe? I don't have enough overview.
That's a new layout-engine, as I understand it?
I know Java/Swing. It uses a viewport. It has always been difficult to use when you have
That's out of scope, I think.
What's meant with that one?
|
to my knowledge infinite scroll is basically loading more content on-demand seamlessly without paging when reaching the end (or near the end), for example on a list. (also see the wikipedia page) |
Ok, so that's the "let's see if the logfile got longer in the meantime" case. This could be part of every run of event-handling or some timer based refresh or some external Or, with infinite scrolling the trigger would be some threshold that is reached, after handling This should probably update the data-model and render the list-item with the new data. Seen this way it is probably the normal workload of the application, not something that must |
Update on
Looked into creating a Viewport that renders into a temp buffer. |
I tried something like this for a SQL client (scrollable.rs), but a major constraint I ran into is the max area of the temporary In practice, at least for a The only workaround I could think of was to make the caller responsible for offsetting / "windowing" the content it asked to be rendered, but that was quite messy (viewer beware; caller is data.rs) and seemed like it defeated the purpose of the viewport approach. I gave up and plan on using the built in I'm a beginner at Rust/ratatui though, so if there's a different viable workaround, would appreciate any tips/pointers 🙏 |
Yeah, fixing the area = u16 constraint is something that needs to happen someday. It's less about just making the change an more about working out why it's there in the first place (no one really knows the history of why it's there). Re: long tables (and large widgets in general), the right approach is to not render stuff which is off screen rather than render everything to a buffer that is big. |
I saw that quirks too, and using buffers that large just to throw most of it away If your dataset is big enough, just creating all the Row and Cell structs is noticeable. (On my computer it takes 3000x5 cells to reach render times of about 10ms).
You might be interested in https://github.com/thscharler/rat-ftable. It's almost ready for beta, but I'm just working on the docs anymore. The rest should be quite stable. It has row-wise scrolling on the y-axis and char-wise scrolling on the x-axis, among other things. |
Oh, I don't think I put the name together until just now. That's a neat set of libs you've got going on there. Lots of neat ideas about how to do things. Keep at it :) |
Thanks, and some kudos back to you. I took quite a few ideas from your various sketches floating around. |
To my understanding, the reason Ratatui does this is the following: Most terminals do not support more than 16-bit cursor indexes. The decision to use The "proper" way to solve this would be to only use |
Using a u16 for positions is not a noticeable limitation, for relative positions I use an i16, and it works fine. The problem above is that Rect::area() calculates the area and limits that result to an u16 too. Which probably works for a Buffer for a terminal window, but is a real limit for a background buffer for scrolling purposes. |
Problem
There are a few widgets that need scrolling behavior. It would be wasteful to implement this on each widget rather than doing it properly once. There is existing scroll behavior in Table, Paragraph and List. There are a few issues / PRs related to scrolling currently in the queue
Paragraph
. #136Elsewhere:
Current Behavior
Paragraph
:Paragraph::default().scroll((1,2))
)Table
:{ selected, offset }
List
{ selected, offset }
Elsewhere:
tui_rs_tree_widget::Tree
tui-logger
tui-textarea
tui-input
I'm sure there are other things to look at. Please add them if you know of them.
Features of solution
Brainstorming ideas
TODO: refine these as must-have, should-have, could-have, won't have (MoSCoW)
Questions / things to look into / choices
render
method just fix the invalid scroll?a. if the scroll returns a result, we can beep / flash / take some action to load more results when a user tries to scroll past the end / beginning of a list of items.
a. Maybe the flexbox PR? wip(layout): flex layout - draft / poc / rfc #23
a. libraries in other languages?
Possible implementation approaches
Pros:
Cons:
Pros:
Cons:
Pros:
list_state.scroll_down()
)Cons:
ScrollState
type with all the scrolling behavior and include this in ListState, TableState etc.Pros:
Cons:
list_state.scroll_state.scroll_down()
And for scrollbar:
The text was updated successfully, but these errors were encountered: