Re: [pve-devel] [PATCH widget-toolkit 1/1] close #3181: ui: add guest name to safe destroy dialog window

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 25.03.25 um 16:01 schrieb Michael Köppl:
> While the format_task_description function is used in other parts of the
> UI, this still leaves these use cases intact. The guest name is an
> optional addition in parantheses.

s/parantheses/parentheses/

> 
> Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
> ---
>  src/Utils.js              | 8 ++++++--
>  src/window/SafeDestroy.js | 3 ++-
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/src/Utils.js b/src/Utils.js
> index c873c85..a64d07f 100644
> --- a/src/Utils.js
> +++ b/src/Utils.js
> @@ -716,7 +716,7 @@ utilities: {
>  	}
>      },
>  
> -    format_task_description: function(type, id) {
> +    format_task_description: function(type, id, name) {

FYI in pve-manager's Utils we got the following in formatGuestTaskConfirmation
already:

return Proxmox.Utils.format_task_description(taskType, `${vmid} (${guestName})`);

And that seems to do the same thing and comes from a not so old commit
31965684c ("fix #5787: ui: display guest name in confirmation dialog")
https://git.proxmox.com/?p=pve-manager.git;a=commitdiff;h=31965684c

Could make sense to use the same approach here and also to reference that
commit in the commit message for posterity's sake.


>  	let farray = Proxmox.Utils.task_desc_table[type];
>  	let text;
>  	if (!farray) {
> @@ -731,7 +731,11 @@ utilities: {
>  	let prefix = farray[0];
>  	text = farray[1];
>  	if (prefix && id !== undefined) {
> -	    return prefix + ' ' + id + ' - ' + text;
> +	    let fullText = prefix + ' ' + id;
> +	    if (name) {
> +		fullText += " (" + name + ")";
> +	    }
> +	    return fullText + " - " + text;

small nit that I already had written before noticing above, so mostly relevant
if we keep this in the first place (or similar changes in the future):

I'd slightly favor directly returning and using template strings [0] for new
interpolation of variables with strings.

And FWIW, it might be a nice small polishing if we allow the caller to
determine if the VMID should be put in parentheses, e.g. for the case where the
user configured the resource tree to use the name as sort key, OTOH., that might
be a relatively big amount of complexity for little gain – just mentioning it
for the sake of completeness.

A simple variant without that extra polishing could look like:

if (prefix && id !== undefined) {
    let fullId = name ? `${id} (${name}) : id;
    return `${prefix} ${fullId} - ${text}`;
}

Ore more verbose:

if (prefix && id !== undefined) {
    let fullId = name ? `${id} (${name}) : id;
    if (name) {
        return `${prefix} ${id} (${name}) - ${text}`;
    } else {
        return `${prefix} ${id} - ${text}`;
    }    
}

but no hard feelings in any way.

[0]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals

>  	}
>  	return text;
>      },
> diff --git a/src/window/SafeDestroy.js b/src/window/SafeDestroy.js
> index c058465..d980a52 100644
> --- a/src/window/SafeDestroy.js
> +++ b/src/window/SafeDestroy.js
> @@ -187,9 +187,10 @@ Ext.define('Proxmox.window.SafeDestroy', {
>  	}
>  
>  	let taskName = me.getTaskName();
> +	const itemName = me.getItem().name;
>  	if (Ext.isDefined(taskName)) {
>  	    me.lookupReference('messageCmp').setHtml(
> -		Proxmox.Utils.format_task_description(taskName, itemId),
> +		Proxmox.Utils.format_task_description(taskName, itemId, itemName),
>  	    );
>  	} else {
>  	    throw "no task name specified";



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