public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Shannon Sterz" <s.sterz@proxmox.com>
To: "Dominik Csapak" <d.csapak@proxmox.com>
Cc: Proxmox Datacenter Manager development discussion
	<pdm-devel@lists.proxmox.com>
Subject: Re: [pdm-devel] [PATCH datacenter-manager v2 02/16] ui: dashboard: refactor creating the node panel into its own module
Date: Thu, 23 Oct 2025 13:19:06 +0200	[thread overview]
Message-ID: <DDPNSQJVP78P.3IZPYQOOPC9KL@proxmox.com> (raw)
In-Reply-To: <20251023083253.1038119-3-d.csapak@proxmox.com>

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 <d.csapak@proxmox.com>
> ---
>  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<Self>, 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! {<i class={"pwt-loading-icon"} />}
> -                    } 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<RemoteType>,
> +    status: Option<ResourcesStatus>,
> +}
> +
> +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<RemoteType>, status: Option<ResourcesStatus>) -> Self {
> +        Self {
> +            remote_type,
> +            status,
> +        }
> +    }
> +}
> +
> +impl From<NodePanel> for VNode {
> +    fn from(value: NodePanel) -> Self {
> +        let comp = VComp::new::<PdmNodePanel>(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 {
> +        Self {}
> +    }
> +
> +    fn update(&mut self, ctx: &Context<Self>, msg: Self::Message) -> bool {
> +        if let Some(provider) = get_search_provider(ctx) {
> +            provider.search(msg);
> +        }
> +        false
> +    }
> +
> +    fn view(&self, ctx: &Context<Self>) -> 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! {<i class={"pwt-loading-icon"} />}
> +            } else {
> +                status_icon.large_4x().into()
> +            })
> +            .with_optional_child((!loading).then_some(text))
> +            .into()
> +    }
> +}
> +
> +pub fn create_node_panel(
> +    remote_type: Option<RemoteType>,
> +    status: Option<ResourcesStatus>,
> +) -> 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


  reply	other threads:[~2025-10-23 11:18 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-23  8:28 [pdm-devel] [PATCH datacenter-manager v2 00/16] prepare ui for customizable views Dominik Csapak
2025-10-23  8:28 ` [pdm-devel] [PATCH datacenter-manager v2 01/16] ui: dashboard: refactor guest panel creation to its own module Dominik Csapak
2025-10-23 11:19   ` Shannon Sterz
2025-10-23  8:28 ` [pdm-devel] [PATCH datacenter-manager v2 02/16] ui: dashboard: refactor creating the node panel into " Dominik Csapak
2025-10-23 11:19   ` Shannon Sterz [this message]
2025-10-23  8:28 ` [pdm-devel] [PATCH datacenter-manager v2 03/16] ui: dashboard: refactor remote panel creation " Dominik Csapak
2025-10-23  8:28 ` [pdm-devel] [PATCH datacenter-manager v2 04/16] ui: dashboard: remote panel: make wizard menu optional Dominik Csapak
2025-10-23  8:28 ` [pdm-devel] [PATCH datacenter-manager v2 05/16] ui: dashboard: refactor sdn panel creation into its own module Dominik Csapak
2025-10-23  8:28 ` [pdm-devel] [PATCH datacenter-manager v2 06/16] ui: dashboard: refactor task summary panel creation to " Dominik Csapak
2025-10-23 11:19   ` Shannon Sterz
2025-10-23  8:28 ` [pdm-devel] [PATCH datacenter-manager v2 07/16] ui: dashboard: task summary: disable virtual scrolling Dominik Csapak
2025-10-23  8:28 ` [pdm-devel] [PATCH datacenter-manager v2 08/16] ui: dashboard: refactor subscription panel creation to its own module Dominik Csapak
2025-10-23  8:28 ` [pdm-devel] [PATCH datacenter-manager v2 09/16] ui: dashboard: refactor top entities " Dominik Csapak
2025-10-23  8:28 ` [pdm-devel] [PATCH datacenter-manager v2 10/16] ui: dashboard: refactor DashboardConfig editing/constants to their module Dominik Csapak
2025-10-23  8:28 ` [pdm-devel] [PATCH datacenter-manager v2 11/16] ui: dashboard: factor out task parameter calculation Dominik Csapak
2025-10-23  8:28 ` [pdm-devel] [PATCH datacenter-manager v2 12/16] ui: dashboard: remove unused remote list Dominik Csapak
2025-10-23  8:28 ` [pdm-devel] [PATCH datacenter-manager v2 13/16] ui: dashboard: status row: make loading less jarring Dominik Csapak
2025-10-23 11:19   ` Shannon Sterz
2025-10-23  8:28 ` [pdm-devel] [RFC PATCH datacenter-manager v2 14/16] ui: introduce `LoadResult` helper type Dominik Csapak
2025-10-23 11:19   ` Shannon Sterz
2025-10-23  8:28 ` [pdm-devel] [RFC PATCH datacenter-manager v2 15/16] ui: dashboard: implement 'View' Dominik Csapak
2025-10-23 11:19   ` Shannon Sterz
2025-10-23 11:44     ` Dominik Csapak
2025-10-23 11:48       ` Dominik Csapak
2025-10-24 10:17         ` Shannon Sterz
2025-10-23  8:28 ` [pdm-devel] [RFC PATCH datacenter-manager v2 16/16] ui: dashboard: use 'View' instead of the Dashboard Dominik Csapak
2025-10-23 11:19   ` Shannon Sterz
2025-10-23 11:20 ` [pdm-devel] [PATCH datacenter-manager v2 00/16] prepare ui for customizable views Shannon Sterz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DDPNSQJVP78P.3IZPYQOOPC9KL@proxmox.com \
    --to=s.sterz@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal