public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Hannes Laimer <h.laimer@proxmox.com>
To: pdm-devel@lists.proxmox.com
Subject: [pdm-devel] [PATCH proxmox-datacenter-manager 1/2] ui: firewall: use property view for options instead of dialog
Date: Mon,  1 Dec 2025 16:31:08 +0100	[thread overview]
Message-ID: <20251201153109.274759-2-h.laimer@proxmox.com> (raw)
In-Reply-To: <20251201153109.274759-1-h.laimer@proxmox.com>

Replace the edit `EditFirewallOptions` dialog form with inline property
view panels (`FirewallOptions{Cluster,Node,Guest}Panel`). The UI is
restructured to use a `TabPanel` with "Rules" and "Options" tabs, making
firewall options directly accessible alongside the rules list. The
"Options" are read-only. Also removes the cog icon in the tree.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 ui/src/remotes/firewall/columns.rs    |  37 +-----
 ui/src/remotes/firewall/tree.rs       | 162 ++++++++++++++++++--------
 ui/src/remotes/firewall/types.rs      |  64 +++-------
 ui/src/remotes/firewall/ui_helpers.rs |  46 +-------
 4 files changed, 134 insertions(+), 175 deletions(-)

diff --git a/ui/src/remotes/firewall/columns.rs b/ui/src/remotes/firewall/columns.rs
index 454fd81..41cca33 100644
--- a/ui/src/remotes/firewall/columns.rs
+++ b/ui/src/remotes/firewall/columns.rs
@@ -1,4 +1,3 @@
-use proxmox_yew_comp::LoadableComponentContext;
 use pwt::css::AlignItems;
 use pwt::prelude::*;
 use pwt::state::TreeStore;
@@ -8,15 +7,12 @@ use pwt::widget::{Container, Fa, Row};
 use std::rc::Rc;
 use yew::Html;
 
-use super::types::{Scope, TreeEntry, ViewState};
+use super::types::{Scope, TreeEntry};
 use super::ui_helpers::{
     render_firewall_status, render_load_error_message, render_rule_stats, render_warning_icon,
 };
 
-use crate::remotes::firewall::tree::FirewallTreeComponent;
-
 pub fn create_columns(
-    ctx: &LoadableComponentContext<FirewallTreeComponent>,
     store: TreeStore<TreeEntry>,
     loading: bool,
     scope: &Scope,
@@ -27,7 +23,6 @@ pub fn create_columns(
         create_name_column(store, loading, scope.clone()),
         create_enabled_column(scope.clone()),
         create_rules_column(scope),
-        create_actions_column(ctx),
     ])
 }
 
@@ -121,33 +116,3 @@ fn create_rules_column(scope: Rc<Scope>) -> DataTableHeader<TreeEntry> {
         })
         .into()
 }
-
-fn create_actions_column(
-    ctx: &LoadableComponentContext<FirewallTreeComponent>,
-) -> DataTableHeader<TreeEntry> {
-    let link = ctx.link().clone();
-
-    DataTableColumn::new(tr!("Actions"))
-        .width("50px")
-        .justify("right")
-        .render(move |entry: &TreeEntry| {
-            if !entry.is_editable() {
-                return Html::default();
-            }
-
-            let view_state = match ViewState::from_entry(entry) {
-                Some(state) => state,
-                None => return Html::default(),
-            };
-
-            let link_clone = link.clone();
-            pwt::widget::Tooltip::new(pwt::widget::ActionIcon::new("fa fa-fw fa-cog").on_activate(
-                move |_| {
-                    link_clone.change_view(Some(view_state.clone()));
-                },
-            ))
-            .tip(tr!("Edit Options"))
-            .into()
-        })
-        .into()
-}
diff --git a/ui/src/remotes/firewall/tree.rs b/ui/src/remotes/firewall/tree.rs
index 8d0b73d..18510b5 100644
--- a/ui/src/remotes/firewall/tree.rs
+++ b/ui/src/remotes/firewall/tree.rs
@@ -3,7 +3,7 @@ use std::pin::Pin;
 use std::rc::Rc;
 use yew::{ContextHandle, Html};
 
-use proxmox_yew_comp::{EditFirewallOptions, LoadableComponent, LoadableComponentContext};
+use proxmox_yew_comp::{LoadableComponent, LoadableComponentContext};
 use pwt::css;
 use pwt::prelude::*;
 use pwt::props::{FieldBuilder, WidgetBuilder};
@@ -11,13 +11,13 @@ use pwt::state::{Selection, TreeStore};
 use pwt::tr;
 use pwt::widget::data_table::DataTable;
 use pwt::widget::form::{Combobox, Field};
-use pwt::widget::{Button, Container, Panel, Toolbar, Trigger};
+use pwt::widget::{Button, Container, Panel, TabBarItem, TabPanel, Toolbar, Trigger};
 
 use crate::RemoteList;
 
 use super::columns::create_columns;
 use super::types::{
-    FirewallError, GuestEntry, LoadState, NodeEntry, RemoteEntry, Scope, TreeEntry, ViewState,
+    FirewallError, GuestEntry, LoadState, NodeEntry, RemoteEntry, Scope, TreeEntry,
 };
 use super::ui_helpers::PanelConfig;
 
@@ -25,6 +25,11 @@ use pdm_api_types::firewall::{GuestKind, NodeFirewallStatus, RemoteFirewallStatu
 use pdm_api_types::remotes::RemoteType;
 use std::cmp::Ordering;
 
+use proxmox_yew_comp::configuration::pve::{
+    FirewallOptionsClusterPanel, FirewallOptionsGuestPanel, FirewallOptionsNodePanel,
+};
+use proxmox_yew_comp::form::pve::PveGuestType;
+
 fn create_loading_tree() -> pwt::state::SlabTree<TreeEntry> {
     let mut tree = pwt::state::SlabTree::new();
     tree.set_root(TreeEntry::Root);
@@ -206,7 +211,7 @@ impl FirewallTreeComponent {
     }
 
     fn render_tree_panel(&self, ctx: &LoadableComponentContext<Self>) -> Panel {
-        let columns = create_columns(ctx, self.store.clone(), ctx.loading(), &self.scope);
+        let columns = create_columns(self.store.clone(), ctx.loading(), &self.scope);
         let table = DataTable::new(columns, self.store.clone())
             .selection(self.selection.clone())
             .striped(false)
@@ -272,29 +277,121 @@ impl FirewallTreeComponent {
         Panel::new().border(true).with_child(column)
     }
 
-    fn render_rules_panel(&self, ctx: &LoadableComponentContext<Self>) -> Panel {
-        let mut config = match &self.selected_entry {
-            Some(entry) => PanelConfig::from_entry(entry, self.load_state.data_generation),
-            None => PanelConfig::for_no_selection(),
+    fn render_content_panel(&self, ctx: &LoadableComponentContext<Self>) -> Html {
+        let entry = match &self.selected_entry {
+            Some(entry) => entry,
+            None => return PanelConfig::for_no_selection().content.into(),
         };
 
-        if self.tree_collapsed {
+        let config = PanelConfig::from_entry(entry, self.load_state.data_generation);
+
+        let title = if self.tree_collapsed {
             let expand_button: Html = Button::new_icon("fa fa-angle-double-right")
                 .onclick(ctx.link().callback(|_| Msg::ToggleTreePanel))
                 .aria_label(tr!("Show tree panel"))
                 .into();
 
-            config.title_prefix = Some(expand_button);
-        }
+            pwt::widget::Row::new()
+                .gap(2)
+                .class(pwt::css::AlignItems::Baseline)
+                .with_child(expand_button)
+                .with_child(config.title)
+                .into()
+        } else {
+            config.title
+        };
+
+        let mut tab_panel = TabPanel::new()
+            .class(css::FlexFit)
+            .class(css::ColorScheme::Neutral)
+            .title(title)
+            .with_item_builder(
+                TabBarItem::new()
+                    .key("rules")
+                    .label(tr!("Rules"))
+                    .icon_class("fa fa-list"),
+                {
+                    let content = config.content;
+                    let key = format!(
+                        "{}-{}-{}",
+                        entry.type_name(),
+                        entry.name(),
+                        self.load_state.data_generation
+                    );
+                    move |_| {
+                        Container::new()
+                            .key(key.clone())
+                            .class(css::FlexFit)
+                            .with_child(content.clone())
+                            .into()
+                    }
+                },
+            );
+
+        let add_options_tab = |panel: TabPanel, content: Html| {
+            panel.with_item_builder(
+                TabBarItem::new()
+                    .key("options")
+                    .label(tr!("Options"))
+                    .icon_class("fa fa-cog"),
+                move |_| content.clone(),
+            )
+        };
+
+        tab_panel = match entry {
+            TreeEntry::Remote(remote) => {
+                let remote_name = remote.name.clone();
+                add_options_tab(
+                    tab_panel,
+                    FirewallOptionsClusterPanel::new()
+                        .remote(remote_name.clone())
+                        .readonly(true)
+                        .into(),
+                )
+            }
+            TreeEntry::Node(node) => {
+                let remote_name = node.remote.clone();
+                let node_name = node.name.clone();
+                add_options_tab(
+                    tab_panel,
+                    FirewallOptionsNodePanel::new(yew::AttrValue::from(node_name.clone()))
+                        .remote(remote_name.clone())
+                        .readonly(true)
+                        .into(),
+                )
+            }
+            TreeEntry::Guest(guest, kind) => {
+                let remote_name = guest.remote.clone();
+                let node_name = guest.node.clone();
+                let vmid = guest.guest.vmid;
+                let guest_type = match kind {
+                    GuestKind::Lxc => PveGuestType::Lxc,
+                    GuestKind::Qemu => PveGuestType::Qemu,
+                };
+
+                add_options_tab(
+                    tab_panel,
+                    FirewallOptionsGuestPanel::new(
+                        guest_type,
+                        yew::AttrValue::from(node_name.clone()),
+                        vmid as u32,
+                    )
+                    .remote(remote_name.clone())
+                    .readonly(true)
+                    .into(),
+                )
+            }
+            _ => tab_panel,
+        };
 
-        config.build()
+        tab_panel.into()
     }
 }
 
 impl LoadableComponent for FirewallTreeComponent {
     type Properties = super::FirewallTree;
     type Message = Msg;
-    type ViewState = ViewState;
+    type ViewState = ();
 
     fn create(ctx: &LoadableComponentContext<Self>) -> Self {
         let tree = create_loading_tree();
@@ -391,7 +488,7 @@ impl LoadableComponent for FirewallTreeComponent {
                 if self.filter_text.is_empty() {
                     self.store.set_filter(None);
                 } else {
-                    let filter_text = Rc::new(self.filter_text.clone());
+                    let filter_text = Rc::new(self.filter_text.to_lowercase());
                     self.store
                         .set_filter(move |entry: &TreeEntry| entry.matches_filter(&filter_text));
                 }
@@ -460,42 +557,7 @@ impl LoadableComponent for FirewallTreeComponent {
             container = container.with_child(self.render_tree_panel(ctx));
         }
 
-        container.with_child(self.render_rules_panel(ctx)).into()
-    }
-
-    fn dialog_view(
-        &self,
-        ctx: &LoadableComponentContext<Self>,
-        view_state: &Self::ViewState,
-    ) -> Option<Html> {
-        let dialog = match view_state {
-            ViewState::EditRemote { remote } => EditFirewallOptions::cluster(remote.to_string())
-                .on_close(ctx.link().change_view_callback(|_| None))
-                .into(),
-            ViewState::EditNode { remote, node } => {
-                EditFirewallOptions::node(remote.to_string(), node.to_string())
-                    .on_close(ctx.link().change_view_callback(|_| None))
-                    .into()
-            }
-            ViewState::EditGuest {
-                remote,
-                node,
-                vmid,
-                ty,
-            } => {
-                let vmtype: &str = ty.into();
-                EditFirewallOptions::guest(
-                    remote.to_string(),
-                    node.to_string(),
-                    *vmid as u64,
-                    vmtype,
-                )
-                .on_close(ctx.link().change_view_callback(|_| None))
-                .into()
-            }
-        };
-
-        Some(dialog)
+        container.with_child(self.render_content_panel(ctx)).into()
     }
 }
 
diff --git a/ui/src/remotes/firewall/types.rs b/ui/src/remotes/firewall/types.rs
index 84aa657..aac9587 100644
--- a/ui/src/remotes/firewall/types.rs
+++ b/ui/src/remotes/firewall/types.rs
@@ -148,17 +148,15 @@ impl TreeEntry {
     }
 
     pub fn matches_filter(&self, filter_text: &str) -> bool {
-        let text = filter_text.to_lowercase();
-
         match self {
             Self::Root | Self::Remote(..) | Self::Node(..) => true,
             Self::Guest(guest, kind) => {
                 let type_name = kind.as_str();
-                guest.guest.name.to_lowercase().contains(&text)
-                    || guest.guest.vmid.to_string().contains(&text)
-                    || type_name.contains(&text)
-                    || guest.node.to_lowercase().contains(&text)
-                    || guest.remote.to_lowercase().contains(&text)
+                guest.guest.name.to_lowercase().contains(filter_text)
+                    || guest.guest.vmid.to_string().contains(filter_text)
+                    || type_name.contains(filter_text)
+                    || guest.node.to_lowercase().contains(filter_text)
+                    || guest.remote.to_lowercase().contains(filter_text)
             }
         }
     }
@@ -173,10 +171,6 @@ impl TreeEntry {
         }
     }
 
-    pub fn is_editable(&self) -> bool {
-        !matches!(self, Self::Root)
-    }
-
     pub fn firewall_status(&self) -> Option<(&FirewallStatus, bool)> {
         match self {
             Self::Remote(entry) => entry.status.as_ref().map(|s| (s, false)),
@@ -199,6 +193,16 @@ impl TreeEntry {
             Self::Guest(_, GuestKind::Qemu) => 4,
         }
     }
+
+    pub fn type_name(&self) -> &'static str {
+        match self {
+            Self::Root => "root",
+            Self::Remote(..) => "remote",
+            Self::Node(..) => "node",
+            Self::Guest(_, GuestKind::Lxc) => "lxc",
+            Self::Guest(_, GuestKind::Qemu) => "qemu",
+        }
+    }
 }
 
 impl ExtractPrimaryKey for TreeEntry {
@@ -216,44 +220,6 @@ impl ExtractPrimaryKey for TreeEntry {
     }
 }
 
-#[derive(PartialEq, Debug, Clone)]
-pub enum ViewState {
-    EditRemote {
-        remote: String,
-    },
-    EditNode {
-        remote: String,
-        node: String,
-    },
-    EditGuest {
-        remote: String,
-        node: String,
-        vmid: u32,
-        ty: GuestKind,
-    },
-}
-
-impl ViewState {
-    pub fn from_entry(entry: &TreeEntry) -> Option<Self> {
-        match entry {
-            TreeEntry::Remote(e) => Some(Self::EditRemote {
-                remote: e.name.clone(),
-            }),
-            TreeEntry::Node(e) => Some(Self::EditNode {
-                remote: e.remote.clone(),
-                node: e.name.clone(),
-            }),
-            TreeEntry::Guest(guest, _) => Some(Self::EditGuest {
-                remote: guest.remote.clone(),
-                node: guest.node.clone(),
-                vmid: guest.guest.vmid,
-                ty: guest.guest.kind,
-            }),
-            TreeEntry::Root => None,
-        }
-    }
-}
-
 #[derive(Debug, Clone)]
 pub enum FirewallError {
     RemoteListLoadFailed(String),
diff --git a/ui/src/remotes/firewall/ui_helpers.rs b/ui/src/remotes/firewall/ui_helpers.rs
index 741064d..8d2202b 100644
--- a/ui/src/remotes/firewall/ui_helpers.rs
+++ b/ui/src/remotes/firewall/ui_helpers.rs
@@ -2,7 +2,7 @@ use pdm_api_types::firewall::{FirewallStatus, GuestKind, RuleStat};
 use pwt::css::{AlignItems, FontColor};
 use pwt::prelude::*;
 use pwt::tr;
-use pwt::widget::{Container, Fa, Panel, Row};
+use pwt::widget::{Container, Fa, Row};
 use yew::{html, Html};
 
 use super::types::TreeEntry;
@@ -56,21 +56,9 @@ pub fn create_panel_title(icon_name: &str, title_text: String) -> Html {
         .into()
 }
 
-pub fn create_rules_panel(title: Html, key: String, content: Html) -> Panel {
-    Panel::new()
-        .class(pwt::css::FlexFit)
-        .title(title)
-        .border(true)
-        .min_width(500)
-        .with_child(Container::new().key(key).with_child(content))
-        .style("flex", "1 1 0")
-}
-
 pub struct PanelConfig {
     pub title: Html,
-    pub key: String,
     pub content: Html,
-    pub title_prefix: Option<Html>,
 }
 
 impl PanelConfig {
@@ -78,10 +66,8 @@ impl PanelConfig {
         let mut rules = proxmox_yew_comp::FirewallRules::cluster(remote.to_string());
         rules.reload_token = reload_token;
         Self {
-            title: create_panel_title("list", tr!("Cluster Firewall Rules - {}", remote)),
-            key: format!("cluster-{}", remote),
+            title: create_panel_title("server", tr!("Cluster Firewall - {}", remote)),
             content: rules.into(),
-            title_prefix: None,
         }
     }
 
@@ -89,10 +75,8 @@ impl PanelConfig {
         let mut rules = proxmox_yew_comp::FirewallRules::node(remote.to_string(), node.to_string());
         rules.reload_token = reload_token;
         Self {
-            title: create_panel_title("list", tr!("Node Firewall Rules - {}/{}", remote, node)),
-            key: format!("node-{}-{}", remote, node),
+            title: create_panel_title("building", tr!("Node Firewall - {}/{}", remote, node)),
             content: rules.into(),
-            title_prefix: None,
         }
     }
 
@@ -113,18 +97,16 @@ impl PanelConfig {
         rules.reload_token = reload_token;
         Self {
             title: create_panel_title(
-                "list",
+                if vmtype == "lxc" { "cube" } else { "desktop" },
                 tr!(
-                    "Guest Firewall Rules - {}/{}/{} {}",
+                    "Guest Firewall - {}/{}/{} {}",
                     remote,
                     node,
                     vmtype.to_uppercase(),
                     vmid
                 ),
             ),
-            key: format!("guest-{}-{}-{}-{}", remote, node, vmid, vmtype),
             content: rules.into(),
-            title_prefix: None,
         }
     }
 
@@ -142,10 +124,8 @@ impl PanelConfig {
             .into();
 
         Self {
-            title: create_panel_title("list", tr!("Firewall Rules")),
-            key: String::new(),
+            title: create_panel_title("shield", tr!("Firewall")),
             content,
-            title_prefix: None,
         }
     }
 
@@ -165,18 +145,4 @@ impl PanelConfig {
             TreeEntry::Root => Self::for_no_selection(),
         }
     }
-
-    pub fn build(self) -> Panel {
-        let title = if let Some(prefix) = self.title_prefix {
-            Row::new()
-                .gap(2)
-                .class(AlignItems::Baseline)
-                .with_child(prefix)
-                .with_child(self.title)
-                .into()
-        } else {
-            self.title
-        };
-        create_rules_panel(title, self.key, self.content)
-    }
 }
-- 
2.47.3



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


  reply	other threads:[~2025-12-01 15:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-01 15:31 [pdm-devel] [PATCH proxmox-datacenter-manager 0/2] change to tabbed panel when selecting an item in the firewall tree Hannes Laimer
2025-12-01 15:31 ` Hannes Laimer [this message]
2025-12-01 15:31 ` [pdm-devel] [PATCH proxmox-datacenter-manager 2/2] ui: firewall: add link to pve to rules/options panel Hannes Laimer
2025-12-01 23:40 ` [pdm-devel] applied: [PATCH proxmox-datacenter-manager 0/2] change to tabbed panel when selecting an item in the firewall tree Thomas Lamprecht

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=20251201153109.274759-2-h.laimer@proxmox.com \
    --to=h.laimer@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