Microsoft-ui-xaml: Proposal: Improve ContentDialog API and Prevent Crashes

[robloo]

Proposal: Improve ContentDialog API and Prevent Crashes (applies to MessageDialog as well)

Multiple ContentDialog instances are not supported which leads to crashes when the app attempts to open the second one. While the framework does not support multiple dialogs, it also does not provide a reliable way for closing/detecting an already open one. This creates a bad combination for application developers.

To make this more robust we need two things:

1. Provide a rock-solid API for the app to manage already open ContentDialogs/MessageDialogs
2. Allow multiple ContentDialogs (and, if possible and not removed from the platform, MessageDialogs) to be displayed on top of each other (like classic Windows allows) and make it the responsibility of the App to manage them correctly. This is only to prevent crashes when the app doesn't manage things correctly -- it's a fail-safe for robustness.

Note: Number 2 would be difficult to do on mobile platforms with the Uno architecture. (We'll see how WinUI evolves for Surface Duo!)

Summary

1. Provide a rock-solid API for the app to manage already open ContentDialogs.

// As named, returns true if a ContentDialog is already open
Application.IsContentDialogOpen

// Will not directly close the ContentDialog but will synchronously return a reference 
// to it so the application developer can decide how best to close it.
Application.GetOpenContentDialog() 

// Overload to ShowAsync() to specify what to do if a ContentDialog is already open
ContentDialog.ShowAsync(HandleExistingDialogOption existingDialogOption) 

enum HandleExistingDialogOption
{
    /// <summary>
    /// Closes any already open dialog and returns ContentDialogResult.None 
    /// (or an empty command for MessageDialog)
    /// </summary>
    CloseExisting, 

    /// <summary>
    /// Does not show the new dialog and immediately returns ContentDialogResult.None
    /// (or an empty command for MessageDialog)
    /// </summary>
    DoNotShow,

    /// <summary>
    /// Dialog will be enqueued and shown once all previous dialogs are closed.
    /// </summary>
    Enqueue
}

1. As stated, allow multiple ContentDialogs to be displayed on top of each other

Remove the platform restriction of only one ContentDialog being open. Also definitely prevent a hard-crash if the application attempt to open a second. Two ContentDialogs is understood to be bad practice but there are some edge cases this helps protect against. Number 1 is still the primary solution.

Rationale

Currently, only one content dialog can be open at a time in the UI. This makes sense as it's meant to be blocking waiting for user input.

However, there are some scenarios where notifications are being sent from the backend to the frontend (not as a result of direct user input) and they should be displayed in a blocking content dialog to the user.

The problem occurs when multiple notifications are sent from the backend and the user hasn't yet closed the first one. In this case the app crashes when it tries to create the second ContentDialog. As a work around, apps can attempt to close dialogs themselves; however, this does not always work. If a request to open a content dialog comes fast enough a crash still occurs.

Applications should not crash so easily when opening a ContentDialog. Work-arounds such as the example code below are not reliable as they are somewhat dependent on timing.

    public static bool IsContentDialogOpen(Window window = null)
        {
            // By default use the current window
            if (window == null)
            {
                window = Window.Current;
            }

            // Get all open popups in the window. A ContentDialog is a popup.
            var popups = VisualTreeHelper.GetOpenPopups(window);

            foreach (var popup in popups)
            {
               if(popup.Child is ContentDialog)
               {
                  // A content dialog is open
                  return (true);
               }
            }

            return (false);
        }

or

var popups = VisualTreeHelper.GetOpenPopups(Window.Current);
foreach(var popup in popups)
{
    if (popup.Child is ContentDialog)
    {
        ((ContentDialog)popup.Child).Hide();
    }
}

[mrlacey]

That trying to open multiple dialogs causes a crash is a bug and should be treated as such.

I'm not convinced by the rationale for adding this.

• How does an app indicate the context of each dialog when multiple ones are shown? I'd expect having multiple dialogs open at once to be potentially confusing to the user and the developer/designer would likely need to do a lot of work to make sure this is always clear to all users. I can easily see the amount of work needed for this as being much more than ensuring that the app doesn't try and open a dialog when another is already open.
• Having the "backend" open dialogs seems wrong. You risk all sorts of potential issues when the backend is directly controlling the UI in this way due to not separating the responsibilities. The "backend" also becomes tightly coupled to the UI layer. At best I would expect the backend to tell the frontend that it needs some information and the frontend is responsible for requesting and returning that information. This leaves the frontend responsible for not prompting for multiple things at the same time.

[yaichenbaum]

I have a Notepad app that has a content dialog with a text box that lets the user goto a different line on the main editing text box. If that dialog is open and the user presses the close button on the app it will try to open a confirm dialog if there are unsaved changes. The second that second dialog comes up the app will crash because there is already an existing content dialog open.
In my case I need to close any open dialogs which as mentioned doesn't always work. I don't know if allowing more then one content dialog to be open at a time is a good idea, but there should definitely be a reliable way to close existing content dialogs so another one can be shown.

[robloo]

@mrlacey We could talk about application architecture endlessly but that would be a bit out of scope I think. However, since you are curious here is the full scenario:

• User starts a search which runs in the backend (Here it is possible to press ‘Enter’ very rapidly in the frontend and schedule two searches one after the other. Yes, that could be changed/filtered but complexity goes up a lot compared to just allowing it.)
• The backend completes the first search but finds no results. It notifies the frontend that ‘Search is complete with no results’
• The frontend displays the confirmation ContentDialog ‘Search is complete with no results’. The user does nothing (and has no chance to due to the speed)
• The backend completes the second search but finds no results. It again notifies the frontend that ‘Search is complete with no results’
• The frontent detects the first ContentDialog is open and attempts to close it. However, a lot of the WinUI code is not syncronous and there are some timing issues I cannot control. It's possible without any async code on my part that the second ContentDialog is opened before the first has a chance to fully close. (I would have to wait for some ridiculous long time to be safe which is just bad.)
• In the majority of cases the first ContentDialog is closed successfully by my applicaiton using the example code above. However, in some cases -- just depending on timing -- the app will crash.

So I try to close any already open ContentDialogs but the APIs to do so just are not there. Eventually the app will crash if requests are coming fast enough. I do of course agree that showing multiple ContentDialogs is bad but there are some edge cases as @yaichenbaum also mentions.

The point is we can’t adequately manage ContentDialogs because the API just isn't there:

1. There is no way to reliably detects when a ContentDialog is open
2. There is no way to reliably close the open ContentDialog (synchronously before the next opens)
3. There is also no parameter in ContentDialgo.ShowAsync() specifying what should happen to any existing ContentDialogs.

You can't have the platform hard-crash when a second ContentDialog is being opened and then not provide an API to manage ContentDialogs (instead relying on the user to do it). On top of that – the limit of one ContentDialog open should not be enforced by crashing… classic Windows or WPF never did that. The framework should allow multiple ContentDialogs to be open for the cases where the app doesn’t manage them correctly (assuming they get the API to do so).

Crashes are bad, ContentDialogs should not be this fragile.

I'm going to reword my proposal a bit asking for the API to manage ContentDialogs and also for the app not to crash when a second is opened. I think we need both to robustly fix this for application developers.

[eugenegff]

User attention is a resource, and as always, exclusive access to resource should be queued.

That is especially true when current user activity contains asynchronous continuations, like in chain like 'Ctrl + S shortcut' => FileSaveDialog.ShowAsync => StorageFile.WriteAsync => 'HideModifiedMark or ShowError'. Any background activity should queue attempt to access user till the current and all previously queued activities would be completed.

[robloo]

@eugenegff I agree with everything you said and that is certainly the best way of handling it. Implementing a queuing system would be a lot easier if there was an 'IsContentDialogOpen' API some place in the platform though. Otherwise developers have to write extra code to manage this themselves.

The app also should be able to make a judgement call of whether to close an existing ContentDialog and show another without waiting for user input. The API's to do that cleanly don't exist.

And finally, I still feel as a fail-safe the platform should not crash if a second ContentDialog is opened.

[jevansaks]

We agree that multiple ContentDialogs should not cause a crash. There's additionally a bug in the platform today with XAML Islands where each island can't have a dialog open because the tracking of this state is per-thread.

[zhuxb711]

Here is my solution for multiple ContentDialog, hope it would help you @robloo :

    public class QueueContentDialog : ContentDialog
    {
        private static readonly AutoResetEvent Locker = new AutoResetEvent(true);

        private static int WaitCount = 0;

        public static bool IsRunningOrWaiting
        {
            get
            {
                return WaitCount != 0;
            }
        }

        private bool IsCloseRequested = false;
        private ContentDialogResult CloseWithResult;

        public new async Task<ContentDialogResult> ShowAsync()
        {
            _ = Interlocked.Increment(ref WaitCount);

            await Task.Run(() =>
            {
                Locker.WaitOne();
            });

            var Result = await base.ShowAsync();

            _ = Interlocked.Decrement(ref WaitCount);

            Locker.Set();

            if (IsCloseRequested)
            {
                IsCloseRequested = false;
                return CloseWithResult;
            }
            else
            {
                return Result;
            }
        }

        public void Close(ContentDialogResult CloseWithResult)
        {
            IsCloseRequested = true;
            this.CloseWithResult = CloseWithResult;
            Hide();
        }

        public QueueContentDialog()
        {
            Background = Application.Current.Resources["DialogAcrylicBrush"] as Brush;
        }
    }

[lukasf]

I have also run into this trouble. Sure this can be handled in the app somehow, but it is tedious work that must be repeated in every single app. A framework solution would be highly appreciated!

I like the proposal, but I have some remarks:

1. This also affects MessageDialog! The solution must handle both ContentDialog and MessageDialog.

2. The CloseExisting enum value should close the existing dialog with ContentDialogResult.None (MessageDialog would return null).

3. Please also consider an enum value Enqueue. Then the dialog would be enqueued and shown once all previous dialogs are closed. The method is async anyways, so we just have to wait a little longer. I think this should be the default! I implemented this in my app using AsyncLock, it was pretty easy to do.

4. If people really want multiple dialogs at the same time, this could be a fourth option. But I think it should not be the default, because it is a pretty irritating user experience (and maybe this should not be implemented at all).

Scenarios such as @yaichenbaum's could perhaps be better implemented by a control that is placed in the normal view (on top level, hovering the normal content), in a Popup, or placed in a specific search area inside normal app layout.

[robloo]

@zhuxb711 Thanks for the code. Tracking open content dialogs is totally possible to do on the app-side. However, it's added work that shouldn't be required. This also gets more complex to coordinate as apps become larger/teams grow. In the older frameworks we never had to worry about such things and could just show dialogs at will without crashes. It would be ideal to have a framework solution in WinUI.

@lukasf

1. True! MessageDialog is right now part of the OS but with WinUI 3.0 it can be fixed as well. I've also moved on to ContentDialog exclusively in all my code as MessageDialog seems to be much less preferred to use (actually it's deprecated as mentioned at the bottom of the page). I'll add some comments including MessageDialog to the description.

2. Yes, I stated 'default button press' but had I thought about it even 2 seconds longer you are of course correct.

3. Good idea, I'll add it! Hate the enum name I came up with but, well, it's a work-in-progress.

4. I don't think multiple dialogs are a good thing at all. My point was it's up to the app to prevent it not the platform. This will align with WPF for people bringing over old apps too. Certainly the platform shouldn't crash on trying to open the second dialog!

[JasonStein]

This is easily achievable on user end with "queuing logic". Here is the class my friend and I wrote in Notepads to handle two scenarios perfectly (you can choose to wait on previous one or not), it has been running in production for half a year and I am never seeing exception again:

https://github.com/JasonStein/Notepads/blob/f127d170c16cbf0831c2cddb480a3ea05e202930/src/Notepads/Utilities/DialogManager.cs

OpenDialogAsync(NotepadsDialog dialog, bool awaitPreviousDialog)

    public class NotepadsDialog : ContentDialog
    {
        public bool IsAborted = false;
    }

"IsAborted" here is to keep track if current dialog is cancelled and aborted by DialogManager, so you are fully aware of the context.

public static class DialogManager
    {
        public static NotepadsDialog ActiveDialog;

        private static TaskCompletionSource<bool> _dialogAwaiter = new TaskCompletionSource<bool>();

        public static async Task<ContentDialogResult> OpenDialogAsync(NotepadsDialog dialog, bool awaitPreviousDialog)
        {
            return await OpenDialog(dialog, awaitPreviousDialog);
        }

        static async Task<ContentDialogResult> OpenDialog(NotepadsDialog dialog, bool awaitPreviousDialog)
        {
            TaskCompletionSource<bool> currentAwaiter = _dialogAwaiter;
            TaskCompletionSource<bool> nextAwaiter = new TaskCompletionSource<bool>();
            _dialogAwaiter = nextAwaiter;

            if (ActiveDialog != null)
            {
                if (awaitPreviousDialog)
                {
                    await currentAwaiter.Task;
                }
                else
                {
                    ActiveDialog.IsAborted = true;
                    ActiveDialog.Hide();
                }
            }

            ActiveDialog = dialog;
            var result = await ActiveDialog.ShowAsync();
            nextAwaiter.SetResult(true);
            return result;
        }
    }

I am using this code to cover two scenarios:

1. User opens multiple documents. Let's say 3 tabs in total and all of them have unsaved content. User hit App exit button, Three ContentDialogs will be shown one by one to inform user to take actions before closing (save or discard or cancel). So I need to call:

await DialogManager.OpenDialogAsync(setCloseSaveReminderDialog, awaitPreviousDialog: true);

1. User opens one document, made some changes and wanted to revert. He clicks revert button, a dialog will be shown to ask user to confirm the "revert" action. Now, user can still click App exit button. In this case, a new dialog that reminds user to save the file will be shown immediately. So I need to call:

await DialogManager.OpenDialogAsync(setCloseSaveReminderDialog, awaitPreviousDialog: false);

[mdtauk]

I think what the request here is something between a single, one at a time, ContentDialog - and not allowing an app to spam the window with endless dialogs, preventing the user from feeling in control of the experience.

A queue sounds like a sensible idea. One dialog visible at a time, but the closing of it, will spawn the next.

Perhaps there should be a limit on the number of spaces in a queue?
Maybe there could be a permission to allow more than say 4 at a time?

For scenarios with multiple documents displaying confirmation dialogs on closure - there could be guidance to combine those into a single dialog maybe.

[chingucoding]

I totally agree with @mdtauk here, using a queue seems to be the best choice UX wise (in my opinion). Having multiple ContentDialogs open at the same time can end up just being confusing.

Also a limitation to the maximum amount of ContentDialogs seems very reasonable, as to many dialogs might overwhelm the user.

[eugenegff]

I totally agree with @mdtauk here, using a queue seems to be the best choice UX wise (in my opinion). Having multiple ContentDialogs open at the same time can end up just being confusing.

Also a limitation to the maximum amount of ContentDialogs seems very reasonable, as to many dialogs might overwhelm the user.

That is so only till you think about multi window situation. Having one ContentDialog per window is perfectly reasonable, and not being able to continue work in one window due to the not closed content dialog in some other window - is not acceptable.

MacOS handles this situation especially well. Programmer choose does it want to show dialog in application modal way, or in window modal way. In first case dialog got own top level modal window, in second case it is shown as panel sliding from titlebar of owning window, preventing from interaction with this window, but not preventing interaction with other windows. For example, all Save dialogs are shown this way. It is called "Sheet", and there could be only one sheet per window.

https://developer.apple.com/design/human-interface-guidelines/macos/windows-and-views/sheets/

All of this is done in single threaded macOS application, that makes it easy to share resources between documents.

[mdtauk]

In UWP there is AppWindow which shares the UI Thread, so if ContentDialog is thread based, then that would indeed cause problems for all windows the app has created.

In WinUI Desktop / Win32 each window has it's own thread, so these queues would be per thread/per window.

UWP would be limited unless these queues are not per thread, but per Window - and were not blocking of other windows.

[eugenegff]

In Win32 you can have several HWND windows in single thread, as well as non modal dialogs.
https://docs.microsoft.com/en-us/windows/win32/dlgbox/using-dialog-boxes#creating-a-modeless-dialog-box

[JasonStein]

I think what the request here is something between a single, one at a time, ContentDialog - and not allowing an app to spam the window with endless dialogs, preventing the user from feeling in control of the experience.

A queue sounds like a sensible idea. One dialog visible at a time, but the closing of it, will spawn the next.

Perhaps there should be a limit on the number of spaces in a queue?
Maybe there could be a permission to allow more than say 4 at a time?

For scenarios with multiple documents displaying confirmation dialogs on closure - there could be guidance to combine those into a single dialog maybe.

You don't need to create a queue just for this and maintaining the life cycle of the queue is error prone in terms of app logic and will make users and devs to confuse. Dev should just call OpenDialog whenever he or she want at the right moment instead of putting it in a queue and make things complicated. And this is what I am doing for Notepads. The solution I have worked perfectly and I do not need to care about when l can or what should I do before calling. Here are some screen records:

Wait scenario:

Overwrite scenarios: