Re: [pve-devel] applied: [PATCH manager v4 0/2] fix #474: allow transfer from container/vms

[Philipp Hufnagl <p.hufnagl@proxmox.com>]

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 :)