From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id E79381FF16B for ; Fri, 7 Nov 2025 13:27:19 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 53B1010347; Fri, 7 Nov 2025 13:28:02 +0100 (CET) Date: Fri, 07 Nov 2025 13:27:28 +0100 Message-Id: From: "Lukas Wagner" To: "Proxmox Datacenter Manager development discussion" Mime-Version: 1.0 X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20251105163546.450094-1-h.laimer@proxmox.com> <20251105163546.450094-13-h.laimer@proxmox.com> In-Reply-To: <20251105163546.450094-13-h.laimer@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1762518428671 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.029 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pdm-devel] [PATCH proxmox-datacenter-manager v2 4/4] ui: add firewall status tree X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" 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. 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. 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, > + }, > + RemoteListChanged, > + Reload, > + FilterChanged(String), > + ScopeChanged(Scope), > + RemotesLoaded(Vec), > + NodesLoaded { > + generation: usize, > + nodes: Vec, > + }, > + SelectionChanged(Option), > + ToggleTreePanel, > + Error(FirewallError), > + NoOp, > +} > + [...] > + > +fn create_remote_combobox( > + ctx: &LoadableComponentContext, > + 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 = 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