Re: [pdm-devel] [RFC PATCH datacenter-manager v2 15/16] ui: dashboard: implement 'View'

[Dominik Csapak <d.csapak@proxmox.com>]

On 10/23/25 1:19 PM, Shannon Sterz wrote:
> On Thu Oct 23, 2025 at 10:28 AM CEST, Dominik Csapak wrote:
[snip]
>> +
>> +struct ViewComp {
>> +    template: LoadResult<ViewTemplate, Error>,
>> +
>> +    // various api call results
>> +    status: LoadResult<ResourcesStatus, Error>,
>> +    top_entities: LoadResult<TopEntities, proxmox_client::Error>,
>> +    statistics: LoadResult<TaskStatistics, Error>,
> 
> this is fine, but i just had an idea, maybe this isn't too useful right
> now, but might be worth exploring: we could turn this into a HashMap
> with something like this:
> 
> HashMap<ToQuery, LoadResult<ApiResponseData, Error>>
> 
> then loading could become iterating over the keys and calling a function
> on them. with a wrapper type we could even implement a getter that
> transforms the ApiResponseData to a concrete type. might cut down on the
> loading logic below and make this more easily extensible in the future.
> 
> the required_api_calls below could then just return such a hashmap with
> only the necessary keys. what do you think (note i haven't tested any of
> this)?

i don't think this will work, since ApiResponseData itself takes a
generic parameter too, and we can't use different ones for different
values of the same hashmap AFAIK

but yeah, we should think about how we could generalize this
instead of just adding on new members...

[snip]
>> +        match self.template.data.as_ref().map(|template| &template.layout) {
>> +            Some(ViewLayout::Rows { rows }) => {
>> +                for items in rows {
>> +                    let mut row = Row::new()
>> +                        .gap(4)
>> +                        .padding_top(0)
>> +                        .class("pwt-content-spacer")
> 
> since this is used here quite extensively, might make sense to also give
> that a type in the `css` module, but that's unrelated to this series

yes, i agree (we have quite some classes that would IMHO benefit
from that)

> 
>> +                        .class(css::FlexDirection::Row)
> 
> just something i'm curious about, but is this necessary? shouldn't a
> `Row` already be `FlexDirection::Row`? or more accurately, isn't it by
> default?

you're right, this seems to be a leftover from some older versions i had



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