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 087501FF165 for ; Thu, 23 Oct 2025 13:18:44 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 050F37C0D; Thu, 23 Oct 2025 13:19:11 +0200 (CEST) Mime-Version: 1.0 Date: Thu, 23 Oct 2025 13:19:06 +0200 Message-Id: To: "Dominik Csapak" X-Mailer: aerc 0.20.0 References: <20251023083253.1038119-1-d.csapak@proxmox.com> <20251023083253.1038119-3-d.csapak@proxmox.com> In-Reply-To: <20251023083253.1038119-3-d.csapak@proxmox.com> From: "Shannon Sterz" X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1761218338084 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.057 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 02/16] ui: dashboard: refactor creating the node panel into its own module 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 Cc: 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" On Thu Oct 23, 2025 at 10:28 AM CEST, Dominik Csapak wrote: > so we can easily reuse it outside the Dashboard struct. Since this > shifts the search logic there, it can be removed from the Dashboard > struct. > correct me if i'm wrong, but this also add support for pbs remotes here if i'm not mistaken? in that case adding that to the commit message would be helpful. > Signed-off-by: Dominik Csapak > --- > ui/src/dashboard/mod.rs | 106 +++-------------------- > ui/src/dashboard/node_panel.rs | 150 +++++++++++++++++++++++++++++++++ > 2 files changed, 161 insertions(+), 95 deletions(-) > create mode 100644 ui/src/dashboard/node_panel.rs > > diff --git a/ui/src/dashboard/mod.rs b/ui/src/dashboard/mod.rs > index 602c8a3b..2e170443 100644 > --- a/ui/src/dashboard/mod.rs > +++ b/ui/src/dashboard/mod.rs > @@ -10,7 +10,7 @@ use yew::{ > Component, > }; > > -use proxmox_yew_comp::{http_get, EditWindow, Status}; > +use proxmox_yew_comp::{http_get, EditWindow}; > use pwt::{ > css::{AlignItems, FlexDirection, FlexFit, FlexWrap, JustifyContent}, > prelude::*, > @@ -25,16 +25,11 @@ use pwt::{ > AsyncPool, > }; > > -use pdm_api_types::{ > - remotes::RemoteType, > - resource::{NodeStatusCount, ResourcesStatus}, > - TaskStatistics, > -}; > +use pdm_api_types::{remotes::RemoteType, resource::ResourcesStatus, TaskStatistics}; > use pdm_client::types::TopEntity; > -use pdm_search::{Search, SearchTerm}; > use proxmox_client::ApiResponseData; > > -use crate::{pve::GuestType, remotes::AddWizard, search_provider::get_search_provider, RemoteList}; > +use crate::{pve::GuestType, remotes::AddWizard, RemoteList}; > > mod top_entities; > pub use top_entities::TopEntities; > @@ -42,6 +37,9 @@ pub use top_entities::TopEntities; > mod subscription_info; > pub use subscription_info::SubscriptionInfo; > > +mod node_panel; > +pub use node_panel::create_node_panel; > + > mod remote_panel; > use remote_panel::RemotePanel; > > @@ -123,7 +121,6 @@ pub enum Msg { > ForceReload, > UpdateConfig(DashboardConfig), > ConfigWindow(bool), > - Search(Search), > } > > struct StatisticsOptions { > @@ -149,81 +146,6 @@ pub struct PdmDashboard { > } > > impl PdmDashboard { > - fn create_node_panel(&self, ctx: &yew::Context, icon: &str, title: String) -> Panel { > - let mut search_terms = vec![SearchTerm::new("node").category(Some("type"))]; > - let (status_icon, text): (Fa, String) = match &self.status { > - Some(status) => { > - match status.pve_nodes { > - NodeStatusCount { > - online, > - offline, > - unknown, > - } if offline > 0 => { > - search_terms.push(SearchTerm::new("offline").category(Some("status"))); > - ( > - Status::Error.into(), > - tr!( > - "{0} of {1} nodes are offline", > - offline, > - online + offline + unknown, > - ), > - ) > - } > - NodeStatusCount { unknown, .. } if unknown > 0 => { > - search_terms.push(SearchTerm::new("unknown").category(Some("status"))); > - ( > - Status::Warning.into(), > - tr!("{0} nodes have an unknown status", unknown), > - ) > - } > - // FIXME, get more detailed status about the failed remotes (name, type, error)? > - NodeStatusCount { online, .. } if status.failed_remotes > 0 => ( > - Status::Unknown.into(), > - tr!("{0} of an unknown number of nodes online", online), > - ), > - NodeStatusCount { online, .. } => { > - (Status::Success.into(), tr!("{0} nodes online", online)) > - } > - } > - } > - None => (Status::Unknown.into(), String::new()), > - }; > - > - let loading = self.status.is_none(); > - let search = Search::with_terms(search_terms); > - Panel::new() > - .flex(1.0) > - .width(300) > - .title(create_title_with_icon(icon, title)) > - .border(true) > - .with_child( > - Column::new() > - .padding(4) > - .class("pwt-pointer") > - .onclick(ctx.link().callback({ > - let search = search.clone(); > - move |_| Msg::Search(search.clone()) > - })) > - .onkeydown(ctx.link().batch_callback({ > - let search = search.clone(); > - move |event: KeyboardEvent| match event.key().as_str() { > - "Enter" | " " => Some(Msg::Search(search.clone())), > - _ => None, > - } > - })) > - .class(FlexFit) > - .class(AlignItems::Center) > - .class(JustifyContent::Center) > - .gap(2) > - .with_child(if loading { > - html! {} > - } else { > - status_icon.large_4x().into() > - }) > - .with_optional_child((!loading).then_some(text)), > - ) > - } > - > fn create_sdn_panel(&self) -> Panel { > let sdn_zones_status = self.status.as_ref().map(|status| status.sdn_zones.clone()); > > @@ -471,12 +393,6 @@ impl Component for PdmDashboard { > self.show_config_window = false; > true > } > - Msg::Search(search_term) => { > - if let Some(provider) = get_search_provider(ctx) { > - provider.search(search_term.into()); > - } > - false > - } > } > } > > @@ -533,11 +449,11 @@ impl Component for PdmDashboard { > ) > .with_child(RemotePanel::new(self.status.clone())), > ) > - .with_child(self.create_node_panel( > - ctx, > - "building", > - tr!("Virtual Environment Nodes"), > - )) > + .with_child( > + create_node_panel(Some(RemoteType::Pve), self.status.clone()) > + .flex(1.0) > + .width(300), > + ) > .with_child( > create_guest_panel(Some(GuestType::Qemu), self.status.clone()) > .flex(1.0) > diff --git a/ui/src/dashboard/node_panel.rs b/ui/src/dashboard/node_panel.rs > new file mode 100644 > index 00000000..76ff424e > --- /dev/null > +++ b/ui/src/dashboard/node_panel.rs > @@ -0,0 +1,150 @@ > +use std::rc::Rc; > + > +use yew::virtual_dom::{VComp, VNode}; > + > +use proxmox_yew_comp::Status; > +use pwt::widget::{Column, Fa, Panel}; > +use pwt::{css, prelude::*}; > + > +use pdm_api_types::remotes::RemoteType; > +use pdm_api_types::resource::{NodeStatusCount, ResourcesStatus}; > +use pdm_search::{Search, SearchTerm}; > + > +use crate::dashboard::create_title_with_icon; > +use crate::search_provider::get_search_provider; > + > +#[derive(Properties, PartialEq)] > +pub struct NodePanel { > + remote_type: Option, > + status: Option, > +} > + > +impl NodePanel { nit: similar to patch one, would be nice to document that remote_type: None means "all" remotes :) > + pub fn new(remote_type: Option, status: Option) -> Self { > + Self { > + remote_type, > + status, > + } > + } > +} > + > +impl From for VNode { > + fn from(value: NodePanel) -> Self { > + let comp = VComp::new::(Rc::new(value), None); > + VNode::from(comp) > + } > +} > + > +pub struct PdmNodePanel {} > + > +impl Component for PdmNodePanel { > + type Message = Search; > + type Properties = NodePanel; > + > + fn create(_ctx: &Context) -> Self { > + Self {} > + } > + > + fn update(&mut self, ctx: &Context, msg: Self::Message) -> bool { > + if let Some(provider) = get_search_provider(ctx) { > + provider.search(msg); > + } > + false > + } > + > + fn view(&self, ctx: &Context) -> Html { > + let props = ctx.props(); > + let remote_type = &props.remote_type; > + let status = &props.status; > + let mut search_terms = vec![SearchTerm::new("node").category(Some("type"))]; > + let (status_icon, text): (Fa, String) = match &status { > + Some(status) => { > + let count = match remote_type { > + Some(RemoteType::Pve) => status.pve_nodes.clone(), > + Some(RemoteType::Pbs) => status.pbs_nodes.clone(), > + None => NodeStatusCount { > + online: status.pve_nodes.online + status.pbs_nodes.online, > + offline: status.pve_nodes.offline + status.pbs_nodes.offline, > + unknown: status.pve_nodes.unknown + status.pbs_nodes.offline, > + }, > + }; > + match count { > + NodeStatusCount { > + online, > + offline, > + unknown, > + } if offline > 0 => { > + search_terms.push(SearchTerm::new("offline").category(Some("status"))); > + ( > + Status::Error.into(), > + tr!( > + "{0} of {1} nodes are offline", > + offline, > + online + offline + unknown, > + ), > + ) > + } > + NodeStatusCount { unknown, .. } if unknown > 0 => { > + search_terms.push(SearchTerm::new("unknown").category(Some("status"))); > + ( > + Status::Warning.into(), > + tr!("{0} nodes have an unknown status", unknown), > + ) > + } > + // FIXME, get more detailed status about the failed remotes (name, type, error)? > + NodeStatusCount { online, .. } if status.failed_remotes > 0 => ( > + Status::Unknown.into(), > + tr!("{0} of an unknown number of nodes online", online), > + ), > + NodeStatusCount { online, .. } => { > + (Status::Success.into(), tr!("{0} nodes online", online)) > + } > + } > + } > + None => (Status::Unknown.into(), String::new()), > + }; > + > + let loading = status.is_none(); > + let search = Search::with_terms(search_terms); > + Column::new() > + .padding(4) > + .class("pwt-pointer") > + .onclick(ctx.link().callback({ > + let search = search.clone(); > + move |_| search.clone() > + })) > + .onkeydown(ctx.link().batch_callback({ > + let search = search.clone(); > + move |event: KeyboardEvent| match event.key().as_str() { > + "Enter" | " " => Some(search.clone()), > + _ => None, > + } > + })) > + .class(css::FlexFit) > + .class(css::AlignItems::Center) > + .class(css::JustifyContent::Center) > + .gap(2) > + .with_child(if loading { > + html! {} > + } else { > + status_icon.large_4x().into() > + }) > + .with_optional_child((!loading).then_some(text)) > + .into() > + } > +} > + > +pub fn create_node_panel( > + remote_type: Option, > + status: Option, > +) -> Panel { > + let (icon, title) = match remote_type { > + Some(RemoteType::Pve) => ("building", tr!("Virtual Environment Nodes")), > + Some(RemoteType::Pbs) => ("building-o", tr!("Backup Server Nodes")), should `building-o` be `floppy-o` here? we tend to use the floppy icon for pbs a bit more often iirc. > + None => ("building", tr!("Nodes")), > + }; > + Panel::new() > + .title(create_title_with_icon(icon, title)) > + .border(true) > + .with_child(NodePanel::new(remote_type, status)) > +} _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel