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:44 PM, Dominik Csapak wrote:
> 
> 
> 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
> 
> 
> 

actually no, the 'pwt-content-spacer' sets the direction to column, so
this is necessary here..


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