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

["Michael Köppl" <m.koeppl@proxmox.com>]

On 3/25/25 19:27, Thomas Lamprecht wrote:
> 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/

This is embarrassing...

> 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.

Thanks for your suggestion. I think it makes more sense to adopt this 
approach than change format_task_description. I'll send a v2 that just 
adds the guest name as part of the id input. However, the current 
implementation of formatGuestTaskConfirmation prints the parentheses 
even if the guest name is undefined. I suppose it mostly feels a bit 
unpolished if the confirmation dialog displays "VM 100 () - Remove" 
(this occurred to me only right after rebooting a node). Would it make 
sense to also update the implementation of formatGuestTaskConfirmation 
to only conditionally display the parentheses as part of a v2 and make 
it consistent across all confirmation dialogs, or is it better to 
consider this a separate patch?

> 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.

Yes, I already pondered a bit on which way around it makes more sense 
(ID (name) or name (ID)) or if the user should even be able to choose. I 
think doing it dependent on the sort key is a good idea, but it would 
add complexity to the code, as you said. I'll check it out and, if I 
find a sensible solution, send a separate patch so the added complexity 
can be properly considered.

Thank you for your feedback!


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