Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Preview 4] MessageDialog is no longer functional #4167

Open
dotMorten opened this issue Feb 12, 2021 · 16 comments
Open

[Preview 4] MessageDialog is no longer functional #4167

dotMorten opened this issue Feb 12, 2021 · 16 comments
Labels
area-Dialogs bug Something isn't working product-winui3 WinUI 3 issues team-Controls Issue for the Controls team version-winui3preview4 WinUI 3 Preview 4 issues

Comments

@dotMorten
Copy link
Contributor

dotMorten commented Feb 12, 2021

Describe the bug
Showing a message dialog no longer works in a Win32 app using Preview 4

Steps to reproduce the bug
Run the following code, for instance in a button click event:

            var dlg = new MessageDialog("Test");
            try
            {
                await dlg.ShowAsync();
            }
            catch (System.Exception ex)
            {
                Debug.WriteLine(ex.Message);
            }

See the following exception thrown in the catch:

Invalid window handle. (0x80070578)

Expected behavior
Dialog displayed

Version Info

NuGet package version:
[Microsoft.WinUI 3.0.0-preview4.210210.4]

Windows app type:

UWP Win32
Yes
Windows 10 version Saw the problem?
Insider Build (xxxxx)
October 2020 Update (19042)
May 2020 Update (19041)
November 2019 Update (18363)
May 2019 Update (18362)
October 2018 Update (17763)
April 2018 Update (17134)
Fall Creators Update (16299)
Creators Update (15063)
Device form factor Saw the problem?
Desktop
Xbox
Surface Hub
IoT

Additional context
Workaround:

        [ComImport]
        [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)]
        [Guid("3E68D4BD-7135-4D10-8018-9FB6D9F33FA1")]
        internal interface IInitializeWithWindow
        {
            void Initialize(IntPtr hwnd);
        }

        [ComImport]
        [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)]
        [Guid("EECDBF0E-BAE9-4CB6-A68E-9598E1CB57BB")]
        internal interface IWindowNative
        {
            IntPtr WindowHandle { get; }
        }


// ...
            var dlg = new MessageDialog("Test");
            IInitializeWithWindow withWindow = dlg.As<IInitializeWithWindow>();
            var handle = this.As<IWindowNative>().WindowHandle;
            try
            {
                withWindow.Initialize(handle);
                await dlg.ShowAsync();
            }
            catch (System.Exception ex)
            {
                Debug.WriteLine(ex.Message);
            }

Alternative workaround without requiring knowledge of the current window:

        public static IAsyncOperation<IUICommand> ShowDialogAsync(string content, string title = null)
        {
            var dlg = new MessageDialog(content, title ?? "");
            var handle = GetActiveWindow();
            if (handle == IntPtr.Zero)
                throw new InvalidOperationException();
            dlg.As<IInitializeWithWindow>().Initialize(handle);
            return dlg.ShowAsync();
        }

        [ComImport]
        [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)]
        [Guid("3E68D4BD-7135-4D10-8018-9FB6D9F33FA1")]
        internal interface IInitializeWithWindow
        {
            void Initialize(IntPtr hwnd);
        }

        [DllImport("user32.dll")]
        private static extern IntPtr GetActiveWindow();
@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Feb 12, 2021
@StephenLPeters StephenLPeters added area-Dialogs version-winui3preview4 WinUI 3 Preview 4 issues product-winui3 WinUI 3 issues and removed needs-triage Issue needs to be triaged by the area owners labels Feb 12, 2021
@dotMorten
Copy link
Contributor Author

It should be added that ContentDialog exhibit similar brokenness, however the above workaround doesn't work, as it couldn't be cast to IInitializeWithWindow.

@huoyaoyuan
Copy link

FYI: The IInitializeWithWindow workaround still works for FolderPicker.

@nickrandolph
Copy link

Was this supposed to have been fixed? or is this still a known issue in ProjectReunion 0.8.1?

@dotMorten
Copy link
Contributor Author

@nickrandolph I hit it too the other day with 0.8.1. It's overly complicated to do a quick message box to a user. You can't use the simple message dialog, so you have to use the content dialog, and also remember to set the XamlRoot.

Example:
https://github.com/dotMorten/UnifiClient/blob/9be1a1ebf48ce207734170da32a1f700316ffd5b/src/UnifiClientApp/UnifiClientApp/MainWindow.xaml.cs#L73-L80

I really wish they'd make a workflow as common as a message box a lot easier.

@krschau krschau added the team-Controls Issue for the Controls team label Jul 19, 2021
@nickrandolph
Copy link

MessageDialog with the hack worked for me. Wonder if there are edge cases where it doesn't work.

@sylveon
Copy link
Contributor

sylveon commented Jul 29, 2021

That's not really a hack, it's the proper way to use MessageDialog in the absence of a CoreWindow. For ContentDialog, if you declare it in XAML the XamlRoot is automatically set so you can just call ShowAsync from code-behind.

@dotMorten
Copy link
Contributor Author

That's not really a hack, it's the proper way

potato potahto. That's merely an excuse for a poorly designed / hard to use api. Calling it a hack because of how using this undiscoverable API feels like, seems fair.

@sylveon
Copy link
Contributor

sylveon commented Jul 30, 2021

Sure, there's a point to be made for the API to be made more discoverable, I opened a docs issue about it: MicrosoftDocs/winrt-api#2047

There's helpers in CsWinRT (WinRT.Interop.InitializeWithWindow.Initialize and WinRT.Interop.WindowNative.GetWindowHandle, which are documented here) that avoids needing to manually import COM interfaces, so the code can be written as the following, making it much less hard to use.

public static IAsyncOperation<IUICommand> ShowDialogAsync(string content, string title = null)
{
    var dlg = new MessageDialog(content, title ?? "");
    var handle = GetActiveWindow();
    if (handle == IntPtr.Zero)
        throw new InvalidOperationException();
    InitializeWithWindow.Initialize(dlg, handle);
    return dlg.ShowAsync();
}


// or
var dlg = new MessageDialog("Test");
InitializeWithWindow.Initialize(dlg, WindowNative.GetWindowHandle(this));
await dlg.ShowAsync();

@dotMorten
Copy link
Contributor Author

dotMorten commented Jul 30, 2021

Yup I'm aware of the new helper, but it doesn't really address the problem. If the dialog took the xamlroot as a parameter it would at least be a bit more obvious, perhaps combined with hiding the empty constructor from intellisense (since it needs to stay there for xaml support).

And speaking of that helper: It isn't even clear what type it takes as a parameter (just any object), nor is there any intellisense to tell me anything:
image
image

When you don't hook up the window handle or root (because how are you supposed to know you need to?), you get really really poor error messages, not help messages (it's not invalid, it's just not set):

image

At least you can get a bit further with some extension methods, but we shouldn't have to do this:
image

All of this ultimately leads to an extremely frustrating and off-putting developer experience. I feel a bit like beating a dead horse, since I've been making this same point over and over. It's one thing that WinUI "works" and can do what it needs to, but that doesn't help, if the developer experience is so frustrating we give up before we really get into using it.

@sylveon
Copy link
Contributor

sylveon commented Jul 30, 2021

Remember that MessageDialog is an old Windows 8 era system API. So at this point it's effectively frozen and most likely won't be seeing any new changes, especially considering it's deprecated. There are other issues to the XamlRoot constructor idea:

  • A system API can't take a type from what is effectively a third party library as part of its API surface
  • MessageDialog and the picker types predate XAML and can run fully independently of it. In fact in WinUI 3 it runs without system XAML at all, it has no idea WinUI 3 is a thing. Coupling it to XAML this late isn't a good idea for existing consumers.
  • I haven't found a (public) way to extract a window handle out of a XamlRoot yet. Just taking a window handle from the constructor also doesn't work because pointer sized integers (IntPtr/std::intptr_t) or raw pointers aren't supported in WinRT APIs. This last detail is why a raw COM interface is used for this, since it was expected to be used mostly for interop and COM does support raw pointers (of which window handles are)

@dotMorten
Copy link
Contributor Author

Remember that MessageDialog is an old Windows 8 era system API

Again just an excuse. WinUI needs a message dialog that fits into the WinUI model of things. We shouldn't have to be doing these crazy hacks to make basic things work. But we're currently left with no choice.

@sylveon
Copy link
Contributor

sylveon commented Jul 30, 2021

Then wouldn't the better idea be to suggest a modern WinUI API for this, or improvements to ContentDialog that makes it easier to use, rather than complain about an old API.

@JaiganeshKumaran
Copy link
Contributor

JaiganeshKumaran commented Feb 26, 2023

@dotMorten What's the problem with this?

ContentDialog dialog = new()
{
    Title = "your title",
    Content = "your message",
    PrimaryButtonText = "OK",
    SecondaryButtonText = "Cancel",
    XamlRoot = this.XamlRoot
};

var result = await dialog.ShowAsync();

@sylveon
Copy link
Contributor

sylveon commented Feb 26, 2023

Nice necro

@mikernet
Copy link

@JaiganeshKumaran

What's the problem with this?

  1. You need to jump through annoying hoops that may or may not play nicely with your MVVM library dialog flow mechanism to show a simple message dialog when you're already showing a content dialog, because you can't stack content dialogs on top of each other, so you end up with convoluted code to hide the current content dialog, then show the message dialog, and then re-show the content dialog. If any code is awaiting the original dialog ShowAsync() then that becomes a big problem.
  2. If you are building a cross-platform app with Uno+WinUI, MessageDialog gets translated into the native message dialog functionality on each platform, which is desirable in a lot of situations.

@ranjeshj
Copy link
Contributor

ranjeshj commented Oct 9, 2023

Ideally, we should make ContentDialog support this model in WinUI3 desktop, multiple islands scenarios.

@duncanmacmichael duncanmacmichael added the bug Something isn't working label Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Dialogs bug Something isn't working product-winui3 WinUI 3 issues team-Controls Issue for the Controls team version-winui3preview4 WinUI 3 Preview 4 issues
Projects
None yet
Development

No branches or pull requests

10 participants