Re: [pdm-devel] [PATCH proxmox-datacenter-manager v2 4/4] ui: add firewall status tree

[Hannes Laimer <h.laimer@proxmox.com>]

On 11/7/25 13:27, Lukas Wagner wrote:
> Looks good from what I can tell (not a super deep review though), some
> notes inline.
> 
> There is some small bug that I've found, when you use the << button to
> collapse the tree and then expand it again, it is slightly smaller than
> before, noticable from the "x out of y rules enabled" text not fitting
> the view any more.
> 

hmm, I couldn't reproduce this here. For v3 I'll replace the .flex(1) 
with a fixed size, that should eliminate any potential _wonkiness_

> Also we need to find some way to handle unavailable remotes more
> gracefully. Right now, a single unavailable remote can block the entire
> loading progress until that one eventually times out; this is definitely
> not great UX-wise. But I think this can also be improved in a future
> patch series, IMO no need to block this until then.
> 

yes. they are loaded in parallel, so max wait time is ~whatever our
timeout is. Maybe a lower timeout could make sense? (I *think* we have
that for remote resources...)

> On Wed Nov 5, 2025 at 5:35 PM CET, Hannes Laimer wrote:
>> +    let mut remote_handle = root.append(remote_entry);
>> +    remote_handle.set_expanded(cluster_is_enabled);
>> +
>> +    for node_status in remote_status.nodes {
>> +        let node_name = node_status.node.clone();
>> +        let node_firewall_status = node_status.status;
>> +
>> +        let node_entry = TreeEntry::Node(NodeEntry {
>> +            remote: remote_name.clone(),
>> +            name: node_name.clone(),
>> +            status: node_firewall_status,
>> +            masked: !cluster_is_enabled,
>> +        });
>> +
>> +        let mut node_handle = remote_handle.append(node_entry);
>> +        node_handle.set_expanded(!node_status.guests.is_empty());
>> +
>> +        for guest in node_status.guests {
>> +            let guest_entry = GuestEntry::new(
>> +                guest.clone(),
>> +                node_name.clone(),
>> +                remote_name.clone(),
>> +                !cluster_is_enabled,
>> +            );
>> +
>> +            let tree_entry = match guest.kind {
>> +                GuestKind::Lxc => TreeEntry::Guest(guest_entry, GuestKind::Lxc),
>> +                GuestKind::Qemu => TreeEntry::Guest(guest_entry, GuestKind::Qemu),
>> +            };
> 
> This can just be:
> 
>              let tree_entry = TreeEntry::Guest(guest_entry, guest.kind);
> 
>> +
>> +            node_handle.append(tree_entry);
>> +        }
>> +    }
>> +}
>> +
>> +fn sort_entries(a: &TreeEntry, b: &TreeEntry) -> Ordering {
>> +    let rank_a = a.sort_rank();
>> +    let rank_b = b.sort_rank();
>> +    match rank_a.cmp(&rank_b) {
>> +        Ordering::Equal => a.name().cmp(&b.name()),
>> +        other => other,
>> +    }
>> +}
>> +
>> +pub enum Msg {
>> +    DataLoaded {
>> +        generation: usize,
>> +        data: Vec<pdm_api_types::firewall::RemoteFirewallStatus>,
>> +    },
>> +    RemoteListChanged,
>> +    Reload,
>> +    FilterChanged(String),
>> +    ScopeChanged(Scope),
>> +    RemotesLoaded(Vec<String>),
>> +    NodesLoaded {
>> +        generation: usize,
>> +        nodes: Vec<String>,
>> +    },
>> +    SelectionChanged(Option<yew::virtual_dom::Key>),
>> +    ToggleTreePanel,
>> +    Error(FirewallError),
>> +    NoOp,
>> +}
>> +
> 
>   [...]
> 
>> +
>> +fn create_remote_combobox(
>> +    ctx: &LoadableComponentContext<FirewallTreeComponent>,
>> +    available_remotes: &[String],
>> +    options_loading: bool,
>> +    current_scope: &Scope,
>> +) -> Html {
>> +    if options_loading {
>> +        return Combobox::new()
>> +            .items(Rc::new(vec![]))
>> +            .placeholder(tr!("Loading..."))
>> +            .disabled(true)
>> +            .key("remote-combobox-loading")
>> +            .on_change(ctx.link().callback(|_: String| Msg::NoOp))
>> +            .into();
>> +    }
>> +
>> +    let items: Vec<yew::AttrValue> = available_remotes
>> +        .iter()
>> +        .map(|remote| yew::AttrValue::from(remote.clone()))
>> +        .collect();
>> +
>> +    let current_value = current_scope
>> +        .remote_name()
>> +        .map(|s| yew::AttrValue::from(s.to_string()));
>> +
>> +    Combobox::new()
>> +        .items(Rc::new(items))
>> +        .default(current_value)
>> +        .placeholder(tr!("All remotes"))
> 
> This combobox also shows PBS remotes, I guess they need to be filtered
> out somewhere along the way.
> 
>> +        .disabled(false)
>> +        .key("remote-combobox")
>> +        .on_change(ctx.link().callback(move |value: String| {
>> +            if value.is_empty() {
>> +                Msg::ScopeChanged(Scope::All)
>> +            } else {
>> +                Msg::ScopeChanged(Scope::Remote { name: value })
>> +            }
>> +        }))
>> +        .into()
>> +}
>> +
> 
> 
> 
> _______________________________________________
> pdm-devel mailing list
> pdm-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
> 
> 



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