feat(widgets)!: make widgets optionally reusable #122
Conversation
|
The |
|
Hey, can you revert the formatting changes? |
|
@orhun bam. |
|
I like this change - it enables some things that were a pain to work around the widget being consumed on render calls or when you can get a reference but it's more difficult to get something consumable (e.g. joshka@e56d8ea where I can easily call render on a boxed dyn widget which means the List Widget can be extended to display any widget and not just specific ones). One possible other approach to implementing a new method would to actually break callers by making render_widget and render_stateful_widget actually borrow the widget (and probably area too) instead of having two methods. We're in a spot where it could be reasonable to make a choice to break consumers here if the outcome enables many more scenarios and makes the library more ergonomic - this is probably controversial and I'd defer to those who have more experience than I do on this. If we keep the two methods naming, do you think |
Would it be possible to have them accept either a
You're absolutely right, |
|
I couldn’t quite get AsRed to work - if you can that would be definitely be the best way to make this work. |
|
@joshka I think you got in the playground is because |
joshka
changed the title
joshka
added
the
Type: Breaking Change
|
Unfortunately, you can't make a blanket implementation for a foreign trait like that. What you can do is make your own trait that mimics |
Nice! That sounds like a realistic approach! |
joshka
force-pushed
the
main
branch
2 times, most recently
from
229d3eb to
907bf3e
Compare
Codecov Report
@@ Coverage Diff @@
## main #122 +/- ##
==========================================
+ Coverage 81.63% 81.79% +0.15%
==========================================
Files 34 34
Lines 6574 6648 +74
==========================================
+ Hits 5367 5438 +71
- Misses 1207 1210 +3
|
|
Thanks for the fix @aschey. That approach works pretty nicely (I updated code in this PR to use that approach). I think it's worth playing with this more to understand some of the pros and cons. We probably want to also make the area Rect param pass by ref also (that's particularly useful when rendering inner blocks). |
In some situations, ratatui's render functions can make it difficult to
implement specific behavior or can make it difficult and inefficient to
reuse widgets. E.g. for a list, you have to clone either the vec or it's
items again each frame to make the new list object. Another example is
rendering a widget in a loop, where you have to clone the widget each
time.
This commit makes it so the `Widget`/`StatefulWidget``::render`
functions only take `self` by immutable reference, meaning they're not
consumed and can be reused.
The `Frame::render_{,stateful_}widget` functions take `widget` by value
though. To allow these functions to be called with references or values,
we added a new trait `AsWidgetRef` which is implemented for `&W` and `W`
where `W: Widget`/`StatefulWidget. This trait is then used to call the
`render` functions. Thus backwards compatibility is maintained for the
`Frame` API.
This also removes any calls to `take()` in the `render` functions since
this cannot happen when the widget is passed by reference.
BREAKING CHANGE: `Widget`/`StatefulWidget`'s `render` functions now take
`self` by immutable reference instead of by value. To update your custom
widgets simply add `&` before the `self` parameter in the `render`
function.
|
Updated Widget list PoC based on this: joshka@e911bee |
Playing with this in the Widget List PoC and noticing what I needed to change in the tests to make them work nicely, I definitely agree with that @sophacles |
|
This PR looks good. The scope is limited. We may need to accompany this change with many other related PRs, but as far as this PR goes, LGTM. |
joshka
requested review from
orhun,
mindoodoo and
sayanarijit
as code owners
kdheepak
changed the title
|
In terms of the |
Many widgets can be rendered without changing their state.
This commit implements The `Widget` trait for various references to
widgets and changes their implementations to be immutable.
This allows us to render widgets without consuming them by passing a ref
to the widget when calling `Frame::render_widget()`.
```rust
// this might be stored in a struct
let paragraph = Paragraph::new("Hello world!");
let [left, right] = area.split(&Layout::horizontal([20, 20]));
frame.render_widget(¶graph, left);
frame.render_widget(¶graph, right); // we can reuse the widget
```
- Clear
- Block
- Tabs
- Sparkline
- Paragraph
- Gauge
- Calendar
Other widgets will be implemented in follow up commits.
Fixes: #164
Replaces PRs: #122 and
#16
Enables: #132
Validated as a viable working solution by: #836
|
Closing with deference to #833 which achieves the same result by implementing |
Many widgets can be rendered without changing their state.
This commit implements The `Widget` trait for various references to
widgets and changes their implementations to be immutable.
This allows us to render widgets without consuming them by passing a ref
to the widget when calling `Frame::render_widget()`.
```rust
// this might be stored in a struct
let paragraph = Paragraph::new("Hello world!");
let [left, right] = area.split(&Layout::horizontal([20, 20]));
frame.render_widget(¶graph, left);
frame.render_widget(¶graph, right); // we can reuse the widget
```
- Clear
- Block
- Tabs
- Sparkline
- Paragraph
- Gauge
- Calendar
Other widgets will be implemented in follow up commits.
Fixes: #164
Replaces PRs: #122 and
#16
Enables: #132
Validated as a viable working solution by: #836
Many widgets can be rendered without changing their state.
This commit implements The `Widget` trait for references to
widgets and changes their implementations to be immutable.
This allows us to render widgets without consuming them by passing a ref
to the widget when calling `Frame::render_widget()`.
```rust
// this might be stored in a struct
let paragraph = Paragraph::new("Hello world!");
let [left, right] = area.split(&Layout::horizontal([20, 20]));
frame.render_widget(¶graph, left);
frame.render_widget(¶graph, right); // we can reuse the widget
```
Implemented for all widgets except BarChart (which has an implementation
that modifies the internal state and requires a rewrite to fix.
Other widgets will be implemented in follow up commits.
Fixes: #164
Replaces PRs: #122 and
#16
Enables: #132
Validated as a viable working solution by:
#836
In some situations, ratatui's immediate-mode rendering can be inefficient, i.e. for a list, you have to clone either the vec or it's items again each frame to make the new list object.
This commit makes it so the
{,Stateful}Widget::renderfunctions only takeselfby immutable reference, meaning they're not consumed and can be reused.The
Frame::render_{,stateful_}widgetfunctions takewidgetby value though, and changing it would break all code using it, so I've added separate functions:render_{,stateful_}widget_reusablefunctions which take thewidgetby reference, allowing it to be reused.This could also allow
StatefulWidgets to store their state themselves, rather than relying on the user to store it.I also had to remove any
takes in therenderfunctions since they would invalidate the widget.I also ran
cargo fmtwith the default config to remove formatting accidentally done under my config. I can undo the formatting if needed.