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 29F841FF178 for ; Mon, 1 Dec 2025 16:31:23 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2C53520FBF; Mon, 1 Dec 2025 16:31:47 +0100 (CET) From: Hannes Laimer To: pdm-devel@lists.proxmox.com Date: Mon, 1 Dec 2025 16:31:08 +0100 Message-ID: <20251201153109.274759-2-h.laimer@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20251201153109.274759-1-h.laimer@proxmox.com> References: <20251201153109.274759-1-h.laimer@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1764603029552 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.051 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: [pdm-devel] [PATCH proxmox-datacenter-manager 1/2] ui: firewall: use property view for options instead of dialog 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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" 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 --- 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, store: TreeStore, 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) -> DataTableHeader { }) .into() } - -fn create_actions_column( - ctx: &LoadableComponentContext, -) -> DataTableHeader { - 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 { 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) -> 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) -> 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) -> 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 { 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, - view_state: &Self::ViewState, - ) -> Option { - 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 { - 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, } 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