-
Notifications
You must be signed in to change notification settings - Fork 275
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
feat: impl WidgetRef for Fn(Rect, &mut Buffer) #1154
base: main
Are you sure you want to change the base?
Conversation
This allows you to treat any function that takes a `Rect` and a mutable reference a `Buffer` as a widget in situations where you don't want to create a new type for a widget. Example: ```rust fn hello(area: Rect, buf: &mut Buffer) { Line::raw("Hello").render(area, buf); } frame.render_widget(&hello, frame.size()); frame.render_widget_ref(hello, frame.size()); ``` Related to: <https://forum.ratatui.rs/t/idea-functionwidget-was-thoughts-on-tui-react/59/2>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1154 +/- ##
=====================================
Coverage 94.2% 94.2%
=====================================
Files 60 60
Lines 14509 14525 +16
=====================================
+ Hits 13670 13686 +16
Misses 839 839 ☔ View full report in Codecov by Sentry. |
This is mainly an experiment. In theory Widget possibly should be Implemented for FnOnce, but that runs into a conflict with the blanket implementation of Widget for &W: WidgetRef. (Can't implement FnOnce for WidgetRef as the self parameter is a ref.) |
Just tossing out my thoughts as a new rust and ratatui user... I think it would more natural to add a method to
I think the
At that point, the Frame's render functions accepting Side note: it seems like Side side note: It seems like
It took me quite a while to realize that all of the Widget/Frame rendering machinery was essentially a wrapper around the above. I don't really see the point of the |
I think perhaps this could be worth considering as an additional thing rather than an alternate.
Makes sense. The frame owns the buffer, but it doesn't own the area, so being
Can you expand on this? How would the use cases that require widget ref be gracefully handled by closures? For perspective those are:
There's a lot of history and context to this that's difficult to summarize neatly. If you're curious though: TL;DR:
|
Thanks again for such a great response. I read the previous discussions you linked to and have a better grasp of the background and goals. I don't want to derail this PR with a broad discussion of issues, so after this reply I'll attempt to politely step back and let a more focused discussion continue.
With the
Ahh, now I see the "consume self" problem. It is unfortunate.
I still can't shake the idea that "widget" idea as currently expressed brings with it some accidental complexity. For example, I'm not seeing a lot of metaprogramming need for a widget trait, so why are there four traits (granted, two experimental) to express the idea of a simple function call? One way it could be simpler...just maybe.... If the receiver of the render calls were flipped, and each Widget implemented a
In fact, this might be quite nice in the common case:
I think this might satisfy the I think this might satisfy the This direction is also backward compatible. Both |
No problem. Let's continue if necessary after this at https://forum.ratatui.rs/t/idea-functionwidget-was-thoughts-on-tui-react/59
Adding a functionally equivalent method that swaps the order of parameters would be bad for the UX of widgets in general. Giving multiple ways to do the same exact thing is confusing and requires more explanation than the simple widgets are types that render to a location in a buffer. All of these options work today already: fn render_app(frame: &mut Frame) {
// existing code
frame.render_widget(Paragraph::new("Hello World!"), frame.size());
// ignoring render_widget
Paragraph::new("Hello World!").render(frame.size(), frame.buffer_mut());
// renderging in a function
hello("world!", frame.size(), frame.buffer_mut());
// returning a widget from a function
hello2("world!").render(frame.size(), frame.buffer_mut());
}
fn hello(name: &str, area: Rect, buffer: &mut Buffer) {
Paragraph::new(format!("Hello, {}!", name)).render(area, buffer);
}
fn hello2(name: &str) -> Paragraph {
Paragraph::new(format!("Hello, {}!", name))
} Using a convention approach doesn't work for boxed widgets. Boxed widgets enables a bunch of future functionality, like to being able to define containers e.g. https://docs.rs/ratatui-widgets/latest/ratatui_widgets/stack_container/struct.StackContainer.html, and creating lists that accept any widget as the content instead of just |
This allows you to treat any function that takes a
Rect
and a mutablereference a
Buffer
as a widget in situations where you don't want tocreate a new type for a widget.
Example:
Related to: https://forum.ratatui.rs/t/idea-functionwidget-was-thoughts-on-tui-react/59/2