public inbox for yew-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [yew-devel] [PATCH yew-widget-toolkit] props: replace `BuilderFn` with `RenderFn`
@ 2025-12-11 12:59 Dominik Csapak
  2025-12-11 13:49 ` Dietmar Maurer
  2025-12-11 13:54 ` Dietmar Maurer
  0 siblings, 2 replies; 4+ messages in thread
From: Dominik Csapak @ 2025-12-11 12:59 UTC (permalink / raw)
  To: yew-devel

Since `RenderFn` is now generic also in it's output, we can replace the
use of `BuilderFn` with it.

Simply using `()` as it's input, and using whatever output we had
before.

The only slightly awkward changes are:
* we now have to use `apply(&())` in this case
* when giving a callback to such a builder we have to use `|_: &_|` now,
  but this was also the case for any closure that got converted into a
  `RenderFn`.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/props/mod.rs                          |  2 +-
 src/props/render_function.rs              | 47 -----------------------
 src/widget/data_table/header_widget.rs    |  2 +-
 src/widget/data_table/resizable_header.rs |  8 ++--
 src/widget/menu/menu_button.rs            | 10 ++---
 src/widget/menu/split_button.rs           | 10 ++---
 6 files changed, 16 insertions(+), 63 deletions(-)

diff --git a/src/props/mod.rs b/src/props/mod.rs
index 254cb4e..c541110 100644
--- a/src/props/mod.rs
+++ b/src/props/mod.rs
@@ -200,7 +200,7 @@ mod padding;
 pub use padding::CssPaddingBuilder;
 
 mod render_function;
-pub use render_function::{BuilderFn, IntoOptionalBuilderFn, IntoOptionalRenderFn, RenderFn};
+pub use render_function::{IntoOptionalRenderFn, RenderFn};
 
 mod sorter_function;
 pub use sorter_function::{IntoSorterFn, SorterFn};
diff --git a/src/props/render_function.rs b/src/props/render_function.rs
index dd5632d..5caf3c4 100644
--- a/src/props/render_function.rs
+++ b/src/props/render_function.rs
@@ -46,50 +46,3 @@ impl<IN, OUT, R: Into<RenderFn<IN, OUT>>> IntoOptionalRenderFn<IN, OUT> for Opti
         self.map(|me| me.into())
     }
 }
-
-/// A [BuilderFn] function is a callback that returns a generic type.
-///
-/// Wraps `Rc` around `Fn` so it can be passed as a prop.
-#[derive(Derivative)]
-#[derivative(Clone(bound = ""), PartialEq(bound = ""))]
-pub struct BuilderFn<T>(#[derivative(PartialEq(compare_with = "Rc::ptr_eq"))] Rc<dyn Fn() -> T>);
-
-impl<T: Into<Html>> From<BuilderFn<T>> for Html {
-    fn from(val: BuilderFn<T>) -> Self {
-        val.apply().into()
-    }
-}
-
-impl<T> BuilderFn<T> {
-    /// Creates a new [`BuilderFn`]
-    pub fn new(builder: impl 'static + Fn() -> T) -> Self {
-        Self(Rc::new(builder))
-    }
-    /// Apply the builder function
-    pub fn apply(&self) -> T {
-        (self.0)()
-    }
-}
-
-impl<T, F: 'static + Fn() -> T> From<F> for BuilderFn<T> {
-    fn from(f: F) -> Self {
-        BuilderFn::new(f)
-    }
-}
-
-/// Helper trait to create an optional [BuilderFn] property.
-pub trait IntoOptionalBuilderFn<T> {
-    fn into_optional_builder_fn(self) -> Option<BuilderFn<T>>;
-}
-
-impl<T> IntoOptionalBuilderFn<T> for Option<BuilderFn<T>> {
-    fn into_optional_builder_fn(self) -> Option<BuilderFn<T>> {
-        self
-    }
-}
-
-impl<T, F: 'static + Fn() -> T> IntoOptionalBuilderFn<T> for F {
-    fn into_optional_builder_fn(self) -> Option<BuilderFn<T>> {
-        Some(BuilderFn::new(self))
-    }
-}
diff --git a/src/widget/data_table/header_widget.rs b/src/widget/data_table/header_widget.rs
index c1fd806..c4a5541 100644
--- a/src/widget/data_table/header_widget.rs
+++ b/src/widget/data_table/header_widget.rs
@@ -311,7 +311,7 @@ impl<T: 'static> PwtHeaderWidget<T> {
                             let headers = Rc::clone(&props.headers);
                             let link = link.clone();
                             let hidden_cells = self.state.hidden_cells();
-                            move || build_header_menu(&headers, &link, cell_idx, &hidden_cells)
+                            move |_: &_| build_header_menu(&headers, &link, cell_idx, &hidden_cells)
                         }),
                 )
                 .onfocusin(link.callback(move |_| Msg::FocusCell(cell_idx)))
diff --git a/src/widget/data_table/resizable_header.rs b/src/widget/data_table/resizable_header.rs
index 301adce..88d624f 100644
--- a/src/widget/data_table/resizable_header.rs
+++ b/src/widget/data_table/resizable_header.rs
@@ -12,7 +12,7 @@ use crate::css::ColorScheme;
 use crate::dom::element_direction_rtl;
 use crate::dom::focus::FocusTracker;
 use crate::dom::DomSizeObserver;
-use crate::props::{BuilderFn, IntoOptionalBuilderFn};
+use crate::props::{IntoOptionalRenderFn, RenderFn};
 use crate::widget::menu::{Menu, MenuButton, MenuController};
 use crate::widget::{Container, Row};
 use crate::{impl_class_prop_builder, impl_yew_std_props_builder, prelude::*};
@@ -53,7 +53,7 @@ pub struct ResizableHeader {
 
     /// Function to generate the header menu.
     #[prop_or_default]
-    pub menu_builder: Option<BuilderFn<Menu>>,
+    pub menu_builder: Option<RenderFn<(), Menu>>,
 }
 
 impl Default for ResizableHeader {
@@ -135,8 +135,8 @@ impl ResizableHeader {
     }
 
     /// Builder style method to set the menu builder.
-    pub fn menu_builder(mut self, builder: impl IntoOptionalBuilderFn<Menu>) -> Self {
-        self.menu_builder = builder.into_optional_builder_fn();
+    pub fn menu_builder(mut self, builder: impl IntoOptionalRenderFn<(), Menu>) -> Self {
+        self.menu_builder = builder.into_optional_render_fn();
         self
     }
 }
diff --git a/src/widget/menu/menu_button.rs b/src/widget/menu/menu_button.rs
index 7428ce4..b3ca233 100644
--- a/src/widget/menu/menu_button.rs
+++ b/src/widget/menu/menu_button.rs
@@ -5,7 +5,7 @@ use yew::prelude::*;
 
 use crate::dom::focus::FocusTracker;
 use crate::prelude::*;
-use crate::props::{BuilderFn, IntoOptionalBuilderFn};
+use crate::props::{IntoOptionalRenderFn, RenderFn};
 use crate::state::SharedStateObserver;
 use crate::widget::menu::MenuController;
 use crate::widget::{Button, Container};
@@ -37,7 +37,7 @@ pub struct MenuButton {
     /// To create menus dynamically. If specified, the 'menu' property
     /// is ignored.
     #[prop_or_default]
-    pub menu_builder: Option<BuilderFn<Menu>>,
+    pub menu_builder: Option<RenderFn<(), Menu>>,
 
     /// Automatically popup menu when receiving focus
     ///
@@ -94,8 +94,8 @@ impl MenuButton {
     }
 
     /// Builder style method to set the menu builder.
-    pub fn menu_builder(mut self, builder: impl IntoOptionalBuilderFn<Menu>) -> Self {
-        self.menu_builder = builder.into_optional_builder_fn();
+    pub fn menu_builder(mut self, builder: impl IntoOptionalRenderFn<(), Menu>) -> Self {
+        self.menu_builder = builder.into_optional_render_fn();
         self
     }
 }
@@ -223,7 +223,7 @@ impl Component for PwtMenuButton {
             let menu = if let Some(menu_builder) = &props.menu_builder {
                 Some(
                     menu_builder
-                        .apply()
+                        .apply(&())
                         .autofocus(true)
                         .menu_controller(self.menu_controller.clone())
                         .on_close(ctx.link().callback(|_| Msg::CloseMenu)),
diff --git a/src/widget/menu/split_button.rs b/src/widget/menu/split_button.rs
index fb84734..32b0336 100644
--- a/src/widget/menu/split_button.rs
+++ b/src/widget/menu/split_button.rs
@@ -5,7 +5,7 @@ use yew::prelude::*;
 
 use crate::dom::focus::FocusTracker;
 use crate::prelude::*;
-use crate::props::{BuilderFn, IntoOptionalBuilderFn};
+use crate::props::{IntoOptionalRenderFn, RenderFn};
 use crate::state::SharedStateObserver;
 use crate::widget::menu::MenuController;
 use crate::widget::{Button, Container};
@@ -39,7 +39,7 @@ pub struct SplitButton {
     /// To create menus dynamically. If specified, the 'menu' property
     /// is ignored.
     #[prop_or_default]
-    pub menu_builder: Option<BuilderFn<Menu>>,
+    pub menu_builder: Option<RenderFn<(), Menu>>,
 
     #[prop_or_default]
     #[builder]
@@ -88,8 +88,8 @@ impl SplitButton {
     }
 
     /// Builder style method to set the menu builder.
-    pub fn menu_builder(mut self, builder: impl IntoOptionalBuilderFn<Menu>) -> Self {
-        self.menu_builder = builder.into_optional_builder_fn();
+    pub fn menu_builder(mut self, builder: impl IntoOptionalRenderFn<(), Menu>) -> Self {
+        self.menu_builder = builder.into_optional_render_fn();
         self
     }
 }
@@ -234,7 +234,7 @@ impl Component for PwtSplitButton {
             let menu = if let Some(menu_builder) = &props.menu_builder {
                 Some(
                     menu_builder
-                        .apply()
+                        .apply(&())
                         .autofocus(true)
                         .menu_controller(self.menu_controller.clone())
                         .on_close(ctx.link().callback(|_| Msg::CloseMenu)),
-- 
2.47.3



_______________________________________________
yew-devel mailing list
yew-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/yew-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [yew-devel] [PATCH yew-widget-toolkit] props: replace `BuilderFn` with `RenderFn`
  2025-12-11 12:59 [yew-devel] [PATCH yew-widget-toolkit] props: replace `BuilderFn` with `RenderFn` Dominik Csapak
@ 2025-12-11 13:49 ` Dietmar Maurer
  2025-12-12  7:31   ` Dominik Csapak
  2025-12-11 13:54 ` Dietmar Maurer
  1 sibling, 1 reply; 4+ messages in thread
From: Dietmar Maurer @ 2025-12-11 13:49 UTC (permalink / raw)
  To: Yew framework devel list at Proxmox


[-- Attachment #1.1: Type: text/plain, Size: 439 bytes --]

I think this was the reason I invented the whole thing.
> The only slightly awkward changes are: * we now have to use `apply(&())` in this case * when giving a callback to such a builder we have to use `|_: &_|` now, but this was also the case for any closure that got converted into a `RenderFn`.
Being forced to use |_: &_| is ugly, and the compiler errors for that are even more ugly and confusing. So I am not sure about this change.

[-- Attachment #1.2: Type: text/html, Size: 852 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
yew-devel mailing list
yew-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/yew-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [yew-devel] [PATCH yew-widget-toolkit] props: replace `BuilderFn` with `RenderFn`
  2025-12-11 12:59 [yew-devel] [PATCH yew-widget-toolkit] props: replace `BuilderFn` with `RenderFn` Dominik Csapak
  2025-12-11 13:49 ` Dietmar Maurer
@ 2025-12-11 13:54 ` Dietmar Maurer
  1 sibling, 0 replies; 4+ messages in thread
From: Dietmar Maurer @ 2025-12-11 13:54 UTC (permalink / raw)
  To: Yew framework devel list at Proxmox


[-- Attachment #1.1: Type: text/plain, Size: 78 bytes --]

Note: private mail
I thought we can simply use Callback to replace BuilderFn?

[-- Attachment #1.2: Type: text/html, Size: 611 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
yew-devel mailing list
yew-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/yew-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [yew-devel] [PATCH yew-widget-toolkit] props: replace `BuilderFn` with `RenderFn`
  2025-12-11 13:49 ` Dietmar Maurer
@ 2025-12-12  7:31   ` Dominik Csapak
  0 siblings, 0 replies; 4+ messages in thread
From: Dominik Csapak @ 2025-12-12  7:31 UTC (permalink / raw)
  To: Yew framework devel list at Proxmox, Dietmar Maurer



On 12/11/25 2:50 PM, Dietmar Maurer wrote:
> 
> I think this was the reason I invented the whole thing.
> 
>  > The only slightly awkward changes are: * we now have to use 
> `apply(&())` in this case * when giving a callback to such a builder we 
> have to use `|_: &_|` now,  but this was also the case for any closure 
> that got converted into a  `RenderFn`.
> 
> Being forced to use |_: &_| is ugly, and the compiler errors for that 
> are even more ugly and confusing. So I am not sure about this change.
> 
as i wrote this is already the case for every user of the RenderFn, and
we only use the BuilderFn internally in pwt currently and only in three 
places, so I don't think that this is too bad.

On 12/11/25 2:54 PM, Dietmar Maurer wrote:
 >
 > I thought we can simply use Callback to replace BuilderFn?

i tried, but this would result in other weird usage, e.g.
we'd have to use `emit(())` instead if `apply(&())` which
is also not very intuitive.

also there is no 'into trait' for callbacks with a generic
return value AFAICS


but as discussed off-list, let's wait for yew 0.22 which
introduces a `CallbackRef` that seemingly does something
very similar to our `RenderFn`, then we can maybe
get rid of both RenderFn and BuilderFn


_______________________________________________
yew-devel mailing list
yew-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/yew-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-12-12  7:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-11 12:59 [yew-devel] [PATCH yew-widget-toolkit] props: replace `BuilderFn` with `RenderFn` Dominik Csapak
2025-12-11 13:49 ` Dietmar Maurer
2025-12-12  7:31   ` Dominik Csapak
2025-12-11 13:54 ` Dietmar Maurer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal