[pve-devel] [PATCH widget-toolkit 1/1] close #3181: ui: add guest name to safe destroy dialog window
Michael Köppl
m.koeppl at proxmox.com
Fri Mar 28 13:03:57 CET 2025
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!
More information about the pve-devel
mailing list