public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Stefan Hanreich <s.hanreich@proxmox.com>
To: Dominik Csapak <d.csapak@proxmox.com>,
	Proxmox Datacenter Manager development discussion
	<pdm-devel@lists.proxmox.com>
Subject: Re: [pdm-devel] [PATCH proxmox-datacenter-manager 5/5] ui: sdn: add zone tree
Date: Tue, 9 Sep 2025 15:57:23 +0200	[thread overview]
Message-ID: <e0a14d0c-0a75-42ac-9ff9-86dea9b127b9@proxmox.com> (raw)
In-Reply-To: <78de94a6-ee2b-41e9-b53f-d2019f9a98db@proxmox.com>

On 9/9/25 3:41 PM, Dominik Csapak wrote:
> looks mostly fine to me (see some comments inline)
> 
> but the toolbar looks weird if it's empty this way
> 
> i know we always have the refresh button on the right, but
> maybe putting it on the left for this single panel could make
> sense, just so the toolbar isn't empty on the left hand side?> alternatively we could put a short title there?

I thought about it too, but generally we talked once that we usually
don't use titles and it seems weird to break that here - as does
breaking the position of the refresh button imo. But I agree that it
does look barren atm.

> On 9/9/25 12:08 PM, Stefan Hanreich wrote:
>> This shows an overview of the state of all zones across all remotes,
>> similar to the current overview in Proxmox VE, in the SDN tab. Add it
>> as a top-level container and move the EVPN section below that, to
>> mimic the menu structure from Proxmox VE.
>>
>> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
>> ---
>>   ui/src/main_menu.rs     |  15 +-
>>   ui/src/sdn/mod.rs       |   3 +
>>   ui/src/sdn/zone_tree.rs | 299 ++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 316 insertions(+), 1 deletion(-)
>>   create mode 100644 ui/src/sdn/zone_tree.rs
>>
>> diff --git a/ui/src/main_menu.rs b/ui/src/main_menu.rs
>> index 7eac775..f440be8 100644
>> --- a/ui/src/main_menu.rs
>> +++ b/ui/src/main_menu.rs
>> @@ -18,6 +18,7 @@ use pdm_api_types::remotes::RemoteType;
>>     use crate::remotes::RemotesPanel;
>>   use crate::sdn::evpn::EvpnPanel;
>> +use crate::sdn::ZoneTree;
>>   use crate::{
>>       AccessControl, CertificatesPanel, Dashboard, RemoteList,
>> ServerAdministration,
>>       SystemConfiguration,
>> @@ -248,8 +249,10 @@ impl Component for PdmMainMenu {
>>               admin_submenu,
>>           );
>>   +        let mut sdn_submenu = Menu::new();
>> +
>>           register_view(
>> -            &mut menu,
>> +            &mut sdn_submenu,
>>               &mut content,
>>               tr!("EVPN"),
>>               "evpn",
>> @@ -257,6 +260,16 @@ impl Component for PdmMainMenu {
>>               |_| EvpnPanel::new().into(),
>>           );
>>   +        register_submenu(
>> +            &mut menu,
>> +            &mut content,
>> +            tr!("SDN"),
>> +            "sdn",
>> +            Some("fa fa-sdn"),
>> +            |_| ZoneTree::new().into(),
>> +            sdn_submenu,
>> +        );
>> +
>>           let mut remote_submenu = Menu::new();
>>             for remote in self.remote_list_cache.iter() {
>> diff --git a/ui/src/sdn/mod.rs b/ui/src/sdn/mod.rs
>> index ef2eab9..b6ab8ad 100644
>> --- a/ui/src/sdn/mod.rs
>> +++ b/ui/src/sdn/mod.rs
>> @@ -1 +1,4 @@
>>   pub mod evpn;
>> +
>> +mod zone_tree;
>> +pub use zone_tree::ZoneTree;
>> diff --git a/ui/src/sdn/zone_tree.rs b/ui/src/sdn/zone_tree.rs
>> new file mode 100644
>> index 0000000..55a5889
>> --- /dev/null
>> +++ b/ui/src/sdn/zone_tree.rs
>> @@ -0,0 +1,299 @@
>> +use futures::Future;
>> +use std::cmp::Ordering;
>> +use std::pin::Pin;
>> +use std::rc::Rc;
>> +
>> +use yew::virtual_dom::{Key, VComp, VNode};
>> +use yew::{Html, Properties};
>> +
>> +use pdm_api_types::resource::{
>> +    PveSdnResource, RemoteResources, ResourceType, SdnStatus,
>> SdnZoneResource,
>> +};
>> +use pdm_client::types::Resource;
>> +use proxmox_yew_comp::{LoadableComponent, LoadableComponentContext,
>> LoadableComponentMaster};
>> +use pwt::props::EventSubscriber;
>> +use pwt::widget::{Button, Toolbar};
>> +use pwt::{
>> +    css,
>> +    css::FontColor,
>> +    props::{ContainerBuilder, ExtractPrimaryKey, WidgetBuilder},
>> +    state::{Selection, SlabTree, TreeStore},
>> +    tr,
>> +    widget::{
>> +        data_table::{DataTable, DataTableColumn, DataTableHeader},
>> +        error_message, Column, Fa, Row,
>> +    },
>> +};
>> +
>> +use crate::pdm_client;
>> +
>> +#[derive(PartialEq, Properties)]
>> +pub struct ZoneTree {}
>> +
>> +impl ZoneTree {
>> +    pub fn new() -> Self {
>> +        yew::props!(Self {})
>> +    }
>> +}
>> +
>> +impl From<ZoneTree> for VNode {
>> +    fn from(value: ZoneTree) -> Self {
>> +        let comp =
>> VComp::new::<LoadableComponentMaster<ZoneTreeComponent>>(Rc::new(value), None);
>> +        VNode::from(comp)
>> +    }
>> +}
>> +
>> +#[derive(Clone, PartialEq, Debug)]
>> +struct ZoneData {
>> +    remote: String,
>> +    node: String,
>> +    name: String,
>> +    status: SdnStatus,
>> +}
>> +
>> +#[derive(Clone, PartialEq, Debug)]
>> +enum ZoneTreeEntry {
>> +    Root,
>> +    Remote(String),
>> +    Node(String, String),
>> +    Zone(ZoneData),
>> +}
>> +
>> +impl ZoneTreeEntry {
>> +    fn from_zone_resource(remote: String, value: SdnZoneResource) ->
>> Self {
>> +        Self::Zone(ZoneData {
>> +            remote,
>> +            node: value.node.clone(),
>> +            name: value.name.clone(),
>> +            status: value.status,
>> +        })
>> +    }
>> +
>> +    fn name(&self) -> &str {
>> +        match &self {
>> +            Self::Root => "",
>> +            Self::Remote(name) => name,
>> +            Self::Node(_, name) => name,
>> +            Self::Zone(zone) => &zone.name,
>> +        }
>> +    }
>> +}
>> +
>> +impl ExtractPrimaryKey for ZoneTreeEntry {
>> +    fn extract_key(&self) -> yew::virtual_dom::Key {
>> +        Key::from(match self {
>> +            ZoneTreeEntry::Root => "/".to_string(),
>> +            ZoneTreeEntry::Remote(name) => format!("/{name}"),
>> +            ZoneTreeEntry::Node(remote_name, name) => format!("/
>> {remote_name}/{name}"),
>> +            ZoneTreeEntry::Zone(zone) => format!("/{}/{}/{}",
>> zone.remote, zone.node, zone.name),
>> +        })
>> +    }
>> +}
>> +
>> +pub enum ZoneTreeMsg {
>> +    LoadFinished(Vec<RemoteResources>),
>> +    Reload,
>> +}
>> +
>> +pub struct ZoneTreeComponent {
>> +    store: TreeStore<ZoneTreeEntry>,
>> +    selection: Selection,
>> +    remote_errors: Vec<String>,
>> +    columns: Rc<Vec<DataTableHeader<ZoneTreeEntry>>>,
>> +}
>> +
>> +fn default_sorter(a: &ZoneTreeEntry, b: &ZoneTreeEntry) -> Ordering {
>> +    a.name().cmp(b.name())
>> +}
>> +
>> +impl ZoneTreeComponent {
>> +    fn columns(store: TreeStore<ZoneTreeEntry>) ->
>> Rc<Vec<DataTableHeader<ZoneTreeEntry>>> {
>> +        Rc::new(vec![
>> +            DataTableColumn::new(tr!("Name"))
>> +                .tree_column(store)
>> +                .render(|entry: &ZoneTreeEntry| {
>> +                    let mut row =
>> Row::new().class(css::AlignItems::Baseline).gap(2);
>> +                    let name = entry.name();
>> +
>> +                    row = match entry {
>> +                        ZoneTreeEntry::Remote(_) =>
>> row.with_child(Fa::new("server")),
>> +                        ZoneTreeEntry::Node(_, _) =>
>> row.with_child(Fa::new("building")),
>> +                        ZoneTreeEntry::Zone(_) =>
>> row.with_child(Fa::new("th")),
>> +                        _ => row,
>> +                    };
>> +
>> +                    row.with_child(name).into()
> 
> i mean it works, but i'd probably write this part a bit differently:
> 
> let icon = match entry {
>     ... => Some("server"),
>     ... => Some("building"),
>     ... => Some("th"),
>      _ => None,
> };
> 
> Row::new()
>     .class(...)
>     .gap(...)
>     .with_optional_child(icon.map(|icon|Fa::new(icon))
>     .with_child(name).into()
> 
> 
> would that too work?
> 

yeah, that seems a lot better.

>> +                })
>> +                .sorter(default_sorter)
>> +                .into(),
>> +            DataTableColumn::new(tr!("Status"))
>> +                .render(|entry: &ZoneTreeEntry| {
>> +                    let mut row =
>> Row::new().class(css::AlignItems::Baseline).gap(2);
>> +
>> +                    if let ZoneTreeEntry::Zone(zone) = entry {
>> +                        row = match zone.status {
>> +                            SdnStatus::Available => {
>> +                               
>> row.with_child(Fa::new("check").class(FontColor::Success))
>> +                            }
>> +                            SdnStatus::Error => {
>> +                                row.with_child(Fa::new("times-
>> circle").class(FontColor::Error))
>> +                            }
>> +                            _ => row,
>> +                        };
>> +
>> +                        row = row.with_child(zone.status);
>> +                    } else {
>> +                        row = row.with_child("");
>> +                    }
>> +
>> +                    row.into()
>> +                })
>> +                .into(),
>> +        ])
>> +    }
>> +}
>> +
>> +fn build_store_from_response(
>> +    remote_resources: Vec<RemoteResources>,
>> +) -> (SlabTree<ZoneTreeEntry>, Vec<String>) {
>> +    let mut tree = SlabTree::new();
>> +
>> +    let mut root = tree.set_root(ZoneTreeEntry::Root);
>> +    root.set_expanded(true);
>> +
>> +    let mut remote_errors = Vec::new();
>> +
>> +    for resources in remote_resources {
>> +        if let Some(error) = resources.error {
>> +            remote_errors.push(format!(
>> +                "could not fetch resources from remote {}: {error}",
>> +                resources.remote,
>> +            ));
>> +            continue;
>> +        }
>> +
>> +        let mut remote =
>> root.append(ZoneTreeEntry::Remote(resources.remote.clone()));
>> +        remote.set_expanded(true);
>> +
>> +        for resource in resources.resources {
>> +            if let
>> Resource::PveSdn(PveSdnResource::Zone(zone_resource)) = resource {
>> +                let node_entry = remote.children_mut().find(|entry| {
>> +                    if let ZoneTreeEntry::Node(_, name) =
>> entry.record() {
>> +                        if name == &zone_resource.node {
>> +                            return true;
>> +                        }
>> +                    }
>> +
>> +                    false
>> +                });
>> +
>> +                let node_name = zone_resource.node.clone();
>> +
>> +                let entry =
>> +                   
>> ZoneTreeEntry::from_zone_resource(resources.remote.clone(),
>> zone_resource);
>> +
>> +                match node_entry {
>> +                    Some(mut node_entry) => {
>> +                        node_entry.append(entry);
>> +                    }
>> +                    None => {
>> +                        let mut node_entry =
>> +                           
>> remote.append(ZoneTreeEntry::Node(resources.remote.clone(), node_name));
>> +
>> +                        node_entry.set_expanded(true);
>> +
>> +                        node_entry.append(entry);
>> +                    }
>> +                };
>> +            }
>> +        }
>> +    }
>> +
>> +    (tree, remote_errors)
>> +}
>> +
>> +impl LoadableComponent for ZoneTreeComponent {
>> +    type Properties = ZoneTree;
>> +    type Message = ZoneTreeMsg;
>> +    type ViewState = ();
>> +
>> +    fn create(_ctx: &LoadableComponentContext<Self>) -> Self {
>> +        let store = TreeStore::new().view_root(false);
>> +        store.set_sorter(default_sorter);
>> +
>> +        let selection = Selection::new();
>> +
>> +        Self {
>> +            store: store.clone(),
>> +            selection,
>> +            remote_errors: Vec::new(),
>> +            columns: Self::columns(store),
>> +        }
>> +    }
>> +
>> +    fn load(
>> +        &self,
>> +        ctx: &LoadableComponentContext<Self>,
>> +    ) -> Pin<Box<dyn Future<Output = Result<(), anyhow::Error>>>> {
>> +        let link = ctx.link().clone();
>> +
>> +        Box::pin(async move {
>> +            let client = pdm_client();
>> +            let remote_resources = client
>> +                .resources_by_type(None, ResourceType::PveSdnZone)
>> +                .await?;
>> +           
>> link.send_message(Self::Message::LoadFinished(remote_resources));
>> +
>> +            Ok(())
>> +        })
>> +    }
>> +
>> +    fn update(&mut self, ctx: &LoadableComponentContext<Self>, msg:
>> Self::Message) -> bool {
>> +        match msg {
>> +            Self::Message::LoadFinished(remote_resources) => {
>> +                let (data, remote_errors) =
>> build_store_from_response(remote_resources);
>> +                self.store.write().update_root_tree(data);
>> +                self.store.set_sorter(default_sorter);
>> +
>> +                self.remote_errors = remote_errors;
>> +
>> +                return true;
>> +            }
>> +            Self::Message::Reload => {
>> +                ctx.link().send_reload();
>> +            }
>> +        }
>> +
>> +        false
>> +    }
>> +
>> +    #[allow(unused_variables)]
> 
> is this necessary? or is there some clippy weirdness going on?

will double-check



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

  reply	other threads:[~2025-09-09 13:57 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-09 10:08 [pdm-devel] [PATCH manager/proxmox-datacenter-manager 0/6] Add SDN resources to dashboard + SDN zone overview tree Stefan Hanreich
2025-09-09 10:08 ` [pdm-devel] [PATCH pve-manager 1/1] cluster: resources: add sdn property to cluster resources schema Stefan Hanreich
2025-09-09 10:08 ` [pdm-devel] [PATCH proxmox-datacenter-manager 1/5] pdm-api-types: add sdn cluster resource Stefan Hanreich
2025-09-09 11:13   ` Stefan Hanreich
2025-09-09 11:24     ` Thomas Lamprecht
2025-09-09 11:26       ` Stefan Hanreich
2025-09-09 10:08 ` [pdm-devel] [PATCH proxmox-datacenter-manager 2/5] server: api: add resources_by_type api call Stefan Hanreich
2025-09-09 10:08 ` [pdm-devel] [PATCH proxmox-datacenter-manager 3/5] ui: add sdn status report to dashboard Stefan Hanreich
2025-09-09 13:10   ` Dominik Csapak
2025-09-09 13:22     ` Stefan Hanreich
2025-09-09 10:08 ` [pdm-devel] [PATCH proxmox-datacenter-manager 4/5] ui: images: add sdn icon Stefan Hanreich
2025-09-09 13:16   ` Dominik Csapak
2025-09-09 13:21     ` Stefan Hanreich
2025-09-09 10:08 ` [pdm-devel] [PATCH proxmox-datacenter-manager 5/5] ui: sdn: add zone tree Stefan Hanreich
2025-09-09 13:41   ` Dominik Csapak
2025-09-09 13:57     ` Stefan Hanreich [this message]
2025-09-09 13:43 ` [pdm-devel] [PATCH manager/proxmox-datacenter-manager 0/6] Add SDN resources to dashboard + SDN zone overview tree Dominik Csapak
2025-09-09 14:06   ` Stefan Hanreich

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=e0a14d0c-0a75-42ac-9ff9-86dea9b127b9@proxmox.com \
    --to=s.hanreich@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