[pve-devel] [PATCH manager 2/3] Fix #1526: Use 'detach' instead of 'remove' when the disk is used

[Dominik Csapak]

Comments inline
On 11/13/2017 12:26 PM, Emmanuel Kasper wrote:
> Signed-off-by: Emmanuel Kasper <e.kasper at proxmox.com>
> ---
>   www/manager6/qemu/HardwareView.js | 25 ++++++++++++++++++++-----
>   1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
> index 43337c36..3b0f00f6 100644
> --- a/www/manager6/qemu/HardwareView.js
> +++ b/www/manager6/qemu/HardwareView.js
> @@ -396,6 +396,8 @@ Ext.define('PVE.qemu.HardwareView', {
>   
>   	var remove_btn = new PVE.button.Button({
>   	    text: gettext('Remove'),
> +	    defaultText: gettext('Remove'),
> +	    altText: gettext('Detach'),
>   	    selModel: sm,
>   	    disabled: true,
>   	    dangerous: true,
> @@ -423,6 +425,19 @@ Ext.define('PVE.qemu.HardwareView', {
>   			Ext.Msg.alert('Error', response.htmlStatus);
>   		    }
>   		});
> +	    },
> +	    listeners: {
> +		boxready: function(btn, initWidth, initHeight) {

i am not entirely happy with this event as it seems it should not work 
with buttons (but does?)
from the extjs documentation:

This event does not fire on components that use liquidLayout, such as 
Ext.button.Button and Ext.form.field.Base.

so maybe it works because it is inside a toolbar?
i know our possibilities are limited, so maybe this is the best way


> +		    // hack: calculate an optimal button width on first display
> +		    // to prevent the whole toolbar to move when we switch
> +		    // between the "Remove" and "Detach" labels
> +		    btn.setText(btn.altText);
> +		    var newWidth = btn.getSize().width;
> +		    btn.setText(btn.defaultText);
> +
> +		    btn.optimalWidth = newWidth > initWidth ? newWidth : initWidth;
> +		    btn.setSize({ width: btn.optimalWidth});
> +		}
>   	    }
>   	});
>   
> @@ -504,22 +519,22 @@ Ext.define('PVE.qemu.HardwareView', {
>   	    var rowdef = rows[key];
>   
>   	    var pending = rec.data['delete'] || me.hasPendingChanges(key);
> -	    var isDisk = !key.match(/^unused\d+/) &&
> +	    var isUsedDisk = !key.match(/^unused\d+/) &&
>   		rowdef.tdCls == 'pve-itype-icon-storage' &&
>   		(value && !value.match(/media=cdrom/));
>   
>   	    var isEfi = (key === 'efidisk0');
>   
> -
>   	    remove_btn.setDisabled(rec.data['delete'] || (rowdef.never_delete === true));
> +	    remove_btn.setText(gettext(isUsedDisk ? remove_btn.altText : remove_btn.defaultText));

this gettext is wrong, because we have the text already translated in 
the definition (so this will only work for english)

>   
>   	    edit_btn.setDisabled(rec.data['delete'] || !rowdef.editor);
>   
> -	    resize_btn.setDisabled(pending || !isDisk);
> +	    resize_btn.setDisabled(pending || !isUsedDisk);
>   
> -	    move_btn.setDisabled(pending || !isDisk);
> +	    move_btn.setDisabled(pending || !isUsedDisk);
>   
> -	    diskthrottle_btn.setDisabled(pending || !isDisk || isEfi);
> +	    diskthrottle_btn.setDisabled(pending || !isUsedDisk || isEfi);
>   
>   	    revert_btn.setDisabled(!pending);
>   
>