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
Rename render_ref
for StatefulWidgetRef
to render_stateful_ref
#996
Comments
+1 on this. I think while it's inconsistent with the StatefulWidget, it's the right choice. One alternative worth considering is naming the traits: |
The |
Maybe:
? And at some point in the future we can deprecate |
The hassle is that doing this breaks everything conceptually (as well as semver). The pay off seems low as it's easy to work around. |
Im not sure that we actually need |
A trait that accepts |
(Removed "good first issue" label while we're discussing this) |
If we are renaming these methods, can we do something like this instead? pub trait RenderRef {
type State;
fn render_ref_with_state(&self, area: Rect, buf: &mut Buffer, state: Option<&mut Self::State>) {
// default empty implementation
}
fn render_ref(&self area: Rect, buf: &mut Buffer) {
self.render_ref_with_state(area, buf, None);
}
} This way we can have just one trait, and get rid of the Widget developers would have to only implement |
There's a tracking issue for associated type defaults which if merged would allow for a defaults for pub trait RenderRef {
type State = (); // `State` can default to empty tuple if tracking issue is merged
fn render_ref_with_state(&self, area: Rect, buf: &mut Buffer, state: Option<&mut Self::State>) {
// default empty implementation
}
fn render_ref(&self area: Rect, buf: &mut Buffer) {
self.render_ref_with_state(area, buf, None);
}
} And this will save one line for widgets that only want to implement |
On a full redo, I think I'd prefer something like: trait Render {
fn render(&self, context: &mut RenderContext);
}
trait RenderMut {
fn render_mut(&mut self, context: &mut RenderContext);
} But I'm not sure yet whether it's also worth including things like |
Why not use a single trait and decide about ref or whatever on the implementation? |
Can we combine them into one trait? trait Render {
fn render_mut(&mut self, context: &mut RenderContext) {
self.render(context)
}
fn render(&self, context: &mut RenderContext) {
}
}
@EdJoPaTo I believe it was not possible to use For example, my understanding is that this code is not possible with the current |
Is StatefulWidgetRef useful for anything at all? With the associated type State I can't have a
instead I would need a
Which means I can't have a Vec with mixed widgets, so why not just use
instead. |
Another thing is unclear to me. If I create a composite widget I end up cloning the contained widget like in use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use ratatui::style::{Style, Styled, Stylize};
use ratatui::text::Text;
use ratatui::widgets::StatefulWidgetRef;
#[derive(Default, Clone)]
struct Button<'a> {
text: Text<'a>,
style: Style,
}
struct ButtonState {
pub armed: bool,
}
impl<'a> StatefulWidgetRef for Button<'a> {
type State = ButtonState;
fn render_ref(&self, area: Rect, buf: &mut Buffer, state: &mut Self::State) {
// ...
}
}
#[derive(Default, Clone)]
struct FocusableButton<'a> {
widget: Button<'a>,
}
struct FocusableButtonState {
pub widget: ButtonState,
pub focus: bool,
}
impl<'a> StatefulWidgetRef for FocusableButton<'a> {
type State = FocusableButtonState;
fn render_ref(&self, area: Rect, buf: &mut Buffer, state: &mut Self::State) {
let w = if state.focus {
let mut w = self.widget.clone();
w.style = w.style.on_light_blue();
w
} else {
self.widget.clone()
};
w.render_ref(area, buf, &mut state.widget)
}
} In this case that clone() doesn't matter much, but if I have a List or Table instead this ends up cloning the whole thing. Is this intended, or how am I supposed to do something like that? |
This is one of the reasons why this is unstable for now. I just made an example in tui-scrollview, where there are different widgets but they share the same state type. But I can see how this might be useless if they had to be different. The context of this is:
|
Tui-rs was created around the idea of recreating widgets on every frame, WidgetRef relaxes that idea somewhat, but it's not the only approach that works. Another approach is to have the state have a method that returns the widget, and ensure that the widget is fairly lightweight (i.e. the data is stored in the state, not the widget): struct ButtonState {
armed: bool,
}
struct ButtonWidget<'a> {
style: Style,
state: &'a ButtonState,
}
impl ButtonState {
fn widget(&self) -> ButtonWidget {
style: ...,
state: self,
}
}
impl FocusedButtonState {
fn widget(&self) -> ButtonWidget {
let button = self.button_state.widget();
if ... { button.style = ... }
button.render(...);
}
} Combine this with mutable refs where necessary. |
I didn't know the exact history of this all, good to know. I tried, and found one way that might work: pub trait AnyWidget: StatefulWidgetRef<State = dyn AnyWidgetState> {}
pub trait AnyWidgetState: Any {
fn as_any_mut(&mut self) -> &mut dyn Any;
} it almost works with StatefulWidgetRef, except this requires And you need I think I will stay with |
At some point this might make a good writeup for the website. Especially combined with some of the ideas / code from https://forum.ratatui.rs/t/question-how-can-i-representing-applicatoin-state-ergonomically-in-ratatui-programs/54/4 |
I've pondered a bit more about this dyn Any stuff. Once you add one or more traits to the state things start to get complicated really fast, without providing a satisfying solution. Maybe this might work if we had one all encompassing trait for the widget, with all the batteries included. But as I tried working with two traits I came to the limits of the poor excuse for RTTI that we have with Any. After my brain started fogging and my eyes drifted of into the 4th dimension, I remembered the first rule of rust: Try something else. |
RIIR -> Rewrite it again when the borrow checker slaps you in the face |
Given the confusions about the render traits… Should it be a trait in the first place? A struct feels long-term, so people expect them to be long-term. struct TableState {offset: usize, …}
// Should own its content, so no references needed. Probably created once for the whole time of the application.
struct TableOptions {highlight_style: Style, headers: Vec<Cell>, …}
fn ratatui::table::render(
buffer: Buffer,
area: Rect,
state: &mut TableState,
rows: &[Row],
&options: &TableOptions,
) {…} The downside is the grouping of many widgets in a Vec like @thscharler tried to achieve. Having a struct named clearly for this purpose with a single trait for this would achieve that too: // Keep renferences on everything instead of owning things
struct TableRenderInformation<'a> {
state: &'a mut TableState,
rows: &'a [Row],
options: &'a TableOptions,
}
impl Render for TableRenderInformation {
// self as the TableRenderInformation should only keep references.
fn render(self, area: Rect, buffer: Buffer);
} This would also remove the need for the Context suggested in #1044 as every widget knows which information like state or not are required to render. |
Not only that, but you'll need a trait for widgets like a ScrollPanel or a Viewport too. Container<T> {
inner: T
} you'll need a trait to render the inner T.
This wouldn't remove the need for Context, but the need for StatefulWidget. The point for the Context was more to have extra information like the frame-count or the cursor position. On a sidenote, I wonder why it was never considered to use Frame |
This is effectively isomorphic (or functionally equivalent) to what we already have.
I've used a similar approach before (I can't find the code right now - it's likely I migrated that code from something like Table {
state: TableState,
rows: Vec<Row>,
options: Options
}
impl WidgetMut for Table {
fn render_mut(&mut self, area: Rect, buffer: Buffer);
}
Context is the context of the rendering process, not the context for the widget. It should contain things like what frame we're on, when was the last frame rendered, the buffer, precomputed layout information, ...
Basically historical reasons. If I'd redesign Ratatui entirely based on current knowledge:
But we've got what we've got, and so we implement things in an additive way that hopefully avoids breaking the world. |
Thats the point.
|
My point is that renaming all the types in Ratatui to add Options / RenderInformation to them adds no value. It's a nit pick.
WidgetRef makes this expectation hold true. |
Problem
Currently both
WidgetRef
andStatefulWidgetRef
userender_ref
method names.ratatui/src/widgets.rs
Lines 298 to 303 in 8719608
ratatui/src/widgets.rs
Lines 395 to 404 in 8719608
If a user imports both traits, then they have to explicitly call the method using the trait name, i.e. they have to write
StatefulWidgetRef::render_ref(widget, area, buf, state)
instead ofwidget.render_ref(area, buf, state)
.Renaming
render_ref
torender_stateful_ref
will prevent the ambiguity betweenWidgetRef
andStatefulWidgetRef
.It would be nice to disambiguate it for
Widget
andStatefulWidget
as well but that would be a big breaking change. ForWidgetRef
andStatefulWidgetRef
, these traits are still unstable at the moment so we can make this change.The text was updated successfully, but these errors were encountered: