[pdm-devel] [PATCH proxmox-yew-widget-toolkit 1/1] data table: add get_property helper for displaying optional values

[Dominik Csapak]

On 8/20/25 12:29, Stefan Hanreich wrote:
> 
> 
> On 8/20/25 11:12 AM, Dominik Csapak wrote:
>> wouldn't it also be possible to simply unwrap the option on the source
>> side?
>>
>>
>> e.g. for a string
>> ...
>> .get_property(|rec: &Record| rec.option.as_deref().unwrap_or(""))
>> ```
> 
> This doesn't work for &str because the size is not known at compilation
> time, at least this produces an error for me (am I holding it wrong?):
> 
>    Some("qwe").as_deref().unwrap_or("")
> 
> error[E0277]: the size for values of type `str` cannot be known at
> compilation time
>     --> src/sdn/evpn/remote_tree.rs:276:18
>      |
> 276 |                 .get_property(|item: &RemoteTreeEntry| {
>      |                  ^^^^^^^^^^^^ doesn't have a size known at
> compile-time
>      |
>      = help: the trait `Sized` is not implemented for `str`
> note: required by an implicit `Sized` bound in
> `DataTableColumn::<T>::get_property`
>     -->
> /usr/share/cargo/registry/pwt-0.6.4/src/widget/data_table/column.rs:303:25
>      |
> 303 |     pub fn get_property<E: Ord + std::fmt::Display>(
>      |                         ^ required by the implicit `Sized`
> requirement on this type parameter in `DataTableColumn::<T>::get_property`

if you have an Option<&str> you can omit the .as_deref()
```
Some("foo").unwrap_or("")
```

works

> 
> The only other option I see is using get_property_owned and cloning
> every time:
> 
>    Some("qwe")
>      .map(str::to_string)
>      .unwrap_or_else(|| "".to_string())
> 
> But since sort calls the function multiple times we'd clone multiple
> times per cell when sorting? Which could be avoided with native support
> for Option<&E>.
> 
> Of course another alternative is to use Cow / IAttr and the likes
> instead if we want to minimize the overhead of cloning but afaict we're
> currently using String everywhere? Might be worthwhile to consider
> though imo.
> 
>> of course you can always implement the renderer and sorter yourself
>> (which should only be a single line each if the value has Display + Ord)
> 
> which becomes verbose quite quickly for sparsely populated trees - why
> not introduce a simple helper to avoid such repetitive code? It avoids
> unnecessary cloning (or at least, is one way to do that) and makes the
> code more succinct at the same time.
> 
>> in general I'm not sure we want to show None values as nothing, this
>> largely depends on the data (e.g. valid things to show/expect could be
>> '-', 'N/A', etc.)
> 
> fair point, but that should be solvable by introducing a default
> parameter / callback and using that instead for generating the default
> value.
> 
>> Also one comment inline
>>
>> On 8/19/25 15:25, Stefan Hanreich wrote:
>>> In some cases, particularly for tree views, it might make sense to not
>>> show values for some cells. In order to simplify handling optional
>>> values, add a get_property helper that can work with optional values.
>>> It renders the values if it exists and nothing otherwise. Since Option
>>> implements Ord, the widget can use its Ord implementation for sorting.
>>> The current helpers cannot be used, since Option does not implement
>>> Display, so the trait bounds are not satisfied.
>>>
>>> Signed-off-by: Stefan Hanreich <s.hanreich at proxmox.com>
>>> ---
>>>    src/widget/data_table/column.rs | 18 +++++++++++++++++-
>>>    1 file changed, 17 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/widget/data_table/column.rs b/src/widget/data_table/
>>> column.rs
>>> index 016be99..770d7c7 100644
>>> --- a/src/widget/data_table/column.rs
>>> +++ b/src/widget/data_table/column.rs
>>> @@ -5,7 +5,7 @@ use derivative::Derivative;
>>>    use yew::html::IntoPropValue;
>>>    use yew::prelude::*;
>>>    -use yew::virtual_dom::Key;
>>> +use yew::virtual_dom::{Key, VNode};
>>>      use crate::props::{CallbackMut, IntoEventCallbackMut,
>>> IntoSorterFn, RenderFn, SorterFn};
>>>    use crate::state::TreeStore;
>>> @@ -328,6 +328,22 @@ impl<T: 'static> DataTableColumn<T> {
>>>            .render(move |item: &T| html! {{get_property_fn(item)}})
>>>        }
>>>    +    /// Builder style method to set a get_property_fn for renderer
>>> and sorter
>>> +    /// the given fn must return the value as an Option
>>> +    pub fn get_property_optional<E: Ord + std::fmt::Display>(
>>> +        self,
>>> +        get_property_fn: impl 'static + Fn(&T) -> Option<&E>,
>>> +    ) -> Self {
>>> +        let get_property_fn = Rc::new(get_property_fn);
>>> +        self.sorter({
>>> +            let get_property_fn = get_property_fn.clone();
>>> +            move |itema: &T, itemb: &T|
>>> get_property_fn(itema).cmp(&get_property_fn(itemb))
>>> +        })
>>> +        .render(
>>> +            move |item: &T| html!
>>> {{get_property_fn(item).map(VNode::from).unwrap_or_default()}},
>>
>> mhmm since we require the Display trait as boundary, wouldn't a
>> to_string() be better here? The default for VNode is a VList, but an
>> empty string produces a VText
> 
> this allocates a string for every cell, whereas VList doesn't and both
> are effectively the same (no HTML produced)

yes it does, VNode::from for T which impl ToString (implicitly by 
Display) uses to_string too:

https://docs.rs/yew/latest/src/yew/virtual_dom/vnode.rs.html#164-166

> 
>>> +        )
>>> +    }
>>> +
>>>        /// Builder style method for [`Self::set_tree_column`]
>>>        pub fn tree_column(mut self, store: impl
>>> IntoPropValue<Option<TreeStore<T>>>) -> Self {
>>>            self.set_tree_column(store);
>>
>