From: Hannes Laimer <h.laimer@proxmox.com>
To: Lukas Wagner <l.wagner@proxmox.com>,
Proxmox Datacenter Manager development discussion
<pdm-devel@lists.proxmox.com>
Subject: Re: [pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list
Date: Tue, 9 Dec 2025 17:45:28 +0100 [thread overview]
Message-ID: <b85c23f9-38e3-4b11-ab20-2b03926e259c@proxmox.com> (raw)
In-Reply-To: <DETSFU51BOKO.2JJS3I3SNMNED@proxmox.com>
Thanks for the feedback! some comments inline
On 12/9/25 16:23, Lukas Wagner wrote:
> On Fri Dec 5, 2025 at 4:25 PM CET, Hannes Laimer wrote:
>> This contains some rough edges, mostly UI wise, but I'd like to get some
>> feedback on if we like this approach. Currently we don't really
>> know what a security group actually contains, in the list currently it's
>> a bit of a black box what a group actually does. Finding out what rules it
>> contains is a little cumbersome. This should make that easier. It seemed
>> like a good place too, I considerd an extra tab maybe. But especially
>> for read-only I think this is better.
>
> Yeah, I think approach could work. But I'd also be interested what other
> think.
>
> If we ever change the view so that rules are editable, I guess we could
> keep it as is, but should probably keep the rules from the group
> read-only in their expanded form. To edit the actual rules of the group,
> some other form of UI would be good, maybe like you suggested, in a
> separate tab on the 'Datacenter'/'Remote' level.
Since rules in groups are just rules so technically editing isn't a
problem, but yes, might be a little confusing cause it would edit the
rule for all instances where the group is used...
maybe some "reduced editing" could also make sense, maybe just order and
on/off, but that's a not relevant for this
> If we go this route, the 'expanded' rules could maybe use some form of
> visual distinction from the rest, to avoid confusion about their
> read-only status.
>
yes, will add that for v2.
> Regarding the UI, I'd maybe put the caret into a separate column; the
> discontinued numbering seems a bit odd to me. Alternatively, the caret
> could stay in the same column, but the group keeps its numbering, e.g.
> like this
>
> 1 .....
> 2 .....
>> 3 .....
> 4 .....
>
> expanded it could maybe look like
>
>
> 1 .....
> 2 .....
> v 3 .....
> 3.1 .....
> 3.2 .....
> 4 .....
>
I like this!
> But that's just some idea; maybe somebody has some other input here.
>
>>
>> This also contains a renaming, mostly cause I had it in the same repo
>> already. If wanted, I can split that and send it separately. The
>> pve-api.json patch contains changes from [1].
>>
>> [1] https://lore.proxmox.com/pve-devel/20251128145846.328173-1-h.laimer@proxmox.com/T/#u
>>
>
>
> Tested-by: Lukas Wagner <l.wagner@proxmox.com>
>
> UI code was only skimmed, but I couldn't really find anything to
> complain about:
>
> Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
next prev parent reply other threads:[~2025-12-09 16:45 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-05 15:25 Hannes Laimer
2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 1/4] pve-api-types: rename ListFirewallRules to FirewallRule Hannes Laimer
2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 2/4] pve-api-types: update pve-api.json Hannes Laimer
2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 3/4] pve-api-types: add security group GET endpoints Hannes Laimer
2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 4/4] pve-api-types: regenerate Hannes Laimer
2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 1/2] pdm: rename ListFirewallRules to FirewallRule Hannes Laimer
2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 2/2] api: firewall: add pve firewall security group GET endpoints Hannes Laimer
2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox-yew-comp 1/2] firewall: rules: rename ListFirewallRules to FirewallRule Hannes Laimer
2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox-yew-comp 2/2] firewall: rules: make security group entries expandable Hannes Laimer
2025-12-09 15:23 ` [pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list Lukas Wagner
2025-12-09 16:45 ` Hannes Laimer [this message]
2025-12-11 9:20 ` Lukas Wagner
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=b85c23f9-38e3-4b11-ab20-2b03926e259c@proxmox.com \
--to=h.laimer@proxmox.com \
--cc=l.wagner@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.