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 9FDE31FF139 for ; Mon, 12 Jan 2026 13:29:14 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id BF5131DB64; Mon, 12 Jan 2026 13:29:15 +0100 (CET) Mime-Version: 1.0 Date: Mon, 12 Jan 2026 13:28:41 +0100 Message-Id: To: "Proxmox Datacenter Manager development discussion" , "Shan Shaji" From: "Lukas Wagner" X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20251203164337.434833-1-s.shaji@proxmox.com> In-Reply-To: <20251203164337.434833-1-s.shaji@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1768220878602 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.038 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 datacenter-manager v2] fix: show empty-state message inside the top entities panels 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" Looking good mostly. Regarding the commit message, usually we only use the 'fix' prefix only if there is some concrete BZ entry that can be referenced. Also, usually we use 'subsystem tags', for this commit this could be (slightly reworded) ui: top entities: show hint message if there are no entities On Wed Dec 3, 2025 at 5:43 PM CET, Shan Shaji wrote: > When the entities list was empty, the dashboard did not display any > message inside the Top Entities panels. This change adds a empty-state > message so the panels provide proper feedback when no entries are > available. > > Signed-off-by: Shan Shaji > --- > changes since v1: Thanks @Dominik > - Removed the extra property inside the TopEntities struct. > - Added optional_child to the Panel to show the empty error message. > - There was an empty space caused by the `TopEntities` component when the > entities list was empty. Inorder to fix that now the TopEntities will > only be rendered if the entities list is not empty. > > ui/src/dashboard/top_entities.rs | 68 ++++++++++++++++++++------------ > 1 file changed, 42 insertions(+), 26 deletions(-) > > diff --git a/ui/src/dashboard/top_entities.rs b/ui/src/dashboard/top_entities.rs > index ab8c703..f79dab7 100644 > --- a/ui/src/dashboard/top_entities.rs > +++ b/ui/src/dashboard/top_entities.rs > @@ -1,6 +1,7 @@ > use std::rc::Rc; > > use pwt::state::SharedState; > +use pwt::widget::Fa; > use web_sys::HtmlElement; > use yew::virtual_dom::{VComp, VNode}; > > @@ -132,6 +133,7 @@ impl Component for TopEntitiesComp { > .style("gap", "var(--pwt-spacer-3)"); > let mut tooltip = None; > let data = &props.entities; > + Unrelated newline change; please remove this one for v3 or at least split it out to a separate patch, in case you think the improved code formatting is worth it. > for entity in data.iter().rev() { > let resource = &entity.resource; > let rrd = &entity.rrd_data; > @@ -199,6 +201,7 @@ impl Component for TopEntitiesComp { > .with_optional_child(tooltip_anchor), > ); > } > + Same here :) > Container::new() > .class(FlexFit) > .with_child(list) > @@ -336,34 +339,47 @@ pub fn create_top_entities_panel( > leaderboard_type: LeaderboardType, > ) -> Panel { > let top_entities = top_entities.read(); > - let (entities, icon, title, metrics_title, threshold) = match leaderboard_type { > - LeaderboardType::GuestCpu => ( > - top_entities.data.as_ref().map(|e| e.guest_cpu.clone()), > - "desktop", > - tr!("Guests With the Highest CPU Usage"), > - tr!("CPU usage"), > - 0.85, > - ), > - LeaderboardType::NodeCpu => ( > - top_entities.data.as_ref().map(|e| e.node_cpu.clone()), > - "building", > - tr!("Nodes With the Highest CPU Usage"), > - tr!("CPU usage"), > - 0.85, > - ), > - LeaderboardType::NodeMemory => ( > - top_entities.data.as_ref().map(|e| e.node_memory.clone()), > - "building", > - tr!("Nodes With the Highest Memory Usage"), > - tr!("Memory usage"), > - 0.95, > - ), > - }; > + let (entities, icon, title, metrics_title, metrics_empty_message, threshold) = > + match leaderboard_type { > + LeaderboardType::GuestCpu => ( > + top_entities.data.as_ref().map(|e| e.guest_cpu.clone()), > + "desktop", > + tr!("Guests With the Highest CPU Usage"), > + tr!("CPU usage"), > + tr!("No guests available"), > + 0.85, > + ), > + LeaderboardType::NodeCpu => ( > + top_entities.data.as_ref().map(|e| e.node_cpu.clone()), > + "building", > + tr!("Nodes With the Highest CPU Usage"), > + tr!("CPU usage"), > + tr!("No nodes available"), > + 0.85, > + ), > + LeaderboardType::NodeMemory => ( > + top_entities.data.as_ref().map(|e| e.node_memory.clone()), > + "building", > + tr!("Nodes With the Highest Memory Usage"), > + tr!("Memory usage"), > + tr!("No nodes available"), > + 0.95, > + ), > + }; > Panel::new() > .title(create_title_with_icon(icon, title)) > - .with_optional_child( > - entities.map(|entities| TopEntities::new(entities, metrics_title, threshold)), > - ) > + .with_optional_child(entities.as_ref().and_then(|entities| { > + entities.is_empty().then_some( > + Row::new() > + .padding(4) > + .gap(2) > + .with_child(Fa::new("info-circle").fixed_width()) > + .with_child(&metrics_empty_message), > + ) > + })) > + .with_optional_child(entities.and_then(|entities| { > + (!entities.is_empty()).then_some(TopEntities::new(entities, metrics_title, threshold)) > + })) I think it would a bit more readable if you used `if` to distinguish both cases, since it's always one of the two (assuming that `entities` is not None) cases. e.g. like this: .with_optional_child(entities.as_ref().and_then(|entities| { if entities.is_empty() { Row::new() .padding(4) .gap(2) .with_child(Fa::new("info-circle").fixed_width()) .with_child(&metrics_empty_message) } else { TopEntities::new(entities, metrics_title, threshold) } })) What do you think? > .with_optional_child((!top_entities.has_data()).then_some(loading_column())) > .with_optional_child( > top_entities _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel