Thanks for the detailed feedback! Sent a v2 with a better commit message and only uncheckedValue set explicitly.
https://lore.proxmox.com/pve-devel/[email protected] On Mon Nov 17, 2025 at 5:47 PM CET, Thomas Lamprecht wrote: > The commit subject could be a bit improved, as of now we do not use plain > "fix" commits, besides that it affects the ui subsystem, which is a major > one in this repo, so naming it makes sense, then it's not really for ha > itself but rather for the (guest) create wizard, and finally, the commit > message and it's subject should focus on what's fixed and ideally (in short) > why thefix works and was chosen rather than describing how it is done, > especially not if that's just then a rather literal description of the > actual diff itself. > > E.g. here the following would be a lot clearer to me: > > "ui: create wizard: only submit ha-managed if checked for better compat" > > > Am 17.11.25 um 14:17 schrieb Michael Köppl: >> This avoids sending the parameter with the default value when it's not >> necessary, since the API already defines the param as optional and >> assigns a default value of 0. > > Why does this work though, 0 and undefined seem close enough for anybody > without in-depth knowledge about ExtJS and/or our widget-toolkit. > > Would be nice to reference upstream docs for chechbox's uncheckedValue > config parameter, which plays the core part here: > >> By default this is undefined, which results in nothing being submitted >> for the checkbox field when the form is submitted > > -- > https://docs.sencha.com/extjs/7.0.0/classic/Ext.form.field.Checkbox.html#cfg-uncheckedValue > >> >> Signed-off-by: Michael Köppl <[email protected]> >> --- >> Thanks to @Friedrich for bringing this to my attention. A case where >> this occurs is in clusters where at least one of the nodes does not yet >> support the ha-managed param and a VM is created on that node from the >> web UI of a node that already supports it. Although in general it can >> probably be expected that all nodes should be upgraded to a new version, >> this avoids failing the CreateWizard dialog entirely for cases where the >> "Add to HA" checkbox wasn't even checked at basically no cost because >> the API already defines a default value of 0 and considers the param >> optional. >> >> www/manager6/lxc/CreateWizard.js | 4 ++-- >> www/manager6/qemu/CreateWizard.js | 4 ++-- >> 2 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/www/manager6/lxc/CreateWizard.js >> b/www/manager6/lxc/CreateWizard.js >> index f35709f14..024536c58 100644 >> --- a/www/manager6/lxc/CreateWizard.js >> +++ b/www/manager6/lxc/CreateWizard.js >> @@ -85,8 +85,8 @@ Ext.define('PVE.lxc.CreateWizard', { >> { >> xtype: 'proxmoxcheckbox', >> name: 'ha-managed', >> - uncheckedValue: 0, >> - defaultValue: 0, >> + uncheckedValue: undefined, >> + defaultValue: undefined, > > Both are the default for (proxmox)checkbox, so could be just omitted? Or if, > then > I'd just set "uncheckedValue" explicitly and add a small comment there so > that it's > a bit more explicit and telling. > >> fieldLabel: gettext('Add to HA'), >> }, >> ], >> diff --git a/www/manager6/qemu/CreateWizard.js >> b/www/manager6/qemu/CreateWizard.js >> index e0c56bc0b..564ca1176 100644 >> --- a/www/manager6/qemu/CreateWizard.js >> +++ b/www/manager6/qemu/CreateWizard.js >> @@ -99,8 +99,8 @@ Ext.define('PVE.qemu.CreateWizard', { >> { >> xtype: 'proxmoxcheckbox', >> name: 'ha-managed', >> - uncheckedValue: 0, >> - defaultValue: 0, >> + uncheckedValue: undefined, >> + defaultValue: undefined, > > Same here. > >> fieldLabel: gettext('Add to HA'), >> }, >> ], _______________________________________________ pve-devel mailing list [email protected] https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
