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/28/25 13:03, Michael Köppl wrote:
> 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

Superseded by 
https://lore.proxmox.com/pve-devel/20250331133154.148713-1-m.koeppl@proxmox.com


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