Superseded
byhttps://lore.proxmox.com/pve-devel/20250321103511.66722-1-m.koe...@proxmox.com/
Thank you for having a look and your detailed suggestions!
style nit: we mainly use `let` variables for new code as that has less confusing
scope rules, i.e. variables declared through var are also available in the
parent
block scope.
Also thanks for this hint. Makes sense, will adhere to it for future patches.
On 3/21/25 08:58, Thomas Lamprecht wrote:
Thanks for this patch, some comments inline.
Am 18.03.25 um 17:14 schrieb Michael Köppl:
The current implementation is slightly misleading. When creating a
privileged container, the nesting checkbox is disabled but keeps its
current state. However, nesting is not enabled for privileged containers
even if the checkbox was set to true. Enabling nesting is still possible
through the Options menu.
Signed-off-by: Michael Köppl<m.koe...@proxmox.com>
---
As an alternative to this, since we already discourage
the use of privileged containers [0], removing the checkbox for creating
privileged
containers in the web UI might make sense. For the rare cases where they
are required, they can still be created using pct (although the question
remains whether privileged should be the default for pct create).
[0]https://forum.proxmox.com/threads/debian-12-lxc-template-systemd-failures.151630/post-686850
www/manager6/lxc/CreateWizard.js | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/www/manager6/lxc/CreateWizard.js b/www/manager6/lxc/CreateWizard.js
index 62cda27a..c7ee56f7 100644
--- a/www/manager6/lxc/CreateWizard.js
+++ b/www/manager6/lxc/CreateWizard.js
@@ -7,6 +7,7 @@ Ext.define('PVE.lxc.CreateWizard', {
nodename: '',
storage: '',
unprivileged: true,
+ nestingEnabled: true,
This works fine as is, but I think there is a better solution available here
(see below) and such use of viewModel data binding can come back and bite one
when extending/refactoring such code, as the code using it needs to be careful
to not interfere with the automatic two-way binding to the field's value.
I do not want to discourage using the viewModel, most of the time it can be
fine and result in even nicer code overall, but it has its oddities.
},
formulas: {
cgroupMode: function(get) {
@@ -69,14 +70,22 @@ Ext.define('PVE.lxc.CreateWizard', {
value: '{unprivileged}',
},
fieldLabel: gettext('Unprivileged container'),
+ listeners: {
+ change: function(checkbox, value) {
+ var viewModel = checkbox.lookupViewModel();
style nit: we mainly use `let` variables for new code as that has less confusing
scope rules, i.e. variables declared through var are also available in the
parent
block scope.
+ if (!value && viewModel) {
+ viewModel.set('nestingEnabled', false);
You could also avoid the intermediate viewModel variable that is used just once
and use an optional-chaining operator, like:
checkbox.lookupViewModel()?.set('nestingEnabled', false);
+ }
+ },
+ },
},
{
xtype: 'proxmoxcheckbox',
FYI that component comes from our proxmox-widget-toolkit and is a small override
of the original ExtJS checkbox. One thing it provides is a `clearOnDisable`
config flag that if set to true should exactly provide the behavior you want
here without needing to hook into the unprivileged change listener.
See src/form/Checkbox.js in the proxmox-widget-toolkit for the rather trivial
implementation of that flag.
name: 'features',
inputValue: 'nesting=1',
- value: true,
bind: {
disabled: '{!unprivileged}',
+ value: '{nestingEnabled}',
},
fieldLabel: gettext('Nesting'),
},
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
