On 8/24/23 16:46, Thomas Lamprecht wrote:
Am 14/08/2023 um 12:42 schrieb Dominik Csapak:
On 8/14/23 12:36, Wolfgang Bumiller wrote:
applied, thanks

@Dominik: does extjs have an 'enableFn' for rows in a grid?
IMO we should either disable the ones with pools when the transfer
checkbox is not checked, or hide them (but when hiding them after
already checking them... it's weird)
Or disable the 'Add' button if a VM with a pool is checked?

'enableFn' is our invention ;) and no that only works for some of our components


looking just now at the gui patch, i would have approached it a bit differently:

always enable the 'transfer' property but show a 'warning' box when one is 
selected
with an old pool

since 'Allow Transfer' is rather non-descriptive (and no documentation is 
included)
+1, FWIW I had no idea what this series is about from just reading the subject,
as "pool" is not mentioned there.

and it adds needless friction on change
(i select a vm, click, get an error, have to select the vm again, click 
transfer, click button..)


We normally use "move" or "migrate", not "transfer", or "reassign" (like for
moving a guest disk to another guest) and it has some merits to not expand the
commonly used (parameter) naming scheme to much, but oh well it's already 
released
and a naming nit that doesn't matters _that_ much.

But the default isn't declared in the schema, please send a follow up for that.

And I agree with Dominik, UX isn't ideal, a warning that one or more VMID will
be moved out of there old Pool, if any, would be sufficient. Not sure if it'd be
better if that's a per-row hint, shown if the row is ticked (e.g., instead of 
the
Pool column) or a edit-window wide warning hint that gets made visible if any of
the selected VMIDs is in a Pool already.

FWIW, and not directly related (i.e., can be it's own series), you could also 
fix
the s/Virtual Machine/Virtual Guest/ wording to avoid the confusion that one 
also
adds Container over this interface.

Sorry for the issue. It has been my first Patch on this scale.

I will make a new patch Improving the user experience by renaming to "migrate" or "migrate from other pool" and fix the schema

As for the feature design itself: The UI could be improved by only showing vms assinged to a pool when the transfer/migrade check box is checked. This way it should be clear if it is a migration without the use of a popup.

Would that work? Feedback is most welcome :)



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to