all lists on 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal