[pve-devel] [PATCH FOLLOW-UP manager/widget-toolkit 0/2] replace SafeDestroy

[Michael Köppl]

On Tue Sep 30, 2025 at 12:00 PM CEST, Thomas Lamprecht wrote:
> Am 24.09.25 um 18:17 schrieb Michael Köppl:
>> The ConfirmRemoveDialog window covers the SafeDestroy window's
>> functionality, but makes it more general-purpose/flexible. It can be
>> used for both yes/no dialog as well as the SafeDestroy dialogs already
>> used for guests/storages. The reason for adding ConfirmRemoveDialog was
>> that Ext.Msg.MessageBox is not properly extendable and simply adding an
>> additional dialog would result in duplicate functionality covered in
>> both SafeDestroy and ConfirmRemoveDialog. Thus, replace SafeDestroy with
>> ConfirmRemoveDialog.
>
> Why not keep SafeDestroy but make it base off the new ConfirmRemoveDialog
> so that it's basically just a very thin wrapper that does not cost us
> much to have? It's frequently used after all, so going that route would
> reduce churn quite a bit.

Dano and I discussed this off-list before and I considered it during the
development process, but wanted to avoid the additional layer because I
thought it might make the implementation more confusing. But now that
I'm working on an updated version of these patches, I also noticed that
it would make things easier. Then I can even skip the follow-up
patches entirely. So I'll do

                ConfirmRemoveDialog
                |                 |
                |                 |
            SafeDestroy     ConfirmRemoveResource
            |         |
            |         |
SafeDestroyGuest    SafeDestroyStorage

Thanks for the feedback, will send an updated series today.