havit / Havit.Blazor

Free Bootstrap 5 components for ASP.NET Blazor + optional enterprise-level stack for Blazor development (gRPC code-first, layered architecture, localization, auth, ...)
https://havit.blazor.eu
MIT License
463 stars 63 forks source link

[HxModal] Application gets locked because of remaining backdrop #810

Closed CryptKat closed 2 months ago

CryptKat commented 2 months ago

If a modal ShowAsync method gets called twice because of an accidental doubleclick, then only one instance of modal is shown, but with two backdrops. When user closes the modal, one backdrop gets dismissed, but the other remains, and the app becomes unusable.

<div @onclick="TestClick">Test</div>

@code {
    private async Task TestClick()
    {
        await Task.Delay(500); // Simulate some API call.
        await MessageBox.ConfirmAsync("Test"); // MessageBox is taken for the sake of simplicity.
    }
}

Solution is very simple. HxModal.js: if a modal instance for that DOM element already exists, then do nothing.

export function show(element, hxModalDotnetObjectReference, closeOnEscape, subscribeToHideEvent) {
    if (!element || bootstrap.Modal.getInstance(element)) {
        return;
    }
    ...
}
hakenr commented 2 months ago

@CryptKat We are handling this in HxOffcanvas, ensuring the previous offcanvas is closed before opening a new one. This isn't restricted to the same offcanvas component and prevents any two offcanvases from being shown simultaneously.

In the case of HxModal (which shares much of its logic with HxOffcanvas in both Havit.Blazor and Bootstrap), we decided not to include these protections as it was considered much less likely to occur in real-world UX scenarios, and we didn't want to add any additional logic unless it proves to be needed.

The condition you propose seems safe and improves the usability of HxModal. We will give it a try.