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

[Philipp Hufnagl]

On 8/14/23 12:42, Dominik Csapak wrote:

> 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)
> 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..)
>
> also there is some whitespace error (missing space between && and 
> 'item.data.poll')
> don't know why eslint did not pick that up...


I considered the option of a warning box. I decided against it because I 
thought it too verbose throwing a warning box every time the user wants 
to transfer/migrate.

As for the UI I agree that it could be improved. Would it solve it if 
the vms currently part of a pool are only shown AFTER the check box is 
checked? This way the User could never experience this error.